Skip to content

Handle the indices pattern ["*", "-*"] when grouping indices by cluster name #128610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 3, 2025
Merged

Handle the indices pattern ["*", "-*"] when grouping indices by cluster name #128610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b3c3f9d
Handle the indices pattern `["*", "-*"]` when grouping indices by clu…
pawankartik-elastic May 29, 2025
cd388ae
Apply review comments
pawankartik-elastic Jun 2, 2025
d29a9c3
Drop redundant code comment
pawankartik-elastic Jun 2, 2025
7a76fca
Merge branch 'main' into pkar/detect-no-indices-aliases-pattern
pawankartik-elastic Jun 2, 2025
bac027a
Update docs/changelog/128610.yaml
pawankartik-elastic Jun 2, 2025
00020a3
Apply review comments
pawankartik-elastic Jun 3, 2025
07449bb
Merge branch 'main' into pkar/detect-no-indices-aliases-pattern
pawankartik-elastic Jun 3, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/128610.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 128610
summary: "Handle the indices pattern `[\"*\", \"-*\"]` when grouping indices by cluster\
\ name"
area: CCS
type: bug
issues: []
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,7 +1433,7 @@ static <T> boolean isExplicitAllPattern(Collection<T> 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]);
}

Expand Down
19 changes: 18 additions & 1 deletion server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -201,7 +202,23 @@ public Map<String, OriginalIndices> groupIndices(
boolean returnLocalAll
) {
final Map<String, OriginalIndices> originalIndicesMap = new HashMap<>();
final Map<String, List<String>> groupedIndices = groupClusterIndices(remoteClusterNames, indices);
final Map<String, List<String>> 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps add a comment here, that explains the semantics of returnLocalAll and where it's used set to false.

I wonder if in the longer run, we should consider making resolve cluster go through a specific method that resolves clusters instead of indices, and remove this flag that makes things a little difficult to read. What do you think? Do we ever consume the indices names (values in the map) or only the cluster aliases (keys in the map)?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we don't return the indices names directly, the _resolve/cluster call's response has a key matching_indices that tells if the specified cluster has the specified index.
groupedIndices = Map.of();
} else {
groupedIndices = groupClusterIndices(remoteClusterNames, indices);
}

if (groupedIndices.isEmpty()) {
if (returnLocalAll) {
// search on _all in the local cluster if neither local indices nor remote indices were specified
Expand Down
24 changes: 9 additions & 15 deletions ...java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1ResolveClusterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> responseMap = responseAsMap(response);
// assertThat(responseMap.get(LOCAL_CLUSTER_NAME_REPRESENTATION), nullValue());
// Map<String, Object> remoteMap = (Map<String, Object>) 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<String, Object> responseMap = responseAsMap(response);
assertThat(responseMap.isEmpty(), is(true));
}
}

private Response performRequestWithRemoteSearchUser(final Request request) throws IOException {
Expand Down
22 changes: 9 additions & 13 deletions ...java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2ResolveClusterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> responseMap = responseAsMap(response);
// assertThat(responseMap.get(LOCAL_CLUSTER_NAME_REPRESENTATION), nullValue());
// Map<String, Object> remoteMap = (Map<String, Object>) 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<String, Object> responseMap = responseAsMap(response);
assertThat(responseMap.isEmpty(), is(true));
}
}

private Response performRequestWithRemoteSearchUser(final Request request) throws IOException {
Expand Down
Loading