Skip to content

fix: ensuring truststore initialization happens before factory init - #51405

Merged
shawkins merged 2 commits into
keycloak:mainfrom
shawkins:iss51396
Aug 6, 2026
Merged

fix: ensuring truststore initialization happens before factory init#51405
shawkins merged 2 commits into
keycloak:mainfrom
shawkins:iss51396

Conversation

@shawkins

@shawkins shawkins commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

closes: #51396

Copilot AI balanced review requested due to automatic review settings August 3, 2026 15:24
@shawkins
shawkins requested review from a team as code owners August 3, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Ensures truststore setup completes before provider factories initialize.

Changes:

  • Combines crypto-provider and truststore initialization.
  • Makes CryptoProviderInitBuildItem enforce both operations’ completion.

Copilot AI review requested due to automatic review settings August 3, 2026 16:02
closes: keycloak#51396

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.

@Pepo48 Pepo48 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The core fix looks correct, one issue with the test assertion though - String.matches() returns a boolean that's never asserted on, so the test passes regardless (even with nonsensical string).

.get("https://mykeycloak.org:8443").then().body(Matchers.containsString("https://mykeycloak.org"));

// ensure that the provider factories init with the correct truststore
result.getOutput().matches("File truststore provider initialized: [^\\n]*keycloak-truststore.p12");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
result.getOutput().matches("File truststore provider initialized: [^\\n]*keycloak-truststore.p12");
result.assertStringCount("File truststore provider initialized: [^\\n]*keycloak-truststore\\.p12", 1);

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Copilot AI review requested due to automatic review settings August 4, 2026 13:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 4 changed files in this pull request and generated no new comments.

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserCredentialVaultAuthenticationSimpleEncryptionStartTLS

Keycloak CI - Java Distribution IT (windows-latest - temurin - 17)

org.openqa.selenium.TimeoutException: 
java.net.SocketTimeoutException: Read timed out
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Windows Server 2025', os.arch: 'amd64', os.version: '10.0', java.version: '17.0.19'
Driver info: driver.version: HtmlUnitDriver
...

Report flaky test

@Pepo48 Pepo48 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM now, thanks!

@shawkins
shawkins merged commit 1712fae into keycloak:main Aug 6, 2026
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants