Centralize SSL Context Handling and Client SSL Provider #17358
+1,035
−198
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17358 +/- ##
============================================
- Coverage 63.28% 63.24% -0.04%
- Complexity 1474 1475 +1
============================================
Files 3161 3164 +3
Lines 188588 188808 +220
Branches 28857 28892 +35
============================================
+ Hits 119351 119416 +65
- Misses 59983 60136 +153
- Partials 9254 9256 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xiangfu0
force-pushed
the
ssl-context-provider
branch
2 times, most recently
from
9b2b3e9 to
ed0217a
Compare
xiangfu0
changed the title
xiangfu0
force-pushed
the
ssl-context-provider
branch
from
ed0217a to
f158985
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes SSL/TLS context handling across Pinot components by introducing module-local context singletons (BrokerContext, ControllerContext, ServerContext) that delegate to a shared QueryRuntimeContext for gRPC SSL contexts. It also adds a pluggable SslContextProvider mechanism for client SSL configuration with JDK/BCJSSE support.
Key Changes:
- Centralized SSL context management through module-specific context classes backed by QueryRuntimeContext
- Introduced SslContextProvider interface with factory-based lookup and default JDK implementation
- Enhanced client SSL configuration with protocol/keystore/algorithm customization
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pinot-server/src/main/java/org/apache/pinot/server/ServerContext.java | New singleton context for server SSL contexts delegating to QueryRuntimeContext |
| pinot-broker/src/main/java/org/apache/pinot/broker/BrokerContext.java | New singleton context for broker SSL contexts delegating to QueryRuntimeContext |
| pinot-controller/src/main/java/org/apache/pinot/controller/ControllerContext.java | New singleton context for controller SSL contexts delegating to QueryRuntimeContext |
| pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/context/QueryRuntimeContext.java | Renamed from BrokerContext, added gRPC SSL context fields |
| pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/context/ServerContext.java | Removed (functionality moved to module-specific contexts) |
| pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/context/ControllerContext.java | Removed (functionality moved to module-specific contexts) |
| pinot-server/src/main/java/org/apache/pinot/server/worker/WorkerQueryServer.java | Initialize server context SSL contexts during worker query server setup |
| pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java | Initialize server context HTTPS contexts on startup |
| pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java | Initialize controller context SSL contexts on startup |
| pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java | Initialize broker context HTTPS contexts on startup |
| pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java | Initialize broker context SSL contexts in multi-stage handler |
| pinot-query-runtime/.../QueryServer.java | Reuse cached server gRPC SSL context from QueryRuntimeContext |
| pinot-query-runtime/.../QueryDispatcher.java | Initialize and reuse client gRPC SSL context from QueryRuntimeContext |
| pinot-query-runtime/.../DispatchClient.java | Accept optional SslContext parameter to reuse cached context |
| pinot-query-runtime/.../MailboxService.java | Resolve SSL contexts from QueryRuntimeContext for broker/server/controller instances |
| pinot-query-runtime/.../GrpcMailboxServer.java | Accept optional SslContext parameter to reuse cached context |
| pinot-query-runtime/.../ChannelManager.java | Accept optional SslContext parameter to reuse cached context |
| pinot-query-runtime/.../OpChainExecutionContext.java | Simplified to use QueryRuntimeContext instead of separate broker/server contexts |
| pinot-clients/.../SslContextProvider.java | New interface for pluggable SSL configuration |
| pinot-clients/.../SslContextProviderFactory.java | Factory for loading SslContextProvider via property/ServiceLoader |
| pinot-clients/.../DefaultSslContextProvider.java | Default JDK/BCJSSE provider implementation |
| pinot-clients/.../JsonAsyncHttpPinotClientTransport.java | Updated to use SslContextProvider for SSL configuration |
| pinot-common/.../ClientSSLContextGenerator.java | Added configurable protocol/keystore/algorithm with try-with-resources |
| pinot-broker/.../BrokerGrpcServer.java | Added warning for non-JDK providers and provider-aware GrpcSslContexts configuration |
| pinot-controller/pom.xml | Added pinot-query-runtime dependency |
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/MailboxService.java
Outdated
Show resolved
Hide resolved
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/MailboxService.java
Outdated
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/common/utils/ClientSSLContextGenerator.java
Outdated
Show resolved
Hide resolved
xiangfu0
force-pushed
the
ssl-context-provider
branch
from
f158985 to
82c7250
Compare
xiangfu0
force-pushed
the
ssl-context-provider
branch
from
82c7250 to
8fc4852
Compare
xiangfu0
force-pushed
the
ssl-context-provider
branch
from
8fc4852 to
f2dc959
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
BrokerContext,ControllerContext, andServerContextbacked byQueryRuntimeContextfor shared gRPC SSL contexts and HTTPS SSL contexts, and wire controller to depend on query-runtime.SslContextProviderwith default JDK/BCJSSE implementation plus configurable lookup (system property/ServiceLoader) and update the async HTTP client transport to use it.