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

Incorrect matching precedence order #755

Closed
Tracked by #608
kate-osborn opened this issue Jun 15, 2023 · 0 comments · Fixed by #789
Closed
Tracked by #608

Incorrect matching precedence order #755

kate-osborn opened this issue Jun 15, 2023 · 0 comments · Fixed by #789
Assignees
Labels
area/httproute/core Relates to all Core features of HTTPRoute bug Something isn't working refined Requirements are refined and the issue is ready to be implemented.
Milestone

Comments

@kate-osborn
Copy link
Contributor

Describe the bug
In the Gateway 0.7.1 release, the HTTPRoute matching precedence order was changed, giving "method" higher precedence than headers and query params.

This change makes our matching precedence logic incorrect.

To Reproduce
Steps to reproduce the behavior:

  1. Deploy NKG
  2. Create the default http gateway
  3. Create this cafe app:
apiVersion: apps/v1
kind: Deployment
metadata:
  name: coffee-method
spec:
  replicas: 1
  selector:
    matchLabels:
      app: coffee-method
  template:
    metadata:
      labels:
        app: coffee-method
    spec:
      containers:
      - name: coffee-method
        image: nginxdemos/nginx-hello:plain-text
        ports:
        - containerPort: 8080
---
apiVersion: v1
kind: Service
metadata:
  name: coffee-method
spec:
  ports:
  - port: 80
    targetPort: 8080
    protocol: TCP
    name: http
  selector:
    app: coffee-method
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: coffee-headers
spec:
  replicas: 1
  selector:
    matchLabels:
      app: coffee-headers
  template:
    metadata:
      labels:
        app: coffee-headers
    spec:
      containers:
      - name: coffee-headers
        image: nginxdemos/nginx-hello:plain-text
        ports:
        - containerPort: 8080
---
apiVersion: v1
kind: Service
metadata:
  name: coffee-headers
spec:
  ports:
  - port: 80
    targetPort: 8080
    protocol: TCP
    name: http
  selector:
    app: coffee-headers
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: coffee-params
spec:
  replicas: 1
  selector:
    matchLabels:
      app: coffee-params
  template:
    metadata:
      labels:
        app: coffee-params
    spec:
      containers:
      - name: coffee-params
        image: nginxdemos/nginx-hello:plain-text
        ports:
        - containerPort: 8080
---
apiVersion: v1
kind: Service
metadata:
  name: coffee-params
spec:
  ports:
  - port: 80
    targetPort: 8080
    protocol: TCP
    name: http
  selector:
    app: coffee-params

and this HTTPRoute:

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: coffee
spec:
  parentRefs:
  - name: gateway
  hostnames:
  - "cafe.example.com"
  rules:
  - matches: # /coffee/ -X POST -H "version:v2" or curl /coffee\?TEST=v2 -X POST -H "version:v2"
    - path:
        type: PathPrefix
        value: /coffee
      method: POST
    backendRefs:
    - name: coffee-method
      port: 80
  - matches: # /coffee/\?TEST=v2 -H "version:v2"
    - path:
        type: PathPrefix
        value: /coffee
      headers:
      - name: version
        value: v2
    backendRefs:
    - name: coffee-headers
      port: 80
  - matches: # /coffee/\?TEST=v2
    - path:
        type: PathPrefix
        value: /coffee
      queryParams:
      - name: TEST
        value: v2
    backendRefs:
    - name: coffee-params
      port: 80

Then execute the following curl command:

curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/coffee/\?TEST=v2 -H "version:v2"  -X POST

You will see a response from coffee-headers, but the response should be from coffee-method.

Note: All the matches in the HTTPRoute have a comment with the requests that should match.

Expected behavior
When a request satisfies multiple matches, the match should be selected according to the criteria in the spec: https://github.com/kubernetes-sigs/gateway-api/blob/ab03a594e7db13b8d2579929b204d8d10990fd2b/apis/v1beta1/httproute_types.go#L158

Your environment

  • Edge (main)
@mpstefan mpstefan added this to the v0.5.0 milestone Jun 15, 2023
@mpstefan mpstefan added refined Requirements are refined and the issue is ready to be implemented. area/httproute/core Relates to all Core features of HTTPRoute bug Something isn't working labels Jun 15, 2023
@sjberman sjberman self-assigned this Jun 26, 2023
@sjberman sjberman moved this from 🆕 New to 🏗 In Progress in NGINX Gateway Fabric Jun 26, 2023
@sjberman sjberman moved this from 🏗 In Progress to 👀 In Review in NGINX Gateway Fabric Jun 26, 2023
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in NGINX Gateway Fabric Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/httproute/core Relates to all Core features of HTTPRoute bug Something isn't working refined Requirements are refined and the issue is ready to be implemented.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants