Hot reload remote cluster credentials

n1v0lg · Nov 29, 2023 · 93485f8 · 93485f8
1 parent 8ffd630
commit 93485f8
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 40 deletions.
diff --git a/...sticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java b/...sticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java
@@ -132,9 +132,15 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(
                     exceptions.add(e);
                 }
             });
-            ExceptionsHelper.rethrowAndSuppress(exceptions);
+            // TODO feels hacky
+            try {
+                transportService.getRemoteClusterService().updateRemoteClusterCredentials(settingsWithKeystore);
+            } catch (final Exception e) {
+                logger.warn(() -> "Reload failed for remote cluster credentials", e);
+                exceptions.add(e);
+            }
 
-            transportService.getRemoteClusterService().updateRemoteClusterCredentials(settingsWithKeystore);
+            ExceptionsHelper.rethrowAndSuppress(exceptions);
 
             return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), null);
         } catch (final Exception e) {

diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java
@@ -12,10 +12,8 @@
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.set.Sets;
 
 import java.util.Map;
-import java.util.Set;
 
 import static org.elasticsearch.transport.RemoteClusterService.REMOTE_CLUSTER_CREDENTIALS;
 
@@ -29,13 +27,9 @@ public RemoteClusterCredentialsManager(Settings settings) {
         updateClusterCredentials(settings);
     }
 
-    public Set<String> updateClusterCredentials(Settings settings) {
-        var newCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings);
-        // TODO this is not exactly accurate
-        var updated = Sets.difference(newCredentials.keySet(), clusterCredentials.keySet());
-        clusterCredentials = newCredentials;
-        logger.info("Read remote cluster credentials: {}", clusterCredentials);
-        return updated;
+    public void updateClusterCredentials(Settings settings) {
+        clusterCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings);
+        logger.info("Updated remote cluster credentials: [{}]", clusterCredentials);
     }
 
     public SecureString resolveCredentials(String clusterAlias) {

diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
@@ -155,16 +155,11 @@ public boolean isRemoteClusterServerEnabled() {
         this.remoteClusterServerEnabled = REMOTE_CLUSTER_SERVER_ENABLED.get(settings);
         this.transportService = transportService;
         this.remoteClusterCredentialsManager = new RemoteClusterCredentialsManager(settings);
-
         if (remoteClusterServerEnabled) {
             registerRemoteClusterHandshakeRequestHandler(transportService);
         }
     }
 
-    public RemoteClusterCredentialsManager getRemoteClusterCredentialsManager() {
-        return remoteClusterCredentialsManager;
-    }
-
     public DiscoveryNode getLocalNode() {
         return transportService.getLocalNode();
     }
@@ -310,21 +305,13 @@ private synchronized void updateSkipUnavailable(String clusterAlias, Boolean ski
     }
 
     public void updateRemoteClusterCredentials(Settings settings) {
-        var updatedClusters = remoteClusterCredentialsManager.updateClusterCredentials(settings);
-        for (var clusterAlias : updatedClusters) {
-            logger.info("Credentials changed for cluster [{}], updating cluster", clusterAlias);
-            updateRemoteCluster(clusterAlias, settings, true);
-        }
+        remoteClusterCredentialsManager.updateClusterCredentials(settings);
     }
 
     @Override
     protected void updateRemoteCluster(String clusterAlias, Settings settings) {
-        updateRemoteCluster(clusterAlias, settings, false);
-    }
-
-    void updateRemoteCluster(String clusterAlias, Settings settings, boolean forceRebuildConnection) {
         CountDownLatch latch = new CountDownLatch(1);
-        updateRemoteCluster(clusterAlias, settings, forceRebuildConnection, ActionListener.runAfter(new ActionListener<>() {
+        updateRemoteCluster(clusterAlias, settings, ActionListener.runAfter(new ActionListener<>() {
             @Override
             public void onResponse(RemoteClusterConnectionStatus status) {
                 logger.info("remote cluster connection [{}] updated: {}", clusterAlias, status);
@@ -407,17 +394,6 @@ enum RemoteClusterConnectionStatus {
         UNCHANGED
     }
 
-    /**
-     * This method updates the list of remote clusters. It's intended to be used as an update consumer on the settings infrastructure
-     *
-     * @param clusterAlias a cluster alias to discovery node mapping representing the remote clusters seeds nodes
-     * @param newSettings the updated settings for the remote connection
-     * @param listener a listener invoked once every configured cluster has been connected to
-     */
-    synchronized void updateRemoteCluster(String clusterAlias, Settings newSettings, ActionListener<Void> listener) {
-        updateRemoteCluster(clusterAlias, newSettings, false, listener);
-    }
-
     /**
      * Connects to all remote clusters in a blocking fashion. This should be called on node startup to establish an initial connection
      * to all configured seed nodes.

diff --git a/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java b/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java
@@ -535,7 +535,11 @@ public void onNodeConnected(DiscoveryNode node, Transport.Connection connection)
                 });
 
                 try (
-                    var remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager);
+                    var remoteConnectionManager = new RemoteConnectionManager(
+                        clusterAlias,
+                        RemoteClusterCredentialsManager.EMPTY,
+                        connectionManager
+                    );
                     var strategy = new ProxyConnectionStrategy(
                         clusterAlias,
                         localService,

diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
@@ -995,7 +995,6 @@ Collection<Object> createComponents(
                 securityContext.get(),
                 destructiveOperations,
                 crossClusterAccessAuthcService.get(),
-                remoteClusterCredentialsResolver,
                 getLicenseState()
             )
         );

diff --git a/...n/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java b/...n/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java
@@ -65,7 +65,6 @@
 import static org.elasticsearch.transport.RemoteClusterPortSettings.REMOTE_CLUSTER_PROFILE;
 import static org.elasticsearch.transport.RemoteClusterPortSettings.REMOTE_CLUSTER_SERVER_ENABLED;
 import static org.elasticsearch.transport.RemoteClusterPortSettings.TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY;
-import static org.elasticsearch.xpack.security.transport.RemoteClusterCredentialsResolver.RemoteClusterCredentials;
 
 public class SecurityServerTransportInterceptor implements TransportInterceptor {