-
Notifications
You must be signed in to change notification settings - Fork 612
tests: Add conformace tests for listenersets #3890
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
3b1f44b
9e130f2
77ebf23
1d02411
c108086
fddf426
9ae1b17
c2c76b6
435a0fd
88cba54
79f9b52
8d27faa
6473f3e
3ec5493
f9423b7
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,203 @@ | ||
| /* | ||
| Copyright 2025 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" | ||
|
|
||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
|
|
||
| gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" | ||
| gatewayxv1a1 "sigs.k8s.io/gateway-api/apisx/v1alpha1" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/http" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/suite" | ||
| "sigs.k8s.io/gateway-api/pkg/features" | ||
| ) | ||
|
|
||
| func init() { | ||
| ConformanceTests = append(ConformanceTests, ListenerSetHostnameConflict) | ||
| } | ||
|
|
||
| var ListenerSetHostnameConflict = suite.ConformanceTest{ | ||
| ShortName: "ListenerSetHostnameConflict", | ||
| Description: "Listener Set listener with hostname conflicts to validate Listener Precedence", | ||
| Features: []features.FeatureName{ | ||
| features.SupportGateway, | ||
| features.SupportGatewayListenerSet, | ||
| features.SupportHTTPRoute, | ||
| }, | ||
| Manifests: []string{ | ||
| "tests/listenerset-hostname-conflict.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 listeners without conflicts should work | ||
| { | ||
| Request: http.Request{Host: "gateway-listener.com", Path: "/gateway-route"}, | ||
| Backend: "infra-backend-v1", | ||
| Namespace: ns, | ||
| }, | ||
| { | ||
| Request: http.Request{Host: "listenerset-1-listener.com", Path: "/listenerset-1-route"}, | ||
| Backend: "infra-backend-v2", | ||
| Namespace: ns, | ||
| }, | ||
| { | ||
| Request: http.Request{Host: "listenerset-2-listener.com", Path: "/listenerset-2-route"}, | ||
| Backend: "infra-backend-v3", | ||
| Namespace: ns, | ||
| }, | ||
| // Requests to the listener with domain name conflict should work on the first listener (based on listener precedence - gateway listener) | ||
| { | ||
| Request: http.Request{Host: "hostname-conflict-listener-1.com", Path: "/gateway-route"}, | ||
| Backend: "infra-backend-v1", | ||
| Namespace: ns, | ||
| }, | ||
| { | ||
| Request: http.Request{Host: "hostname-conflict-listener-1.com", Path: "/listenerset-1-route"}, | ||
| Response: http.Response{StatusCode: 404}, | ||
| }, | ||
| { | ||
| Request: http.Request{Host: "hostname-conflict-listener-1.com", Path: "/listenerset-2-route"}, | ||
| Response: http.Response{StatusCode: 404}, | ||
| }, | ||
| // Requests to the listener with domain name conflict should work on the first listener (based on listener precedence - alphabetic / creation time) | ||
| { | ||
| Request: http.Request{Host: "hostname-conflict-listener-2.com", Path: "/listenerset-1-route"}, | ||
| Backend: "infra-backend-v2", | ||
| Namespace: ns, | ||
| }, | ||
| { | ||
| Request: http.Request{Host: "hostname-conflict-listener-2.com", Path: "/listenerset-2-route"}, | ||
| Response: http.Response{StatusCode: 404}, | ||
| }, | ||
| } | ||
|
|
||
| acceptedListenerConditions := []metav1.Condition{ | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionResolvedRefs), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: "", // any reason | ||
| }, | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionAccepted), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: "", // any reason | ||
| }, | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionProgrammed), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: "", // any reason | ||
| }, | ||
| } | ||
| hostnameConflictedListenerConditions := []metav1.Condition{ | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionAccepted), | ||
| Status: metav1.ConditionFalse, | ||
| Reason: string(gatewayv1.ListenerReasonHostnameConflict), | ||
| }, | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionProgrammed), | ||
| Status: metav1.ConditionFalse, | ||
| Reason: string(gatewayv1.ListenerReasonHostnameConflict), | ||
| }, | ||
| { | ||
| Type: string(gatewayv1.ListenerConditionConflicted), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayv1.ListenerReasonHostnameConflict), | ||
| }, | ||
| } | ||
|
|
||
| // Gateway, route and conditions | ||
| gwNN := types.NamespacedName{Name: "gateway-with-listenerset-http-listener", Namespace: ns} | ||
| gwRoutes := []types.NamespacedName{ | ||
| {Name: "gateway-route", Namespace: ns}, | ||
| } | ||
| gwAddr := kubernetes.GatewayAndRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), &gatewayv1.HTTPRoute{}, false, gwRoutes...) | ||
| kubernetes.GatewayMustHaveCondition(t, suite.Client, suite.TimeoutConfig, gwNN, metav1.Condition{ | ||
| Type: string(gatewayv1.GatewayConditionAttachedListenerSets), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayv1.GatewayReasonListenerSetsAttached), | ||
| }) | ||
| kubernetes.GatewayListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, gwNN, acceptedListenerConditions, "gateway-listener") | ||
| // The first conflicted listener is accepted based on Listener precedence | ||
| kubernetes.GatewayListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, gwNN, acceptedListenerConditions, "hostname-conflict-listener-1") | ||
|
|
||
| // ListenerSet1, route and conditions | ||
| ls1NN := types.NamespacedName{Name: "listenerset-with-conflict-1", Namespace: ns} | ||
| ls1Routes := []types.NamespacedName{ | ||
| {Namespace: ns, Name: "listenerset-with-conflict-1-route"}, | ||
| } | ||
| for _, routeNN := range ls1Routes { | ||
| kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, ls1NN) | ||
| } | ||
| kubernetes.ListenerSetMustHaveCondition(t, suite.Client, suite.TimeoutConfig, ls1NN, metav1.Condition{ | ||
| Type: string(gatewayxv1a1.ListenerSetConditionAccepted), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayxv1a1.ListenerSetReasonListenersNotValid), | ||
| }) | ||
| kubernetes.ListenerSetMustHaveCondition(t, suite.Client, suite.TimeoutConfig, ls1NN, metav1.Condition{ | ||
| Type: string(gatewayxv1a1.ListenerSetConditionProgrammed), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayxv1a1.ListenerSetReasonListenersNotValid), | ||
| }) | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls1NN, acceptedListenerConditions, "listenerset-1-listener") | ||
| // The conflicted listener should not be accepted | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls1NN, hostnameConflictedListenerConditions, "hostname-conflict-listener-1") | ||
| // The first conflicted listener is accepted based on Listener precedence | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls1NN, acceptedListenerConditions, "hostname-conflict-listener-2") | ||
|
|
||
| // ListenerSet2, route and conditions | ||
| ls2NN := types.NamespacedName{Name: "listenerset-with-conflict-2", Namespace: ns} | ||
| ls2Routes := []types.NamespacedName{ | ||
| {Namespace: ns, Name: "listenerset-with-conflict-2-route"}, | ||
| } | ||
| for _, routeNN := range ls2Routes { | ||
| kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, ls2NN) | ||
| } | ||
| kubernetes.ListenerSetMustHaveCondition(t, suite.Client, suite.TimeoutConfig, ls2NN, metav1.Condition{ | ||
| Type: string(gatewayxv1a1.ListenerSetConditionAccepted), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayxv1a1.ListenerSetReasonListenersNotValid), | ||
| }) | ||
| kubernetes.ListenerSetMustHaveCondition(t, suite.Client, suite.TimeoutConfig, ls2NN, metav1.Condition{ | ||
| Type: string(gatewayxv1a1.ListenerSetConditionProgrammed), | ||
| Status: metav1.ConditionTrue, | ||
| Reason: string(gatewayxv1a1.ListenerSetReasonListenersNotValid), | ||
| }) | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls2NN, acceptedListenerConditions, "listenerset-2-listener") | ||
| // The conflicted listeners should not be accepted | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls2NN, hostnameConflictedListenerConditions, "hostname-conflict-listener-1") | ||
| kubernetes.ListenerSetListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, ls2NN, hostnameConflictedListenerConditions, "hostname-conflict-listener-2") | ||
|
|
||
| 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) | ||
| }) | ||
| } | ||
| }, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: Gateway | ||
| metadata: | ||
| name: gateway-with-listenerset-http-listener | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| gatewayClassName: "{GATEWAY_CLASS_NAME}" | ||
| allowedListeners: | ||
| namespaces: | ||
| from: Same | ||
| listeners: | ||
| - name: gateway-listener | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "gateway-listener.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| - name: hostname-conflict-listener-1 | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "hostname-conflict-listener-1.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| --- | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: HTTPRoute | ||
| metadata: | ||
| name: gateway-route | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRefs: | ||
| - name: gateway-with-listenerset-http-listener | ||
| namespace: gateway-conformance-infra | ||
| rules: | ||
| - matches: | ||
| - path: | ||
| type: PathPrefix | ||
| value: /gateway-route | ||
| backendRefs: | ||
| - name: infra-backend-v1 | ||
| port: 8080 | ||
| --- | ||
| apiVersion: gateway.networking.x-k8s.io/v1alpha1 | ||
| kind: XListenerSet | ||
| metadata: | ||
| name: listenerset-with-conflict-1 | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRef: | ||
| kind: Gateway | ||
| group: gateway.networking.k8s.io | ||
| name: gateway-with-listenerset-http-listener | ||
| namespace: gateway-conformance-infra | ||
| listeners: | ||
| - name: listenerset-1-listener | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "listenerset-1-listener.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| - name: hostname-conflict-listener-1 | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "hostname-conflict-listener-1.com" | ||
| allowedRoutes: | ||
|
Member
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. unrelated with this PR, but this got my attention: @dprotaso @youngnick do we really want this kind of deep allowance on a ListenerSet? The way I see a ListenerSet, it is mostly related to "Ana" persona, allowing all of these dereference will be really hard for controllers to track. Imagine this situation:
IMO ListenerSet should be scoped to the namespace they belong, and in case of a cross-namespace need of a Listener this should then be a case where a Route attaches not to a ListenerSet, but to a Gateway listener as it is today. |
||
| namespaces: | ||
| from: All | ||
| - name: hostname-conflict-listener-2 | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "hostname-conflict-listener-2.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| --- | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: HTTPRoute | ||
| metadata: | ||
| name: listenerset-with-conflict-1-route | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRefs: | ||
| - kind: XListenerSet | ||
| group: gateway.networking.x-k8s.io | ||
| name: listenerset-with-conflict-1 | ||
| namespace: gateway-conformance-infra | ||
| rules: | ||
| - matches: | ||
| - path: | ||
| type: PathPrefix | ||
| value: /listenerset-1-route | ||
| backendRefs: | ||
| - name: infra-backend-v2 | ||
| port: 8080 | ||
| --- | ||
| apiVersion: gateway.networking.x-k8s.io/v1alpha1 | ||
| kind: XListenerSet | ||
| metadata: | ||
| name: listenerset-with-conflict-2 | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRef: | ||
| kind: Gateway | ||
| group: gateway.networking.k8s.io | ||
| name: gateway-with-listenerset-http-listener | ||
| namespace: gateway-conformance-infra | ||
| listeners: | ||
| - name: listenerset-2-listener | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "listenerset-2-listener.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| - name: hostname-conflict-listener-1 | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "hostname-conflict-listener-1.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| - name: hostname-conflict-listener-2 | ||
| port: 80 | ||
| protocol: HTTP | ||
| hostname: "hostname-conflict-listener-2.com" | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| --- | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: HTTPRoute | ||
| metadata: | ||
| name: listenerset-with-conflict-2-route | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRefs: | ||
| - kind: XListenerSet | ||
| group: gateway.networking.x-k8s.io | ||
| name: listenerset-with-conflict-2 | ||
| namespace: gateway-conformance-infra | ||
| rules: | ||
| - matches: | ||
| - path: | ||
| type: PathPrefix | ||
| value: /listenerset-2-route | ||
| backendRefs: | ||
| - name: infra-backend-v3 | ||
| port: 8080 | ||
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.
I was now wondering on something on GEP that should be clarified again (sorry @davidjumani !)
I think this conflict management may be a bit unsecure. The CreationTimestamp reflects when the resource was created, but we can modify it in a way that a Listener can be stolen:
Now, given the situation above, Erin's ListenerSet is older than Ana's ListenerSet. We don't have a conflict here, but what happens if Erin then changes her ListenerSet to something like:
I don't think we have a way to say "Erin's resource is older, but the ListenerSet inside Ana's array is older" which means effectively per the conflict management that Erin's ListenerSet will win the "conflict resolution" per the resource age, and steal www.mything.tld from Ana
Am I missing something here?