Separate configuration of underlying Quarkus from main configuration #9592

stianst · 2022-01-17T08:24:17Z

stianst
Jan 17, 2022
Maintainer

As a general rule Keycloak should be configurable as Keycloak, not as something that is powered by Quarkus. This includes defining the configuration attributes at a Keycloak level, and clearly documenting them (in CLI as well a documentation).

There may be times when there is a need to apply Quarkus configuration though. This could be configuration that haven't made available in Keycloak yet (for example Hashicorp Vault), or it may be special cases that are not generic enough to officially support. For the latter it is important to remember that Keycloak should be opinionated and smart, requiring as little configuration as possible.

At the moment it is permitted to use a mix of Keycloak and Quarkus configuration within keycloak.properties. For example:

db.url.host=mydatabase
quarkus.http.idle-timeout=10M

The proposal here is to split the configuration where keycloak.properties can only be used for official Keycloak configuration, with a separate quarkus.properties file to apply unsupported Quarkus configuration. An example quarkus.properties file will be included in the distribution with a header warning that usage of this file is unsupported, and is used at your own risk. Further, changing quarkus.properties will only be supported at build-time, as usage of Quarkus properties is considered as customising Keycloak, rather than as configuring Keycloak.

With two configuration files a clear separation is provided, and it is more obvious that one contains official configuration options, while the other is customising and is not supported at the same level. This should make it more obvious that whenever quarkus.properties is used a request to add additional configuration to Keycloak should be considered. Further, this is also aligned with the approach taken to configure the embedded Infinispan server, where there is Keycloak level-configuration on the caches using the standard Keycloak configuration mechanisms, with a separate cache.xml configuration file following the Infinispan configuration format.

stianst · 2022-01-17T08:27:23Z

stianst
Jan 17, 2022
Maintainer Author

@DGuhr @pedroigor FIY

0 replies

stianst · 2022-01-17T09:21:21Z

stianst
Jan 17, 2022
Maintainer Author

This is related to the An alternative configuration file format for the Quarkus distribution proposal.

0 replies

andreaTP · 2022-01-18T09:56:08Z

andreaTP
Jan 18, 2022

I understand the willingness of separating the core Keycloak configurations from what is provided by the framework (and possibly unsupported).
Although in this specific case the settings are, often, intertwined and having two files can lead to major confusions.

e.g. @stianst how do you see the system working if db.url.host is defined in keycloak.properties and quarkus.datasource.jdbc.url is defined in quarkus.properties?

2 replies

pedroigor Jan 20, 2022
Collaborator

@andreaTP Our configuration sources have higher precedence over Quarkus specific ones. In this case, the value from our properties file will be taken.

In general, you won't use the quarkus.properties file as we should be providing everything you need using our own options. In case we are missing something, we should discuss and try to incorporate it into our configuration.

Although I see your point, we are going to have this "advanced usage" properly documented. As well as what people should do in case we are missing some configuration.

IMO, the confusion you are talking about is manageable and should be properly addressed.

DGuhr Jan 20, 2022

+1 to what pedro states. This is basically the one way to use quarkus internals directly, if you really really want to. it should have a comment along the lines of

Here be dragons. Yep, you heard right! Use at your own risk and be aware that the actual Keycloak config overrides the internal quarkus properties and this file may be deleted in the future. We're happy if you open a github discussion instead to let us know where keycloaks capabilities fall short for your use-case, so we could incorporate your feedback into one of the next versions.

sschu · 2022-01-21T15:23:15Z

sschu
Jan 21, 2022
Collaborator

Is there any specific reason why quarkus.properties is not picked up as configuration? I think the separation is clear with the different files and the warning text. I assume changing build options has a higher price then changing configuration options so why pay that price here?

11 replies

sschu Jan 24, 2022
Collaborator

@DGuhr Thanks for the explanation. Do I understand this right that according to this proposal Keycloak will transform Quarkus runtime configuration options into built time options that will only be picked up in a reaugmentation step? This would essentially force us to always run the build as part of container startup (which is not suggested, as you say) because:
a) We run > 60 instances and having a custom optimized image for every instance is not feasible.
b) I can foresee production problems on certain instances that we would need to fix by adapting options that are only available as quarkus properties (e.g. the example with the http timeout). These should be configurable per Keycloak instance we run and picked up by a rolling restart as fast as possible.
c) Looking at https://quarkus.io/guides/all-config I don't see Keycloak picking up all these properties (that's surely not the goal) so Keycloak will always be less flexible configuration-wise. Still, some of these switches might be useful in very rare scenarios. Forcing to have a slow container startup to keep this flexibility is a pretty high price to pay. If I get it right, the only benefit is people are more motivated to contribute turning Quarkus options into Keycloak options?

DGuhr Jan 24, 2022

to a) I don't quite get the point. You would not build >60 optimized images, you would rather create the image once or maybe twice, depending on the build options. You would do this e.g. in your CI/CD pipeline, push it to a container registry and use it as your base image, and apply the run configuration for the specific instances. Or do you have >60 differing base images with distinct build options? Would love to know the use case around this. :)
to b) Yes, that's a fair and valid point. Another approach might be, that in "troubleshooting" cases, you might want to deploy using auto-build until you found the root cause and then create a new optimized image for all the instances?
to c) No, it's not the goal. Yes, Keycloak will always be less flexible, and hopefully more convenient, as it's not a framework (yet?), it's a product. To be honest here, I am not hard about letting people who want to use quarkus "as is" use it really as is, without the "what was run config in quarkus must be a build option in keycloak", but I totally understand and support the wish to get more feedback about missing configuration which should be made available in a (perhaps even more) convenient way instead of just using quarkus as-is. @stianst @pedroigor wdyt?

sschu Jan 24, 2022
Collaborator

a) It is as you say, since 60 images is not feasible, we would use one or two images maybe. I did a quick scan over the quarkus runtime options and I do think most of these options would probably make it to several/all instances and not be configured on an individual level. With the exception of
b) troubleshooting where I really see this. I was already thinking about having a mechanism in the image that tries to detect whether any quarkus options were used and to trigger a build in that case. If auto-build already does that based on a pre-optimized image...great.
c) I don't really see people come up with a meaningful abstraction for Keycloak config based on Quarkus config (e.g. one switch in Keycloak abstracts a few switches in Quarkus in a meaningful way, to give somewhat more flexibility in an opinionated way). With a given problem, people will just want the flexibility and all they do is add a Quarkus options to Keycloak 1:1 without taking the extra effort of designing something around this. If that happens, you can just keep the Quarkus runtime options as Keycloak runtime options without the extra effort to maintain it. ;)

pedroigor Jan 25, 2022
Collaborator

@DGuhr As discussed before, we should favor forcing build. For several reasons, some of them herein mentioned and also here #9682.

@sschu You are right, if you run auto-build you should sacrifice startup time a bit in favor of UX. Thanks for highlighting this, I'm updating the PR to enable this usage.

I'm also wondering if you could open an issue with the properties you are using and not available from our configuration. Depending on how it goes we can plan to include them as fully supported.

sschu Jan 26, 2022
Collaborator

Hi @pedroigor!
I don't have a list of properties, yet. Looking at https://quarkus.io/guides/all-config, I would say everything around DB configuration (esp. connection pooling) and log configuration should be as flexible as possible...

Separate configuration of underlying Quarkus from main configuration #9592

Uh oh!

Uh oh!

stianst Jan 17, 2022 Maintainer

Replies: 4 comments · 13 replies

Uh oh!

stianst Jan 17, 2022 Maintainer Author

Uh oh!

stianst Jan 17, 2022 Maintainer Author

Uh oh!

andreaTP Jan 18, 2022

Uh oh!

pedroigor Jan 20, 2022 Collaborator

Uh oh!

Uh oh!

DGuhr Jan 20, 2022

Uh oh!

sschu Jan 21, 2022 Collaborator

Uh oh!

sschu Jan 24, 2022 Collaborator

Uh oh!

Uh oh!

DGuhr Jan 24, 2022

Uh oh!

sschu Jan 24, 2022 Collaborator

Uh oh!

pedroigor Jan 25, 2022 Collaborator

Uh oh!

sschu Jan 26, 2022 Collaborator

stianst
Jan 17, 2022
Maintainer

Replies: 4 comments 13 replies

stianst
Jan 17, 2022
Maintainer Author

stianst
Jan 17, 2022
Maintainer Author

andreaTP
Jan 18, 2022

pedroigor Jan 20, 2022
Collaborator

sschu
Jan 21, 2022
Collaborator

sschu Jan 24, 2022
Collaborator

sschu Jan 24, 2022
Collaborator

pedroigor Jan 25, 2022
Collaborator

sschu Jan 26, 2022
Collaborator