Centralize SSL Context Handling and Client SSL Provider

[xiangfu0]

This pull request introduces a more flexible and configurable approach to SSL/TLS context management for both the Pinot broker and the Java client. The main changes include adding pluggable SSL context providers for the client, refactoring how SSL contexts are set up and used in the broker, and improving support for custom TLS protocols and providers. These updates enhance security, allow easier integration with different environments (e.g., FIPS/BCJSSE), and make it possible to override SSL handling via configuration or service loader.

SSL/TLS Context Provider Framework (Client):

• Added a new pluggable SslContextProvider interface and a default implementation (DefaultSslContextProvider) to allow custom configuration of the AsyncHttpClient's SSL/TLS settings. A factory (SslContextProviderFactory) resolves the provider using system properties or Java service loader, falling back to the default. [1] [2] [3]
• Refactored JsonAsyncHttpPinotClientTransport to use the new SslContextProvider for configuring SSL/TLS, replacing direct Netty context usage and improving protocol selection. [1] [2]

Broker SSL/TLS Context Management:

• Updated broker startup (BaseBrokerStarter) and request handler (MultiStageBrokerRequestHandler) to set up and cache SSLContext and GRPC SslContext in the singleton BrokerContext, supporting both HTTPS and GRPC with auto-renewal and insecure mode handling. [1] [2]
• Improved GRPC server SSL context creation to log warnings for non-JDK providers and support FIPS/BCJSSE environments, passing the provider explicitly to GrpcSslContexts.configure.

Dependency and Import Updates:

• Added necessary imports for SSL/TLS context management and refactored package usage to support the new context provider and broker context handling. [1] [2] [3] [4] [5]

[codecov-commenter]

Codecov Report

❌ Patch coverage is 31.20301% with 183 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.17%. Comparing base (92b6182) to head (db413f5).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
...org/apache/pinot/query/mailbox/MailboxService.java	19.56%	35 Missing and 2 partials ⚠️
.../pinot/common/utils/ClientSSLContextGenerator.java	24.13%	22 Missing ⚠️
...apache/pinot/controller/BaseControllerStarter.java	0.00%	18 Missing ⚠️
...che/pinot/core/instance/context/ServerContext.java	16.66%	15 Missing ⚠️
...pinot/core/instance/context/ControllerContext.java	0.00%	14 Missing ⚠️
...che/pinot/core/instance/context/BrokerContext.java	33.33%	12 Missing ⚠️
...e/pinot/broker/broker/helix/BaseBrokerStarter.java	0.00%	10 Missing ⚠️
.../pinot/server/starter/helix/BaseServerStarter.java	0.00%	10 Missing ⚠️
...apache/pinot/client/SslContextProviderFactory.java	75.67%	7 Missing and 2 partials ⚠️
.../pinot/query/service/dispatch/QueryDispatcher.java	27.27%	7 Missing and 1 partial ⚠️
... and 7 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #17358      +/-   ##
============================================
- Coverage     63.20%   63.17%   -0.03%     
- Complexity     1476     1477       +1     
============================================
  Files          3170     3172       +2     
  Lines        189523   189751     +228     
  Branches      28997    29036      +39     
============================================
+ Hits         119779   119877      +98     
- Misses        60451    60566     +115     
- Partials       9293     9308      +15     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	63.13% <31.20%> (-0.03%)	⬇️
java-21	63.12% <31.20%> (-0.06%)	⬇️
temurin	63.17% <31.20%> (-0.03%)	⬇️
unittests	63.17% <31.20%> (-0.03%)	⬇️
unittests1	55.50% <19.37%> (-0.05%)	⬇️
unittests2	34.04% <25.56%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

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

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

pinot-common/src/main/java/org/apache/pinot/common/utils/ClientSSLContextGenerator.java:1

• The setCertificateEntry method is being called with the loop index i instead of the certificate cert as the second argument. This will cause a runtime error since setCertificateEntry expects a Certificate object, not an integer. Change line 122 to trustStore.setCertificateEntry(serverKey, cert);.

/**

[Jackie-Jiang]

Putting context class in the component module could potentially make them hard to use. pinot-core won't be able to access them. We probably need to put them into lower module so that they are more accessible.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.