From ac0c5239ad23d52582a47c62268aa20626a965aa Mon Sep 17 00:00:00 2001 From: Jeremy Dahlgren Date: Thu, 25 Sep 2025 17:26:21 -0400 Subject: [PATCH 1/3] Move credentials settings merging out of RemoteClusterService Addresses a second settings cleanup request comment from PR 133834, moving the settings merging and creation of a LinkedProjectConfig out of RemoteClusterService and into TransportReloadRemoteClusterCredentialsAction. This is one step closer to eliminating the Settings based code in RemoteClusterService so it operates on LinkedProjectConfig objects only. Resolves: ES-12864 --- .../transport/RemoteClusterService.java | 37 ++++++++++++------- .../transport/RemoteClusterServiceTests.java | 12 +++++- ...tReloadRemoteClusterCredentialsAction.java | 17 ++++++++- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index d2ad9f95f23e7..aaee222bbac34 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.EsExecutors; @@ -106,6 +107,10 @@ public boolean isRemoteClusterServerEnabled() { this.crossProjectEnabled = settings.getAsBoolean("serverless.cross_project.enabled", false); } + public ProjectResolver getProjectResolver() { + return projectResolver; + } + /** * Group indices by cluster alias mapped to OriginalIndices for that cluster. * @param remoteClusterNames Set of configured remote cluster names. @@ -302,11 +307,20 @@ public void skipUnavailableChanged( } } - @FixForMultiProject(description = "Refactor as needed to support project specific changes to linked remotes.") - public synchronized void updateRemoteClusterCredentials(Supplier settingsSupplier, ActionListener listener) { + /** + * Rebuilds linked project connections as needed for updated remote cluster credentials. + * @param settingsSupplier A {@link Supplier} for the updated credentials {@link Settings}. + * @param configFunction A function that builds a {@link LinkedProjectConfig} for an alias, static settings, and new settings. + * @param listener An {@link ActionListener} invoked once all connection modifications have been completed. + */ + public synchronized void updateRemoteClusterCredentials( + Supplier settingsSupplier, + TriFunction configFunction, + ActionListener listener + ) { final var projectId = projectResolver.getProjectId(); - final Settings settings = settingsSupplier.get(); - final UpdateRemoteClusterCredentialsResult result = remoteClusterCredentialsManager.updateClusterCredentials(settings); + final Settings newSettings = settingsSupplier.get(); + final UpdateRemoteClusterCredentialsResult result = remoteClusterCredentialsManager.updateClusterCredentials(newSettings); // We only need to rebuild connections when a credential was newly added or removed for a cluster alias, not if the credential // value was updated. Therefore, only consider added or removed aliases final int totalConnectionsToRebuild = result.addedClusterAliases().size() + result.removedClusterAliases().size(); @@ -318,20 +332,17 @@ public synchronized void updateRemoteClusterCredentials(Supplier setti logger.info("project [{}] rebuilding [{}] connections after credentials update", projectId, totalConnectionsToRebuild); try (var connectionRefs = new RefCountingRunnable(() -> listener.onResponse(null))) { for (var clusterAlias : result.addedClusterAliases()) { - maybeRebuildConnectionOnCredentialsChange(projectId, clusterAlias, settings, connectionRefs); + maybeRebuildConnectionOnCredentialsChange(configFunction.apply(clusterAlias, settings, newSettings), connectionRefs); } for (var clusterAlias : result.removedClusterAliases()) { - maybeRebuildConnectionOnCredentialsChange(projectId, clusterAlias, settings, connectionRefs); + maybeRebuildConnectionOnCredentialsChange(configFunction.apply(clusterAlias, settings, newSettings), connectionRefs); } } } - private void maybeRebuildConnectionOnCredentialsChange( - ProjectId projectId, - String clusterAlias, - Settings newSettings, - RefCountingRunnable connectionRefs - ) { + private void maybeRebuildConnectionOnCredentialsChange(LinkedProjectConfig config, RefCountingRunnable connectionRefs) { + final var projectId = config.originProjectId(); + final var clusterAlias = config.linkedProjectAlias(); final var connectionsMap = getConnectionsMapForProject(projectId); if (false == connectionsMap.containsKey(clusterAlias)) { // A credential was added or removed before a remote connection was configured. @@ -344,8 +355,6 @@ private void maybeRebuildConnectionOnCredentialsChange( return; } - final var mergedSettings = Settings.builder().put(settings, false).put(newSettings, false).build(); - final var config = RemoteClusterSettings.toConfig(projectId, ProjectId.DEFAULT, clusterAlias, mergedSettings); updateRemoteCluster(config, true, ActionListener.releaseAfter(new ActionListener<>() { @Override public void onResponse(RemoteClusterConnectionStatus status) { diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 709d2c791b114..7b005b695b0af 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -1627,7 +1627,7 @@ public void testUpdateRemoteClusterCredentialsRebuildsConnectionWithCorrectProfi secureSettings.setString("cluster.remote.cluster_1.credentials", randomAlphaOfLength(10)); final PlainActionFuture listener = new PlainActionFuture<>(); final Settings settings = Settings.builder().put(clusterSettings).setSecureSettings(secureSettings).build(); - service.updateRemoteClusterCredentials(() -> settings, listener); + service.updateRemoteClusterCredentials(() -> settings, this::buildLinkedProjectConfig, listener); listener.actionGet(10, TimeUnit.SECONDS); } @@ -1641,6 +1641,7 @@ public void testUpdateRemoteClusterCredentialsRebuildsConnectionWithCorrectProfi service.updateRemoteClusterCredentials( // Settings without credentials constitute credentials removal () -> clusterSettings, + this::buildLinkedProjectConfig, listener ); listener.actionGet(10, TimeUnit.SECONDS); @@ -1730,7 +1731,7 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite .put(cluster2Settings) .setSecureSettings(secureSettings) .build(); - service.updateRemoteClusterCredentials(() -> settings, listener); + service.updateRemoteClusterCredentials(() -> settings, this::buildLinkedProjectConfig, listener); listener.actionGet(10, TimeUnit.SECONDS); } @@ -1750,6 +1751,7 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite service.updateRemoteClusterCredentials( // Settings without credentials constitute credentials removal () -> settings, + this::buildLinkedProjectConfig, listener ); listener.actionGet(10, TimeUnit.SECONDS); @@ -1828,6 +1830,12 @@ public void testLogsConnectionResult() throws IOException { } } + @FixForMultiProject(description = "Refactor to add the linked project ID associated with the alias.") + private LinkedProjectConfig buildLinkedProjectConfig(String alias, Settings staticSettings, Settings newSettings) { + final var mergedSettings = Settings.builder().put(staticSettings, false).put(newSettings, false).build(); + return RemoteClusterSettings.toConfig(projectResolver.getProjectId(), ProjectId.DEFAULT, alias, mergedSettings); + } + private void updateRemoteCluster( RemoteClusterService service, String alias, diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java index 5f5b082dee2ce..e33c6bd4f8b8d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java @@ -16,14 +16,19 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; +import org.elasticsearch.core.FixForMultiProject; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.rest.action.admin.cluster.RestReloadSecureSettingsAction; import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.LinkedProjectConfig; import org.elasticsearch.transport.RemoteClusterService; +import org.elasticsearch.transport.RemoteClusterSettings; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.Transports; import org.elasticsearch.xpack.core.security.action.ActionTypes; @@ -79,7 +84,17 @@ protected void doExecute(Task task, Request request, ActionListener ActionResponse.Empty.INSTANCE)); + @FixForMultiProject(description = "Supply the linked project ID when building the LinkedProjectConfig object.") + final TriFunction configBuilder = (clusterAlias, staticSettings, newSettings) -> { + final var projectId = remoteClusterService.getProjectResolver().getProjectId(); + final var mergedSettings = Settings.builder().put(staticSettings, false).put(newSettings, false).build(); + return RemoteClusterSettings.toConfig(projectId, ProjectId.DEFAULT, clusterAlias, mergedSettings); + }; + remoteClusterService.updateRemoteClusterCredentials( + settingsSupplier, + configBuilder, + listener.safeMap(ignored -> ActionResponse.Empty.INSTANCE) + ); } private ClusterBlockException checkBlock(ClusterState clusterState) { From 2bbe86993e67616044804f0b4022a7ddcfee263d Mon Sep 17 00:00:00 2001 From: Jeremy Dahlgren Date: Wed, 1 Oct 2025 14:29:49 -0400 Subject: [PATCH 2/3] Move credentials update code into TransportReloadRemoteClusterCredentialsAction --- .../RemoteClusterCredentialsManager.java | 6 ++ .../transport/RemoteClusterService.java | 97 ++--------------- .../transport/RemoteClusterServiceTests.java | 54 ++++++---- ...tReloadRemoteClusterCredentialsAction.java | 101 +++++++++++++++--- 4 files changed, 139 insertions(+), 119 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java index 33ac9718d829e..9d340798923fa 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java @@ -26,13 +26,19 @@ public class RemoteClusterCredentialsManager { private static final Logger logger = LogManager.getLogger(RemoteClusterCredentialsManager.class); + private final Settings settings; private volatile Map clusterCredentials = Collections.emptyMap(); @SuppressWarnings("this-escape") public RemoteClusterCredentialsManager(Settings settings) { + this.settings = settings; updateClusterCredentials(settings); } + public Settings getSettings() { + return settings; + } + public final synchronized UpdateRemoteClusterCredentialsResult updateClusterCredentials(Settings settings) { final Map newClusterCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings); if (clusterCredentials.isEmpty()) { diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index aaee222bbac34..1f699195259c1 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -18,7 +18,6 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.RefCountingListener; -import org.elasticsearch.action.support.RefCountingRunnable; import org.elasticsearch.client.internal.RemoteClusterClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.ProjectId; @@ -26,7 +25,6 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.EsExecutors; @@ -35,7 +33,6 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.indices.IndicesExpressionGrouper; import org.elasticsearch.node.ReportingService; -import org.elasticsearch.transport.RemoteClusterCredentialsManager.UpdateRemoteClusterCredentialsResult; import java.io.Closeable; import java.io.IOException; @@ -51,7 +48,6 @@ import java.util.concurrent.TimeoutException; import java.util.function.BiFunction; import java.util.function.Function; -import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -107,8 +103,8 @@ public boolean isRemoteClusterServerEnabled() { this.crossProjectEnabled = settings.getAsBoolean("serverless.cross_project.enabled", false); } - public ProjectResolver getProjectResolver() { - return projectResolver; + public RemoteClusterCredentialsManager getRemoteClusterCredentialsManager() { + return remoteClusterCredentialsManager; } /** @@ -194,6 +190,13 @@ public Set getRegisteredRemoteClusterNames() { return getConnectionsMapForCurrentProject().keySet(); } + /** + * Returns the registered linked project aliases for the provided origin Project ID. + */ + public Set getRegisteredRemoteClusterNames(ProjectId originProjectId) { + return getConnectionsMapForProject(originProjectId).keySet(); + } + /** * Returns a connection to the given node on the given remote cluster * @@ -307,83 +310,6 @@ public void skipUnavailableChanged( } } - /** - * Rebuilds linked project connections as needed for updated remote cluster credentials. - * @param settingsSupplier A {@link Supplier} for the updated credentials {@link Settings}. - * @param configFunction A function that builds a {@link LinkedProjectConfig} for an alias, static settings, and new settings. - * @param listener An {@link ActionListener} invoked once all connection modifications have been completed. - */ - public synchronized void updateRemoteClusterCredentials( - Supplier settingsSupplier, - TriFunction configFunction, - ActionListener listener - ) { - final var projectId = projectResolver.getProjectId(); - final Settings newSettings = settingsSupplier.get(); - final UpdateRemoteClusterCredentialsResult result = remoteClusterCredentialsManager.updateClusterCredentials(newSettings); - // We only need to rebuild connections when a credential was newly added or removed for a cluster alias, not if the credential - // value was updated. Therefore, only consider added or removed aliases - final int totalConnectionsToRebuild = result.addedClusterAliases().size() + result.removedClusterAliases().size(); - if (totalConnectionsToRebuild == 0) { - logger.debug("project [{}] no connection rebuilding required after credentials update", projectId); - listener.onResponse(null); - return; - } - logger.info("project [{}] rebuilding [{}] connections after credentials update", projectId, totalConnectionsToRebuild); - try (var connectionRefs = new RefCountingRunnable(() -> listener.onResponse(null))) { - for (var clusterAlias : result.addedClusterAliases()) { - maybeRebuildConnectionOnCredentialsChange(configFunction.apply(clusterAlias, settings, newSettings), connectionRefs); - } - for (var clusterAlias : result.removedClusterAliases()) { - maybeRebuildConnectionOnCredentialsChange(configFunction.apply(clusterAlias, settings, newSettings), connectionRefs); - } - } - } - - private void maybeRebuildConnectionOnCredentialsChange(LinkedProjectConfig config, RefCountingRunnable connectionRefs) { - final var projectId = config.originProjectId(); - final var clusterAlias = config.linkedProjectAlias(); - final var connectionsMap = getConnectionsMapForProject(projectId); - if (false == connectionsMap.containsKey(clusterAlias)) { - // A credential was added or removed before a remote connection was configured. - // Without an existing connection, there is nothing to rebuild. - logger.info( - "project [{}] no connection rebuild required for remote cluster [{}] after credentials change", - projectId, - clusterAlias - ); - return; - } - - updateRemoteCluster(config, true, ActionListener.releaseAfter(new ActionListener<>() { - @Override - public void onResponse(RemoteClusterConnectionStatus status) { - logger.info( - "project [{}] remote cluster connection [{}] updated after credentials change: [{}]", - projectId, - clusterAlias, - status - ); - } - - @Override - public void onFailure(Exception e) { - // We don't want to return an error to the upstream listener here since a connection rebuild failure - // does *not* imply a failure to reload secure settings; however, that's how it would surface in the reload-settings call. - // Instead, we log a warning which is also consistent with how we handle remote cluster settings updates (logging instead of - // returning an error) - logger.warn( - () -> "project [" - + projectId - + "] failed to update remote cluster connection [" - + clusterAlias - + "] after credentials change", - e - ); - } - }, connectionRefs.acquire())); - } - @Override public void updateLinkedProject(LinkedProjectConfig config) { final var projectId = config.originProjectId(); @@ -425,8 +351,7 @@ public void onFailure(Exception e) { * @param forceRebuild Forces an existing connection to be closed and reconnected even if the connection strategy does not require it. * @param listener The listener invoked once the configured cluster has been connected. */ - // Package-access for testing. - synchronized void updateRemoteCluster( + public synchronized void updateRemoteCluster( LinkedProjectConfig config, boolean forceRebuild, ActionListener listener @@ -468,7 +393,7 @@ synchronized void updateRemoteCluster( } } - enum RemoteClusterConnectionStatus { + public enum RemoteClusterConnectionStatus { CONNECTED, DISCONNECTED, RECONNECTED, diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 7b005b695b0af..252bd0e2d2c42 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.support.ActionTestUtils; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.action.support.RefCountingRunnable; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; @@ -1625,9 +1626,12 @@ public void testUpdateRemoteClusterCredentialsRebuildsConnectionWithCorrectProfi { final MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("cluster.remote.cluster_1.credentials", randomAlphaOfLength(10)); - final PlainActionFuture listener = new PlainActionFuture<>(); + final PlainActionFuture listener = new PlainActionFuture<>(); final Settings settings = Settings.builder().put(clusterSettings).setSecureSettings(secureSettings).build(); - service.updateRemoteClusterCredentials(() -> settings, this::buildLinkedProjectConfig, listener); + final var result = service.getRemoteClusterCredentialsManager().updateClusterCredentials(settings); + assertThat(result.addedClusterAliases(), equalTo(Set.of("cluster_1"))); + final var config = buildLinkedProjectConfig("cluster_1", Settings.EMPTY, settings); + service.updateRemoteCluster(config, true, listener); listener.actionGet(10, TimeUnit.SECONDS); } @@ -1637,13 +1641,13 @@ public void testUpdateRemoteClusterCredentialsRebuildsConnectionWithCorrectProfi ); { - final PlainActionFuture listener = new PlainActionFuture<>(); - service.updateRemoteClusterCredentials( - // Settings without credentials constitute credentials removal - () -> clusterSettings, - this::buildLinkedProjectConfig, - listener - ); + final PlainActionFuture listener = new PlainActionFuture<>(); + // Settings without credentials constitute credentials removal + final var result = service.getRemoteClusterCredentialsManager().updateClusterCredentials(clusterSettings); + assertThat(result.addedClusterAliases().size(), equalTo(0)); + assertThat(result.removedClusterAliases(), equalTo(Set.of("cluster_1"))); + final var config = buildLinkedProjectConfig("cluster_1", Settings.EMPTY, clusterSettings); + service.updateRemoteCluster(config, true, listener); listener.actionGet(10, TimeUnit.SECONDS); } @@ -1719,6 +1723,8 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite assertConnectionHasProfile(service.getRemoteClusterConnection(goodCluster), "default"); assertConnectionHasProfile(service.getRemoteClusterConnection(badCluster), "default"); expectThrows(NoSuchRemoteClusterException.class, () -> service.getRemoteClusterConnection(missingCluster)); + final Set aliases = Set.of(badCluster, goodCluster, missingCluster); + final ActionListener noop = ActionListener.noop(); { final MockSecureSettings secureSettings = new MockSecureSettings(); @@ -1731,7 +1737,14 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite .put(cluster2Settings) .setSecureSettings(secureSettings) .build(); - service.updateRemoteClusterCredentials(() -> settings, this::buildLinkedProjectConfig, listener); + final var result = service.getRemoteClusterCredentialsManager().updateClusterCredentials(settings); + assertThat(result.addedClusterAliases(), equalTo(aliases)); + try (var connectionRefs = new RefCountingRunnable(() -> listener.onResponse(null))) { + for (String alias : aliases) { + final var config = buildLinkedProjectConfig(alias, Settings.EMPTY, settings); + service.updateRemoteCluster(config, true, ActionListener.releaseAfter(noop, connectionRefs.acquire())); + } + } listener.actionGet(10, TimeUnit.SECONDS); } @@ -1748,12 +1761,16 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite { final PlainActionFuture listener = new PlainActionFuture<>(); final Settings settings = Settings.builder().put(cluster1Settings).put(cluster2Settings).build(); - service.updateRemoteClusterCredentials( - // Settings without credentials constitute credentials removal - () -> settings, - this::buildLinkedProjectConfig, - listener - ); + // Settings without credentials constitute credentials removal + final var result = service.getRemoteClusterCredentialsManager().updateClusterCredentials(settings); + assertThat(result.addedClusterAliases().size(), equalTo(0)); + assertThat(result.removedClusterAliases(), equalTo(aliases)); + try (var connectionRefs = new RefCountingRunnable(() -> listener.onResponse(null))) { + for (String alias : aliases) { + final var config = buildLinkedProjectConfig(alias, Settings.EMPTY, settings); + service.updateRemoteCluster(config, true, ActionListener.releaseAfter(noop, connectionRefs.acquire())); + } + } listener.actionGet(10, TimeUnit.SECONDS); } @@ -1854,10 +1871,7 @@ private void updateRemoteCluster( Settings newSettings, ActionListener listener ) { - final var mergedSettings = Settings.builder().put(settings, false).put(newSettings, false).build(); - @FixForMultiProject(description = "Refactor to add the linked project ID associated with the alias.") - final var config = RemoteClusterSettings.toConfig(projectResolver.getProjectId(), ProjectId.DEFAULT, alias, mergedSettings); - service.updateRemoteCluster(config, false, listener); + service.updateRemoteCluster(buildLinkedProjectConfig(alias, settings, newSettings), false, listener); } private void initializeRemoteClusters(RemoteClusterService remoteClusterService) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java index e33c6bd4f8b8d..6b9a36aca47ef 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java @@ -7,18 +7,21 @@ package org.elasticsearch.xpack.security.action.settings; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.LegacyActionRequest; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.RefCountingRunnable; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.ProjectId; +import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; @@ -51,14 +54,18 @@ public class TransportReloadRemoteClusterCredentialsAction extends TransportActi TransportReloadRemoteClusterCredentialsAction.Request, ActionResponse.Empty> { + private static final Logger logger = LogManager.getLogger(TransportReloadRemoteClusterCredentialsAction.class); + private final RemoteClusterService remoteClusterService; private final ClusterService clusterService; + private final ProjectResolver projectResolver; @Inject public TransportReloadRemoteClusterCredentialsAction( TransportService transportService, ClusterService clusterService, - ActionFilters actionFilters + ActionFilters actionFilters, + ProjectResolver projectResolver ) { super( ActionTypes.RELOAD_REMOTE_CLUSTER_CREDENTIALS_ACTION.name(), @@ -68,6 +75,7 @@ public TransportReloadRemoteClusterCredentialsAction( ); this.remoteClusterService = transportService.getRemoteClusterService(); this.clusterService = clusterService; + this.projectResolver = projectResolver; } @Override @@ -84,17 +92,84 @@ protected void doExecute(Task task, Request request, ActionListener configBuilder = (clusterAlias, staticSettings, newSettings) -> { - final var projectId = remoteClusterService.getProjectResolver().getProjectId(); - final var mergedSettings = Settings.builder().put(staticSettings, false).put(newSettings, false).build(); - return RemoteClusterSettings.toConfig(projectId, ProjectId.DEFAULT, clusterAlias, mergedSettings); - }; - remoteClusterService.updateRemoteClusterCredentials( - settingsSupplier, - configBuilder, - listener.safeMap(ignored -> ActionResponse.Empty.INSTANCE) - ); + // Synchronized on the RCS instance to match previous behavior where this functionality was in a synchronized RCS method. + synchronized (remoteClusterService) { + updateClusterCredentials(settingsSupplier, listener); + } + } + + private void updateClusterCredentials(Supplier settingsSupplier, ActionListener listener) { + final var projectId = projectResolver.getProjectId(); + final var credentialsManager = remoteClusterService.getRemoteClusterCredentialsManager(); + final var staticSettings = credentialsManager.getSettings(); + final var newSettings = settingsSupplier.get(); + final var result = credentialsManager.updateClusterCredentials(newSettings); + // We only need to rebuild connections when a credential was newly added or removed for a cluster alias, not if the credential + // value was updated. Therefore, only consider added or removed aliases + final int totalConnectionsToRebuild = result.addedClusterAliases().size() + result.removedClusterAliases().size(); + if (totalConnectionsToRebuild == 0) { + logger.debug("project [{}] no connection rebuilding required after credentials update", projectId); + listener.onResponse(ActionResponse.Empty.INSTANCE); + return; + } + logger.info("project [{}] rebuilding [{}] connections after credentials update", projectId, totalConnectionsToRebuild); + try (var connectionRefs = new RefCountingRunnable(() -> listener.onResponse(ActionResponse.Empty.INSTANCE))) { + for (var clusterAlias : result.addedClusterAliases()) { + maybeRebuildConnectionOnCredentialsChange(toConfig(projectId, clusterAlias, staticSettings, newSettings), connectionRefs); + } + for (var clusterAlias : result.removedClusterAliases()) { + maybeRebuildConnectionOnCredentialsChange(toConfig(projectId, clusterAlias, staticSettings, newSettings), connectionRefs); + } + } + } + + private void maybeRebuildConnectionOnCredentialsChange(LinkedProjectConfig config, RefCountingRunnable connectionRefs) { + final var projectId = config.originProjectId(); + final var clusterAlias = config.linkedProjectAlias(); + if (false == remoteClusterService.getRegisteredRemoteClusterNames(projectId).contains(clusterAlias)) { + // A credential was added or removed before a remote connection was configured. + // Without an existing connection, there is nothing to rebuild. + logger.info( + "project [{}] no connection rebuild required for remote cluster [{}] after credentials change", + projectId, + clusterAlias + ); + return; + } + + remoteClusterService.updateRemoteCluster(config, true, ActionListener.releaseAfter(new ActionListener<>() { + @Override + public void onResponse(RemoteClusterService.RemoteClusterConnectionStatus status) { + logger.info( + "project [{}] remote cluster connection [{}] updated after credentials change: [{}]", + projectId, + clusterAlias, + status + ); + } + + @Override + public void onFailure(Exception e) { + // We don't want to return an error to the upstream listener here since a connection rebuild failure + // does *not* imply a failure to reload secure settings; however, that's how it would surface in the reload-settings call. + // Instead, we log a warning which is also consistent with how we handle remote cluster settings updates (logging instead of + // returning an error) + logger.warn( + () -> "project [" + + projectId + + "] failed to update remote cluster connection [" + + clusterAlias + + "] after credentials change", + e + ); + } + }, connectionRefs.acquire())); + } + + @FixForMultiProject(description = "Supply the linked project ID when building the LinkedProjectConfig object.") + private LinkedProjectConfig toConfig(ProjectId projectId, String clusterAlias, Settings staticSettings, Settings newSettings) { + final var mergedSettings = Settings.builder().put(staticSettings, false).put(newSettings, false).build(); + return RemoteClusterSettings.toConfig(projectId, ProjectId.DEFAULT, clusterAlias, mergedSettings); } private ClusterBlockException checkBlock(ClusterState clusterState) { From 129ad47891cd9cc332bc39cded6d8282a5b930b5 Mon Sep 17 00:00:00 2001 From: Jeremy Dahlgren Date: Mon, 6 Oct 2025 12:11:46 -0400 Subject: [PATCH 3/3] Use clusterService.getSettings() in updateClusterCredentials() --- .../transport/RemoteClusterCredentialsManager.java | 6 ------ .../TransportReloadRemoteClusterCredentialsAction.java | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java index 9d340798923fa..33ac9718d829e 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java @@ -26,19 +26,13 @@ public class RemoteClusterCredentialsManager { private static final Logger logger = LogManager.getLogger(RemoteClusterCredentialsManager.class); - private final Settings settings; private volatile Map clusterCredentials = Collections.emptyMap(); @SuppressWarnings("this-escape") public RemoteClusterCredentialsManager(Settings settings) { - this.settings = settings; updateClusterCredentials(settings); } - public Settings getSettings() { - return settings; - } - public final synchronized UpdateRemoteClusterCredentialsResult updateClusterCredentials(Settings settings) { final Map newClusterCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings); if (clusterCredentials.isEmpty()) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java index 6b9a36aca47ef..bc3c969ef26e3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/settings/TransportReloadRemoteClusterCredentialsAction.java @@ -101,7 +101,7 @@ protected void doExecute(Task task, Request request, ActionListener settingsSupplier, ActionListener listener) { final var projectId = projectResolver.getProjectId(); final var credentialsManager = remoteClusterService.getRemoteClusterCredentialsManager(); - final var staticSettings = credentialsManager.getSettings(); + final var staticSettings = clusterService.getSettings(); final var newSettings = settingsSupplier.get(); final var result = credentialsManager.updateClusterCredentials(newSettings); // We only need to rebuild connections when a credential was newly added or removed for a cluster alias, not if the credential