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

Allow mongo client to be configured by a tls registry #46293

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

Malandril
Copy link
Contributor

@Malandril Malandril commented Feb 15, 2025

This PR allows the mongodb client to be configured using a tls registry configuration. This allows simpler ways to configure the trustores, and mtls connections.

By default the mongodb client dost not use the default tls registry.

@Malandril Malandril changed the title Add tls regsitry to mongo client Allow mongo client to be configured by a tls registry Feb 15, 2025

This comment has been minimized.

@Malandril Malandril force-pushed the mongo-tls-registry branch 3 times, most recently from 80e1c7e to 930883d Compare February 15, 2025 20:59

This comment has been minimized.

Copy link

github-actions bot commented Feb 15, 2025

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

import io.smallrye.certs.junit5.Certificate;
import io.smallrye.certs.junit5.Certificates;

@DisabledOnOs(value = OS.WINDOWS, disabledReason = "Tests don't pass on windows CI")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests on windows CI don't seem to work correctly, I cannot reproduce it, on my local windows.
It may be because the generated certs use relative paths.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the report. It might be something related to the Junit 5 plugin generating certificates.

This comment has been minimized.

This comment has been minimized.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

This is really nice, thanks!

I added a few minor suggestions and questions. And it's probably a good idea to let @cescoffier have a look.

docs/src/main/asciidoc/mongodb.adoc Outdated Show resolved Hide resolved

If you want to use SSL/TLS encryption, you need to add these properties in your `application.properties`:
You can configure the client to trust servers with certificates with invalid hostnames:

[source,properties]
----
quarkus.mongodb.tls=true
quarkus.mongodb.tls-insecure=true # only if TLS certificate cannot be validated
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed when you are using a TLS configuration from the registry? IIRC, it's configured there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This option configures the mongo builder with the method invalidHostNameAllowed.
The tls registry config is applied after, if the quarkus.mongodb.tls is true, the registry can replace this because it can be configured with quarkus.tls.hostname-verification-algorithm=NONE.

docs/src/main/asciidoc/mongodb.adoc Outdated Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

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.

Thanks! Looks good! I'm just wondering about the dedicated tls config of Mongo.

docs/src/main/asciidoc/mongodb.adoc Outdated Show resolved Hide resolved
import io.smallrye.certs.junit5.Certificate;
import io.smallrye.certs.junit5.Certificates;

@DisabledOnOs(value = OS.WINDOWS, disabledReason = "Tests don't pass on windows CI")
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the report. It might be something related to the Junit 5 plugin generating certificates.

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Feb 18, 2025

When all is done, we'll need to have the commits squashed

This comment has been minimized.

@Malandril Malandril force-pushed the mongo-tls-registry branch 2 times, most recently from 840f7c6 to a08bd7e Compare February 18, 2025 13:35
@geoand geoand requested a review from cescoffier February 18, 2025 13:38

This comment has been minimized.

This comment has been minimized.

test: Disable tls test on windows

Apply suggestions from code review

Co-authored-by: Guillaume Smet <[email protected]>

Deprecate tlsInsecure mongo configuration

Review comments, enable tls  when configuration name set
Copy link

quarkus-bot bot commented Feb 18, 2025

Status for workflow Quarkus Documentation CI

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

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

This comment has been minimized.

@geoand geoand added release/noteworthy-feature triage/waiting-for-ci Ready to merge when CI successfully finishes labels Feb 19, 2025
Copy link

quarkus-bot bot commented Feb 19, 2025

Status for workflow Quarkus CI

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

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

@geoand geoand merged commit 863abd5 into quarkusio:main Feb 19, 2025
59 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 19, 2025
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 19, 2025
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