Skip to content

Support index and project exclusions for CPS #139230

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

Jump to bottom
Open
ywangd wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Support index and project exclusions for CPS #139230

ywangd wants to merge 6 commits into from
+500 −93

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Dec 9, 2025

Add general support for index and project exclusions for the CPS index resolution process. Exclusion follows the same flat-world resolution model so that an expression such as -foo exclude foo from matching projects.

Note it currently throws exception on following cases:

  1. Double exclusion on project and index expression (new rule)
  2. Unmatched project in both exclusion and inclusion (new and existing rule)
  3. Excluding a project without including it first (existing rule)
  4. Project exclusion results into empty (existing rule)
  5. Project exclusion with index expression other than * (existing rule)

Other than the first case, we might want to discuss relaxing other cases in the future.

Relates: ES-12692

@ywangd ywangd added >non-issue :Security/Security Security issues without another label v9.3.0 labels Dec 9, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Dec 9, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Dec 9, 2025

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

ywangd
ywangd commented Dec 9, 2025
View reviewed changes
...rc/main/java/org/elasticsearch/search/crossproject/CrossProjectIndexExpressionsRewriter.java
Comment on lines -53 to -54
// TODO remove me: only used in tests
public static Map<String, IndexRewriteResult> rewriteIndexExpressions(
Copy link
Member Author

@ywangd ywangd Dec 9, 2025

Choose a reason for hiding this comment

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

Moved this method to the corresponding test since it's test only.

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Dec 9, 2025
@idegtiarenko idegtiarenko mentioned this pull request Dec 9, 2025
Merged
idegtiarenko
idegtiarenko reviewed Dec 9, 2025
View reviewed changes
...st/java/org/elasticsearch/search/crossproject/CrossProjectIndexExpressionsRewriterTests.java Outdated
// Cannot apply exclusion for both the project and the index
expectThrows(IllegalArgumentException.class, () -> rewriteIndexExpressions(origin, linked, "-_origin:-metrics*"));
expectThrows(IllegalArgumentException.class, () -> rewriteIndexExpressions(origin, linked, "-P0:-metrics*"));
expectThrows(IllegalArgumentException.class, () -> rewriteIndexExpressions(origin, linked, "-P1:-metrics*"));
Copy link
Contributor

@idegtiarenko idegtiarenko Dec 9, 2025

Choose a reason for hiding this comment

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

NIT: Consider using org.elasticsearch.test.ESTestCase#expectThrows that can assert error message as well

Copy link
Member Author

@ywangd ywangd Dec 9, 2025

Choose a reason for hiding this comment

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

TIL. Pushed 78d2d86

@idegtiarenko
Copy link
Contributor

idegtiarenko commented Dec 9, 2025

Is this updating only CPS resolvedTo data structure?

@ywangd
Copy link
Member Author

ywangd commented Dec 9, 2025

Is this updating only CPS resolvedTo data structure?

It should be reflected in both IndicesRequest#indices() and Replaceable#getResolvedIndexExpressions(). I believe the later is what you referred as resolvedTo for FieldCaps.

@idegtiarenko
Copy link
Contributor

idegtiarenko commented Dec 11, 2025

Is this updating only CPS resolvedTo data structure?

It should be reflected in both IndicesRequest#indices() and Replaceable#getResolvedIndexExpressions(). I believe the later is what you referred as resolvedTo for FieldCaps.

Is it going to be reflected when running in non CPS mode?

@ywangd
Copy link
Member Author

ywangd commented Dec 11, 2025

Is it going to be reflected when running in non CPS mode?

If CPS is disabled, ResolvedIndexExpressions won't be populated and is always null. It might change in future. But we haven't decided on that yet. It will always be reflected in IndicesRequest#indices() regardless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@idegtiarenko idegtiarenko idegtiarenko left review comments

@quux00 quux00 Awaiting requested review from quux00

@richard-dennehy richard-dennehy Awaiting requested review from richard-dennehy

@piergm piergm Awaiting requested review from piergm

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

>non-issue :Security/Security Security issues without another label serverless-linked Added by automation, don't add manually Team:Security Meta label for security team v9.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ywangd @elasticsearchmachine @idegtiarenko