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

Support CDI providers of key/trust stores in TLS registry #45937

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

emattheis
Copy link
Contributor

@emattheis emattheis commented Jan 29, 2025

Per discussion #41024, this PR provides a mechanism for application code to supply keystores or truststores to the TLS registry.

Copy link

quarkus-bot bot commented Jan 29, 2025

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • description should not be empty, describe your intent or provide links to the issues this PR is fixing (using Fixes #NNNNN) or changelogs

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jan 29, 2025
Copy link
Member

@cescoffier cescoffier left a 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

@cescoffier
Copy link
Member

I forgot to mention that this deserves a small section in the TLS registry documentation.

This comment has been minimized.

Copy link

github-actions bot commented Jan 29, 2025

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

@emattheis emattheis force-pushed the tls-config-providers branch 2 times, most recently from 176ca4f to 0ab88e3 Compare January 30, 2025 23:57
Copy link
Member

@cescoffier cescoffier left a 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)

@emattheis
Copy link
Contributor Author

it's loaded only once, at startup time

Also on reloads, but certainly not dynamic from the SSLContext point of view.

I'll mention the reload behavior in the documentation updates.

@cescoffier
Copy link
Member

@emattheis Can you squash your commits?

@emattheis
Copy link
Contributor Author

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 InstanceHandle vs InjectableInstance. I probably just need to RTFM 😉

This comment has been minimized.

This comment has been minimized.

@emattheis emattheis force-pushed the tls-config-providers branch 2 times, most recently from 7637996 to af5eceb Compare January 31, 2025 19:58
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Jan 31, 2025
@emattheis
Copy link
Contributor Author

@cescoffier I updated the docs and squashed the commits.

This comment has been minimized.

This comment has been minimized.

@emattheis emattheis force-pushed the tls-config-providers branch from af5eceb to 27997d3 Compare February 6, 2025 15:24
Copy link

quarkus-bot bot commented Feb 6, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 27997d3.

✅ 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.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Feb 6, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 27997d3.

✅ 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.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/virtual-threads/mailer-virtual-threads

io.quarkus.virtual.mail.RunOnVirtualThreadTest.test - History

  • Read timed out - java.net.SocketTimeoutException
java.net.SocketTimeoutException: Read timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:278)
	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:304)
	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346)
	at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796)
	at java.base/java.net.Socket$SocketInputStream.read(Socket.java:1099)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:161)
	at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)

@cescoffier cescoffier merged commit 3b3e7f0 into quarkusio:main Feb 7, 2025
55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Feb 7, 2025
@cescoffier
Copy link
Member

Thanks! Sorry for the delay for the merge; the last few days have been a bit chaotic.

@emattheis emattheis deleted the tls-config-providers branch February 7, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/docstyle issues related for manual docstyle review area/documentation triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants