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

Fix header present match #47704

Merged
merged 4 commits into from
Nov 3, 2023
Merged

Fix header present match #47704

merged 4 commits into from
Nov 3, 2023

Conversation

hzxuzhonghu
Copy link
Member

@hzxuzhonghu hzxuzhonghu commented Nov 2, 2023 •
edited by istio-policy-bot
Loading

Please provide a description of this PR:

According to https://istio.io/latest/docs/reference/config/networking/virtual-service/

The header keys must be lowercase and use hyphen as the separator, e.g. x-request-id.

Header values are case-sensitive and formatted as follows:

exact: "value" for exact string match

prefix: "value" for prefix-based match

regex: "value" for RE2 style regex-based match (https://github.com/google/re2/wiki/Syntax).

If the value is empty and only the name of header is specified, presence of the header is checked. To provide an empty value, use {}, for example:

 - match:
   - headers:
       myheader: {}

Without this, the generated route match does not contain presentMatch: true .

 - match:
   - headers:
       myheader: {}

    routes:
    - decorator:
        operation: sleep.default.svc.cluster.local:80/ratings/v2/*
      match:
        caseSensitive: false
        headers:
        - name: myheader
          presentMatch: true

@hzxuzhonghu hzxuzhonghu requested a review from a team as a code owner November 2, 2023 07:23
@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 2, 2023
@hzxuzhonghu
Copy link
Member Author

@ramaraochavali ptal

@hzxuzhonghu
Copy link
Member Author

/cherry-pick release-1.20

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: once the present PR merges, I will cherry-pick it on top of release-1.20 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.20

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@hzxuzhonghu
Copy link
Member Author

/cherry-pick release-1.19

/cherry-pick release-1.18

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: once the present PR merges, I will cherry-pick it on top of release-1.19 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.19

/cherry-pick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ramaraochavali
Copy link
Contributor

Change LGTM. Can you please add a test?

@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 3, 2023
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Nov 3, 2023
@istio-testing istio-testing merged commit 0945c97 into istio:master Nov 3, 2023
1 check passed
@istio-testing
Copy link
Collaborator

@hzxuzhonghu: #47704 failed to apply on top of branch "release-1.20":

Applying: Fix header present match
Applying: add release note
Applying: Add ut
Using index info to reconstruct a base tree...
M	pilot/pkg/networking/core/v1alpha3/route/route_test.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/networking/core/v1alpha3/route/route_test.go
CONFLICT (content): Merge conflict in pilot/pkg/networking/core/v1alpha3/route/route_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Add ut
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.20

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: new issue created for failed cherrypick: #47715

In response to this:

/cherry-pick release-1.20

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing istio-testing mentioned this pull request Nov 3, 2023
Closed
@istio-testing
Copy link
Collaborator

@hzxuzhonghu: #47704 failed to apply on top of branch "release-1.19":

Applying: Fix header present match
Applying: add release note
Applying: Add ut
Using index info to reconstruct a base tree...
M	pilot/pkg/networking/core/v1alpha3/route/route_test.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/networking/core/v1alpha3/route/route_test.go
CONFLICT (content): Merge conflict in pilot/pkg/networking/core/v1alpha3/route/route_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Add ut
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.19

/cherry-pick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing istio-testing mentioned this pull request Nov 3, 2023
Closed
@istio-testing
Copy link
Collaborator

@hzxuzhonghu: new issue created for failed cherrypick: #47716

In response to this:

/cherry-pick release-1.19

/cherry-pick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: #47704 failed to apply on top of branch "release-1.18":

Applying: Fix header present match
Applying: add release note
Applying: Add ut
Using index info to reconstruct a base tree...
M	pilot/pkg/networking/core/v1alpha3/route/route_test.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/networking/core/v1alpha3/route/route_test.go
CONFLICT (content): Merge conflict in pilot/pkg/networking/core/v1alpha3/route/route_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Add ut
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.19

/cherry-pick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: new issue created for failed cherrypick: #47717

In response to this:

/cherry-pick release-1.19

/cherry-pick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing istio-testing mentioned this pull request Nov 3, 2023
Closed
@hzxuzhonghu hzxuzhonghu deleted the header-present branch November 3, 2023 07:20
hzxuzhonghu added a commit to hzxuzhonghu/istio that referenced this pull request Nov 6, 2023
@hzxuzhonghu
Fix header present match (istio#47704)
40a9ce5 
* Fix header present match

* add release note

* Add ut

* fix lint
@hzxuzhonghu hzxuzhonghu mentioned this pull request Nov 6, 2023
Merged
hzxuzhonghu added a commit to hzxuzhonghu/istio that referenced this pull request Nov 6, 2023
@hzxuzhonghu
Fix header present match (istio#47704)
ce4b82e 
* Fix header present match

* add release note

* Add ut

* fix lint
@hzxuzhonghu hzxuzhonghu mentioned this pull request Nov 6, 2023
Merged
hzxuzhonghu added a commit to hzxuzhonghu/istio that referenced this pull request Nov 6, 2023
@hzxuzhonghu
Fix header present match (istio#47704)
9cb93fd 
* Fix header present match

* add release note

* Add ut

* fix lint
istio-testing pushed a commit that referenced this pull request Nov 6, 2023
@hzxuzhonghu
Fix header present match (#47704) (#47736)
d3b9bc8 
* Fix header present match

* add release note

* Add ut

* fix lint
istio-testing pushed a commit that referenced this pull request Nov 6, 2023
@hzxuzhonghu
Fix header present match (#47704) (#47737)
2035c75 
* Fix header present match

* add release note

* Add ut

* fix lint
istio-testing pushed a commit that referenced this pull request Nov 14, 2023
@hzxuzhonghu
Fix header present match (#47704) (#47735)
6cf85b4 
* Fix header present match

* add release note

* Add ut

* fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ramaraochavali ramaraochavali ramaraochavali approved these changes

Assignees
No one assigned
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hzxuzhonghu @istio-testing @ramaraochavali