Skip to content
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.

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

(rbac) add principal matchers for dynamic downstream IPs #37374

Closed
wants to merge 1 commit into from

Conversation

antoniovleonti
Copy link
Contributor

Commit Message: (rbac) add principal matchers for dynamic downstream IPs
Additional Description:

This PR adds two matchers to RBAC (filter_state_ip, dynamic_metadata_ip) which compare an IP from filter state / dynamic metadata to a CIDR block. If the IP is not found where specified in configuration, the matcher fails. The IP is assumed to be a downstream IP, so the matchers are principal matchers.

Risk Level: low
Testing: unit tested
Docs Changes: none
Release Notes: changelog updated
Platform Specific Features: none

Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #37374 was opened by antoniovleonti.

see: more, trace.

@antoniovleonti
Copy link
Contributor Author

/retest

core.v3.CidrRange ip_range = 2;
}

message DynamicMetadataIPMatcherConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a fairly specific use case for the general mechanism provided by the existing SourcedMetadata matcher. Instead of adding this special case, can we generalize type.matcher.v3.MetadataMatcher to allow arbitrary plugin matchers, and then define a plugin that matches an IP range?

@@ -338,6 +338,23 @@ message Principal {
type.matcher.v3.StringMatcher principal_name = 2;
}

message FilterStateIPMatcherConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Along similar lines, I think it would be better to add a pluggable matcher to type.matcher.v3.FilterStateMatcher rather than add this as a special case here.

@markdroth
Copy link
Contributor

/wait

@antoniovleonti antoniovleonti marked this pull request as draft December 13, 2024 16:01
@antoniovleonti
Copy link
Contributor Author

succeeded by #37637

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

Successfully merging this pull request may close these issues.

3 participants