Skip to content

api: adjust the behavior of header's present_match#16627

Merged
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
soulxu:correct_present_behavior
May 27, 2021
Merged

api: adjust the behavior of header's present_match#16627
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
soulxu:correct_present_behavior

Conversation

@soulxu
Copy link
Copy Markdown
Member

@soulxu soulxu commented May 22, 2021

Commit Message: api: adjust the behavior of header's present_match

Additional Description:
Currently, envoy ignores the value of present match. This commit adjust
envoy's behavior. When the value is True, envoy will check the header
present. When the value is False, envoy will check the header absent.
Risk Level: low
Testing: unit test added
Docs Changes: api doc updated
Release Notes: minor behavior change
Fixes #14656

Signed-off-by: He Jie Xu hejie.xu@intel.com

Currently, envoy ignores the value of present match. This commit adjust
envoy's behavior. When the value is True, envoy will check the header
present. When the value is False, envoy will check the header absent.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #16627 was opened by soulxu.

see: more, trace.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@soulxu soulxu changed the title api: adjust the behavior when present match's value is false api: adjust the behavior of header's present_match May 22, 2021
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the fix! A few minor comments.
/wait

// If specified, header match will be performed based on whether the header is in the
// request.
// If specified as true, header match will be performed based on whether the header is in the
// request. If specified as false, header match will be performed based on whether the header is absent.
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.

I think this is technically a data plane behavioral change. That said, I don't see any sensible way that this field could have been configured with false and a meaningful match expected previously, so I don't think we need runtime protection.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the doc is not correct, config present_match: false will not effected, shoud use

present_match: true
invert_match:true

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hanjm I tested it on the main branch, it works as expected. I pasted the detail at #17345 (comment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sorry, my version(1.17) is too low. Thanks you reply.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented May 24, 2021

@htuch thanks for the review, addressed your comment.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Copy link
Copy Markdown
Member

@htuch htuch 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!

@mattklein123 mattklein123 merged commit 9a7a7ed into envoyproxy:main May 27, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Currently, envoy ignores the value of present match. This commit adjust
envoy's behavior. When the value is True, envoy will check the header
present. When the value is False, envoy will check the header absent.

Signed-off-by: He Jie Xu <hejie.xu@intel.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.

API: router HeaderMatch "present_match" field is a bool but value is ignored

4 participants