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

Update keycloak-config-cli using SPIRE example to Keycloak >= 24.0.0 #387

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

moritzschmitz-oviva
Copy link
Contributor

@moritzschmitz-oviva moritzschmitz-oviva commented Jun 20, 2024

Keycloak >= 24.0.0 deprecated the use of password-protected truststores. This makes the integration with SPIRE quite a bit easier. This PR should update the example to reflect the changes.

Signed-off-by: Moritz Schmitz von Hülst <moritz.schmitz@oviva.com>
Signed-off-by: Moritz Schmitz von Hülst <moritz.schmitz@oviva.com>
@kfox1111
Copy link
Collaborator

kfox1111 commented Jul 1, 2024

Thanks for the pr. :)

I like the idea of switching to pems from the java trust store.

I think there is one problem with the pr though.

The example should be able to refresh certificates when spire issues new ones. They expire fairly regularly, so that plumbing still needs to be in place. The spiffe-helper and java-spiffe-helper can do the refresh. I think the spire-agent can not. Could spire-agent be switched out with the spire-helper?

@moritzschmitz-oviva
Copy link
Contributor Author

moritzschmitz-oviva commented Jul 1, 2024

The example should be able to refresh certificates when spire issues new ones.

For this example Keycloak itself most likely is the problem. The certs get loaded at startup and are not reloaded during runtime.

@kfox1111
Copy link
Collaborator

kfox1111 commented Jul 1, 2024

Yeah, there definitely is an issue in Keycloak: keycloak/keycloak#26524

But for the example, we should probably give the recommendation as proper as possible, with rotation working, so when its fixed in Keycloak, those that copied the example don't have to go back and figure out how to fix it.

@faisal-memon
Copy link
Collaborator

@moritzschmitz-oviva Just wanted to check in if youre still working on this?

@faisal-memon
Copy link
Collaborator

@moritzschmitz-oviva Just checking in again if you are interested in finishing off this pr?

@moritzschmitz-oviva
Copy link
Contributor Author

@moritzschmitz-oviva Just checking in again if you are interested in finishing off this pr?

Hi @faisal-memon not at the moment, no. Will probably have a look later if nobody else is interested.

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

Successfully merging this pull request may close these issues.

3 participants