diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index 419aa189e352c..e7a70e4f34085 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; @@ -34,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; @@ -50,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; @@ -106,6 +103,10 @@ public boolean isRemoteClusterServerEnabled() { this.crossProjectEnabled = settings.getAsBoolean("serverless.cross_project.enabled", false); } + public RemoteClusterCredentialsManager getRemoteClusterCredentialsManager() { + return remoteClusterCredentialsManager; + } + /** * Group indices by cluster alias mapped to OriginalIndices for that cluster. * @param remoteClusterNames Set of configured remote cluster names. @@ -185,6 +186,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 * @@ -298,79 +306,6 @@ public void skipUnavailableChanged( } } - @FixForMultiProject(description = "Refactor as needed to support project specific changes to linked remotes.") - public synchronized void updateRemoteClusterCredentials(Supplier settingsSupplier, ActionListener listener) { - final var projectId = projectResolver.getProjectId(); - final Settings settings = settingsSupplier.get(); - final UpdateRemoteClusterCredentialsResult result = remoteClusterCredentialsManager.updateClusterCredentials(settings); - // 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(projectId, clusterAlias, settings, connectionRefs); - } - for (var clusterAlias : result.removedClusterAliases()) { - maybeRebuildConnectionOnCredentialsChange(projectId, clusterAlias, settings, connectionRefs); - } - } - } - - private void maybeRebuildConnectionOnCredentialsChange( - ProjectId projectId, - String clusterAlias, - Settings newSettings, - RefCountingRunnable connectionRefs - ) { - 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; - } - - 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) { - 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(); @@ -412,8 +347,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 @@ -455,7 +389,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 709d2c791b114..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, 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,12 +1641,13 @@ public void testUpdateRemoteClusterCredentialsRebuildsConnectionWithCorrectProfi ); { - final PlainActionFuture listener = new PlainActionFuture<>(); - service.updateRemoteClusterCredentials( - // Settings without credentials constitute credentials removal - () -> clusterSettings, - 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); } @@ -1718,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(); @@ -1730,7 +1737,14 @@ public void testUpdateRemoteClusterCredentialsRebuildsMultipleConnectionsDespite .put(cluster2Settings) .setSecureSettings(secureSettings) .build(); - service.updateRemoteClusterCredentials(() -> settings, 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); } @@ -1747,11 +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, - 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); } @@ -1828,6 +1847,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, @@ -1846,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 5f5b082dee2ce..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 @@ -7,23 +7,31 @@ 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.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; @@ -46,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(), @@ -63,6 +75,7 @@ public TransportReloadRemoteClusterCredentialsAction( ); this.remoteClusterService = transportService.getRemoteClusterService(); this.clusterService = clusterService; + this.projectResolver = projectResolver; } @Override @@ -79,7 +92,84 @@ protected void doExecute(Task task, Request request, ActionListener 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 = 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 + // 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) {