Skip to content

Do not assume we hear back from all linked projects when validating resolved index expressions for CPS #137916

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+194 −11
Open

Do not assume we hear back from all linked projects when validating resolved index expressions for CPS #137916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1581480
Do not assume we hear back from all linked projects when validating r…
pawankartik-elastic Nov 11, 2025
4d0428a
Add test cases with project routing
pawankartik-elastic Nov 11, 2025
c48dec5
Update docs/changelog/137916.yaml
pawankartik-elastic Nov 11, 2025
10dc85e
Merge branch 'main' into pkar/cps-validate-no-response-linked-project
pawankartik-elastic Nov 12, 2025
3162119
`rm` TODO in `_resolve/index`
pawankartik-elastic Nov 12, 2025
0fbe3f0
Merge branch 'main' into pkar/cps-validate-no-response-linked-project
pawankartik-elastic Nov 26, 2025
3f0f215
Merge branch 'main' into pkar/cps-validate-no-response-linked-project
pawankartik-elastic Nov 26, 2025
7d67fb8
Merge branch 'main' into pkar/cps-validate-no-response-linked-project
pawankartik-elastic Nov 28, 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
6 changes: 6 additions & 0 deletions docs/changelog/137916.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 137916
summary: Do not assume we hear back from all linked projects when validating resolved
index expressions for CPS
area: CCS
type: enhancement
issues: []
10 changes: 0 additions & 10 deletions server/src/main/java/org/elasticsearch/action/admin/indices/resolve/ResolveIndexAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,16 +635,6 @@ protected void doExecute(Task task, Request request, final ActionListener<Respon
final Runnable terminalHandler = () -> {
if (completionCounter.countDown()) {
if (resolveCrossProject) {
// TODO temporary fix: we need to properly handle the case where a remote does not return a result due to
// a failure -- in the current version of resolve indices though, these are just silently ignored
if (remoteRequests != remoteResponses.size()) {
listener.onFailure(
new IllegalStateException(
"expected [" + remoteRequests + "] remote responses but got only [" + remoteResponses.size() + "]"
)
);
return;
}
final Exception ex = CrossProjectIndexResolutionValidator.validate(
originalIndicesOptions,
request.getProjectRouting(),
Expand Down
11 changes: 10 additions & 1 deletion ...main/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,16 @@ private static ElasticsearchException checkSingleRemoteExpression(
IndicesOptions indicesOptions
) {
ResolvedIndexExpressions resolvedExpressionsInProject = remoteResolvedExpressions.get(projectAlias);
assert resolvedExpressionsInProject != null : "We should always have resolved expressions from linked project";
/*
* We look for an index in the linked projects only after we've ascertained that it does not exist
* on the origin. However, if we couldn't find a valid entry for the same index in the resolved
* expressions `Map<K,V>` from the linked projects, it could mean that we did not hear back from
* the linked project due to some error that occurred on it. In such case, the scenario effectively
* is identical to the one where we could not find an index anywhere.
*/
if (resolvedExpressionsInProject == null) {
return new IndexNotFoundException(remoteExpression);
}

ResolvedIndexExpression.LocalExpressions matchingExpression = findMatchingExpression(resolvedExpressionsInProject, resource);
if (matchingExpression == null) {
Expand Down
178 changes: 178 additions & 0 deletions ...java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,184 @@ public void testMissingFlatExpressionWithStrictIgnoreUnavailable() {
assertThat(e.getMessage(), containsString("no such index [logs]"));
}

public void testMissingResponseFromLinkedProjectsWithStrictIgnoreUnavailable() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"logs",
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE,
null
),
Set.of("P1:logs")
)
)
);

// logs does not exist in the remote responses and indices options are strict. We expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, Map.of());
assertNotNull(e);
assertThat(e, instanceOf(IndexNotFoundException.class));
assertThat(e.getMessage(), containsString("no such index [logs]"));
}

public void testMissingResponseFromLinkedProjectsWithLenientIgnoreUnavailable() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"logs",
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE,
null
),
Set.of("P1:logs")
)
)
);

// logs does not exist in the remote responses and ignore_unavailable is set to true. We do not expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getLenientIndicesOptions(), null, local, Map.of());
assertNull(e);
}

public void testMissingResponseFromLinkedProjectsWithStrictAllowNoIndices() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of("P1:logs*")
)
)
);

// Mimic no response from P1 project.
var remote = Map.of(
"P2",
new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"not-logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of("not-logs"),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of()
)
)
)
);

// Index expression is a wildcard-ed expression but the indices options are strict. We expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getStrictAllowNoIndices(), null, local, remote);
assertNotNull(e);
assertThat(e, instanceOf(IndexNotFoundException.class));
assertThat(e.getMessage(), containsString("no such index [logs*]"));
}

public void testMissingResponseFromLinkedProjectsWithLenientAllowNoIndices() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of("P1:logs*")
)
)
);

// Mimic no response from P1 project.
var remote = Map.of(
"P2",
new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"not-logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of("not-logs"),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of()
)
)
)
);

// Index expression is a wildcard-ed expression but the indices options are lenient. We do not expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getLenientIndicesOptions(), null, local, remote);
assertNull(e);
}

public void testMissingResponseFromLinkedProjectsForQualifiedExpressionWithStrictIgnoreUnavailable() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(new ResolvedIndexExpression("P1:logs", ResolvedIndexExpression.LocalExpressions.NONE, Set.of("P1:logs")))
);

// Mimic no response from P1 project.
var remote = Map.of(
"P2",
new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"not-logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of("not-logs"),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of()
)
)
)
);

// logs does not exist in the remote responses and indices options are strict. We expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote);
assertNotNull(e);
assertThat(e, instanceOf(IndexNotFoundException.class));
assertThat(e.getMessage(), containsString("no such index [P1:logs]"));
}

public void testMissingResponseFromLinkedProjectsForQualifiedExpressionWithLenientIgnoreUnavailable() {
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(new ResolvedIndexExpression("P1:logs", ResolvedIndexExpression.LocalExpressions.NONE, Set.of("P1:logs")))
);

// Mimic no response from P1 project.
var remote = Map.of(
"P2",
new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"not-logs*",
new ResolvedIndexExpression.LocalExpressions(
Set.of("not-logs"),
ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
null
),
Set.of()
)
)
)
);

// logs does not exist in the remote responses and indices options are lenient. We do not expect an error.
var e = CrossProjectIndexResolutionValidator.validate(getLenientIndicesOptions(), null, local, remote);
assertNull(e);
}
Copy link
Contributor

@quux00 quux00 Nov 11, 2025

Choose a reason for hiding this comment

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

These tests are good, but I feel like the overall change would be more convincing if changing this broke some existing test. Since it doesn't can we modify an existing test to test this scenario too?

Copy link
Contributor Author

@pawankartik-elastic pawankartik-elastic Nov 11, 2025

Choose a reason for hiding this comment

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

It never broke any existing test because in our tests, we always assumed:

  • an index exists on the origin,
  • an index exists on a linked project,
  • an index exists nowhere, or,
  • an index exists everywhere.

AFAIK, we did not account for the scenario where we never heard back from a linked project, which could trip checkSingleRemoteExpression() in production. This is why the CPS-resolve index API currently throws an error if the number of responses from the linked projects does not match the number of requests fanned out.

Copy link
Contributor Author

@pawankartik-elastic pawankartik-elastic Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why did we not account for such a scenario? One reason I can think of is that to make that happen, we'd have to make the linked project unresponsive. We currently cannot do that, and it always responds with either "there's no such index" or "there exists such an index" (which happens through SAF and ResolvedIndexExpressions).

Edit: I may have a way to mimic such scenarios. Let me give it a try tomorrow! :)

Copy link
Contributor Author

@pawankartik-elastic pawankartik-elastic Nov 12, 2025

Choose a reason for hiding this comment

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

Okay, so I managed to write a relevant test for Serverless (see the linked PR). Without these changes, the test would ideally crash because we hit the assert and in production, it'd NPE.


public void testUnauthorizedFlatExpressionWithStrictIgnoreUnavailable() {
final var exception = new ElasticsearchSecurityException("authorization errors while resolving [logs]");
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
Expand Down