record index resolution for _all expressions by richard-dennehy · Pull Request #135425 · elastic/elasticsearch

richard-dennehy · 2025-09-25T10:15:38Z

Follow up to #135081

Records resolution for _all index expressions

Resolves: ES-13004

elasticsearchmachine · 2025-09-25T11:52:04Z

Pinging @elastic/es-security (Team:Security)

ywangd

I am not sure about the status of this PR. It has not implemented the CPS resolution yet. Maybe it should be draft?

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

ywangd

I had some more comments.

I think it's also possible to get a pattern like *:_all which takes the other path as opposed to isAllIndices == true. I suspect it may not work as is since indexAbstractionResolver.resolveIndexAbstractions does not handle _all. We can address it separately.

ywangd · 2025-10-10T03:14:46Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

+                var resolvedExpressionsBuilder = ResolvedIndexExpressions.builder();
+                Set<String> remoteIndices = Collections.emptySet();
+                if (crossProjectModeDecider.resolvesCrossProject(replaceable)) {
+                    remoteIndices = CrossProjectIndexExpressionsRewriter.rewriteIndexExpression(
+                        "*",
+                        authorizedProjects.originProjectAlias(),
+                        authorizedProjects.allProjectAliases()
+                    ).remoteExpressions();
+                }
+
+                resolvedExpressionsBuilder.addExpressions(
+                    indicesRequest.indices().length > 0 ? indicesRequest.indices()[0] : Metadata.ALL,
+                    localExpressions,
+                    ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
+                    remoteIndices
+                );
+                var resolved = resolvedExpressionsBuilder.build();
+
+                if (crossProjectModeDecider.crossProjectEnabled()) {
+                    setResolvedIndexExpressionsIfUnset(replaceable, resolved);
+                }
+                resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList());


A couple comments:

It needs to be moved inside the above if (indicesOptions.expandWildcardExpressions()) { block since there is no need to resolve CPS if something like _all is used along with expandWildcards=false, i.e. it resolves to nothing everywhere so that we can short circuit

The first argument to CrossProjectIndexExpressionsRewriter.rewriteIndexExpression should use the actual pattern passed in, i.e. something like indicesRequest.indices() == null || indicesRequest.indices().isEmpty() ? "_all" : indicesRequest.indices()[0]. This should ensure the any selector pattern is included and the current code should throw an exception for them on rewriting as intended.

Code related to CPS is better all put inside the if (crossProjectModeDecider.resolvesCrossProject(replaceable)) { block so that it is clear they do not apply when the request is not CPS relevant. setResolvedIndexExpressionsIfUnset is an exception since it is already wrapped with CPS enabled check.

For 3 - I don't think there's anything I can move; line 395 needs to always happen, to match the existing behaviour - this means everything else currently outside the CPS branch needs to stay out of that branch

I should have been more specific. I had performance in mind. The index resolution can be performance sensitive when a cluster has a large number of indices.

The original code adds each resolved index inline with resolvedIndicesBuilder.addLocal thus build the list of indices only once.

The proposed change builds a HashSet first. It then builds a List (getLocalIndicesList) and finally calls resolvedIndicesBuilder.addLocal(List) which internally builds yet another list. So I was thinking whether we could separate the CPS path a bit more from the current code path so that it does not impact current production. That said, we can defer this as a follow-up.

...ity/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java

ywangd

I think this is almost there for a reasonable first version. Could you please raise a linked serverless side PR to augument the REST test CrossProjectSearchRestIT#testCrossProjectResolveIndexWithUniversalApiKey for the change here?

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

ywangd · 2025-10-13T02:16:05Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

+                var resolvedExpressionsBuilder = ResolvedIndexExpressions.builder();
+                Set<String> remoteIndices = Collections.emptySet();
+                if (crossProjectModeDecider.resolvesCrossProject(replaceable)) {
+                    remoteIndices = CrossProjectIndexExpressionsRewriter.rewriteIndexExpression(
+                        "*",
+                        authorizedProjects.originProjectAlias(),
+                        authorizedProjects.allProjectAliases()
+                    ).remoteExpressions();
+                }
+
+                resolvedExpressionsBuilder.addExpressions(
+                    indicesRequest.indices().length > 0 ? indicesRequest.indices()[0] : Metadata.ALL,
+                    localExpressions,
+                    ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS,
+                    remoteIndices
+                );
+                var resolved = resolvedExpressionsBuilder.build();
+
+                if (crossProjectModeDecider.crossProjectEnabled()) {
+                    setResolvedIndexExpressionsIfUnset(replaceable, resolved);
+                }
+                resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList());


I should have been more specific. I had performance in mind. The index resolution can be performance sensitive when a cluster has a large number of indices.

The original code adds each resolved index inline with resolvedIndicesBuilder.addLocal thus build the list of indices only once.

The proposed change builds a HashSet first. It then builds a List (getLocalIndicesList) and finally calls resolvedIndicesBuilder.addLocal(List) which internally builds yet another list. So I was thinking whether we could separate the CPS path a bit more from the current code path so that it does not impact current production. That said, we can defer this as a follow-up.

…esolution

ywangd · 2025-10-14T02:11:40Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

+                    if (crossProjectModeDecider.crossProjectEnabled()) {
+                        setResolvedIndexExpressionsIfUnset(replaceable, resolved);
+                    }
+                    resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList());


We need to call resolvedIndicesBuilder.addRemote(resolved.getRemoteIndicesList()) as well to ensure remote indices are not lost after rewriting.

ywangd

LGTM

Please make sure CI is green before merging. Thanks!

* record index resolution for _all expressions * [CI] Auto commit changes from spotless * cleanup * fix compile * include remote expressions in _all index resolution * address review comments * clean up tests * [CI] Auto commit changes from spotless * address review comments * [CI] Auto commit changes from spotless * update remote indices list * use order-insensitive assertion --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nikolaj Volgushev <n1v0lg@users.noreply.github.com>

Follow up to #135425 As identified, expressions such as `*:_all` currently don't resolve correctly (it appears that it currently tries to authorize the user against the literal index "_all"). This PR fixes the resolution of prefixed `_all` expressions. Relates: ES-13213

Follow up to elastic#135425 As identified, expressions such as `*:_all` currently don't resolve correctly (it appears that it currently tries to authorize the user against the literal index "_all"). This PR fixes the resolution of prefixed `_all` expressions. Relates: ES-13213

record index resolution for _all expressions

edca92a

richard-dennehy added >non-issue :Security/Security Security issues without another label Team:Security Meta label for security team labels Sep 25, 2025

elasticsearchmachine added the v9.2.0 label Sep 25, 2025

elasticsearchmachine and others added 2 commits September 25, 2025 10:31

[CI] Auto commit changes from spotless

5828ee1

cleanup

ca80d6e

richard-dennehy marked this pull request as ready for review September 25, 2025 11:51

elasticsearchmachine added v9.3.0 and removed v9.2.0 labels Oct 2, 2025

n1v0lg and others added 2 commits October 6, 2025 16:54

Merge branch 'main' into record-_all-index-resolution

c378fb2

fix compile

9162b46

n1v0lg requested a review from ywangd October 7, 2025 16:14

ywangd reviewed Oct 8, 2025

View reviewed changes

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java Outdated Show resolved Hide resolved

...security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java Outdated Show resolved Hide resolved

include remote expressions in _all index resolution

8df95be

ywangd reviewed Oct 10, 2025

View reviewed changes

richard-dennehy and others added 3 commits October 10, 2025 15:07

address review comments

3fbcd81

clean up tests

102bf4d

[CI] Auto commit changes from spotless

079fe33

ywangd reviewed Oct 13, 2025

View reviewed changes

richard-dennehy and others added 3 commits October 13, 2025 10:09

address review comments

424b73b

Merge remote-tracking branch 'upstream/main' into record-_all-index-r…

81b6df1

…esolution

[CI] Auto commit changes from spotless

505d692

elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Oct 13, 2025

ywangd reviewed Oct 14, 2025

View reviewed changes

update remote indices list

612755c

ywangd approved these changes Oct 15, 2025

View reviewed changes

richard-dennehy added 2 commits October 15, 2025 11:35

Merge branch 'main' into record-_all-index-resolution

9eefe44

use order-insensitive assertion

7ec2bb0

richard-dennehy merged commit ec2b7f0 into elastic:main Oct 15, 2025
40 checks passed

richard-dennehy mentioned this pull request Oct 29, 2025

resolve indices for prefixed _all expressions #137330

Merged

Conversation

richard-dennehy commented Sep 25, 2025 • edited by ywangd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 25, 2025

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ywangd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

richard-dennehy Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ywangd Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

richard-dennehy commented Sep 25, 2025 •

edited by ywangd

Loading

ywangd left a comment •

edited

Loading

ywangd left a comment •

edited

Loading