Skip to content
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

Support --optimized (or at least don't prevent it) #166

Open
famod opened this issue Dec 5, 2024 · 2 comments · May be fixed by #170
Open

Support --optimized (or at least don't prevent it) #166

famod opened this issue Dec 5, 2024 · 2 comments · May be fixed by #170
Labels
enhancement New feature or request

Comments

@famod
Copy link

famod commented Dec 5, 2024

Description

ExtendableKeycloakContainer sets a few buid-time properties unconditionally, preventing me from using withProductionMode() plus --optimized (via withCustomCommand()).
Keycloak won't start due to:

The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.https-client-auth, kc.https-management-client-auth, kc.metrics-enabled

I really don't want to rebuild here, I want use my "pre-built" custom docker image (which has all the build-time properties in place).

Motivation

See description.

Details

I'm not sure what's the best way to support --optimized.
There could be a withOptimized() or so. If that's set, any attempt to set buid-time env vars would have to be prevented.

Apart from that I'm not sure why ExtendableKeycloakContainer is setting those properties unconditionally in the first place.
I mean KC_HTTPS_CLIENT_AUTH and KC_HTTPS_MANAGEMENT_CLIENT_AUTH are both none by default anyway.
KC_METRICS_ENABLED shouldn't be set like that either (IMHO). Instead of boolean it could be Boolean so that in case of null the env var wouldn't be set.

KC_HEALTH_ENABLED is a bit more tricky because its needed for the default wait strategy. I think its ok to keep setting it if no custom wait strategy is set. It might then make sense to add withHealthCheck(boolean) (and validate agains the wait strategy accordingly).
As far as I'm concerned personally, I wouldn't mind if KC_HEALTH_ENABLED was kept unconditionally because I'm enabling it in Dockerfile anyway.

@famod famod added the enhancement New feature or request label Dec 5, 2024
@famod
Copy link
Author

famod commented Dec 5, 2024

PS: I've just realized that there is only withEnabledMetrics() and not withMetrics(boolean) so it might make more sense to only set KC_METRICS_ENABLED if the field is true. 🤷‍♂️

@robson90 robson90 linked a pull request Jan 23, 2025 that will close this issue
@robson90
Copy link
Contributor

@famod you mean something like this: #170 ?

Testcase, that reflects ur request: https://github.com/dasniko/testcontainers-keycloak/pull/170/files#diff-63f294533bc217a97b6f3bab89abf6a031ed46db2506832cc7766049efc1bdbfR232

You can then just reference the correct image name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants