From 71a69cbed934e41dd6b6a2f03e1534a65f4356e0 Mon Sep 17 00:00:00 2001 From: Pawan Kartik Date: Tue, 3 Jun 2025 13:51:36 +0100 Subject: [PATCH] Handle the indices pattern `["*", "-*"]` when grouping indices by cluster name (#128610) The auth code injects the pattern `["*", "-*"]` to specify that it's okay to return an empty response because user's patterns did not match any remote clusters. However, we fail to recognise this specific pattern and `groupIndices()` eventually associates it with the local cluster. This causes Elasticsearch to fallback to the local cluster unknowingly and return its details back to the user even though the user hasn't requested any info about the local cluster. (cherry picked from commit 3f1e1b3c305c8e3597326104792dcb53714be30a) # Conflicts: # server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java --- docs/changelog/128610.yaml | 6 +++++ .../metadata/IndexNameExpressionResolver.java | 2 +- .../transport/RemoteClusterService.java | 19 ++++++++++++++- ...teClusterSecurityRCS1ResolveClusterIT.java | 24 +++++++------------ ...teClusterSecurityRCS2ResolveClusterIT.java | 22 +++++++---------- 5 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 docs/changelog/128610.yaml diff --git a/docs/changelog/128610.yaml b/docs/changelog/128610.yaml new file mode 100644 index 0000000000000..7ee83973f77c2 --- /dev/null +++ b/docs/changelog/128610.yaml @@ -0,0 +1,6 @@ +pr: 128610 +summary: "Handle the indices pattern `[\"*\", \"-*\"]` when grouping indices by cluster\ + \ name" +area: CCS +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index d28049f2a6316..afa2332be7cbc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -1246,7 +1246,7 @@ static boolean isExplicitAllPattern(Collection aliasesOrIndices, Function /** * Identifies if this expression list is *,-* which effectively means a request that requests no indices. */ - static boolean isNoneExpression(String[] expressions) { + public static boolean isNoneExpression(String[] expressions) { return expressions.length == 2 && "*".equals(expressions[0]) && "-*".equals(expressions[1]); } diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index 60c295ccb4f5a..81b9b822bc860 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -19,6 +19,7 @@ import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.RefCountingRunnable; import org.elasticsearch.client.internal.RemoteClusterClient; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.common.Strings; @@ -195,7 +196,23 @@ boolean isRemoteNodeConnected(final String remoteCluster, final DiscoveryNode no */ public Map groupIndices(IndicesOptions indicesOptions, String[] indices, boolean returnLocalAll) { final Map originalIndicesMap = new HashMap<>(); - final Map> groupedIndices = groupClusterIndices(getRemoteClusterNames(), indices); + final Map> groupedIndices; + /* + * returnLocalAll is used to control whether we'd like to fallback to the local cluster. + * While this is acceptable in a few cases, there are cases where we should not fallback to the local + * cluster. Consider _resolve/cluster where the specified patterns do not match any remote clusters. + * Falling back to the local cluster and returning its details in such cases is not ok. This is why + * TransportResolveClusterAction sets returnLocalAll to false wherever it uses groupIndices(). + * + * If such a fallback isn't allowed and the given indices match a pattern whose semantics mean that + * it's ok to return an empty result (denoted via ["*", "-*"]), empty groupIndices. + */ + if (returnLocalAll == false && IndexNameExpressionResolver.isNoneExpression(indices)) { + groupedIndices = Map.of(); + } else { + groupedIndices = groupClusterIndices(getRemoteClusterNames(), indices); + } + if (groupedIndices.isEmpty()) { if (returnLocalAll) { // search on _all in the local cluster if neither local indices nor remote indices were specified diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1ResolveClusterIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1ResolveClusterIT.java index dabda4a216dc7..682a2db0cd8a6 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1ResolveClusterIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1ResolveClusterIT.java @@ -281,21 +281,15 @@ public void testResolveClusterUnderRCS1() throws Exception { ); assertThat(exc.getMessage(), containsString(indexOptionTuple.v1())); } - // TODO: The security pathways are not using the new - // RemoteClusterService.groupIndices(IndicesOptions indicesOptions, String[] indices, boolean returnLocalAll) method - // so this use case still behaves badly - fix in follow on PR - // { - // // TEST CASE 13: Resolution against wildcarded remote cluster expression that matches no remotes - // final Request remoteOnly1 = new Request("GET", "_resolve/cluster/no_such_remote*:*"); - // Response response = performRequestWithRemoteSearchUser(remoteOnly1); - // assertOK(response); - // Map responseMap = responseAsMap(response); - // assertThat(responseMap.get(LOCAL_CLUSTER_NAME_REPRESENTATION), nullValue()); - // Map remoteMap = (Map) responseMap.get("my_remote_cluster"); - // assertThat((Boolean) remoteMap.get("connected"), equalTo(true)); - // assertThat((Boolean) remoteMap.get("matching_indices"), equalTo(false)); - // assertThat(remoteMap.get("version"), notNullValue()); - // } + { + // TEST CASE 13: Resolution against wildcarded remote cluster expression that matches no remotes should result in an + // empty response and not fall back to the local cluster. + final Request remoteOnly1 = new Request("GET", "_resolve/cluster/no_such_remote*:*"); + Response response = performRequestWithRemoteSearchUser(remoteOnly1); + assertOK(response); + Map responseMap = responseAsMap(response); + assertThat(responseMap.isEmpty(), is(true)); + } } private Response performRequestWithRemoteSearchUser(final Request request) throws IOException { diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2ResolveClusterIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2ResolveClusterIT.java index fab49c9c2932d..8b3185bb7757b 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2ResolveClusterIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2ResolveClusterIT.java @@ -376,19 +376,15 @@ public void testResolveCluster() throws Exception { ); assertThat(exc.getMessage(), containsString(indexOptionTuple.v1())); } - // TODO: fix this in a follow-on PR - // { - // // TEST CASE 13: Resolution against wildcarded remote cluster expression that matches no remotes - // final Request remoteOnly1 = new Request("GET", "_resolve/cluster/no_such_remote*:*"); - // Response response = performRequestWithRemoteSearchUser(remoteOnly1); - // assertOK(response); - // Map responseMap = responseAsMap(response); - // assertThat(responseMap.get(LOCAL_CLUSTER_NAME_REPRESENTATION), nullValue()); - // Map remoteMap = (Map) responseMap.get("my_remote_cluster"); - // assertThat((Boolean) remoteMap.get("connected"), equalTo(true)); - // assertThat((Boolean) remoteMap.get("matching_indices"), equalTo(false)); - // assertThat(remoteMap.get("version"), notNullValue()); - // } + { + // TEST CASE 13: Resolution against wildcarded remote cluster expression that matches no remotes should result in an + // empty response and not fall back to the local cluster. + final Request remoteOnly1 = new Request("GET", "_resolve/cluster/no_such_remote*:*"); + Response response = performRequestWithRemoteSearchUser(remoteOnly1); + assertOK(response); + Map responseMap = responseAsMap(response); + assertThat(responseMap.isEmpty(), is(true)); + } } private Response performRequestWithRemoteSearchUser(final Request request) throws IOException {