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

feat: add support for path matching precedence #1855

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

Xunzhuo
Copy link
Member

@Xunzhuo Xunzhuo commented Mar 20, 2023

What type of PR is this?

/kind bug
/area conformance

What this PR does / why we need it:

Path matches in following orders:

  1. Exact
  2. Prefix

Which issue(s) this PR fixes:

Fixes #1770

Does this PR introduce a user-facing change?:

* Updated spec to clarify that Exact matches have precedence over Prefix matches and RegularExpression matches have implementation specific precedence.
* Added conformance test to verify that path matching precedence is implemented.

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. area/conformance cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 20, 2023
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 20, 2023
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 20, 2023
@Xunzhuo Xunzhuo marked this pull request as ready for review March 20, 2023 12:00
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 20, 2023
@k8s-ci-robot k8s-ci-robot requested a review from bowei March 20, 2023 12:00
@shaneutt shaneutt added this to the v0.7.0 milestone Mar 20, 2023
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 20, 2023
@Xunzhuo Xunzhuo force-pushed the regex-order branch 2 times, most recently from 99ed7a7 to 190afc2 Compare March 21, 2023 02:33
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 21, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 27, 2023
@k8s-ci-robot k8s-ci-robot merged commit e65388c into kubernetes-sigs:main Mar 27, 2023
@Xunzhuo Xunzhuo deleted the regex-order branch March 1, 2024 06:55
arkodg pushed a commit to envoyproxy/gateway that referenced this pull request Apr 5, 2024
…2579)

* Change route sorting order to Exact > RegularExpression > PathPrefix

kubernetes-sigs/gateway-api#1770
kubernetes-sigs/gateway-api#1855

Signed-off-by: Stéphane Cottin <[email protected]>
arkodg pushed a commit to arkodg/gateway that referenced this pull request Apr 8, 2024
…nvoyproxy#2579)

* Change route sorting order to Exact > RegularExpression > PathPrefix

kubernetes-sigs/gateway-api#1770
kubernetes-sigs/gateway-api#1855

Signed-off-by: Stéphane Cottin <[email protected]>
(cherry picked from commit 11f56fd)
Signed-off-by: Arko Dasgupta <[email protected]>
Xunzhuo added a commit to envoyproxy/gateway that referenced this pull request Apr 8, 2024
* Run certgen when upgrading (#2934)

run certgen when upgrading

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit 62ecf15)
Signed-off-by: Arko Dasgupta <[email protected]>

* Fix: nil secret in resourceversiontable (#2982)

* fix nil secret in resourceversiontable

Signed-off-by: huabing zhao <[email protected]>

* check secrets in the xds result

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit e880439)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: add missing http filters to the http filter chain (#2970)

* fix: add missing http filters to the http filter chain

Signed-off-by: huabing zhao <[email protected]>

* refactor

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: huabing zhao <[email protected]>

* add comments

Signed-off-by: huabing zhao <[email protected]>

* remove refactor

Signed-off-by: huabing zhao <[email protected]>

* remove refactor

Signed-off-by: huabing zhao <[email protected]>

* fix gen

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
(cherry picked from commit f699edf)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: allow websockets in url rewrite (#3022)

allow websockets in url rewrite

Signed-off-by: Jesse Haka <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit 3d51933)
Signed-off-by: Arko Dasgupta <[email protected]>

* Set host for http health checker explicitly to avoid using the cluster name as host header for http health checking request. (#3057)

* Set host for http health checker explictly to avoid using the cluster name as host header for http health checking request

Signed-off-by: lemonlinger <[email protected]>

* fix broken tests

Signed-off-by: lemonlinger <[email protected]>

* fix health-check test case in xds translation

Signed-off-by: lemonlinger <[email protected]>

* Simplify code and concise comments

Signed-off-by: lemonlinger <[email protected]>

---------

Signed-off-by: lemonlinger <[email protected]>
(cherry picked from commit 8f450a9)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: do not create infra resources when missing translated listeners (#3043)

* fix: do not create infra resources when missing translated listeners

Signed-off-by: Karol Szwaj <[email protected]>

* remove empty line

Signed-off-by: Karol Szwaj <[email protected]>

* skip infra creation on empty listeners and log it

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>
(cherry picked from commit 36d7141)
Signed-off-by: Arko Dasgupta <[email protected]>

* Fix: double slashes in redirect URL (#2998)

* fix: double trailing splashs in redirect URL

Signed-off-by: huabing zhao <[email protected]>

* add e2e tests

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* add e2e tests

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* revert

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* remove comments

Signed-off-by: huabing zhao <[email protected]>

* extract method

Signed-off-by: huabing zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit ceb697f)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: Allow Policy to attach to multiple http listeners  (#2967)

* Fixing the clienttrafficpolicy validation.

Signed-off-by: Lior Okman <[email protected]>

* Make SecurityPolicy validate correctly.

Signed-off-by: Lior Okman <[email protected]>

* Reverted the SecurityPolicy validation - handled differently via
another feature.

Signed-off-by: Lior Okman <[email protected]>

* Updated the tests to reflect that this validation isn't required for SecurityPolicy

Signed-off-by: Lior Okman <[email protected]>

* Added some comments to explain the validation being performed.

Signed-off-by: Lior Okman <[email protected]>

* Updated the error message as requested in the review.

Signed-off-by: Lior Okman <[email protected]>

---------

Signed-off-by: Lior Okman <[email protected]>
(cherry picked from commit f9409e4)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: set path prefix for http ext auth service (#3018)

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit 2882b7c)
Signed-off-by: Arko Dasgupta <[email protected]>

* Change route sorting order to Exact > RegularExpression > PathPrefix (#2579)

* Change route sorting order to Exact > RegularExpression > PathPrefix

kubernetes-sigs/gateway-api#1770
kubernetes-sigs/gateway-api#1855

Signed-off-by: Stéphane Cottin <[email protected]>
(cherry picked from commit 11f56fd)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: infraIR duplicate port translation for merged gateways (#3061)

* fix: duplicate port translation for merged gateways

Signed-off-by: Karol Szwaj <[email protected]>

* refactor to map

Signed-off-by: Karol Szwaj <[email protected]>

* rename map

Signed-off-by: Karol Szwaj <[email protected]>

* add seperate testcase

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>
(cherry picked from commit 29946b0)
Signed-off-by: Arko Dasgupta <[email protected]>

* translator: set SpawnUpstreamSpan to true (#3102)

* translator: set SpawnUpstreamSpan to true

Signed-off-by: zirain <[email protected]>

* update

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 635ebfc)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: rate limit doesn't work with two(and more) listeners (#3085)

* fix: rate limit doesn't work with two listeners

Signed-off-by: huabing zhao <[email protected]>

* add e2e test for rate limit on multiple listeners

Signed-off-by: huabing zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Co-authored-by: Xunzhuo <[email protected]>
(cherry picked from commit a5bedbc)
Signed-off-by: Arko Dasgupta <[email protected]>

* rerun make testdata

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Jesse Haka <[email protected]>
Signed-off-by: lemonlinger <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Lior Okman <[email protected]>
Signed-off-by: Stéphane Cottin <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Huabing Zhao <[email protected]>
Co-authored-by: Jesse Haka <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Meng <[email protected]>
Co-authored-by: Karol Szwaj <[email protected]>
Co-authored-by: Lior Okman <[email protected]>
Co-authored-by: vixns <[email protected]>
Co-authored-by: Xunzhuo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-blocker MUST be completed to complete the milestone size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Corner case in HTTPRouteRule match order when RegularExpression is used
7 participants