snleee commented on code in PR #12325:
URL: https://github.com/apache/pinot/pull/12325#discussion_r1468938515
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java:
##########
@@ -322,6 +351,119 @@ public static SslContext buildServerContext(TlsConfig
tlsConfig) {
}
}
+ /**
+ * check if the key store or trust store path is null or has file scheme.
+ *
+ * @param keyOrTrustStorePath key store or trust store path in String format.
+ */
+ public static boolean isKeyOrTrustStorePathNullOrHasFileScheme(String
keyOrTrustStorePath) {
+ try {
+ return keyOrTrustStorePath == null
+ ||
makeKeyOrTrustStoreUrl(keyOrTrustStorePath).toURI().getScheme().startsWith(FILE_SCHEME);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Enables auto renewal of SSLFactory when
+ * 1. the {@link SSLFactory} is created with a key manager and trust manager
swappable
+ * 2. the key store is null or a local file
+ * 3. the trust store is null or a local file
+ * 4. the key store or trust store file changes.
+ * @param sslFactory the {@link SSLFactory} to enable key manager and trust
manager auto renewal
+ * @param tlsConfig the {@link TlsConfig} to get the key store and trust
store information
+ */
+ public static void enableAutoRenewalFromFileStoreForSSLFactory(SSLFactory
sslFactory, TlsConfig tlsConfig) {
+ enableAutoRenewalFromFileStoreForSSLFactory(sslFactory,
+ tlsConfig.getKeyStoreType(), tlsConfig.getKeyStorePath(),
tlsConfig.getKeyStorePassword(),
+ tlsConfig.getTrustStoreType(), tlsConfig.getTrustStorePath(),
tlsConfig.getTrustStorePassword(),
+ null, null);
+ }
+
+ private static void enableAutoRenewalFromFileStoreForSSLFactory(
+ SSLFactory sslFactory,
+ String keyStoreType, String keyStorePath, String keyStorePassword,
+ String trustStoreType, String trustStorePath, String trustStorePassword,
+ String sslContextProtocol, SecureRandom secureRandom) {
+ try {
+ URL keyStoreURL = keyStorePath == null ? null :
makeKeyOrTrustStoreUrl(keyStorePath);
+ URL trustStoreURL = trustStorePath == null ? null :
makeKeyOrTrustStoreUrl(trustStorePath);
+ if (keyStoreURL != null) {
+ Preconditions.checkArgument(
+ keyStoreURL.toURI().getScheme().startsWith(FILE_SCHEME),
+ "key store path must be a local file path or null when SSL auto
renew is enabled");
+ Preconditions.checkArgument(
+ sslFactory.getKeyManager().isPresent()
+ && sslFactory.getKeyManager().get() instanceof
HotSwappableX509ExtendedKeyManager,
+ "key manager of the existing SSLFactory must be swappable"
+ );
+ }
+ if (trustStoreURL != null) {
+ Preconditions.checkArgument(
+ trustStoreURL.toURI().getScheme().startsWith(FILE_SCHEME),
+ "trust store path must be a local file path or null when SSL auto
renew is enabled");
+ Preconditions.checkArgument(
+ sslFactory.getTrustManager().isPresent()
+ && sslFactory.getTrustManager().get() instanceof
HotSwappableX509ExtendedTrustManager,
+ "trust manager of the existing SSLFactory must be swappable"
+ );
+ }
+ // The reloadSslFactoryWhenFileStoreChanges is a blocking call, so we
need to create a new thread to run it.
+ // Creating a new thread to run the reloadSslFactoryWhenFileStoreChanges
is costly; however, unless we
+ // invoke the createAutoRenewedSSLFactoryFromFileStore method crazily,
this should not be a problem.
+ Executors.newSingleThreadExecutor().execute(() -> {
Review Comment:
I checked the usage of `createAutoRenewedSSLFactoryFromFileStore()` and we
may hit this for every single query request? (Since we call it when we
initialize `GrpcQueryClient`. It looks that we will create `GrpcQueryClient`
every time we hit broker query api)
If that's the case, this can hurt the broker (imagine that we get 1000s of
queries per sec and we spin up the new thread for each query for renewing ssl).
Should we consider to add a background periodic task that monitors the ssl
files and make the change if needed?
Let's double check if that's the case and think about the better solution.
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
