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

Generate certificates in the OIDC integration test #45095

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Dec 12, 2024

It proved a bit harder than I thought, interestingly these generated certificates trigger the hostname verification, the older ones don't, but in any case, it is goodbye to the old certificates. I know of one more test with the pregenerated cert, in the oidc-client/deployments, will deal with it a bit later.

It won't backport to 3.15 though, but the old certs are valid till 2029 :-)

@sberyozkin
Copy link
Member Author

Just fixed some formatting issues

This comment has been minimized.

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I am slow Sergey, but you say that old certs are gone (except for one test in the oidc-client/deployments) and that you generate certificates now, but I still can see them here https://github.com/quarkusio/quarkus/tree/ffcd797ca9127b9a02eca170481ffa2988b831a4/integration-tests/oidc/src/main/resources. Maybe they should be removed?

@sberyozkin
Copy link
Member Author

@michalvavrik Yeah, forgot to drop them as resources, will do, thanks

@sberyozkin sberyozkin force-pushed the oidc_test_generate_certs branch from ffcd797 to 80d1a2d Compare December 13, 2024 12:07
Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@@ -1,23 +1,24 @@
quarkus.keycloak.devservices.create-realm=false
quarkus.keycloak.devservices.start-command=start --https-client-auth=required --hostname-strict=false --https-key-store-file=/etc/server-keystore.p12 --https-trust-store-file=/etc/server-truststore.p12 --https-trust-store-password=password --spi-user-profile-declarative-user-profile-config-file=/opt/keycloak/upconfig.json
quarkus.keycloak.devservices.resource-aliases.keystore=server-keystore.p12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as you are removing upconfig.json, I am not certain if this spi-user-profile-declarative-user-profile-config-file still exists and needs to be set judging by this https://www.keycloak.org/server/all-provider-config#_user_profile, but https://github.com/keycloak/keycloak/blob/main/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java is complex for me to determine. we can keep it if you know it does something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalvavrik This resource is shipped with the devservice, I used to keep it here because the devservice was not used in this test earlier. This upconfig.json is about avoiding Keycloak insisting on configuring additional properties for users like email, etc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, thanks for info. strange it is not listed but I probably looked at the wrong page.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalvavrik That is because it is undocumented, it is a non-official property :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalvavrik You may recall you were dealing with QE tests started suddenly failing with the upgrade to the next KC version and this is the resource which helps dealing with those issues :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha

Copy link

quarkus-bot bot commented Dec 13, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 80d1a2d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@sberyozkin sberyozkin merged commit def13dc into quarkusio:main Dec 13, 2024
23 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 13, 2024
@sberyozkin sberyozkin deleted the oidc_test_generate_certs branch December 13, 2024 12:53
@gsmet gsmet modified the milestones: 3.18 - main, 3.17.5 Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants