-
Notifications
You must be signed in to change notification settings - Fork 123
KEYCLOAK-10319 Long-term Keycloak Operator architecture #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
27ffd58 to
4c8251d
Compare
davidffrench
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent overview @slaskawi . Some very small feedback/edit suggestions from me.
stianst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really cover what I had in mind with regards to a vision for the operator. It's missing some details on things like:
- Personas and use-cases - describe related personas and use-cases for the personas
- More details on being opinionated and providing a high-level usability, rather than low-level toggles
- How we will avoid feature creep - I can easily see lots of request for small things to be added, which in the end results in a complicated beast rather than a simple to use opinionated
- Avoiding logic in the operator - a bit more expanding on the section on where features should be implemented to describe that advanced logic doesn't belong in the operator
- Section capability levels and how we plan to properly address them
|
Why .asciidoc when all other designs are in markdown? Would be good to just keep it all in one format |
I'd rather keep things consistent here. If you want to propose changing the format that's fine, but otherwise stick to what is already done. Consistency matters. |
@stianst Added usecases
@stianst added "a way to deliver a high level usability rather than low-level toggles" to the manifesto.
@stianst To me, this sounds exactly the (although in different words) than the previous point.
@stianst Added "the thinnest possible layer, with the least possible logic, on top of Keycloak Container and Keycloak Server." to the manifesto.
@stianst I can add you a dedicated section, but let me push the use cases first. Maybe that will be enough for you. |
4c8251d to
74315bd
Compare
|
@davidffrench @stianst @mposolda Thank you gentlemen for the review. I addressed your comments and pushed a new version. |
|
@slaskawi Does make sense to list the CRDs used by the Operator? I found one quick reference to the Considering they play an important role, maybe a specific section about CRDs could make this part of the architecture more clear. Especially if provided:
|
@pedroigor This is coming in updated documentation, see keycloak/keycloak-documentation#796
@pedroigor I don't want to be too specific about those CRs in this architecture vision as I see them as an implementation detail. However, this is the second time I see someone asking for a list of CRs, so let me add it.
@pedroigor I see those are Keycloak limitations (not necessarily the Operator). Perhaps we should have a Keycloak long term vision somewhere and put this there? What do you think, @stianst ?
@pedroigor I believe this has been covered in Operator SDK manual, so repeating it here doesn't make much sense (to me).
@pedroigor That's a good point. Let me mention here the way it has been implemented. |
74315bd to
a8631dc
Compare
|
@pedroigor Pushed new version. |
|
This looks good to me @slaskawi |
|
|
||
| We envision Keycloak Operator as: | ||
|
|
||
| * an opinionated way of installing and maintaining Keycloak installation on both Kubernetes and OpenShift. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should upgrades be called out or is that covered under maintaining?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm I see it is covered
stianst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline. Perhaps it would also be good to cover a section on CRs vs admin console. One/two-way sync, etc.
|
|
||
| * **Status**: Draft #1 | ||
| * **JIRA**: https://issues.redhat.com/browse/KEYCLOAK-10319[KEYCLOAK-10319] | ||
| * **Source of the images**: https://docs.google.com/presentation/d/13N5ClXXXcxKjXgor72rUdtMBN07_GneUkhTTwBsy_tE/edit?usp=sharing[Google Slides] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this link as it is not public
|
|
||
| * an opinionated way of installing and maintaining Keycloak installation on both Kubernetes and OpenShift. | ||
| * a way to deliver a high level usability rather than low-level toggles | ||
| * an example of a recommended way of installing, configuring, managing custom extension and themes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this line.
| * Cluster Admin - a Kubernetes/OpenShift administrator | ||
| * Service Infrastructure Admin - an administrator who provisions Keycloak Operator as well as Keycloak Cluster | ||
| * Realm Admin - a Keycloak administrator, who can provision realms (both using Custom Resources and Admin UI) | ||
| * Secured Application - either configured by an engineering team (using self registration feature) or by Keycloak Admin (or Keycloak Realm Admin). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not fully convinced about these personas. Looking at the doc I sent you there are:
- Cluster Admin
- Service Infrastructure Admin
- Service Admin
- Service Consumer
- Service End User
I think those are better personas then those you've created. From my perspective the description in context of the Keycloak Operator would be something like:
- Cluster Admin - Provisions cluster resources. Basically, installs the Keycloak Operator.
- Service Infrastructure Admin - Provisions the managed services. Basically, creates the Keycloak CR.
- Service Admin - Manages the Realm. Basically, create the KeycloakRealm CR, but can also manage it through the Keycloak admin console
- Service Consumer - Application developers. Basically, creates the KeycloakClient CR, but can also create clients through other means (admin console, client registration cli, etc.)
- Service End User - Application users. Not sure there are any use-cases for these in this context, but worth mentioning anyways
| * Realm Admin - a Keycloak administrator, who can provision realms (both using Custom Resources and Admin UI) | ||
| * Secured Application - either configured by an engineering team (using self registration feature) or by Keycloak Admin (or Keycloak Realm Admin). | ||
|
|
||
| The use cases we defined are based on Operatr Capability Model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operator, not Operatr
| |Cluster Admin |Full Lifecycle| Configure Storage Class for Local Backups| Yes | ||
|
|
||
| |Service Infrastructure Admin |Basic Install | Install Keycloak Operator | Yes | ||
| |Service Infrastructure Admin |Basic Install | Install and manage Keycloak installation | Yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install and manage Keycloak installation seems a bit broad. What does it cover?
| |Secured Application |Autopilot | Provide stable response time despite the load| No | ||
| |==== | ||
|
|
||
| ## History |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just remove the History section - this is irrelevant information.
|
|
||
| 1. Keycloak Server needs to provide a JSON log based formatter. | ||
| 2. Keycloak Container needs to expose an environment variable, that could be switching on to use JSON formatting | ||
| 3. Keycloak Operator needs to expose proper configuration on Custom Resource level enabling JSON log formatting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the Operator actually be opinionated here and just use JSON logging since that is the recommended on OpenShift/Kubernetes to have a common/parsable logging format?
|
|
||
| image::img/Keycloak-operator-components.png[Keycloak Operator components] | ||
|
|
||
| When Keycloak Operator spins up Keycloak installation, it creates: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should make it a bit more clear in the list what types the things below are. For example keycloak-postgres is listed twice, once it doesn't say what type it is, then in the second it says in the description it's a service. Then for keycloak-discovery it more clearly says it's a Service. I would use the format: name type - description for everything
|
|
||
| When Keycloak Operator spins up Keycloak installation, it creates: | ||
|
|
||
| * `keycloak-db-secret` - used to store database username, password and other properties, such as external address (if used). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do find it a bit weird that the external address for the DB is stored within a secret ;)
| When Keycloak Operator spins up Keycloak installation, it creates: | ||
|
|
||
| * `keycloak-db-secret` - used to store database username, password and other properties, such as external address (if used). | ||
| * `credentials-<<CR Name>>` - Admin user credentials to log into Keycloak installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on this one? I don't understand what this is used for.
|
Please change format to markdown to make this consistent with everything else. |
|
@slaskawi hey there, I'm working on a project where I'm considering using the keycloak operator as a component. In my use case, I think it would be useful to be able to add and update Identity/User Storage Providers via CRDs. Just wondering, would something like that be out of the scope of this project? I mainly just want a way to easily add additional authentication pieces, each bundled with its relevant keycloak configuration, to a big sso system in k8s. And extending this operator to handle those type of CRDs is one idea I had, and doesn't seem terribly hard for me to do, but that could be overkill. Thanks and great job! |
| * an opinionated way of installing and maintaining Keycloak installation on both Kubernetes and OpenShift. | ||
| * a way to deliver a high level usability rather than low-level toggles | ||
| * an example of a recommended way of installing, configuring, managing custom extension and themes. | ||
| * the thinnest possible layer, with the least possible logic, on top of Keycloak Container and Keycloak Server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand what you mean, but this could also be interpreted as just dumping the exported config into map[string]string.
Do you envision being able to set up and configure keycloak just by reading the documentation/CRD and creating a CRD, instead doing the setup in the UI and exporting the configuration (including the "defaults" that are included in the export)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our CRDs reflect Keycloak resources. A good example is RealmRepresentation JSON payload and KeycloakRealm CRD. They are very similar and, in the long-term, should be the same.
So it is perfectly doable to configure Keycloak purely through CRDs (without touching the UI).
This Pull Request contains a High Level Architecture overview of Keycloak Operator and its long-term vision.
Better preview is available here: https://github.com/slaskawi/keycloak-community/blob/Operator-high-level-architecture/design/operator-architecture.asciidoc