[keycloak-dev] Change in enabling / disabling features

[Marko Strukelj]

The more I look at the idea of having '-preview' in feature names the more
I think it will cause more trouble than it's worth.

The reason is it complicates migration when we promote feature to stable -
dropping '-preview' from its name, and it causes configuration names to be
fluid over time.

For example, consider how moving authorization from preview to stable would
look for a user.

User got used to using -Dfeature.authorization-preview=true

Now we change this setting to be called -Dfeature.authorization=true. User
still using -Dfeature.authorization-preview=true, will get a warning upon
startup: 'Unknown feature system property: feature.authorization-preview',
and the system property will have no effect. If user uses Keycloak
distribution then authorization-preview will be enabled by default. So no
need to explicitly enable it - a no-op operation in the first place, and a
no-op operation at upgrade. If user sets the property to false, then after
upgrade the feature will suddenly be enabled when before it was not.

It doesn't seem right to drop configuration like that. If we want old
config to keep working then every migration from preview to stable also has
to add backwards compatibility logic, so that surprises like this don't
happen. Also all documentation needs to be changed wherever the feature
name is referenced.

Whenever we promote any feature from '-preview' to stable this will be
needed, and it may become standard ritual that may to most users seem
completely redundant. Why not just call it -Dfeature.authorization in the
first place, and maybe just display a warning if any 'preview' feature is
enabled, something like:

[WARN] The following preview features are enabled: ...

WDYT?


On Wed, Aug 30, 2017 at 8:28 AM, Stian Thorgersen <sthorger at redhat.com>
wrote:

> In standalone.xml I'd use "feature.scripts" sys properties instead of the
> longer "keycloak.feature.scripts". It's just shorter and simpler and
> won't cause any conflicts.
>
> We need to support the old "keycloak.profile.feature.<featurename>=enabled|disabled"
> as is, including if set in profile.properties. This is important in case
> customers have explicitly disabled impersonation for example. +1 To
> printing a warning in this case. Then let's remove the old deprecated
> properties in Keycloak 3.5x as well as the profile.properties.
>
> On 28 August 2017 at 15:57, Marko Strukelj <mstrukel at redhat.com> wrote:
>
>> This is a heads up about an upcoming change in how to enable or disable
>> features in Keycloak.
>>
>> Keycloak has a notion of features, which makes it possible to disable
>> certain functionality that is enabled by default, or enable certain
>> functionality that is disabled by default.
>>
>> There are four sets of functionality you can enable or disable as
>> features:
>> impersonation, script,  authorization, and docker. See [1] for more info.
>>
>> Currently a file called profile.properties can be used to enable / disable
>> features, or system properties can be used, which override any
>> configuration inside profile.properties as explained in [1].
>>
>> This is an aberration from how other configuration is specified on the
>> server via standalone.xml.
>>
>> Also, the reason config file is called profile.properties, and not
>> features.properties is because the set of enabled/disabled features is
>> different for upstream project (where all but 'docker' are enabled), for
>> product based on Keycloak - RHSSO (where only 'impersonation' is enabled),
>> and for technology preview versions of RHSSO.
>>
>> This additionally complicates things. The idea is to simplify that and
>> remove the notion of profiles, stop using profile.properties, and move all
>> configuration to standalone.xml where all the available features will be
>> listed with default values:
>>
>> <subsystem xmlns="urn:jboss:domain:keycloak-server:1.1">
>>
>> ...
>>
>>   <features>
>>     <feature name="authorization" enabled="true" />
>>     <feature name="impersonation" enabled="true" />
>>     <feature name="scripts" enabled="true" />
>>     <feature name="docker" enabled="false" />
>>   </features>
>>
>> ...
>>
>> </subsystem>
>>
>> This is how configuration would look like in Keycloak distribution. In
>> product distributions different features would be enabled / disabled - no
>> more named profiles.
>>
>> However, in order to allow system properties override the idea is to do it
>> slightly differently:
>>
>>   <features>
>>     <feature name="authorization"
>> enabled="${keycloak.feature.authorization:true}" />
>>     <feature name="impersonation"
>> enabled="${keycloak.feature.impersonation:true}" />
>>     <feature name="scripts" enabled="${keycloak.feature.scripts:true}" />
>>     <feature name="docker" enabled="${keycloak.feature.docker:false}" />
>>   </features>
>>
>>
>> We should probably also facilitate transition for current deployments, and
>> take old style system properties into account if they are used -
>> converting
>> them to the new ones before configuration is applied.
>>
>> Deprecated: -Dkeycloak.profile.feature.impersonation=enabled
>> New style:    -Dkeycloak.feature.impersonation=true
>>
>> We don't want to keep support for this indefinitely (maybe for one minor
>> version?) so if use of deprecated system properties is detected a warning
>> will be logged.
>>
>> Also, Stian proposed that features not considered stable yet, should carry
>> a suffix '-preview' in the name. So the actual feature name, and system
>> property name may still change for some features.
>>
>> You can follow progress on JIRA issue [2].
>>
>> -
>>
>> [1]
>> https://keycloak.gitbooks.io/documentation/server_installati
>> on/topics/profiles.html
>> [2] https://issues.jboss.org/browse/KEYCLOAK-4994
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>
>