From f852c9c2d29683c181f8a44db776fc172575e2aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Thu, 23 May 2024 15:11:23 +0200 Subject: [PATCH] fix: fix attachedRoutes calculation when sectionName does not match --- internal/types/gatewaytypes.go | 5 + pkg/utils/gateway/ownerrefs.go | 28 ++- pkg/utils/gateway/ownerrefs_test.go | 272 ++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 3 deletions(-) create mode 100644 pkg/utils/gateway/ownerrefs_test.go diff --git a/internal/types/gatewaytypes.go b/internal/types/gatewaytypes.go index 885fcf6b9..82aed7c36 100644 --- a/internal/types/gatewaytypes.go +++ b/internal/types/gatewaytypes.go @@ -17,12 +17,17 @@ type ( CommonRouteSpec = gatewayv1.CommonRouteSpec Kind = gatewayv1.Kind Group = gatewayv1.Group + Namespace = gatewayv1.Namespace AllowedRoutes = gatewayv1.AllowedRoutes RouteGroupKind = gatewayv1.RouteGroupKind RouteNamespaces = gatewayv1.RouteNamespaces ObjectName = gatewayv1.ObjectName + SectionName = gatewayv1.SectionName + PortNumber = gatewayv1.PortNumber ) +var GroupVersion = gatewayv1.GroupVersion + const ( HTTPProtocolType = gatewayv1.HTTPProtocolType diff --git a/pkg/utils/gateway/ownerrefs.go b/pkg/utils/gateway/ownerrefs.go index 860007415..c54fd016a 100644 --- a/pkg/utils/gateway/ownerrefs.go +++ b/pkg/utils/gateway/ownerrefs.go @@ -114,9 +114,31 @@ func ListHTTPRoutesForGateway( for _, httpRoute := range httpRoutesList.Items { if !lo.ContainsBy(httpRoute.Spec.ParentRefs, func(parentRef gwtypes.ParentReference) bool { gwGVK := gateway.GroupVersionKind() - return (parentRef.Group != nil && string(*parentRef.Group) == gwGVK.Group) && - (parentRef.Kind != nil && string(*parentRef.Kind) == gwGVK.Kind) && - string(parentRef.Name) == gateway.Name + if parentRef.Group != nil && string(*parentRef.Group) != gwGVK.Group { + return false + } + if parentRef.Kind != nil && string(*parentRef.Kind) != gwGVK.Kind { + return false + } + if string(parentRef.Name) != gateway.Name { + return false + } + + if parentRef.SectionName != nil { + if !lo.ContainsBy(gateway.Spec.Listeners, func(listener gwtypes.Listener) bool { + if listener.Name != *parentRef.SectionName { + return false + } + if parentRef.Port != nil && listener.Port != *parentRef.Port { + return false + } + return true + }) { + return false + } + } + + return true }) { continue } diff --git a/pkg/utils/gateway/ownerrefs_test.go b/pkg/utils/gateway/ownerrefs_test.go new file mode 100644 index 000000000..c9e682027 --- /dev/null +++ b/pkg/utils/gateway/ownerrefs_test.go @@ -0,0 +1,272 @@ +package gateway + +import ( + "context" + "testing" + + "github.com/samber/lo" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + gwtypes "github.com/kong/gateway-operator/internal/types" + "github.com/kong/gateway-operator/modules/manager/scheme" +) + +func TestListHTTPRoutesForGateway(t *testing.T) { + testCases := []struct { + name string + httpRoutes []client.Object + gateway *gwtypes.Gateway + expected []gwtypes.HTTPRoute + expectedErr bool + }{ + { + name: "returns HTTPRoute for a Gateway", + httpRoutes: []client.Object{ + &gwtypes.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + }, + }, + }, + }, + }, + }, + gateway: &gwtypes.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gwtypes.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "default", + }, + }, + expected: []gwtypes.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + }, + }, + }, + }, + }, + }, + }, + { + name: "does not return HTTPRoute for a Gateway when it is not a parent", + httpRoutes: []client.Object{ + &gwtypes.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{}, + }, + }, + gateway: &gwtypes.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gwtypes.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "default", + }, + }, + expected: nil, + }, + { + name: "returns HTTPRoute when section name does match", + httpRoutes: []client.Object{ + &gwtypes.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + SectionName: lo.ToPtr(gwtypes.SectionName("http")), + }, + }, + }, + }, + }, + }, + gateway: &gwtypes.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gwtypes.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "default", + }, + Spec: gwtypes.GatewaySpec{ + Listeners: []gwtypes.Listener{ + { + Name: "http", + Port: 80, + }, + }, + }, + }, + expected: []gwtypes.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + SectionName: lo.ToPtr(gwtypes.SectionName("http")), + }, + }, + }, + }, + }, + }, + }, + { + name: "does not return HTTPRoute when section name does not match", + httpRoutes: []client.Object{ + &gwtypes.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + SectionName: lo.ToPtr(gwtypes.SectionName("http-1")), + }, + }, + }, + }, + }, + }, + gateway: &gwtypes.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gwtypes.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "default", + }, + Spec: gwtypes.GatewaySpec{ + Listeners: []gwtypes.Listener{ + { + Name: "http", + Port: 80, + }, + }, + }, + }, + expected: nil, + }, + { + name: "does not return HTTPRoute when port does not match", + httpRoutes: []client.Object{ + &gwtypes.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: gwtypes.HTTPRouteSpec{ + CommonRouteSpec: gwtypes.CommonRouteSpec{ + ParentRefs: []gwtypes.ParentReference{ + { + Group: lo.ToPtr(gwtypes.Group(gwtypes.GroupVersion.Group)), + Kind: lo.ToPtr(gwtypes.Kind("Gateway")), + Name: gwtypes.ObjectName("gw-1"), + SectionName: lo.ToPtr(gwtypes.SectionName("http")), + Port: lo.ToPtr(gwtypes.PortNumber(8080)), + }, + }, + }, + }, + }, + }, + gateway: &gwtypes.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: gwtypes.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "default", + }, + Spec: gwtypes.GatewaySpec{ + Listeners: []gwtypes.Listener{ + { + Name: "http", + Port: 80, + }, + }, + }, + }, + expected: nil, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(scheme.Get()). + WithObjects(tc.gateway). + WithObjects(tc.httpRoutes...). + Build() + routes, err := ListHTTPRoutesForGateway(context.Background(), cl, tc.gateway) + if tc.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tc.expected, routes) + } + }) + + } +}