Prohibit _reindex, _delete_by_query and _update_by_query from issuing cross-project calls #137203
Conversation
…m issuing cross-project calls
elasticsearchmachine
added
the
serverless-linked
| /** | ||
| * Whether the request supports remote indices in the search request. | ||
| */ | ||
| public boolean supportsRemoteIndicesSearch() { |
There was a problem hiding this comment.
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
);
}
}
alexey-ivanov-es
added
:Distributed Indexing/Reindex
elasticsearchmachine
added
Team:Search Foundations
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
|
Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing) |
| /** | ||
| * Whether the request supports remote indices in the search request. | ||
| */ | ||
| public boolean supportsRemoteIndicesSearch() { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
quux00
left a comment
quux00
left a comment
There was a problem hiding this comment.
Looks good!
One optional suggestion left.
| /** | ||
| * Extracts the list of remote index expressions from the given array of index expressions | ||
| */ | ||
| public static List<String> getRemoteIndexExpressions(String... expressions) { |
There was a problem hiding this comment.
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.
BASE=7e531067b312877ef94cd9cebdca59bcd4951db1 HEAD=23c60f89d411f813a796695fe86322c54a483f73 Branch=main
BASE=7e531067b312877ef94cd9cebdca59bcd4951db1 HEAD=23c60f89d411f813a796695fe86322c54a483f73 Branch=main
… cross-project calls (elastic#137203) This PR introduces an assertion to make sure that delete-by-query/update-by-query can't have indices options enabling flat-world, and also extracts the method to find cross-project/cross-cluster index expressions from a list of index expressions that is used in ActionFilters to disable cross-project requests in Serverless. This change doesn't prohibit cross-cluster update-by-query/delete-by-query requests even though these requests were never supposed to work in CCS, since doing so could lead to a breaking change: elastic/dev#3350 ES-12969
4 participants
This PR introduces an assertion to make sure that delete-by-query/update-by-query can't have indices options enabling flat-world, and also extracts the method to find cross-project/cross-cluster index expressions from a list of index expressions that is used in
ActionFilters to disable cross-project requests in Serverless.This change doesn't prohibit cross-cluster update-by-query/delete-by-query requests even though these requests were never supposed to work in CCS, since doing so could lead to a breaking change: https://github.com/elastic/dev/issues/3350
ES-12969