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.

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

HeaderValueMatch support custom descriptor key #20321

Merged
merged 5 commits into from
Mar 18, 2022
Merged

HeaderValueMatch support custom descriptor key #20321

merged 5 commits into from
Mar 18, 2022

Conversation

zirain
Copy link
Contributor

@zirain zirain commented Mar 14, 2022

Signed-off-by: hejianpeng hejianpeng2@huawei.com

before:

- header_value_match:
      descriptor_value: "svc1"
      expect_match: true
      headers:
        - name: :path
          prefix_match: /svc1
[2022-03-14 13:45:02.793][3414241][debug][filter] [external/envoy/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc:120] populate descriptors: key=request_method value=GET
[2022-03-14 13:45:02.793][3414241][debug][filter] [external/envoy/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc:120] populate descriptors: key=header_match value=svc1

after:

- header_value_match:
      descriptor_key: path_match
      descriptor_value: "svc1"
      expect_match: true
      headers:
        - name: :path
          prefix_match: /svc1

[2022-03-14 13:45:35.923][3414669][debug][filter] [external/envoy/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc:120] populate descriptors: key=request_method value=GET
[2022-03-14 13:45:35.923][3414669][debug][filter] [external/envoy/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc:120] populate descriptors: key=path_match value=svc1

Commit Message: HeaderValueMatch support custom descriptor key
Additional Description:
Risk Level: low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] #20249
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
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 @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #20321 was opened by zirain.

see: more, trace.

adisuissa
adisuissa requested changes Mar 15, 2022
View reviewed changes
Copy link
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.

Overall LGTM, but needs tests, and updated release notes.
API LGTM.
/wait

@zirain
Copy link
Contributor Author

zirain commented Mar 15, 2022

@adisuissa can you rerun the pipeline? I think there's nothing special with tsan.

@adisuissa
Copy link
Contributor

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20321 (comment) was created by @adisuissa.

see: more, trace.

@adisuissa
Copy link
Contributor

Per the contributions guidelines, please avoid force-pushing as it is hard to track changes.

@zirain
Copy link
Contributor Author

zirain commented Mar 15, 2022

Per the contributions guidelines, please avoid force-pushing as it is hard to track changes.

ok, I will not do that next time.

@zirain zirain requested a review from adisuissa March 16, 2022 00:06
@zirain
Copy link
Contributor Author

zirain commented Mar 17, 2022

update tests and release-notes, PTAL @adisuissa

adisuissa
adisuissa previously approved these changes Mar 17, 2022
View reviewed changes
Copy link
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!
/assign-from @envoyproxy/senior-maintainers

@repokitteh-read-only
Copy link

@envoyproxy/senior-maintainers assignee is @mattklein123

🐱

Caused by: a #20321 (review) was submitted by @adisuissa.

see: more, trace.

@adisuissa
Copy link
Contributor

/lgtm api

mattklein123
mattklein123 requested changes Mar 17, 2022
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM with small release note comment.

/wait

docs/root/version_history/current.rst Outdated
@@ -25,6 +25,7 @@ Minor Behavior Changes
* http: when writing custom filters, `injectEncodedDataToFilterChain` and `injectDecodedDataToFilterChain` now trigger sending of headers if they were not yet sent due to `StopIteration`. Previously, calling one of the inject functions in that state would trigger an assertion. See issue #19891 for more details.
* listener: the :ref:`ipv4_compat <envoy_api_field_core.SocketAddress.ipv4_compat>` flag can only be set on Ipv6 address and Ipv4-mapped Ipv6 address. A runtime guard is added ``envoy.reloadable_features.strict_check_on_ipv4_compat`` and the default is true.
* perf: ssl contexts are now tracked without scan based garbage collection and greatly improved the performance on secret update.
* ratelimit: header_value_match support custom descriptor key.
Copy link
Member

Choose a reason for hiding this comment

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

please ref link to the field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, sorry for the mistake about miss signoff, which cause me do another force-pushing

@zirain
HeaderValueMatch support custom descriptor key
a0e3f45 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain
add test & release notes
303442f 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain
fix test
59f182d 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain
uadd field ref link
413a159 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain
fix doc link
4a9c8d0 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
mattklein123
mattklein123 approved these changes Mar 18, 2022
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit b7e499b into envoyproxy:main Mar 18, 2022
@zirain zirain deleted the header_value_match branch March 19, 2022 01:35
@zirain zirain mentioned this pull request Mar 19, 2022
Closed
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
@zirain @ravenblackx
HeaderValueMatch support custom descriptor key (envoyproxy#20321)
9261e2d 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain zirain mentioned this pull request Jan 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@adisuissa adisuissa adisuissa left review comments

Assignees

@adisuissa adisuissa

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zirain @adisuissa @mattklein123