You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "kay-owolabi (via GitHub)" <gi...@apache.org> on 2023/12/21 18:16:11 UTC

[PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

kay-owolabi opened a new pull request, #12194:
URL: https://github.com/apache/pinot/pull/12194

   ## Summary
   This pull request introduces improvements to the SSL context handling in `JsonAsyncHttpPinotClientTransport` and `PinotControllerTransport` within Apache Pinot. The primary goal is to address the issue of SSL handshake failures during dynamic certificate rotation, as discussed in [#12100](https://github.com/apache/pinot/issues/12100).
   
   ## Changes
   - **Dynamic SSL Context**: Implemented a mechanism to dynamically set and refresh the `SSLContext` based on `TlsConfig`. This change ensures that the SSL context is correctly updated in scenarios where the client certificate or TLS configuration changes.
   - **Deprecated Constructor Replacement**: Replaced the deprecated usage of `JdkSslContext` in `AsyncHttpClient` with `TlsUtils.buildClientContext(tlsConfig)`, enhancing the SSL context construction to be more adaptable to changes.
   
   ## Testing and Validation
   - Conducted thorough testing to simulate certificate rotations and validate the successful completion of SSL/TLS handshakes with the updated certificates.
   - Ensured that the new SSL context handling mechanism is consistent with the existing functionalities and does not introduce regressions.
   
   ## Impact and Benefits
   - **Resolves Handshake Failures**: This fix directly addresses the handshake failures in long-running JDBC clients post-certificate rotation, enhancing the stability and reliability of SSL/TLS connections.
   - **Future-Proofing**: The changes lay the groundwork for more dynamic and robust SSL/TLS configuration management within Apache Pinot, accommodating evolving security requirements.
   
   I welcome any feedback or suggestions on these changes and look forward to your reviews. Thank you for considering this contribution to Apache Pinot.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #12194:
URL: https://github.com/apache/pinot/pull/12194#issuecomment-1883885412

   CC @zhtaoxiang who has recently look at this part of the SSL construct. 
   
   note: this solution might be limited to the scope of the original issue. where as there might be a bigger problem in the overall security design. but the goal is to see if this solution can fit into the overall direction towards the larger security scope. if so we should accept it and continue to evolve. 
   
   thanks
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12194:
URL: https://github.com/apache/pinot/pull/12194#issuecomment-1874373814

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: `12 lines` in your changes are missing coverage. Please review.
   > Comparison is base [(`5b4e19a`)](https://app.codecov.io/gh/apache/pinot/commit/5b4e19aa4e3073ce9030d2283fb80bbaf177df21?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.51% compared to head [(`6ec54fb`)](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.41%.
   > Report is 1 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...main/java/org/apache/pinot/client/PinotDriver.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qZGJjLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Bpbm90RHJpdmVyLmphdmE=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...main/java/org/apache/pinot/client/BrokerCache.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlckNhY2hlLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ient/JsonAsyncHttpPinotClientTransportFactory.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydEZhY3RvcnkuamF2YQ==) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...org/apache/pinot/client/utils/ConnectionUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L3V0aWxzL0Nvbm5lY3Rpb25VdGlscy5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ot/client/controller/PinotControllerTransport.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qZGJjLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L2NvbnRyb2xsZXIvUGlub3RDb250cm9sbGVyVHJhbnNwb3J0LmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...nt/controller/PinotControllerTransportFactory.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qZGJjLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L2NvbnRyb2xsZXIvUGlub3RDb250cm9sbGVyVHJhbnNwb3J0RmFjdG9yeS5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ava/org/apache/pinot/client/utils/DriverUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qZGJjLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L3V0aWxzL0RyaXZlclV0aWxzLmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #12194      +/-   ##
   ============================================
   - Coverage     61.51%   61.41%   -0.11%     
   + Complexity     1153      207     -946     
   ============================================
     Files          2415     2415              
     Lines        131157   131157              
     Branches      20245    20245              
   ============================================
   - Hits          80683    80550     -133     
   - Misses        44575    44707     +132     
   - Partials       5899     5900       +1     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-34.77%)` | :arrow_down: |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.41% <7.69%> (+0.01%)` | :arrow_up: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.39% <7.69%> (-0.11%)` | :arrow_down: |
   | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.38% <7.69%> (+<0.01%)` | :arrow_up: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.41% <7.69%> (-0.11%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.41% <7.69%> (-0.11%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.46% <ø> (-0.15%)` | :arrow_down: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12194/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.70% <7.69%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12194?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "kay-owolabi (via GitHub)" <gi...@apache.org>.
kay-owolabi commented on code in PR #12194:
URL: https://github.com/apache/pinot/pull/12194#discussion_r1440898870


##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java:
##########
@@ -70,7 +68,7 @@ public JsonAsyncHttpPinotClientTransport() {
   }
 
   public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String scheme, String extraOptionString,
-      boolean useMultistageEngine, @Nullable SSLContext sslContext, ConnectionTimeouts connectionTimeouts,
+      boolean useMultistageEngine, @Nullable SslContext sslContext, ConnectionTimeouts connectionTimeouts,

Review Comment:
   updated



##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java:
##########
@@ -70,7 +68,7 @@ public JsonAsyncHttpPinotClientTransport() {
   }
 
   public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String scheme, String extraOptionString,
-      boolean useMultistageEngine, @Nullable SSLContext sslContext, ConnectionTimeouts connectionTimeouts,
+      boolean useMultistageEngine, @Nullable SslContext sslContext, ConnectionTimeouts connectionTimeouts,

Review Comment:
   @walterddr updated



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "kay-owolabi (via GitHub)" <gi...@apache.org>.
kay-owolabi commented on PR #12194:
URL: https://github.com/apache/pinot/pull/12194#issuecomment-1904618233

   > @kay-owolabi Thanks for the fix. I am working on a PR to make the KeyManager and TrustManager swappable (as mentioned in #12107). Maybe we can wait a while to see if my fix can fix the problem you are facing?
   
   @zhtaoxiang do you have a pr I could take a look at?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12194:
URL: https://github.com/apache/pinot/pull/12194#discussion_r1439696608


##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/controller/PinotControllerTransport.java:
##########
@@ -51,14 +49,14 @@ public class PinotControllerTransport {
   private final String _scheme;
   private final AsyncHttpClient _httpClient;
 
-  public PinotControllerTransport(Map<String, String> headers, String scheme, @Nullable SSLContext sslContext,
+  public PinotControllerTransport(Map<String, String> headers, String scheme, @Nullable SslContext sslContext,
       ConnectionTimeouts connectionTimeouts, TlsProtocols tlsProtocols, @Nullable String appId) {

Review Comment:
   since this is public API we might need to keep supporting both javax.net.ssl.SSLContext and io.netty.handler.ssl.SslContext and mark the former with deprecated annotation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12194:
URL: https://github.com/apache/pinot/pull/12194#discussion_r1440955683


##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java:
##########
@@ -70,7 +68,7 @@ public JsonAsyncHttpPinotClientTransport() {
   }
 
   public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String scheme, String extraOptionString,
-      boolean useMultistageEngine, @Nullable SSLContext sslContext, ConnectionTimeouts connectionTimeouts,
+      boolean useMultistageEngine, @Nullable SslContext sslContext, ConnectionTimeouts connectionTimeouts,

Review Comment:
   :+1:



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on PR #12194:
URL: https://github.com/apache/pinot/pull/12194#issuecomment-1894683133

   @kay-owolabi Thanks for the fix. I am working on a PR to make the KeyManager and TrustManager swappable. Maybe we can wait a while to see if my fix can fix the problem you are facing?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12194:
URL: https://github.com/apache/pinot/pull/12194#discussion_r1439696608


##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/controller/PinotControllerTransport.java:
##########
@@ -51,14 +49,14 @@ public class PinotControllerTransport {
   private final String _scheme;
   private final AsyncHttpClient _httpClient;
 
-  public PinotControllerTransport(Map<String, String> headers, String scheme, @Nullable SSLContext sslContext,
+  public PinotControllerTransport(Map<String, String> headers, String scheme, @Nullable SslContext sslContext,
       ConnectionTimeouts connectionTimeouts, TlsProtocols tlsProtocols, @Nullable String appId) {

Review Comment:
   since this is public API we might need to keep supporting both javax.net.ssl.SSLContext and io.netty.handler.ssl.SslContext and mark the former with deprecated annotation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12194:
URL: https://github.com/apache/pinot/pull/12194#discussion_r1439698356


##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java:
##########
@@ -70,7 +68,7 @@ public JsonAsyncHttpPinotClientTransport() {
   }
 
   public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String scheme, String extraOptionString,
-      boolean useMultistageEngine, @Nullable SSLContext sslContext, ConnectionTimeouts connectionTimeouts,
+      boolean useMultistageEngine, @Nullable SslContext sslContext, ConnectionTimeouts connectionTimeouts,

Review Comment:
   since this is public API we might need to keep supporting both javax.net.ssl.SSLContext and io.netty.handler.ssl.SslContext and mark the former with deprecated annotation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Enhance SSL Context Handling in Client Transports for Dynamic Certificate Rotation [pinot]

Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on PR #12194:
URL: https://github.com/apache/pinot/pull/12194#issuecomment-1939494062

   > > @kay-owolabi Thanks for the fix. I am working on a PR to make the KeyManager and TrustManager swappable (as mentioned in #12107). Maybe we can wait a while to see if my fix can fix the problem you are facing?
   > 
   > @zhtaoxiang do you have a pr I could take a look at?
   
   A list of PRs merged: https://github.com/apache/pinot/pull/12277, https://github.com/apache/pinot/pull/12325, https://github.com/apache/pinot/pull/12357, https://github.com/apache/pinot/pull/12384


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org