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.

Sign up for GitHub

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

Jump to bottom

Add support of mTLS in Spring Cloud Config Client #21513

Merged
merged 1 commit into from
Nov 22, 2021
Merged

Add support of mTLS in Spring Cloud Config Client #21513

merged 1 commit into from
Nov 22, 2021

Conversation

ishumov
Copy link

@ishumov ishumov commented Nov 17, 2021 •
edited by geoand
Loading

Fixes #21512

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 17, 2021

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not contain an issue number (use Fix #1234 in the description instead)

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added the area/spring Issues relating to the Spring integration label Nov 17, 2021
@ishumov ishumov changed the title Add support of mTLS in Spring Cloud Config Client #21512 Add support of mTLS in Spring Cloud Config Client Nov 17, 2021
gsmet
gsmet reviewed Nov 17, 2021
View reviewed changes
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.

Thanks for this. I added a small question inline.

...c/main/java/io/quarkus/spring/cloud/config/client/runtime/VertxSpringCloudConfigGateway.java
} else {
webClientOptions.setTrustStoreOptions((JksOptions) storeOptions);
webClientOptions.setKeyStoreOptions((JksOptions) storeOptions);
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain the change of method? Does it still work in the initial case covered?

Copy link
Author

@ishumov ishumov Nov 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean SSL session without checking client cert on a server? Usual way in Java is using truststore for this purpose (that code is unchanged). In that case keystore not needed. From my point of view using keystore as a store for CA certificate is a mistake.

For this change I was guided by https://vertx.io/docs/vertx-core/java/#ssl

Copy link
Member

Choose a reason for hiding this comment

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

@michalszynkiewicz you probably have more experience than me on the Vert.x web client, could you have a look?

Copy link
Member

Choose a reason for hiding this comment

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

This looks good to me.
Setting trust store options from keystore didn't look good.

@geoand geoand merged commit 4000361 into quarkusio:main Nov 22, 2021
@quarkus-bot quarkus-bot bot added this to the 2.6 - main milestone Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet left review comments

@geoand geoand geoand approved these changes

@michalszynkiewicz michalszynkiewicz michalszynkiewicz approved these changes

Assignees
No one assigned
Labels
area/spring Issues relating to the Spring integration
Projects
None yet
Milestone
2.6.0.CR1
Development

Successfully merging this pull request may close these issues.

Add support of mTLS in Spring Cloud Config Client
4 participants
@ishumov @gsmet @michalszynkiewicz @geoand