From dcd13ea2f2cffecda5ee26564f63ec960cacf5a6 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Tue, 4 Jul 2023 11:37:05 +0100 Subject: [PATCH 1/4] Support SupportedKinds in ListenerStatus --- docs/gateway-api-compatibility.md | 2 +- internal/state/change_processor_test.go | 3 + internal/state/graph/gateway_listener.go | 43 ++++++---- internal/state/graph/gateway_listener_test.go | 49 +++++++++-- internal/state/graph/gateway_test.go | 84 +++++++++++++++++++ internal/state/graph/graph_test.go | 2 + internal/status/gateway.go | 10 +-- internal/status/gateway_test.go | 6 +- internal/status/statuses.go | 6 +- internal/status/updater_test.go | 24 +++--- 10 files changed, 179 insertions(+), 50 deletions(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index fc116cbfbd..5866219693 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -97,7 +97,7 @@ Fields: Gateway. NKG only supports a single Gateway. * `listeners` * `name` - supported. - * `supportedKinds` - not supported. + * `supportedKinds` - supported. * `attachedRoutes` - supported. * `conditions` - Supported (Condition/Status/Reason): * `Accepted/True/Accepted` diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 39d38e5c8c..6c26c0763b 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -451,6 +451,7 @@ var _ = Describe("ChangeProcessor", func() { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: expRouteHR1, }, + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, "listener-443-1": { Source: gw1.Spec.Listeners[1], @@ -459,6 +460,7 @@ var _ = Describe("ChangeProcessor", func() { {Namespace: "test", Name: "hr-1"}: expRouteHR1, }, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(diffNsTLSSecret)), + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, }, Valid: true, @@ -544,6 +546,7 @@ var _ = Describe("ChangeProcessor", func() { Conditions: conditions.NewListenerRefNotPermitted( "Certificate ref to secret cert-ns/different-ns-tls-secret not permitted by any ReferenceGrant", ), + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, } expAttachment := &graph.ParentRefAttachmentStatus{ diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index 0cec44b0fa..879f3b8b06 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -28,6 +28,8 @@ type Listener struct { ResolvedSecret *types.NamespacedName // Conditions holds the conditions of the Listener. Conditions []conditions.Condition + // SupportedKinds is the list of RouteGroupKinds allowed by the listener. + SupportedKinds []v1beta1.RouteGroupKind // Valid shows whether the Listener is valid. // A Listener is considered valid if NKG can generate valid NGINX configuration for it. Valid bool @@ -87,7 +89,6 @@ func newListenerConfiguratorFactory( }, http: &listenerConfigurator{ validators: []listenerValidator{ - validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, validateHTTPListener, @@ -98,7 +99,6 @@ func newListenerConfiguratorFactory( }, https: &listenerConfigurator{ validators: []listenerValidator{ - validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, createHTTPSListenerValidator(), @@ -158,11 +158,15 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { } } + cnds, supportedKinds := getAndValidateSupportedKinds(listener) + conds = append(conds, cnds...) + if len(conds) > 0 { return &Listener{ - Source: listener, - Conditions: conds, - Valid: false, + Source: listener, + Conditions: conds, + Valid: false, + SupportedKinds: supportedKinds, } } @@ -171,6 +175,7 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { AllowedRouteLabelSelector: allowedRouteSelector, Routes: make(map[types.NamespacedName]*Route), Valid: true, + SupportedKinds: supportedKinds, } // resolvers might add different conditions to the listener, so we run them all. @@ -206,7 +211,18 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition return nil } -func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition { +func getAndValidateSupportedKinds(listener v1beta1.Listener) ([]conditions.Condition, []v1beta1.RouteGroupKind) { + if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil { + return nil, []v1beta1.RouteGroupKind{ + { + Kind: "HTTPRoute", + }, + } + } + var conds []conditions.Condition + + supportedKinds := make([]v1beta1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds)) + validHTTPRouteKind := func(kind v1beta1.RouteGroupKind) bool { if kind.Kind != v1beta1.Kind("HTTPRoute") { return false @@ -219,17 +235,16 @@ func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Co switch listener.Protocol { case v1beta1.HTTPProtocolType, v1beta1.HTTPSProtocolType: - if listener.AllowedRoutes != nil { - for _, kind := range listener.AllowedRoutes.Kinds { - if !validHTTPRouteKind(kind) { - msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind) - return conditions.NewListenerInvalidRouteKinds(msg) - } + for _, kind := range listener.AllowedRoutes.Kinds { + if !validHTTPRouteKind(kind) { + msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind) + conds = append(conds, conditions.NewListenerInvalidRouteKinds(msg)...) + continue } + supportedKinds = append(supportedKinds, kind) } } - - return nil + return conds, supportedKinds } func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condition { diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index 0472238a3c..83f16dfe02 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -220,17 +220,27 @@ func TestValidateListenerHostname(t *testing.T) { } func TestValidateListenerAllowedRouteKind(t *testing.T) { + expectedKinds := []v1beta1.RouteGroupKind{ + { + Kind: "HTTPRoute", + Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), + }, + } tests := []struct { protocol v1beta1.ProtocolType kind v1beta1.Kind group v1beta1.Group name string + expected []v1beta1.RouteGroupKind expectErr bool }{ { protocol: v1beta1.TCPProtocolType, expectErr: false, name: "unsupported protocol is ignored", + kind: "TCPRoute", + group: v1beta1.GroupName, + expected: []v1beta1.RouteGroupKind{}, }, { protocol: v1beta1.HTTPProtocolType, @@ -252,6 +262,7 @@ func TestValidateListenerAllowedRouteKind(t *testing.T) { kind: "HTTPRoute", expectErr: false, name: "valid HTTP", + expected: expectedKinds, }, { protocol: v1beta1.HTTPSProtocolType, @@ -259,30 +270,50 @@ func TestValidateListenerAllowedRouteKind(t *testing.T) { kind: "HTTPRoute", expectErr: false, name: "valid HTTPS", + expected: expectedKinds, + }, + { + protocol: v1beta1.HTTPSProtocolType, + expectErr: false, + name: "valid HTTPS no kind specified", + expected: []v1beta1.RouteGroupKind{ + { + Kind: "HTTPRoute", + }, + }, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) + var listener v1beta1.Listener - listener := v1beta1.Listener{ - Protocol: test.protocol, - AllowedRoutes: &v1beta1.AllowedRoutes{ - Kinds: []v1beta1.RouteGroupKind{ - { - Kind: test.kind, - Group: &test.group, + if test.kind != "" { + listener = v1beta1.Listener{ + Protocol: test.protocol, + AllowedRoutes: &v1beta1.AllowedRoutes{ + Kinds: []v1beta1.RouteGroupKind{ + { + Kind: test.kind, + Group: &test.group, + }, }, }, - }, + } + } else { + listener = v1beta1.Listener{ + Protocol: test.protocol, + } } - conds := validateListenerAllowedRouteKind(listener) + conds, kinds := getAndValidateSupportedKinds(listener) if test.expectErr { g.Expect(conds).ToNot(BeEmpty()) + g.Expect(kinds).To(BeEmpty()) } else { g.Expect(conds).To(BeEmpty()) + g.Expect(helpers.Diff(test.expected, kinds)).To(BeEmpty()) } }) } diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index 0d8744cd7d..8738d7d3e3 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -354,11 +354,17 @@ func TestBuildGateway(t *testing.T) { Source: foo80Listener1, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-8080": { Source: foo8080Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -378,12 +384,18 @@ func TestBuildGateway(t *testing.T) { Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-8443-https": { Source: foo8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -401,6 +413,9 @@ func TestBuildGateway(t *testing.T) { Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(labels.Set(labelSet)), Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute", Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName)}, + }, }, }, Valid: true, @@ -442,6 +457,9 @@ func TestBuildGateway(t *testing.T) { Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretDiffNamespace)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -461,6 +479,9 @@ func TestBuildGateway(t *testing.T) { `Certificate ref to secret diff-ns/secret not permitted by any ReferenceGrant`, ), Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -479,6 +500,9 @@ func TestBuildGateway(t *testing.T) { Conditions: conditions.NewListenerUnsupportedValue( `invalid label selector: "invalid" is not a valid label selector operator`, ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute", Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName)}, + }, }, }, Valid: true, @@ -497,6 +521,9 @@ func TestBuildGateway(t *testing.T) { Conditions: conditions.NewListenerUnsupportedProtocol( `protocol: Unsupported value: "TCP": supported values: "HTTP", "HTTPS"`, ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -517,6 +544,9 @@ func TestBuildGateway(t *testing.T) { Conditions: conditions.NewListenerUnsupportedValue( `port: Invalid value: 0: port must be between 1-65535`, ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "invalid-https-port": { Source: invalidHTTPSPortListener, @@ -524,6 +554,9 @@ func TestBuildGateway(t *testing.T) { Conditions: conditions.NewListenerUnsupportedValue( `port: Invalid value: 0: port must be between 1-65535`, ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -542,11 +575,17 @@ func TestBuildGateway(t *testing.T) { Source: invalidHostnameListener, Valid: false, Conditions: conditions.NewListenerUnsupportedValue(invalidHostnameMsg), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "invalid-https-hostname": { Source: invalidHTTPSHostnameListener, Valid: false, Conditions: conditions.NewListenerUnsupportedValue(invalidHostnameMsg), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -566,6 +605,9 @@ func TestBuildGateway(t *testing.T) { Conditions: conditions.NewListenerInvalidCertificateRef( `tls.certificateRefs[0]: Invalid value: test/does-not-exist: secret does not exist`, ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -595,45 +637,69 @@ func TestBuildGateway(t *testing.T) { Source: foo80Listener1, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-8080": { Source: foo8080Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-8081": { Source: foo8081Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "bar-80": { Source: bar80Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-443-https-1": { Source: foo443HTTPSListener1, Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-8443-https": { Source: foo8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "bar-443-https": { Source: bar443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "bar-8443-https": { Source: bar8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, @@ -662,18 +728,27 @@ func TestBuildGateway(t *testing.T) { Valid: false, Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "bar-80": { Source: bar80Listener, Valid: false, Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-443": { Source: foo443Listener, Valid: false, Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-80-https": { Source: foo80HTTPSListener, @@ -681,6 +756,9 @@ func TestBuildGateway(t *testing.T) { Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "foo-443-https-1": { Source: foo443HTTPSListener1, @@ -688,6 +766,9 @@ func TestBuildGateway(t *testing.T) { Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, "bar-443-https": { Source: bar443HTTPSListener, @@ -695,6 +776,9 @@ func TestBuildGateway(t *testing.T) { Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, }, }, Valid: true, diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index 2428eba68c..3bc5bd5847 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -271,6 +271,7 @@ func TestBuildGraph(t *testing.T) { Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, "listener-443-1": { Source: gw1.Spec.Listeners[1], @@ -279,6 +280,7 @@ func TestBuildGraph(t *testing.T) { {Namespace: "test", Name: "hr-3"}: routeHR3, }, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secret)), + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, }, Valid: true, diff --git a/internal/status/gateway.go b/internal/status/gateway.go index aeaf13576e..515b830e48 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -27,14 +27,8 @@ func prepareGatewayStatus( s := gatewayStatus.ListenerStatuses[name] listenerStatuses = append(listenerStatuses, v1beta1.ListenerStatus{ - Name: v1beta1.SectionName(name), - SupportedKinds: []v1beta1.RouteGroupKind{ - { - // FIXME(pleshakov) Set it based on the listener - // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/690 - Kind: "HTTPRoute", - }, - }, + Name: v1beta1.SectionName(name), + SupportedKinds: s.SupportedKinds, AttachedRoutes: s.AttachedRoutes, Conditions: convertConditions(s.Conditions, gatewayStatus.ObservedGeneration, transitionTime), }) diff --git a/internal/status/gateway_test.go b/internal/status/gateway_test.go index 96df92a89f..a3e6d5a1ff 100644 --- a/internal/status/gateway_test.go +++ b/internal/status/gateway_test.go @@ -17,13 +17,17 @@ func TestPrepareGatewayStatus(t *testing.T) { Type: &ipAddrType, Value: "1.2.3.4", } - status := GatewayStatus{ Conditions: CreateTestConditions("GatewayTest"), ListenerStatuses: ListenerStatuses{ "listener": { AttachedRoutes: 3, Conditions: CreateTestConditions("ListenerTest"), + SupportedKinds: []v1beta1.RouteGroupKind{ + { + Kind: v1beta1.Kind("HTTPRoute"), + }, + }, }, }, ObservedGeneration: 1, diff --git a/internal/status/statuses.go b/internal/status/statuses.go index a38b3bac8e..f7c4a8d46a 100644 --- a/internal/status/statuses.go +++ b/internal/status/statuses.go @@ -40,9 +40,8 @@ type GatewayStatus struct { // ListenerStatus holds the status-related information about a listener in the Gateway resource. type ListenerStatus struct { - // Conditions is the list of conditions for this listener. - Conditions []conditions.Condition - // AttachedRoutes is the number of routes attached to the listener. + Conditions []conditions.Condition + SupportedKinds []v1beta1.RouteGroupKind AttachedRoutes int32 } @@ -207,6 +206,7 @@ func buildGatewayStatus(gateway *graph.Gateway, nginxReloadRes NginxReloadResult listenerStatuses[name] = ListenerStatus{ AttachedRoutes: int32(len(l.Routes)), Conditions: conditions.DeduplicateConditions(conds), + SupportedKinds: l.SupportedKinds, } } diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index 43d401a519..4732ff85a9 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -13,7 +13,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/gateway-api/apis/v1beta1" - gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" @@ -34,7 +33,7 @@ var _ = Describe("Updater", func() { BeforeEach(OncePerOrdered, func() { scheme := runtime.NewScheme() - Expect(gatewayv1beta1.AddToScheme(scheme)).Should(Succeed()) + Expect(v1beta1.AddToScheme(scheme)).Should(Succeed()) client = fake.NewClientBuilder(). WithScheme(scheme). @@ -84,6 +83,7 @@ var _ = Describe("Updater", func() { "http": { AttachedRoutes: 1, Conditions: status.CreateTestConditions("Test"), + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, }, ObservedGeneration: gens.gateways, @@ -135,16 +135,12 @@ var _ = Describe("Updater", func() { }, Status: v1beta1.GatewayStatus{ Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), - Listeners: []gatewayv1beta1.ListenerStatus{ + Listeners: []v1beta1.ListenerStatus{ { - Name: "http", - SupportedKinds: []v1beta1.RouteGroupKind{ - { - Kind: "HTTPRoute", - }, - }, + Name: "http", AttachedRoutes: 1, Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), + SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, }, Addresses: []v1beta1.GatewayAddress{addr}, @@ -196,12 +192,12 @@ var _ = Describe("Updater", func() { Kind: "HTTPRoute", APIVersion: "gateway.networking.k8s.io/v1beta1", }, - Status: gatewayv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayv1beta1.RouteStatus{ - Parents: []gatewayv1beta1.RouteParentStatus{ + Status: v1beta1.HTTPRouteStatus{ + RouteStatus: v1beta1.RouteStatus{ + Parents: []v1beta1.RouteParentStatus{ { - ControllerName: gatewayv1beta1.GatewayController(gatewayCtrlName), - ParentRef: gatewayv1beta1.ParentReference{ + ControllerName: v1beta1.GatewayController(gatewayCtrlName), + ParentRef: v1beta1.ParentReference{ Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), Name: "gateway", SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("http")), From 82e45c2822fa70456e80e0f4a319afd80877d782 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Thu, 6 Jul 2023 14:23:25 +0100 Subject: [PATCH 2/4] Update unit test --- internal/state/graph/gateway_listener_test.go | 83 +++++++++++-------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index 83f16dfe02..b6174801d1 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -219,18 +219,23 @@ func TestValidateListenerHostname(t *testing.T) { } } -func TestValidateListenerAllowedRouteKind(t *testing.T) { - expectedKinds := []v1beta1.RouteGroupKind{ +func TestGetAndValidateSupportedKinds(t *testing.T) { + HTTPRouteGroupKind := []v1beta1.RouteGroupKind{ { Kind: "HTTPRoute", Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), }, } + TCPRouteGroupKind := []v1beta1.RouteGroupKind{ + { + Kind: "TCPRoute", + Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), + }, + } tests := []struct { protocol v1beta1.ProtocolType - kind v1beta1.Kind - group v1beta1.Group name string + kind []v1beta1.RouteGroupKind expected []v1beta1.RouteGroupKind expectErr bool }{ @@ -238,39 +243,41 @@ func TestValidateListenerAllowedRouteKind(t *testing.T) { protocol: v1beta1.TCPProtocolType, expectErr: false, name: "unsupported protocol is ignored", - kind: "TCPRoute", - group: v1beta1.GroupName, + kind: TCPRouteGroupKind, expected: []v1beta1.RouteGroupKind{}, }, { - protocol: v1beta1.HTTPProtocolType, - group: "bad-group", - kind: "HTTPRoute", + protocol: v1beta1.HTTPProtocolType, + kind: []v1beta1.RouteGroupKind{ + { + Kind: "HTTPRoute", + Group: helpers.GetPointer[v1beta1.Group]("bad-group"), + }, + }, expectErr: true, name: "invalid group", + expected: []v1beta1.RouteGroupKind{}, }, { protocol: v1beta1.HTTPProtocolType, - group: v1beta1.GroupName, - kind: "TCPRoute", + kind: TCPRouteGroupKind, expectErr: true, name: "invalid kind", + expected: []v1beta1.RouteGroupKind{}, }, { protocol: v1beta1.HTTPProtocolType, - group: v1beta1.GroupName, - kind: "HTTPRoute", + kind: HTTPRouteGroupKind, expectErr: false, name: "valid HTTP", - expected: expectedKinds, + expected: HTTPRouteGroupKind, }, { protocol: v1beta1.HTTPSProtocolType, - group: v1beta1.GroupName, - kind: "HTTPRoute", + kind: HTTPRouteGroupKind, expectErr: false, name: "valid HTTPS", - expected: expectedKinds, + expected: HTTPRouteGroupKind, }, { protocol: v1beta1.HTTPSProtocolType, @@ -282,38 +289,44 @@ func TestValidateListenerAllowedRouteKind(t *testing.T) { }, }, }, + { + protocol: v1beta1.HTTPProtocolType, + kind: []v1beta1.RouteGroupKind{ + { + Kind: "HTTPRoute", + Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), + }, + { + Kind: "bad-kind", + Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName), + }, + }, + expectErr: true, + name: "valid and invalid kinds", + expected: HTTPRouteGroupKind, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - var listener v1beta1.Listener - if test.kind != "" { - listener = v1beta1.Listener{ - Protocol: test.protocol, - AllowedRoutes: &v1beta1.AllowedRoutes{ - Kinds: []v1beta1.RouteGroupKind{ - { - Kind: test.kind, - Group: &test.group, - }, - }, - }, - } - } else { - listener = v1beta1.Listener{ - Protocol: test.protocol, + listener := v1beta1.Listener{ + Protocol: test.protocol, + } + + if test.kind != nil { + listener.AllowedRoutes = &v1beta1.AllowedRoutes{ + Kinds: test.kind, } } conds, kinds := getAndValidateSupportedKinds(listener) + g.Expect(helpers.Diff(test.expected, kinds)).To(BeEmpty()) if test.expectErr { g.Expect(conds).ToNot(BeEmpty()) - g.Expect(kinds).To(BeEmpty()) } else { g.Expect(conds).To(BeEmpty()) - g.Expect(helpers.Diff(test.expected, kinds)).To(BeEmpty()) } }) } From 2a3077290dff297eb8fed22d227eff38a5d74519 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Fri, 7 Jul 2023 12:00:26 +0100 Subject: [PATCH 3/4] Create wrappers for shared function with different return signatures --- internal/state/graph/gateway_listener.go | 19 ++++++++++++++++--- internal/state/graph/gateway_listener_test.go | 4 ++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index 879f3b8b06..38ff7084db 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -89,6 +89,7 @@ func newListenerConfiguratorFactory( }, http: &listenerConfigurator{ validators: []listenerValidator{ + validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, validateHTTPListener, @@ -158,8 +159,7 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { } } - cnds, supportedKinds := getAndValidateSupportedKinds(listener) - conds = append(conds, cnds...) + supportedKinds := getListenerSupportedKinds(listener) if len(conds) > 0 { return &Listener{ @@ -211,7 +211,10 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition return nil } -func getAndValidateSupportedKinds(listener v1beta1.Listener) ([]conditions.Condition, []v1beta1.RouteGroupKind) { +func getAndValidateListenerSupportedKinds(listener v1beta1.Listener) ( + []conditions.Condition, + []v1beta1.RouteGroupKind, +) { if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil { return nil, []v1beta1.RouteGroupKind{ { @@ -247,6 +250,16 @@ func getAndValidateSupportedKinds(listener v1beta1.Listener) ([]conditions.Condi return conds, supportedKinds } +func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition { + conds, _ := getAndValidateListenerSupportedKinds(listener) + return conds +} + +func getListenerSupportedKinds(listener v1beta1.Listener) []v1beta1.RouteGroupKind { + _, kinds := getAndValidateListenerSupportedKinds(listener) + return kinds +} + func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condition { if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil && diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index b6174801d1..bbc7c66a87 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -219,7 +219,7 @@ func TestValidateListenerHostname(t *testing.T) { } } -func TestGetAndValidateSupportedKinds(t *testing.T) { +func TestGetAndValidateListenerSupportedKinds(t *testing.T) { HTTPRouteGroupKind := []v1beta1.RouteGroupKind{ { Kind: "HTTPRoute", @@ -321,7 +321,7 @@ func TestGetAndValidateSupportedKinds(t *testing.T) { } } - conds, kinds := getAndValidateSupportedKinds(listener) + conds, kinds := getAndValidateListenerSupportedKinds(listener) g.Expect(helpers.Diff(test.expected, kinds)).To(BeEmpty()) if test.expectErr { g.Expect(conds).ToNot(BeEmpty()) From e6d57f10ecf8c07d513f4994d03823ca0e70e0c7 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Mon, 10 Jul 2023 09:55:32 +0100 Subject: [PATCH 4/4] Re-add accidently removed validator --- internal/state/graph/gateway_listener.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index 38ff7084db..31119d7abd 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -100,6 +100,7 @@ func newListenerConfiguratorFactory( }, https: &listenerConfigurator{ validators: []listenerValidator{ + validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, createHTTPSListenerValidator(),