-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support to customize httpClientBuilder for s3 client #50
Conversation
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/test/java/com/bakdata/kafka/AmazonS3LargeMessageClientRoundtripTest.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
437a6cf
to
47b61e7
Compare
Hi @philipp94831 I update the PR according to your comments. Can you take a look when you have time. Thanks! |
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
large-message-core/src/test/java/com/bakdata/kafka/AmazonS3LargeMessageClientRoundtripTest.java
Outdated
Show resolved
Hide resolved
large-message-core/src/main/java/com/bakdata/kafka/AbstractLargeMessageConfig.java
Outdated
Show resolved
Hide resolved
…onfiguredSdkHttpClientBuilder
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 should do the trick and then we are good to merge :-)
Remove_NoSdkHttpClientBuilder.patch
Thank you very much for your contribution and patience. I create a release later today |
Also thanks for your review, that is really helpful! |
2.10.0 should be available |
Pull Request
What
Add a new parameter S3_SDK_HTTP_CLIENT_BUILDER_CONFIG
This is a class to customize the httpClient used by the s3 client
Why
Our use case is:
We have a minio server with https endpoint. And the minio server uses self-signed certificate to do the TLS.
When our kafka client uses this lib to put object to that minio, we need to make sure the CA certificate of the minio server should in the truststore used by the s3 client. This is the reason we want to customize the httpClient.
Another limitation is that we need add the CA certificate to the truststore in the code instead of using the keytool command line. Since the command line is hard to automate in our use case. So finally I choose add this support in the this lib.
How
Test
I added this parameter with DefaultTlsTrustManagersProvider in the UT and it works fine.