-
Notifications
You must be signed in to change notification settings - Fork 480
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
Add conformance test for isolation of HTTP listeners #2669
Changes from all commits
f438606
2405c98
749d51e
d6abccd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,131 @@ | ||||||||||||||||||||||
apiVersion: gateway.networking.k8s.io/v1 | ||||||||||||||||||||||
kind: Gateway | ||||||||||||||||||||||
metadata: | ||||||||||||||||||||||
name: http-listener-isolation-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
spec: | ||||||||||||||||||||||
gatewayClassName: "{GATEWAY_CLASS_NAME}" | ||||||||||||||||||||||
listeners: | ||||||||||||||||||||||
- name: empty-hostname | ||||||||||||||||||||||
port: 80 | ||||||||||||||||||||||
protocol: HTTP | ||||||||||||||||||||||
allowedRoutes: | ||||||||||||||||||||||
namespaces: | ||||||||||||||||||||||
from: All | ||||||||||||||||||||||
- name: wildcard-example-com | ||||||||||||||||||||||
port: 80 | ||||||||||||||||||||||
protocol: HTTP | ||||||||||||||||||||||
hostname: "*.example.com" | ||||||||||||||||||||||
allowedRoutes: | ||||||||||||||||||||||
namespaces: | ||||||||||||||||||||||
from: All | ||||||||||||||||||||||
- name: wildcard-foo-example-com | ||||||||||||||||||||||
port: 80 | ||||||||||||||||||||||
protocol: HTTP | ||||||||||||||||||||||
hostname: "*.foo.example.com" | ||||||||||||||||||||||
allowedRoutes: | ||||||||||||||||||||||
namespaces: | ||||||||||||||||||||||
from: All | ||||||||||||||||||||||
- name: abc-foo-example-com | ||||||||||||||||||||||
port: 80 | ||||||||||||||||||||||
protocol: HTTP | ||||||||||||||||||||||
hostname: "abc.foo.example.com" | ||||||||||||||||||||||
allowedRoutes: | ||||||||||||||||||||||
namespaces: | ||||||||||||||||||||||
from: All | ||||||||||||||||||||||
--- | ||||||||||||||||||||||
apiVersion: gateway.networking.k8s.io/v1 | ||||||||||||||||||||||
kind: HTTPRoute | ||||||||||||||||||||||
metadata: | ||||||||||||||||||||||
name: attaches-to-empty-hostname-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
spec: | ||||||||||||||||||||||
parentRefs: | ||||||||||||||||||||||
- name: http-listener-isolation-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
sectionName: empty-hostname | ||||||||||||||||||||||
hostnames: | ||||||||||||||||||||||
- "bar.com" | ||||||||||||||||||||||
- "*.example.com" # request matching is prevented by the isolation wildcard-example-com listener | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these are more tied to hostname intersection b/w listener and route, should this be part of this test or another one ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My interpretation is that listener intersection is about one listener and its HTTPRoutes (so it could be tested separately) -- gateway-api/apis/v1/httproute_types.go Lines 71 to 73 in 4b1a28d
while listener isolation is about multiple listeners -- gateway-api/apis/v1/gateway_types.go Lines 173 to 179 in 4b1a28d
Or to put it differently, correct implementation of listener intersection is required for this test, but ultimately the tests is about listener isolation. |
||||||||||||||||||||||
- "*.foo.example.com" # request matching is prevented by the isolation wildcard-foo-example-com listener | ||||||||||||||||||||||
- "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener | ||||||||||||||||||||||
rules: | ||||||||||||||||||||||
- matches: | ||||||||||||||||||||||
- path: | ||||||||||||||||||||||
type: PathPrefix | ||||||||||||||||||||||
value: /empty-hostname | ||||||||||||||||||||||
backendRefs: | ||||||||||||||||||||||
- name: infra-backend-v1 | ||||||||||||||||||||||
port: 8080 | ||||||||||||||||||||||
--- | ||||||||||||||||||||||
apiVersion: gateway.networking.k8s.io/v1 | ||||||||||||||||||||||
kind: HTTPRoute | ||||||||||||||||||||||
metadata: | ||||||||||||||||||||||
name: attaches-to-wildcard-example-com-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
spec: | ||||||||||||||||||||||
parentRefs: | ||||||||||||||||||||||
- name: http-listener-isolation-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
sectionName: wildcard-example-com | ||||||||||||||||||||||
hostnames: | ||||||||||||||||||||||
- "bar.com" # doesn't match wildcard-example-com listener | ||||||||||||||||||||||
- "*.example.com" | ||||||||||||||||||||||
- "*.foo.example.com" # request matching is prevented by the isolation of wildcard-foo-example-com listener | ||||||||||||||||||||||
- "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener | ||||||||||||||||||||||
rules: | ||||||||||||||||||||||
- matches: | ||||||||||||||||||||||
- path: | ||||||||||||||||||||||
type: PathPrefix | ||||||||||||||||||||||
value: /wildcard-example-com | ||||||||||||||||||||||
backendRefs: | ||||||||||||||||||||||
- name: infra-backend-v1 | ||||||||||||||||||||||
port: 8080 | ||||||||||||||||||||||
--- | ||||||||||||||||||||||
apiVersion: gateway.networking.k8s.io/v1 | ||||||||||||||||||||||
kind: HTTPRoute | ||||||||||||||||||||||
metadata: | ||||||||||||||||||||||
name: attaches-to-wildcard-foo-example-com-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
spec: | ||||||||||||||||||||||
parentRefs: | ||||||||||||||||||||||
- name: http-listener-isolation-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
sectionName: wildcard-foo-example-com | ||||||||||||||||||||||
hostnames: | ||||||||||||||||||||||
- "bar.com" # doesn't match wildcard-foo-example-com listener | ||||||||||||||||||||||
- "*.example.com" # this becomes *.foo.example.com, as the hostname cannot be less specific than *.foo.example.com of the listener | ||||||||||||||||||||||
- "*.foo.example.com" | ||||||||||||||||||||||
- "abc.foo.example.com" # request matching is prevented by the isolation abc-foo-example-com listener | ||||||||||||||||||||||
rules: | ||||||||||||||||||||||
- matches: | ||||||||||||||||||||||
- path: | ||||||||||||||||||||||
type: PathPrefix | ||||||||||||||||||||||
value: /wildcard-foo-example-com | ||||||||||||||||||||||
backendRefs: | ||||||||||||||||||||||
- name: infra-backend-v1 | ||||||||||||||||||||||
port: 8080 | ||||||||||||||||||||||
--- | ||||||||||||||||||||||
apiVersion: gateway.networking.k8s.io/v1 | ||||||||||||||||||||||
kind: HTTPRoute | ||||||||||||||||||||||
metadata: | ||||||||||||||||||||||
name: attaches-to-abc-foo-example-com-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
spec: | ||||||||||||||||||||||
parentRefs: | ||||||||||||||||||||||
- name: http-listener-isolation-with-hostname-intersection | ||||||||||||||||||||||
namespace: gateway-conformance-infra | ||||||||||||||||||||||
sectionName: abc-foo-example-com | ||||||||||||||||||||||
hostnames: | ||||||||||||||||||||||
- "bar.com" # doesn't match abc-foo-example-com listener | ||||||||||||||||||||||
- "*.example.com" # becomes abc.foo.example.com as it cannot be less specific than abc.foo.example.com of the listener | ||||||||||||||||||||||
- "*.foo.example.com" # becomes abc.foo.example.com as it cannot be less specific than abc.foo.example.com of the listener | ||||||||||||||||||||||
- "abc.foo.example.com" | ||||||||||||||||||||||
rules: | ||||||||||||||||||||||
- matches: | ||||||||||||||||||||||
- path: | ||||||||||||||||||||||
type: PathPrefix | ||||||||||||||||||||||
value: /abc-foo-example-com | ||||||||||||||||||||||
backendRefs: | ||||||||||||||||||||||
- name: infra-backend-v1 | ||||||||||||||||||||||
port: 8080 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,176 @@ | ||
/* | ||
Copyright 2024 The Kubernetes Authors. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package tests | ||
|
||
import ( | ||
"testing" | ||
|
||
"k8s.io/apimachinery/pkg/types" | ||
|
||
"sigs.k8s.io/gateway-api/conformance/utils/http" | ||
"sigs.k8s.io/gateway-api/conformance/utils/kubernetes" | ||
"sigs.k8s.io/gateway-api/conformance/utils/suite" | ||
) | ||
|
||
func init() { | ||
ConformanceTests = append(ConformanceTests, GatewayHTTPListenerIsolation) | ||
} | ||
|
||
var GatewayHTTPListenerIsolation = suite.ConformanceTest{ | ||
ShortName: "GatewayHTTPListenerIsolation", | ||
Description: "Listener isolation for HTTP listeners with multiple listeners and HTTPRoutes", | ||
Features: []suite.SupportedFeature{ | ||
suite.SupportGateway, | ||
suite.SupportGatewayHTTPListenerIsolation, | ||
suite.SupportHTTPRoute, | ||
}, | ||
Manifests: []string{ | ||
"tests/gateway-http-listener-isolation.yaml", | ||
"tests/gateway-http-listener-isolation-with-hostname-intersection.yaml", | ||
}, | ||
Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { | ||
ns := "gateway-conformance-infra" | ||
|
||
kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ns}) | ||
|
||
testCases := []http.ExpectedResponse{ | ||
// Requests to the empty-hostname listener | ||
{ | ||
Request: http.Request{Host: "bar.com", Path: "/empty-hostname"}, | ||
Backend: "infra-backend-v1", | ||
Namespace: ns, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.com", Path: "/wildcard-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.com", Path: "/wildcard-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.com", Path: "/abc-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
// Requests to the wildcard-example-com listener | ||
{ | ||
Request: http.Request{Host: "bar.example.com", Path: "/empty-hostname"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.example.com", Path: "/wildcard-example-com"}, | ||
Backend: "infra-backend-v1", | ||
Namespace: ns, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.example.com", Path: "/wildcard-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.example.com", Path: "/abc-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
// Requests to the foo-wildcard-example-com listener | ||
{ | ||
Request: http.Request{Host: "bar.foo.example.com", Path: "/empty-hostname"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.foo.example.com", Path: "/wildcard-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.foo.example.com", Path: "/wildcard-foo-example-com"}, | ||
Backend: "infra-backend-v1", | ||
Namespace: ns, | ||
}, | ||
{ | ||
Request: http.Request{Host: "bar.foo.example.com", Path: "/abc-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
// Requests to the abc-foo-example-com listener | ||
{ | ||
Request: http.Request{Host: "abc.foo.example.com", Path: "/empty-hostname"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "abc.foo.example.com", Path: "/wildcard-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
}, | ||
{ | ||
Request: http.Request{Host: "abc.foo.example.com", Path: "/wildcard-foo-example-com"}, | ||
Response: http.Response{StatusCode: 404}, | ||
Namespace: ns, | ||
}, | ||
{ | ||
Request: http.Request{Host: "abc.foo.example.com", Path: "/abc-foo-example-com"}, | ||
Backend: "infra-backend-v1", | ||
Namespace: ns, | ||
}, | ||
} | ||
|
||
t.Run("hostnames are configured only in listeners", func(t *testing.T) { | ||
gwNN := types.NamespacedName{Name: "http-listener-isolation", Namespace: ns} | ||
routes := []types.NamespacedName{ | ||
{Namespace: ns, Name: "attaches-to-empty-hostname"}, | ||
{Namespace: ns, Name: "attaches-to-wildcard-example-com"}, | ||
{Namespace: ns, Name: "attaches-to-wildcard-foo-example-com"}, | ||
{Namespace: ns, Name: "attaches-to-abc-foo-example-com"}, | ||
} | ||
|
||
gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routes...) | ||
for _, routeNN := range routes { | ||
kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN) | ||
} | ||
|
||
for i := range testCases { | ||
// Declare tc here to avoid loop variable | ||
// reuse issues across parallel tests. | ||
tc := testCases[i] | ||
t.Run(tc.GetTestCaseName(i), func(t *testing.T) { | ||
t.Parallel() | ||
http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, tc) | ||
}) | ||
} | ||
}) | ||
|
||
t.Run("intersecting hostnames are configured in listeners and HTTPRoutes", func(t *testing.T) { | ||
gwNN := types.NamespacedName{Name: "http-listener-isolation-with-hostname-intersection", Namespace: ns} | ||
routes := []types.NamespacedName{ | ||
{Namespace: ns, Name: "attaches-to-empty-hostname-with-hostname-intersection"}, | ||
{Namespace: ns, Name: "attaches-to-wildcard-example-com-with-hostname-intersection"}, | ||
{Namespace: ns, Name: "attaches-to-wildcard-foo-example-com-with-hostname-intersection"}, | ||
{Namespace: ns, Name: "attaches-to-abc-foo-example-com-with-hostname-intersection"}, | ||
} | ||
|
||
gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routes...) | ||
for _, routeNN := range routes { | ||
kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN) | ||
} | ||
|
||
for i := range testCases { | ||
// Declare tc here to avoid loop variable | ||
// reuse issues across parallel tests. | ||
tc := testCases[i] | ||
t.Run(tc.GetTestCaseName(i), func(t *testing.T) { | ||
t.Parallel() | ||
http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, tc) | ||
}) | ||
} | ||
}) | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually need 2 new Gateways for this test? (Asking because this could significantly increase the time it takes to run conformance tests + the overall resources required).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for two Gateways, I can make it work with one to decrease time and required resources 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds great, thanks! Ping me whenever this is ready for another look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pleshakov do you have time to merge these Gateways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robscott
I didn't have enough time to dedicated to this. Apologies!
Is the idea to get the PR merged before the next Gateway API release?
One thing that worries me:
since I already got approvals and confirmations from folks running this against their implementations, although merging should not cause any changes to the test behavior, there is a slight chance for it, since I will be updating code and manifests. So another round reviews and re-runs will be needed.