[keycloak-dev] Change in enabling / disabling features

Tue Oct 3 00:13:18 EDT 2017

Bill - I told you that if you want this changed you need to talk to John
about it, not me.

On 2 October 2017 at 23:45, Bill Burke <bburke at redhat.com> wrote:

> Adding per feature toggles is not a good idea.  We currently have
> feature to feature dependencies.  For example, token exchange depends
> on fine-grain admin permissions, and fine grain admin permissions
> depends on authorization.  I've warned Stian about this and I guess he
> doesn't care?  I still struggle to understand why we want to make
> things more difficult on paying customers.
>
> On Mon, Oct 2, 2017 at 9:37 AM, 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
> >>>
> >>
> >>
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
> --
> Bill Burke
> Red Hat
>