PDavid commented on code in PR #8681:
URL: https://github.com/apache/hbase/pull/8681#discussion_r4256590395
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -422,20 +561,52 @@ private static String[] getCipherSuites(Configuration
config) {
}
/**
- * Enable certificate file reloading by creating FileWatchers for keystore
and truststore.
- * AtomicReferences will be set with the new instances. resetContext - if
not null - will be
- * called when the file has been modified.
+ * Enable certificate file reloading for the RPC <em>client</em> side by
creating FileWatchers for
+ * the keystore and truststore whose paths are resolved by {@link
Role#CLIENT} (role-scoped keys
+ * first, legacy keys as fallback). AtomicReferences will be set with the
new instances.
+ * {@code resetContext} - if not null - will be called when the file has
been modified.
* @param keystoreWatcher Reference to keystoreFileWatcher.
* @param trustStoreWatcher Reference to truststoreFileWatcher.
* @param resetContext Callback for file changes.
*/
- public static void enableCertFileReloading(Configuration config,
+ public static void enableCertFileReloadingForClient(Configuration config,
AtomicReference<FileChangeWatcher> keystoreWatcher,
AtomicReference<FileChangeWatcher> trustStoreWatcher, Runnable
resetContext)
throws IOException {
- String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
+ enableCertFileReloading(config, Role.CLIENT, keystoreWatcher,
trustStoreWatcher, resetContext);
+ }
+
+ /**
+ * Enable certificate file reloading for the RPC <em>server</em> side. See
+ * {@link #enableCertFileReloadingForClient} for parameter semantics; the
only difference is that
+ * paths are resolved via the {@link Role#SERVER} role-scoped keys, falling
back to the legacy
+ * unscoped keys when unset.
+ */
+ public static void enableCertFileReloadingForServer(Configuration config,
+ AtomicReference<FileChangeWatcher> keystoreWatcher,
+ AtomicReference<FileChangeWatcher> trustStoreWatcher, Runnable
resetContext)
+ throws IOException {
+ enableCertFileReloading(config, Role.SERVER, keystoreWatcher,
trustStoreWatcher, resetContext);
+ }
+
+ private static void enableCertFileReloading(Configuration config, Role role,
+ AtomicReference<FileChangeWatcher> keystoreWatcher,
+ AtomicReference<FileChangeWatcher> trustStoreWatcher, Runnable
resetContext)
+ throws IOException {
+ String keyStoreLocation;
+ String trustStoreLocation;
+ if (role == Role.CLIENT) {
+ keyStoreLocation = resolveConfig(config,
TLS_CONFIG_CLIENT_KEYSTORE_LOCATION,
+ TLS_CONFIG_KEYSTORE_LOCATION, "");
+ trustStoreLocation = resolveConfig(config,
TLS_CONFIG_CLIENT_TRUSTSTORE_LOCATION,
+ TLS_CONFIG_TRUSTSTORE_LOCATION, "");
+ } else {
+ keyStoreLocation = resolveConfig(config,
TLS_CONFIG_SERVER_KEYSTORE_LOCATION,
+ TLS_CONFIG_KEYSTORE_LOCATION, "");
+ trustStoreLocation = resolveConfig(config,
TLS_CONFIG_SERVER_TRUSTSTORE_LOCATION,
+ TLS_CONFIG_TRUSTSTORE_LOCATION, "");
Review Comment:
Rather minor question:
Can I ask what is the reason why we don't use `TLSStore.resolve()` here
instead of `resolveConfig()` calls?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
