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

HTTPRoute matching fails HTTPRouteMethodMatching test cases 10 and 11 #4164

Closed
rainest opened this issue Jun 12, 2023 · 2 comments
Closed

HTTPRoute matching fails HTTPRouteMethodMatching test cases 10 and 11 #4164

rainest opened this issue Jun 12, 2023 · 2 comments
Labels
area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API

Comments

@rainest
Copy link
Contributor

rainest commented Jun 12, 2023

Our route matching fails a conformance test from 0.7.1. The actual rule tested here is not clear from the test comment:

For requests that satisfy multiple matches, ensure precedence order defined by the Gateway API spec is maintained.

2023-06-12T19:56:52.2898759Z     --- FAIL: TestGatewayConformance/HTTPRouteMethodMatching (0.08s)
2023-06-12T19:56:52.2899589Z         --- PASS: TestGatewayConformance/HTTPRouteMethodMatching/2_request_to_'/'_should_receive_a_404 (3.03s)
2023-06-12T19:56:52.2900577Z         --- FAIL: TestGatewayConformance/HTTPRouteMethodMatching/11_request_to_'/'_with_headers_should_go_to_infra-backend-v2 (30.00s)
/'_should_go_to_infra-backend-v1 (0.00s)
2023-06-12T19:56:52.2910265Z         --- FAIL: TestGatewayConformance/HTTPRouteMethodMatching/10_request_to_'/path5'_should_go_to_infra-backend-v1 (30.00s)

The test in question checks a request with a method and path versus a request with a method and header, but the exact expected precedence rule or expected rule match isn't immediately clear. Test requests and configuration from a run are attached.

method_route_config_10_11.txt
method_route_routes_10_11.txt
method_route_reqs_10_11.txt

@rainest rainest added the area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API label Jun 12, 2023
@randmonkey
Copy link
Contributor

randmonkey commented Jul 17, 2023

This is also about priorities. The requests in cases 10 and 11 will involve the last 3 matches of the testing HTTPRoute:

  # Matches for checking precedence.
  - matches:
    - path:
        type: PathPrefix
        value: /path5
    backendRefs:
    - name: infra-backend-v1
      port: 8080
  - matches:
    - method: PATCH
    backendRefs:
    - name: infra-backend-v2
      port: 8080
  - matches:
    - headers:
      - name: version
        value: four
    backendRefs:
    - name: infra-backend-v3
      port: 8080

request 10 PATCH /path5 matches the first 2 matches, and the match with non-empty PathPrefix match takes precedence over the other one only specifying the method.

request 11 PATCH / with header version: four matches the last 2 matches, and the match 2 specifying method takes precedence over the last match which only specifies headers.

traditional router and traditional_compatible router cannot support the path > method > header priority. It gets fixed after #4296 & #4359.

@randmonkey
Copy link
Contributor

fixed with Expression router: #4359. will close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API
Projects
None yet
Development

No branches or pull requests

2 participants