Skip to content

Add configuration for RLS response metadata namespace#44531

Open
yanavlasov wants to merge 2 commits intoenvoyproxy:mainfrom
yanavlasov:rl-metadata-namespace-config
Open

Add configuration for RLS response metadata namespace#44531
yanavlasov wants to merge 2 commits intoenvoyproxy:mainfrom
yanavlasov:rl-metadata-namespace-config

Conversation

@yanavlasov
Copy link
Copy Markdown
Contributor

Add an option for modifying the default namespace where HTTP filter stores metadata from the rate limit service response. This allows rate limit results to influence behavior of other extensions.

Risk Level: low (new config option)
Testing: unit tests
Docs Changes: yes
Release Notes: yes
Platform Specific Features: no

Signed-off-by: Yan Avlasov <yavlasov@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

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: #44531 was opened by yanavlasov.

see: more, trace.

Signed-off-by: Yan Avlasov <yavlasov@google.com>
@mathetake
Copy link
Copy Markdown
Member

kindly ping @wbpcode @markdroth

Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

LGTM with minor comment to the comment of API.

/wait

repeated config.route.v3.RateLimit rate_limits = 17;

// The namespace where dynamic metadata from rate limit response is saved.
// If not set, the default is "envoy.extensions.filters.http.ratelimit".
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// If not set, the default is "envoy.extensions.filters.http.ratelimit".
// If not set, the default is "envoy.filters.http.ratelimit".

?

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.

4 participants