Keycloak Operator - synchronizing KeycloakRealms

[Sebastian Łaskawiec]

Hey,

This discussion is related to https://issues.redhat.com/browse/KEYCLOAK-15142

Early on, when designing initial version of Keycloak Operator, we decided to create Realms and never update them from Operator. Our idea was that users will create an empty realm and edit it via Keycloak Admin Console. This breaks symmetry with other CRs (KeycloakUser, KeycloakClient), which are always updated.

After thinking about this for a moment, I'd like to propose introducing a boolean switch to all our CRs, called "managed". If the switch is set to true, Keycloak Operator will synchronize all the updates. If the switch is set to false, Keycloak Operator will only create the resource and then never update it. This mode might be useful if you'd like to create a Realm and manage it via Keycloak Admin Console.

What do you think about this proposal?

Thanks,

Sebastian

[Peter Braun]

Hey Sebastian,

we used the same strategy in our old internal Keycloak Operator. How would this  A few questions:

1) what is the default value if the field is not provided?

2) what if users  are provided in a Realm CR and also in a User CR? Who wins?

3) will other CRs also get this switch or will they always be managed?

I think a default value of 'managed: true' would make sense: otherwise users could run into the situation where they created a realm a while ago, made changes in the admin console and then they all get reset once the flag is switched.

Regards,

Peter

[Jan Lieskovsky]

Hey Sebastian,

  the proposal looks reasonable to me. Comments inline.

On Wed, Aug 12, 2020 at 9:38 AM Peter Braun <pbr...@redhat.com> wrote:

Hey Sebastian,

we used the same strategy in our old internal Keycloak Operator. How would this  A few questions:

1) what is the default value if the field is not provided?

IMHO it should default to "true" (since users might naturally expect performing the changes via the webconsole

or "oc edit" command to work without additional settings need to be performed in advance).

 

2) what if users  are provided in a Realm CR and also in a User CR? Who wins?

Maybe merge / union the requests if intersection of the users definition in both CRs is disjunctive?

If the requests are conflicting ones, I would say the definition in User CR should win (since IMHO

this is what the operator user would expect to happen).

 

3) will other CRs also get this switch or will they always be managed?

To preserve the consistency other CRs should get this switch too IMHO.

 

I think a default value of 'managed: true' would make sense: otherwise users could run into the situation where they created a realm a while ago, made changes in the admin console and then they all get reset once the flag is switched.

+1 on defaulting to "true" setting (the behavior you described [sudden values reset upon flag switch] might be confusing to the users IMHO. Plus one might expect the "oc edit" command on the resource just work [without additional settings need to be performed apriori] if the field is allowed to be edited in CR definition).

 

Regards,

Peter

On Wednesday, August 12, 2020 at 9:25:46 AM UTC+2 sebastian...@gmail.com wrote:

Hey,

This discussion is related to https://issues.redhat.com/browse/KEYCLOAK-15142

Early on, when designing initial version of Keycloak Operator, we decided to create Realms and never update them from Operator. Our idea was that users will create an empty realm and edit it via Keycloak Admin Console. This breaks symmetry with other CRs (KeycloakUser, KeycloakClient), which are always updated.

After thinking about this for a moment, I'd like to propose introducing a boolean switch to all our CRs, called "managed". If the switch is set to true, Keycloak Operator will synchronize all the updates. If the switch is set to false, Keycloak Operator will only create the resource and then never update it. This mode might be useful if you'd like to create a Realm and manage it via Keycloak Admin Console.

What do you think about this proposal?

LGTM AFAICT. Please also see the other comments within reply to Peter's reply above.

 

Thanks,

Sebastian

--

HTH

Thank you && Regards, Jan

--

Jan iankko Lieskovsky / Keycloak / RH-SSO Team

 

You received this message because you are subscribed to the Google Groups "Keycloak Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to keycloak-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/0380c36f-02d9-48bc-9753-dbfaa7be2c42n%40googlegroups.com.

[Till Markus (IOC/PDL22)]

I’m new to the discussion and don’t have a full picture what happened so far in the operator.

So don’t mind to ignore this comment J

 

IMHO CRD are mostly used to describe a desired state. If an “managed” flag is introduced. This would indicate to me that CRD is also not removed if the resource is removed. This brings the question to my mind why the CRD was there in the first place.

 

Idea:

If the CRD is unmanaged it could be a job like interface. E.g. CreateClient CRD CreateRealm CRD etc. as in the K8s Job/CronJob API the Create* CRDs could consist of a higher level strategy as “override vs update” or may how many clients of the same type are created via an iterator etc. The separation of concern for the run once CRDs and the synced CRDs would allow a clear separation of such configurations. Eventually these Create* CRDs would wrap a concrete CRD e.g. a Client etc. After successful creation these CRDs could be dropped or show their final state forever in the state section.

 

In any case something like this would be more idiomatic to k8s IMHO.  Also users would have a clear separation between run once and synced objects.

 

Mit freundlichen Grüßen / Best regards

Markus Till

Project Delivery Berlin 22 (IOC/PDL22)
Bosch.IO GmbH | Ziegelei 7 | 88090 Immenstaad | GERMANY | www.bosch.io
Tel. +49 30 726112-354 | Mobil +49 172 5782078 | Telefax +49 30 726112-100 | Threema / Threema Work: FHKUKVDR | Marku...@bosch.io

Sitz: Berlin, Registergericht: Amtsgericht Charlottenburg; HRB 148411 B
Aufsichtsratsvorsitzender: Dr.-Ing. Thorsten Lücke; Geschäftsführung: Dr. Stefan Ferber, Dr. Aleksandar Mitrovic, Yvonne Reckling

​

--

You received this message because you are subscribed to the Google Groups "Keycloak Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to keycloak-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/c11621ec-bccf-40f5-9f1e-069a45565ddan%40googlegroups.com.

[Sebastian Łaskawiec]

On Wednesday, 12 August 2020 at 10:06:44 UTC+2 jlie...@redhat.com wrote:

Hey Sebastian,

  the proposal looks reasonable to me. Comments inline.

On Wed, Aug 12, 2020 at 9:38 AM Peter Braun <pbr...@redhat.com> wrote:

Hey Sebastian,

we used the same strategy in our old internal Keycloak Operator. How would this  A few questions:

1) what is the default value if the field is not provided?

IMHO it should default to "true" (since users might naturally expect performing the changes via the webconsole

or "oc edit" command to work without additional settings need to be performed in advance).

I agree, "true" seems sensible.

 

 

2) what if users  are provided in a Realm CR and also in a User CR? Who wins?

Maybe merge / union the requests if intersection of the users definition in both CRs is disjunctive?

If the requests are conflicting ones, I would say the definition in User CR should win (since IMHO

this is what the operator user would expect to happen).

That's a very interesting aspect. Analyzing the CRDs and trying to catch this corner case in Operator's code will result in a pretty complex code. The situation gets even worse if we introduce the "managed" flag into the picture. What will happen if a Realm uses managed mode and you create a new Client with managed set to false?

I believe a better approach would be to remove Clients and Users from the Realm CRD. The downside is that it's breaking change and we will probably need to switch to v1alpha2 for all our CRDs (leaving v1alpha1 unsupported). It goes without saying that this change should happen on a major Keycloak release.

 

 

3) will other CRs also get this switch or will they always be managed?

To preserve the consistency other CRs should get this switch too IMHO.

I totally agree with Jan here.

 

 

I think a default value of 'managed: true' would make sense: otherwise users could run into the situation where they created a realm a while ago, made changes in the admin console and then they all get reset once the flag is switched.

+1 on defaulting to "true" setting (the behavior you described [sudden values reset upon flag switch] might be confusing to the users IMHO. Plus one might expect the "oc edit" command on the resource just work [without additional settings need to be performed apriori] if the field is allowed to be edited in CR definition).

I totally agree here.

[Sebastian Łaskawiec]

On Wednesday, 12 August 2020 at 11:47:22 UTC+2 Till Markus (IOC/PDL22) wrote:

I’m new to the discussion and don’t have a full picture what happened so far in the operator.

So don’t mind to ignore this comment J

 

IMHO CRD are mostly used to describe a desired state. If an “managed” flag is introduced. This would indicate to me that CRD is also not removed if the resource is removed. This brings the question to my mind why the CRD was there in the first place.

You're touching two aspects at the same time, so let me take them one by one.

Removing resources associated with a specific CR is something different. This process is handled by Kubernetes Garbage Collection mechanism: https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/

We simply create an "Owner Reference" pointing from all the objects to a CR. Once the CR is removed, Kubernetes terminates all "child" resources automatically. As you can see, the relation is from a CR to specific Kubernetes resources associated with it.

The other aspect is syncing the state. During early days of Keycloak Operator we decided to drop two-way sync. In other words, we always sync resources from a CR to Keycloak and never the other way around. Two-way sync may seem appealing but from the implementation perspective, it's a lot of headache - especially solving race conditions (something gets modified in the Keycloak Admin Console and CR roughly at the same time).

[Till Markus (IOC/PDL22)]

Is there documentation on this design? I would like to dive deeper before my next comment.

 

Anyways an also unqualified addition to my initial comment ;):

 

I was a bit unspecific.

 

I was referring to resources as resources In Keycloak. So if you drop a Client CR in k8s it will not drop the resource in Keycloak. That’s what I understood as “unmanaged”

Therefore if you create 100 clients in k8s it might turn out a lot of them aren’t in Keycloak at all or are changed over time. So the information value of this objects kept in k8s is unclear to me, It just indicates that someone at some point created a Client but not what the state of this client is, so the client fields makes no sense (as you said sync is hard and also a bit against the principles of k8s).

 

To represent a unmanaged Client I would rather expect a UnmanagedClient CR or the proposed Client CR (with unmanaged flag) which holds just the clientid or things which won’t change. In addition I would expect its deleted if the user deletes the client in the admin api.

 

Therefore I would split the CR which have a constant desired state as e.g. a Client which will be changed back if changed on the admin ui. (Keycloak should may support immutable clients for this use case).

And Clients which are created only once. This kind of Clients can disappear after the job is done. So I would design them like a Job in k8s.

 

In general I find it confusing to combine immutable configuration (k8s) and mutable configuration (Keycloak) in the same object.

To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/1e2b674d-ceee-4405-8501-17f39323d8a5n%40googlegroups.com.

[Sebastian Łaskawiec]

On Wed, 12 Aug 2020 at 14:32, Till Markus (IOC/PDL22) <Marku...@bosch.io> wrote:

Is there documentation on this design? I would like to dive deeper before my next comment.

 

Anyways an also unqualified addition to my initial comment ;):

 

I was a bit unspecific.

 

I was referring to resources as resources In Keycloak. So if you drop a Client CR in k8s it will not drop the resource in Keycloak. That’s what I understood as “unmanaged”

Removing things is very tricky. Imagine that you have an unmanaged Client CR pointing to a managed Realm CR (by using a RealmSelector). If someone removes the Realm CR, your client will also be gone. Even though it's unmanaged.

Therefore, I would prefer to leave removing things as they are. In other words - managed/unmanaged flag will apply only to updating resources. Creating/removing remains as it is now. What do you think?

 

Therefore if you create 100 clients in k8s it might turn out a lot of them aren’t in Keycloak at all or are changed over time. So the information value of this objects kept in k8s is unclear to me, It just indicates that someone at some point created a Client but not what the state of this client is, so the client fields makes no sense (as you said sync is hard and also a bit against the principles of k8s).

Exactly! It depends where you want your "source of truth" to be. At the moment, you have two options:

- Modify things using Keycloak Admin UI or Keycloak CLI. In this case your primary source of truth will be the database.

- Modify things using CRs. In this case your primary source of truth is a CR. The database might be recreated at any point of time.

This is a kind of discussion we had with Stian for a very long time. Has CRs become a database from a conceptual perspective already?

 

 

To represent a unmanaged Client I would rather expect a UnmanagedClient CR or the proposed Client CR (with unmanaged flag) which holds just the clientid or things which won’t change. In addition I would expect its deleted if the user deletes the client in the admin api.

 

Therefore I would split the CR which have a constant desired state as e.g. a Client which will be changed back if changed on the admin ui. (Keycloak should may support immutable clients for this use case).

And Clients which are created only once. This kind of Clients can disappear after the job is done. So I would design them like a Job in k8s.

 

In general I find it confusing to combine immutable configuration (k8s) and mutable configuration (Keycloak) in the same object.

Yes, I need to admit it - it is confusing.

A while ago, we Stian and I had an idea that certain resources might be "owned" by a specific management interface in Keycloak. If you create a Client using a CR, it can only be modified by the operator. If a Client has been created by Keycloak Admin Console, it can not be modified by the operator nor by the CLI etc.

This is a very early stage of this idea, but maybe at some point we'll turn it into a feature proposal.

--

Sebastian Łaskawiec

[Stian Thorgersen]

What I would like to see is a write-up on how the different CRs will be used, and have this all a bit more use-case/user-story driven.

To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/CADP5VTUBjeTR1aKjq1ZNgmLZkU2pPp-f0aQifUy41BitTKDAZg%40mail.gmail.com.

[Schuster Sebastian (IOC/PDL22)]

I also like this idea of owned “resources”. Could also be used to prevent people from messing with the internal clients (security-admin-console, account, etc…).

 

Best regards,

Sebastian

 

Mit freundlichen Grüßen / Best regards

Dr.-Ing. Sebastian Schuster

Project Delivery Berlin 22 (IOC/PDL22)

Bosch.IO GmbH | Ullsteinstr. 128 | 12109 Berlin | GERMANY | www.bosch.io
Tel. +49 30 726112-485 | Mobil +49 152 02177668 | Telefax +49 30 726112-100 | Sebastian...@bosch.io

To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/CADP5VTUBjeTR1aKjq1ZNgmLZkU2pPp-f0aQifUy41BitTKDAZg%40mail.gmail.com.