Skip to content

Commit 49a341b

Browse files
Address review comments
1 parent fed05cb commit 49a341b

File tree

3 files changed

+24
-18
lines changed

3 files changed

+24
-18
lines changed

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,15 @@ private void executeRequest(
379379
final ResolvedIndices resolvedIndices;
380380

381381
/*
382-
* Irrespective of whether this is the origin project or a linked project, it's wiser to relax
383-
* the index options to prevent the index resolution APIs from throwing index not found errors.
384-
* Also, we do not replace the indices options on the SearchRequest because we'd be needing it
385-
* downstream when validating the indices from both the origin and the linked projects.
382+
* If this search request is originating from a search endpoint that is CPS compatible, and,
383+
* CPS is enabled, i.e. resolvesCrossProject() returns true, we need to modify and relax the
384+
* indices options. This is to:
385+
* a) Prevent the downstream code from throwing an error if an index is not found since in
386+
* CPS, an index can exist either on the origin or on different linked project(s).
387+
* b) Prevent the linked projects from re-interpreting the index expressions as CPS expressions
388+
* and rather treat them as canonical/standard ones.
386389
*/
387-
IndicesOptions resolutionIdxOpts = crossProjectModeDecider.resolvesCrossProject(original)
390+
IndicesOptions resolutionIdxOpts = resolvesCrossProject
388391
? indicesOptionsForCrossProjectFanout(original.indicesOptions())
389392
: original.indicesOptions();
390393

@@ -964,8 +967,9 @@ static SearchResponseMerger createSearchResponseMerger(
964967
}
965968

966969
/**
967-
* Outside Cross Project Search, we're sure of projects involved and their corresponding indices. However,
968-
* in CPS, it may be possible that indices can exist anywhere:
970+
* Reconciliation is only done when Cross Project Search is enabled and requests originate from a CPS
971+
* compatible endpoints. Outside CPS, we're sure of projects involved and their corresponding indices.
972+
* However, in CPS, it may be possible that indices can exist anywhere:
969973
* <ul>
970974
* <li>Only on the origin</li>
971975
* <li>Only on the linked project(s)</li>
@@ -982,7 +986,7 @@ static SearchResponseMerger createSearchResponseMerger(
982986
* validation is complete.
983987
* @param originResolvedIdxExpressions The resolution result from origin's Security Action Filter.
984988
* @param shardResponses Responses pieced back from SearchShards API.
985-
* @param projects Clusters object to build upon.
989+
* @param projects The clusters originally in scope for the query.
986990
* @return A new Clusters object containing only the Search-participating projects.
987991
*/
988992
static SearchResponse.Clusters reconcileProjects(
@@ -991,7 +995,7 @@ static SearchResponse.Clusters reconcileProjects(
991995
SearchResponse.Clusters projects
992996
) {
993997
/*
994-
* We only fire a SearchShards API for a project if it needs to be searched. This can either mean that it was
998+
* We only fire a SearchShards API call for a project if it needs to be searched. This can either mean that it was
995999
* part of the search due to the flatworld behaviour, or that it was targeted specifically. If it returns an
9961000
* empty response, it's because the project does not host any of our specified indices.
9971001
*/
@@ -1092,7 +1096,7 @@ Map<String, SearchShardsResponse> createFinalResponse() {
10921096
}
10931097
resolvedIndexExpressions.put(entry.getKey(), entry.getValue().getResolvedIndexExpressions());
10941098
}
1095-
// We do not use the related index options here when validating indices' existence.
1099+
// We do not use the relaxed index options here when validating indices' existence.
10961100
ElasticsearchException validationEx = CrossProjectIndexResolutionValidator.validate(
10971101
originalIdxOpts,
10981102
originResolvedIdxExpressions,

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.rest.ServerlessScope;
1717
import org.elasticsearch.rest.action.RestCancellableNodeClient;
1818
import org.elasticsearch.rest.action.RestRefCountedChunkedToXContentListener;
19+
import org.elasticsearch.search.crossproject.CrossProjectModeDecider;
1920
import org.elasticsearch.usage.SearchUsageHolder;
2021
import org.elasticsearch.xpack.core.search.action.AsyncSearchResponse;
2122
import org.elasticsearch.xpack.core.search.action.SubmitAsyncSearchAction;
@@ -38,11 +39,7 @@ public final class RestSubmitAsyncSearchAction extends BaseRestHandler {
3839

3940
private final SearchUsageHolder searchUsageHolder;
4041
private final Predicate<NodeFeature> clusterSupportsFeature;
41-
private final Settings settings;
42-
43-
public RestSubmitAsyncSearchAction(SearchUsageHolder searchUsageHolder, Predicate<NodeFeature> clusterSupportsFeature) {
44-
this(searchUsageHolder, clusterSupportsFeature, null);
45-
}
42+
private final CrossProjectModeDecider crossProjectModeDecider;
4643

4744
public RestSubmitAsyncSearchAction(
4845
SearchUsageHolder searchUsageHolder,
@@ -51,7 +48,7 @@ public RestSubmitAsyncSearchAction(
5148
) {
5249
this.searchUsageHolder = searchUsageHolder;
5350
this.clusterSupportsFeature = clusterSupportsFeature;
54-
this.settings = settings;
51+
this.crossProjectModeDecider = new CrossProjectModeDecider(settings);
5552
}
5653

5754
@Override
@@ -71,7 +68,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
7168
}
7269
SubmitAsyncSearchRequest submit = new SubmitAsyncSearchRequest();
7370

74-
boolean crossProjectEnabled = settings != null && settings.getAsBoolean("serverless.cross_project.enabled", false);
71+
boolean crossProjectEnabled = crossProjectModeDecider.crossProjectEnabled();
7572
if (crossProjectEnabled) {
7673
// accept but drop project_routing param until fully supported
7774
request.param("project_routing");

x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchActionTests.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.lucene.util.SetOnce;
1010
import org.elasticsearch.common.bytes.BytesArray;
11+
import org.elasticsearch.common.settings.Settings;
1112
import org.elasticsearch.common.util.concurrent.ThreadContext;
1213
import org.elasticsearch.core.TimeValue;
1314
import org.elasticsearch.rest.RestRequest;
@@ -31,7 +32,11 @@ public class RestSubmitAsyncSearchActionTests extends RestActionTestCase {
3132

3233
@Before
3334
public void setUpAction() {
34-
RestSubmitAsyncSearchAction action = new RestSubmitAsyncSearchAction(new UsageService().getSearchUsageHolder(), nf -> false);
35+
RestSubmitAsyncSearchAction action = new RestSubmitAsyncSearchAction(
36+
new UsageService().getSearchUsageHolder(),
37+
nf -> false,
38+
Settings.EMPTY
39+
);
3540
controller().registerHandler(action);
3641
}
3742

0 commit comments

Comments
 (0)