[keycloak-dev] Change in enabling / disabling features

Tue Oct 3 07:36:17 EDT 2017

In what you are proposing what happens when a feature goes from tech
preview to supported? It would still be disabled by default right?

On 3 October 2017 at 12:26, Marko Strukelj <mstrukel at redhat.com> wrote:

>
> On Oct 3, 2017 06:11, "Stian Thorgersen" <sthorger at redhat.com> wrote:
>
> Having '-preview' in the name clearly communicates the fact that it's a
> tech preview feature. When it is made fully supported it should be enabled
> out of the box by default. If we don't do this then users that haven't
> enabled the feature when it was tech preview won't have it enabled when
> it's fully supported. Alternatively, if you automatically change it from
> disabled to enabled when it's moved from tech preview you don't know if
> users actually had chosen to disable it or if that was just the default. In
> either case I think it's cleaner and safer to rename, because the feature
> does indeed change from preview to supported.
>
>
> The case you point out with feature not getting disabled ...
>
> Consider the case where user disables a preview feature with
> -Dfeature.authorization-preview=false. It's a no-op for product, but not
> for community distro. When we upgrade feature to stable user will get a
> warning for an unknown feature, and the old config will not turn off the
> feature any more - on product as well. So did we solve anything?
>
> Do we want -Dfeature.authorization-preview=false to keep having effect
> even after it's not called feature.authorization-preview any more? That's
> what I call backwards compatibility. Will it remain in place forever or we
> give ourself extra work not only adding it in the first place, but also
> removing it at some point?
>
> My point is that '-preview' adds complexity to maintain, may introduce
> extra bugs, JIRAs, will require extra documentation ... so what it brings
> to the table should outweigh the burden. Maybe there is another way to
> achieve obviousness of the fact that some feature is 'preview' without
> actually naming the feature, and all config options as such.
>
>
>
> On 2 October 2017 at 15:37, Marko Strukelj <mstrukel at redhat.com> wrote:
>
>> 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
>>>>
>>>
>>>
>>
>
>