-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Upgrade to Quarkus 3.2.2.Final #21912
Conversation
e332dde
to
fbfe788
Compare
@@ -56,7 +56,7 @@ public enum ClientAuth { | |||
public static final Option HTTPS_PROTOCOLS = new OptionBuilder<>("https-protocols", String.class) | |||
.category(OptionCategory.HTTP) | |||
.description("The list of protocols to explicitly enable.") | |||
.defaultValue("TLSv1.3") | |||
.defaultValue("TLSv1.3,TLSv1.2") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to mitigate quarkusio/quarkus#34468 and basically replicate the previous behaviour where TLSv1.2 worked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, as the current NIST SP 800-52 Rev. 2 guideline recommends to enable TLSv1.3 alongside TLSv1.2, so clients can choose the "better" protocol on their capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vmuzikar LGTM, thanks!
@@ -123,7 +123,7 @@ void testUnknownQuarkusBuildTimePropertyApplied(LaunchResult result) { | |||
} | |||
|
|||
@Test | |||
@Launch({ "start", "--http-enabled=true", "--hostname-strict=false", "--config-keystore=keystore" }) | |||
@Launch({ "start", "--http-enabled=true", "--hostname-strict=false", "--config-keystore=../../../../src/test/resources/keystore" }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure, this change was necessary because the invalid keystore path exception would take a precedence?
I haven't gone through the recent smallrye-keystore changes, but this might indicate that there are some improvements when it comes to exception handling, so our custom property validation might be redundant to some extent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure, this change was necessary because the invalid keystore path exception would take a precedence?
Yes but it still triggered our custom validation, not anything in SmallRye.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok then, thanks for clarification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @vmuzikar.
Closes keycloak#21907 (cherry picked from commit ecdf8e8)
Closes #21907