Skip to content

Add conformance test for combination of extended match types with core types #1984

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

Merged
k8s-ci-robot merged 2 commits into from
May 18, 2023
Merged

Add conformance test for combination of extended match types with core types #1984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ca6c59
Add conformance test for combination of extended match types with cor…
gauravkghildiyal May 3, 2023
9d470b4
Add execution bit to hack/verify-yamllint.sh
gauravkghildiyal May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions conformance/tests/httproute-method-matching.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,68 @@ var HTTPRouteMethodMatching = suite.ConformanceTest{
},
}

// Combinations of method matching with other core matches.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path1", Method: "GET"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "one"}, Path: "/", Method: "PUT"},
Backend: "infra-backend-v2",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "two"}, Path: "/path2", Method: "POST"},
Backend: "infra-backend-v3",
Namespace: ns,
},
}...)

// Ensure that combinations of matches which are OR'd together match
// even if only one of them is used in the request.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path3", Method: "PATCH"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "three"}, Path: "/path4", Method: "DELETE"},
Backend: "infra-backend-v1",
Namespace: ns,
},
}...)

// Ensure that combinations of match types which are ANDed together do not match
// when only a subset of match types is used in the request.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/", Method: "PUT"},
Response: http.Response{StatusCode: 404},
},
{
Request: http.Request{Path: "/path4", Method: "DELETE"},
Response: http.Response{StatusCode: 404},
},
}...)

// For requests that satisfy multiple matches, ensure precedence order
// defined by the Gateway API spec is maintained.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path5", Method: "PATCH"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "four"}, Path: "/", Method: "PATCH"},
Backend: "infra-backend-v3",
Namespace: ns,
},
}...)

for i := range testCases {
// Declare tc here to avoid loop variable
// reuse issues across parallel tests.
Expand Down
67 changes: 67 additions & 0 deletions conformance/tests/httproute-method-matching.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,70 @@ spec:
backendRefs:
- name: infra-backend-v2
port: 8080

# Combinations with core match types.
- matches:
- path:
type: PathPrefix
value: /path1
method: GET
backendRefs:
- name: infra-backend-v1
port: 8080
- matches:
- headers:
- name: version
value: one
method: PUT
backendRefs:
- name: infra-backend-v2
port: 8080
- matches:
- path:
type: PathPrefix
value: /path2
headers:
- name: version
value: two
method: POST
backendRefs:
- name: infra-backend-v3
port: 8080

# Match of the form (cond1 AND cond2) OR (cond3 AND cond4 AND cond5)
- matches:
- path:
type: PathPrefix
value: /path3
method: PATCH
- path:
type: PathPrefix
value: /path4
headers:
- name: version
value: three
method: DELETE
backendRefs:
- name: infra-backend-v1
port: 8080

# 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
62 changes: 62 additions & 0 deletions conformance/tests/httproute-query-param-matching.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,68 @@ var HTTPRouteQueryParamMatching = suite.ConformanceTest{
Response: http.Response{StatusCode: 404},
}}

// Combinations of query param matching with other core matches.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path1?animal=whale"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "one"}, Path: "/?animal=whale"},
Backend: "infra-backend-v2",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "two"}, Path: "/path2?animal=whale"},
Backend: "infra-backend-v3",
Namespace: ns,
},
}...)

// Ensure that combinations of matches which are OR'd together match
// even if only one of them is used in the request.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path3?animal=shark"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "three"}, Path: "/path4?animal=kraken"},
Backend: "infra-backend-v1",
Namespace: ns,
},
}...)

// Ensure that combinations of match types which are ANDed together do not match
// when only a subset of match types is used in the request.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/?animal=shark"},
Response: http.Response{StatusCode: 404},
},
{
Request: http.Request{Path: "/path4?animal=kraken"},
Response: http.Response{StatusCode: 404},
},
}...)

// For requests that satisfy multiple matches, ensure precedence order
// defined by the Gateway API spec is maintained.
testCases = append(testCases, []http.ExpectedResponse{
{
Request: http.Request{Path: "/path5?animal=hydra"},
Backend: "infra-backend-v1",
Namespace: ns,
},
{
Request: http.Request{Headers: map[string]string{"version": "four"}, Path: "/?animal=hydra"},
Copy link
Member

@sunjayBhatia sunjayBhatia May 22, 2023

Choose a reason for hiding this comment

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

Looks like this test case added after my review doesn't work with Contour

this assumes header matching gets precedence over method matching, but I'm not sure that is expressed anywhere explicitly in the spec, see #1993

Copy link
Member Author

@gauravkghildiyal gauravkghildiyal May 22, 2023

Choose a reason for hiding this comment

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

Yes that was how I interpreted it -- given method matching is not in the list (and the only thing not in the list), it must have the least priority.

I'm sorry for causing any issues. I've sent #2052 as a temporary solution to help unblock (if this is the path we wish to take)

Copy link
Member

@sunjayBhatia sunjayBhatia May 22, 2023

Choose a reason for hiding this comment

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

thanks! yeah not a super strong opinion on where it lands in terms of priority, but should probably be something explicitly debated/documented 👍🏽

Backend: "infra-backend-v3",
Namespace: ns,
},
}...)

for i := range testCases {
tc := testCases[i]
t.Run(tc.GetTestCaseName(i), func(t *testing.T) {
Expand Down
79 changes: 79 additions & 0 deletions conformance/tests/httproute-query-param-matching.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,82 @@ spec:
backendRefs:
- name: infra-backend-v3
port: 8080

# Combinations with core match types.
- matches:
- path:
type: PathPrefix
value: /path1
queryParams:
- name: animal
value: whale
backendRefs:
- name: infra-backend-v1
port: 8080
- matches:
- headers:
- name: version
value: one
queryParams:
- name: animal
value: whale
backendRefs:
- name: infra-backend-v2
port: 8080
- matches:
- path:
type: PathPrefix
value: /path2
headers:
- name: version
value: two
queryParams:
- name: animal
value: whale
backendRefs:
- name: infra-backend-v3
port: 8080

# Match of the form (cond1 AND cond2) OR (cond3 AND cond4 AND cond5)
- matches:
- path:
type: PathPrefix
value: /path3
queryParams:
- name: animal
value: shark
- path:
type: PathPrefix
value: /path4
headers:
- name: version
value: three
queryParams:
- name: animal
value: kraken
backendRefs:
- name: infra-backend-v1
port: 8080

# Matches for checking precedence.
- matches:
- path:
type: PathPrefix
value: /path5
backendRefs:
- name: infra-backend-v1
port: 8080
- matches:
- queryParams:
- name: animal
value: hydra
backendRefs:
- name: infra-backend-v2
port: 8080
- matches:
- headers:
- name: version
value: four
backendRefs:
- name: infra-backend-v3
port: 8080
Empty file modified hack/verify-yamllint.sh
100644 → 100755
View file
Open in desktop
Empty file.