-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 CDI providers of key/trust stores in TLS registry #45937
Conversation
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
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.
Very nice!
I made a few comment and I added a question for @mkouba
extensions/tls-registry/deployment/src/main/java/io/quarkus/tls/CertificatesProcessor.java
Outdated
Show resolved
Hide resolved
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/CertificateRecorder.java
Outdated
Show resolved
Hide resolved
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/KeyStoreProvider.java
Show resolved
Hide resolved
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/TrustStoreProvider.java
Show resolved
Hide resolved
...nsions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/config/TlsBucketConfig.java
Outdated
Show resolved
Hide resolved
I forgot to mention that this deserves a small section in the TLS registry documentation. |
This comment has been minimized.
This comment has been minimized.
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/CertificateRecorder.java
Outdated
Show resolved
Hide resolved
176ca4f
to
0ab88e3
Compare
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!
I made 2 small suggestions as it's not really dynamic (it's loaded only once, at startup time)
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/KeyStoreProvider.java
Outdated
Show resolved
Hide resolved
extensions/tls-registry/runtime/src/main/java/io/quarkus/tls/runtime/TrustStoreProvider.java
Outdated
Show resolved
Hide resolved
Also on reloads, but certainly not dynamic from the SSLContext point of view. I'll mention the reload behavior in the documentation updates. |
@emattheis Can you squash your commits? |
Yes. I want to address @michalvavrik's point about destroying dependent beans, and then I'll squash everything. I'm still a bit confused about |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
7637996
to
af5eceb
Compare
@cescoffier I updated the docs and squashed the commits. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
af5eceb
to
27997d3
Compare
Status for workflow
|
Status for workflow
|
Thanks! Sorry for the delay for the merge; the last few days have been a bit chaotic. |
Per discussion #41024, this PR provides a mechanism for application code to supply keystores or truststores to the TLS registry.