Skip to content

Prohibit _reindex, _delete_by_query and _update_by_query from issuing cross-project calls #137203

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
alexey-ivanov-es merged 6 commits into from
Nov 7, 2025
Merged

Prohibit _reindex, _delete_by_query and _update_by_query from issuing cross-project calls #137203

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ddf638
ES-12969 Prohibit _reindex, _delete_by_query and _update_by_query fro…
alexey-ivanov-es Oct 27, 2025
ed101ee
[CI] Auto commit changes from spotless
Oct 27, 2025
ce97075
Merge branch 'main' into ES-12969
alexey-ivanov-es Oct 27, 2025
07643bf
Merge remote-tracking branch 'upstream/main' into ES-12969
alexey-ivanov-es Nov 5, 2025
313bed5
Address review comments
alexey-ivanov-es Nov 5, 2025
23c60f8
Merge branch 'main' into ES-12969
alexey-ivanov-es Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/reindex/src/main/java/org/elasticsearch/reindex/AbstractBaseReindexRestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ protected RestChannelConsumer doPrepareRequest(RestRequest request, NodeClient c
// Build the internal request
Request internal = setCommonOptions(request, buildRequest(request));

// Only requests supporting remote indices can have IndicesOptions allowing cross-project index expressions
assert internal.supportsRemoteIndicesSearch()
|| internal.getSearchRequest().indicesOptions().resolveCrossProjectIndexExpression() == false;

// Executes the request and waits for completion
if (request.paramAsBoolean("wait_for_completion", true)) {
Map<String, String> params = new HashMap<>();
Expand Down
20 changes: 14 additions & 6 deletions 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 @@ -479,18 +479,26 @@ private static void ensureRemoteExpressionRequireIgnoreUnavailable(IndicesOption
return;
}
if (RemoteClusterAware.isRemoteIndexName(current)) {
List<String> crossClusterIndices = new ArrayList<>();
for (int i = 0; i < expressions.length; i++) {
if (RemoteClusterAware.isRemoteIndexName(expressions[i])) {
crossClusterIndices.add(expressions[i]);
}
}
List<String> crossClusterIndices = getRemoteIndexExpressions(expressions);
throw new IllegalArgumentException(
"Cross-cluster calls are not supported in this context but remote indices were requested: " + crossClusterIndices
);
}
}

/**
* Extracts the list of remote index expressions from the given array of index expressions
*/
public static List<String> getRemoteIndexExpressions(String... expressions) {
Copy link
Contributor

@quux00 quux00 Oct 28, 2025

Choose a reason for hiding this comment

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

nit (optional): since isRemoteIndexName is in RemoteClusterAware should we move this method to be there as well? If others want to use this in the future, they might search RemoteClusterAware first and not finding it re-implement it.

List<String> crossClusterIndices = new ArrayList<>();
for (int i = 0; i < expressions.length; i++) {
if (RemoteClusterAware.isRemoteIndexName(expressions[i])) {
crossClusterIndices.add(expressions[i]);
}
}
return crossClusterIndices;
}

/**
* Translates the provided index expression into actual concrete indices, properly deduplicated.
*
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ public AbstractBulkByScrollRequest(SearchRequest searchRequest, boolean setDefau
*/
protected abstract Self self();

/**
* Whether the request supports remote indices in the search request.
*/
public boolean supportsRemoteIndicesSearch() {
Copy link
Contributor Author

@alexey-ivanov-es alexey-ivanov-es Oct 28, 2025

Choose a reason for hiding this comment

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

This method can be used in the future to prohibit remote update-by-query/delete-by-query requests in CCS by adding the following code to AbstractBulkByScrollRequest.validate:

        if (supportsRemoteIndicesSearch() == false) {
            List<String> remoteIndices = IndexNameExpressionResolver.getRemoteIndexExpressions(searchRequest.indices());
            if (remoteIndices.isEmpty() == false) {
                e = addValidationError(
                    "Cross-cluster calls are not supported in this context but remote indices were requested: " + remoteIndices,
                    e
                );
            }
        }

Copy link
Contributor Author

@alexey-ivanov-es alexey-ivanov-es Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can IndicesRequest.Replaceable.allowsRemoteIndices and IndicesRequest.Replaceable.allowsCrossProject be used for this? Currently, forDeleteByQueryRequest and UpdateByQueryRequest allowsRemoteIndices() == false, but it doesn't prohibit remote index expressions to be used there. Also, ReindexRequest is not currently IndicesRequest.Replaceable

Copy link
Contributor

@quux00 quux00 Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good question. Given that none of these Request objects implement IndicesRequest.Replaceable, this looks like a good solution. Based on git blame, it looks like AbstractBulkByScrollRequest was created about 6 months before IndicesRequest.Replaceable was created and it was never retrofitted. My guess is that we will likely need to change ReindexRequest to implement IndicesRequest.Replaceable when we enable it to work cross-project in serverless, so that it inherits the new flat world handling in the Security Action Filter. My vote would be to make a note of that somewhere - either as code comments or in some backlog ticket, rather than do that now. We can decide then whether to also add it to UpdateByQueryRequest and DeleteByQueryRequest. My default is to leave those alone since they will never work cross-project and disentangle only Reindex but we can work that out later.

@piergm - What do you think?

Copy link
Member

@piergm piergm Oct 29, 2025

Choose a reason for hiding this comment

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

Correct, IndicesRequest.Replaceable is a must to make CPS work, but that can be done in a later date.
++ on adding it to the backlog.

return false;
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException e = searchRequest.validate();
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ protected ReindexRequest self() {
return this;
}

@Override
public boolean supportsRemoteIndicesSearch() {
return true;
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException e = super.validate();
Expand Down
18 changes: 18 additions & 0 deletions ...er/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
Expand Down Expand Up @@ -3486,6 +3487,23 @@ public void testResolveWriteIndexAbstractionMultipleMatches() {
);
}

public void testGetRemoteIndexExpressions() {
{
List<String> remoteIndexExpressions = IndexNameExpressionResolver.getRemoteIndexExpressions("index-1");
assertThat(remoteIndexExpressions, empty());
}
{
List<String> remoteIndexExpressions = IndexNameExpressionResolver.getRemoteIndexExpressions(
"index-1",
"remote:index-1",
"idx-*",
"remote-2:idx-5"
);
assertThat(remoteIndexExpressions, hasSize(2));
assertThat(remoteIndexExpressions, contains("remote:index-1", "remote-2:idx-5"));
}
}

public static IndexMetadata.Builder indexBuilder(String index) {
return indexBuilder(index, Settings.EMPTY);
}
Expand Down