Skip to content

[ProtoApiScrubber] Allow generic matchers to be configured in filter config.#42276

Merged
adisuissa merged 3 commits intoenvoyproxy:mainfrom
sumitkmr2:match
Dec 16, 2025
Merged

[ProtoApiScrubber] Allow generic matchers to be configured in filter config.#42276
adisuissa merged 3 commits intoenvoyproxy:mainfrom
sumitkmr2:match

Conversation

@sumitkmr2
Copy link
Copy Markdown
Contributor

@sumitkmr2 sumitkmr2 commented Nov 26, 2025

Commit Message: Allow generic matchers to be configured in filter config.
Additional Description: The filter, from inception, supported generic matching. Since it was designed keeping CEL matching in mind, the match input type was restricted to CEL. While writing ITs for other types of matching, it was realized that this is an unnecessary restriction and hence, is being removed. The Integration Tests for other types of matching will be added once Integration Test base PR (#42121) is submitted.
Risk Level: None.
Testing: Negative UTs removed.
Docs Changes: None.
Release Notes: None.
Platform Specific Features: None.
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Sumit Kumar <sumitkmr@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #42276 was opened by sumitkmr2.

see: more, trace.

Signed-off-by: Sumit Kumar <sumitkmr@google.com>
@sumitkmr2 sumitkmr2 marked this pull request as ready for review November 26, 2025 17:43
@sumitkmr2 sumitkmr2 requested a review from adisuissa as a code owner November 26, 2025 17:43
}
}

TEST_F(ProtoApiScrubberFilterConfigTest, MatcherInputTypeValidations) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What will happen now if a non-supported matcher will be used?
Are there tests that cover that? Would adding an integration test for this use-case make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All the matchers will be supported by this filter after this change and all the inputs would be available for this filter to be used by those matchers. I wasn't able to find any matcher that won't work and hence, to validation is added.

PS: There were some bugs I found after this PR which I'm fixing in a parallel PR (#42293) to fix header and trailer propagation but rest of the inputs flow automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds strange, and I may be missing something about the how the matcher is used.

@phlax
Copy link
Copy Markdown
Member

phlax commented Dec 8, 2025

@sumitkmr2 i think this is awaiting your feedback

/wait-any

@sumitkmr2
Copy link
Copy Markdown
Contributor Author

sumitkmr2 commented Dec 8, 2025 via email

@adisuissa
Copy link
Copy Markdown
Contributor

/wait-any

Signed-off-by: Sumit Kumar <sumitkmr@google.com>
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

}
}

TEST_F(ProtoApiScrubberFilterConfigTest, MatcherInputTypeValidations) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds strange, and I may be missing something about the how the matcher is used.

@adisuissa adisuissa merged commit a2cf52a into envoyproxy:main Dec 16, 2025
25 checks passed
grnmeira pushed a commit to grnmeira/envoy that referenced this pull request Mar 20, 2026
…config (envoyproxy#42276)

Signed-off-by: Sumit Kumar <sumitkmr@google.com>
Signed-off-by: Gustavo <grnmeira@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants