From 39c29be10e50112da4b019856976b54339bd6cee Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Thu, 8 Aug 2024 16:46:36 -0600 Subject: [PATCH 1/3] Support cross-namespace routing with TLSRoutes --- .../static/state/change_processor_test.go | 502 +++++++++++++++--- internal/mode/static/state/graph/graph.go | 1 + .../static/state/graph/reference_grant.go | 8 + .../state/graph/reference_grant_test.go | 46 ++ .../mode/static/state/graph/route_common.go | 9 +- .../static/state/graph/route_common_test.go | 3 + internal/mode/static/state/graph/tlsroute.go | 66 ++- .../mode/static/state/graph/tlsroute_test.go | 148 +++++- tests/Makefile | 2 +- 9 files changed, 649 insertions(+), 136 deletions(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 0d55189d41..fe790d6087 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -34,8 +34,11 @@ import ( ) const ( - controllerName = "my.controller" - gcName = "test-class" + controllerName = "my.controller" + gcName = "test-class" + httpListenerName = "listener-80-1" + httpsListenerName = "listener-443-1" + tlsListenerName = "listener-8443-1" ) func createRoute( @@ -57,14 +60,14 @@ func createRoute( Namespace: (*v1.Namespace)(helpers.GetPointer("test")), Name: v1.ObjectName(gateway), SectionName: (*v1.SectionName)( - helpers.GetPointer("listener-80-1"), + helpers.GetPointer(httpListenerName), ), }, { Namespace: (*v1.Namespace)(helpers.GetPointer("test")), Name: v1.ObjectName(gateway), SectionName: (*v1.SectionName)( - helpers.GetPointer("listener-443-1"), + helpers.GetPointer(httpsListenerName), ), }, }, @@ -89,32 +92,49 @@ func createRoute( } } -func createGateway(name string) *v1.Gateway { - return &v1.Gateway{ +func createTLSRoute(name, gateway, hostname string, backendRefs ...v1.BackendRef) *v1alpha2.TLSRoute { + return &v1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", Name: name, Generation: 1, }, - Spec: v1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1.Listener{ + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: v1.CommonRouteSpec{ + ParentRefs: []v1.ParentReference{ + { + Namespace: (*v1.Namespace)(helpers.GetPointer("test")), + Name: v1.ObjectName(gateway), + SectionName: (*v1.SectionName)( + helpers.GetPointer(tlsListenerName), + ), + }, + }, + }, + Hostnames: []v1.Hostname{ + v1.Hostname(hostname), + }, + Rules: []v1alpha2.TLSRouteRule{ { - Name: "listener-80-1", - Hostname: nil, - Port: 80, - Protocol: v1.HTTPProtocolType, + BackendRefs: backendRefs, }, }, }, } } -func createGatewayWithTLSListener(name string, tlsSecret *apiv1.Secret) *v1.Gateway { - gw := createGateway(name) +func createHTTPListener() v1.Listener { + return v1.Listener{ + Name: httpListenerName, + Hostname: nil, + Port: 80, + Protocol: v1.HTTPProtocolType, + } +} - l := v1.Listener{ - Name: "listener-443-1", +func createHTTPSListener(name string, tlsSecret *apiv1.Secret) v1.Listener { + return v1.Listener{ + Name: v1.SectionName(name), Hostname: nil, Port: 443, Protocol: v1.HTTPSProtocolType, @@ -129,9 +149,32 @@ func createGatewayWithTLSListener(name string, tlsSecret *apiv1.Secret) *v1.Gate }, }, } - gw.Spec.Listeners = append(gw.Spec.Listeners, l) +} - return gw +func createTLSListener(name string) v1.Listener { + return v1.Listener{ + Name: v1.SectionName(name), + Hostname: nil, + Port: 8443, + Protocol: v1.TLSProtocolType, + TLS: &v1.GatewayTLSConfig{ + Mode: helpers.GetPointer(v1.TLSModePassthrough), + }, + } +} + +func createGateway(name string, listeners ...v1.Listener) *v1.Gateway { + return &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + Generation: 1, + }, + Spec: v1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: listeners, + }, + } } func createRouteWithMultipleRules( @@ -158,23 +201,41 @@ func createHTTPRule(path string, backendRefs ...v1.HTTPBackendRef) v1.HTTPRouteR } } -func createBackendRef( +func createHTTPBackendRef( kind *v1.Kind, name v1.ObjectName, namespace *v1.Namespace, ) v1.HTTPBackendRef { return v1.HTTPBackendRef{ BackendRef: v1.BackendRef{ - BackendObjectReference: v1.BackendObjectReference{ - Kind: kind, - Name: name, - Namespace: namespace, - Port: helpers.GetPointer[v1.PortNumber](80), - }, + BackendObjectReference: createBackendRefObj(kind, name, namespace), }, } } +func createTLSBackendRef( + name v1.ObjectName, + namespace v1.Namespace, +) v1.BackendRef { + kindSvc := v1.Kind("Service") + return v1.BackendRef{ + BackendObjectReference: createBackendRefObj(&kindSvc, name, &namespace), + } +} + +func createBackendRefObj( + kind *v1.Kind, + name v1.ObjectName, + namespace *v1.Namespace, +) v1.BackendObjectReference { + return v1.BackendObjectReference{ + Kind: kind, + Name: name, + Namespace: namespace, + Port: helpers.GetPointer[v1.PortNumber](80), + } +} + func createRouteBackendRefs(refs []v1.HTTPBackendRef) []graph.RouteBackendRef { rbrs := make([]graph.RouteBackendRef, 0, len(refs)) for _, ref := range refs { @@ -298,15 +359,18 @@ var _ = Describe("ChangeProcessor", func() { Describe("Process gateway resources", Ordered, func() { var ( - gcUpdated *v1.GatewayClass - diffNsTLSSecret, sameNsTLSSecret *apiv1.Secret - hr1, hr1Updated, hr2 *v1.HTTPRoute - gw1, gw1Updated, gw2 *v1.Gateway - refGrant1, refGrant2 *v1beta1.ReferenceGrant - expGraph *graph.Graph - expRouteHR1, expRouteHR2 *graph.L7Route - gatewayAPICRD, gatewayAPICRDUpdated *metav1.PartialObjectMetadata - routeKey1, routeKey2 graph.RouteKey + gcUpdated *v1.GatewayClass + diffNsTLSSecret, sameNsTLSSecret *apiv1.Secret + hr1, hr1Updated, hr2 *v1.HTTPRoute + tr1, tr1Updated, tr2 *v1alpha2.TLSRoute + gw1, gw1Updated, gw2 *v1.Gateway + secretRefGrant, hrServiceRefGrant, trServiceRefGrant *v1beta1.ReferenceGrant + expGraph *graph.Graph + expRouteHR1, expRouteHR2 *graph.L7Route + expRouteTR1, expRouteTR2 *graph.L4Route + gatewayAPICRD, gatewayAPICRDUpdated *metav1.PartialObjectMetadata + routeKey1, routeKey2 graph.RouteKey + trKey1, trKey2 graph.L4RouteKey ) BeforeAll(func() { gcUpdated = gc.DeepCopy() @@ -334,7 +398,20 @@ var _ = Describe("ChangeProcessor", func() { routeKey2 = graph.CreateRouteKey(hr2) - refGrant1 = &v1beta1.ReferenceGrant{ + tlsBackendRef := createTLSBackendRef("tls-service", "tls-service-ns") + + tr1 = createTLSRoute("tr-1", "gateway-1", "foo.tls.com", tlsBackendRef) + + trKey1 = graph.CreateRouteKeyL4(tr1) + + tr1Updated = tr1.DeepCopy() + tr1Updated.Generation++ + + tr2 = createTLSRoute("tr-2", "gateway-2", "bar.tls.com", tlsBackendRef) + + trKey2 = graph.CreateRouteKeyL4(tr2) + + secretRefGrant = &v1beta1.ReferenceGrant{ ObjectMeta: metav1.ObjectMeta{ Namespace: "cert-ns", Name: "ref-grant", @@ -355,7 +432,7 @@ var _ = Describe("ChangeProcessor", func() { }, } - refGrant2 = &v1beta1.ReferenceGrant{ + hrServiceRefGrant = &v1beta1.ReferenceGrant{ ObjectMeta: metav1.ObjectMeta{ Namespace: "service-ns", Name: "ref-grant", @@ -376,6 +453,27 @@ var _ = Describe("ChangeProcessor", func() { }, } + trServiceRefGrant = &v1beta1.ReferenceGrant{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tls-service-ns", + Name: "ref-grant", + }, + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1.GroupName, + Kind: kinds.TLSRoute, + Namespace: "test", + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Kind: "Service", + }, + }, + }, + } + sameNsTLSSecret = &apiv1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "tls-secret", @@ -400,12 +498,22 @@ var _ = Describe("ChangeProcessor", func() { }, } - gw1 = createGatewayWithTLSListener("gateway-1", diffNsTLSSecret) // cert in diff namespace than gw + gw1 = createGateway( + "gateway-1", + createHTTPListener(), + createHTTPSListener(httpsListenerName, diffNsTLSSecret), // cert in diff namespace than gw + createTLSListener(tlsListenerName), + ) gw1Updated = gw1.DeepCopy() gw1Updated.Generation++ - gw2 = createGatewayWithTLSListener("gateway-2", sameNsTLSSecret) + gw2 = createGateway( + "gateway-2", + createHTTPListener(), + createHTTPSListener(httpsListenerName, sameNsTLSSecret), + createTLSListener(tlsListenerName), + ) gatewayAPICRD = &metav1.PartialObjectMetadata{ TypeMeta: metav1.TypeMeta{ @@ -430,7 +538,7 @@ var _ = Describe("ChangeProcessor", func() { ParentRefs: []graph.ParentRef{ { Attachment: &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{"listener-80-1": {"foo.example.com"}}, + AcceptedHostnames: map[string][]string{httpListenerName: {"foo.example.com"}}, Attached: true, ListenerPort: 80, }, @@ -439,7 +547,7 @@ var _ = Describe("ChangeProcessor", func() { }, { Attachment: &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{"listener-443-1": {"foo.example.com"}}, + AcceptedHostnames: map[string][]string{httpsListenerName: {"foo.example.com"}}, Attached: true, ListenerPort: 443, }, @@ -480,7 +588,7 @@ var _ = Describe("ChangeProcessor", func() { ParentRefs: []graph.ParentRef{ { Attachment: &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{"listener-80-1": {"bar.example.com"}}, + AcceptedHostnames: map[string][]string{httpListenerName: {"bar.example.com"}}, Attached: true, ListenerPort: 80, }, @@ -489,7 +597,7 @@ var _ = Describe("ChangeProcessor", func() { }, { Attachment: &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{"listener-443-1": {"bar.example.com"}}, + AcceptedHostnames: map[string][]string{httpsListenerName: {"bar.example.com"}}, Attached: true, ListenerPort: 443, }, @@ -513,6 +621,62 @@ var _ = Describe("ChangeProcessor", func() { Attachable: true, } + expRouteTR1 = &graph.L4Route{ + Source: tr1, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{tlsListenerName: {"foo.tls.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + SectionName: tr1.Spec.ParentRefs[0].SectionName, + }, + }, + Spec: graph.L4RouteSpec{ + Hostnames: tr1.Spec.Hostnames, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"}, + Valid: false, + }, + }, + Valid: true, + Attachable: true, + Conditions: []conditions.Condition{ + staticConds.NewRouteBackendRefRefBackendNotFound( + "spec.rules[0].backendRefs[0].name: Not found: \"tls-service\"", + ), + }, + } + + expRouteTR2 = &graph.L4Route{ + Source: tr2, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{tlsListenerName: {"bar.tls.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + SectionName: tr2.Spec.ParentRefs[0].SectionName, + }, + }, + Spec: graph.L4RouteSpec{ + Hostnames: tr2.Spec.Hostnames, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"}, + Valid: false, + }, + }, + Valid: true, + Attachable: true, + Conditions: []conditions.Condition{ + staticConds.NewRouteBackendRefRefBackendNotFound( + "spec.rules[0].backendRefs[0].name: Not found: \"tls-service\"", + ), + }, + } + // This is the base case expected graph. Tests will manipulate this to add or remove elements // to fit the expected output of the input under test. expGraph = &graph.Graph{ @@ -524,7 +688,7 @@ var _ = Describe("ChangeProcessor", func() { Source: gw1, Listeners: []*graph.Listener{ { - Name: "listener-80-1", + Name: httpListenerName, Source: gw1.Spec.Listeners[0], Valid: true, Attachable: true, @@ -536,7 +700,7 @@ var _ = Describe("ChangeProcessor", func() { }, }, { - Name: "listener-443-1", + Name: httpsListenerName, Source: gw1.Spec.Listeners[1], Valid: true, Attachable: true, @@ -548,11 +712,22 @@ var _ = Describe("ChangeProcessor", func() { {Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, }, }, + { + Name: tlsListenerName, + Source: gw1.Spec.Listeners[2], + Valid: true, + Attachable: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{trKey1: expRouteTR1}, + SupportedKinds: []v1.RouteGroupKind{ + {Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + }, + }, }, Valid: true, }, IgnoredGateways: map[types.NamespacedName]*v1.Gateway{}, - L4Routes: map[graph.L4RouteKey]*graph.L4Route{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{trKey1: expRouteTR1}, Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1}, ReferencedSecrets: map[types.NamespacedName]*graph.Secret{}, ReferencedServices: map[types.NamespacedName]struct{}{ @@ -560,6 +735,10 @@ var _ = Describe("ChangeProcessor", func() { Namespace: "service-ns", Name: "service", }: {}, + { + Namespace: "tls-service-ns", + Name: "tls-service", + }: {}, }, } }) @@ -593,6 +772,16 @@ var _ = Describe("ChangeProcessor", func() { Expect(helpers.Diff(&graph.Graph{}, processor.GetLatestGraph())).To(BeEmpty()) }) }) + When("the first TLSRoute is upserted", func() { + It("returns empty graph", func() { + processor.CaptureUpsertChange(tr1) + + changed, graphCfg := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(helpers.Diff(&graph.Graph{}, graphCfg)).To(BeEmpty()) + Expect(helpers.Diff(&graph.Graph{}, processor.GetLatestGraph())).To(BeEmpty()) + }) + }) When("the different namespace TLS Secret is upserted", func() { It("returns nil graph", func() { processor.CaptureUpsertChange(diffNsTLSSecret) @@ -613,12 +802,20 @@ var _ = Describe("ChangeProcessor", func() { expGraph.Gateway.Valid = false expGraph.Gateway.Listeners = nil - // no ref grant exists yet for hr1 + // no ref grant exists yet for hr1 or tr1 expGraph.Routes[routeKey1].Conditions = []conditions.Condition{ staticConds.NewRouteBackendRefRefNotPermitted( "Backend ref to Service service-ns/service not permitted by any ReferenceGrant", ), } + + expGraph.L4Routes[trKey1].Conditions = []conditions.Condition{ + staticConds.NewRouteBackendRefRefNotPermitted( + "Backend ref to Service tls-service-ns/tls-service not permitted by any ReferenceGrant", + ), + } + + // gateway class does not exist so routes cannot attach expGraph.Routes[routeKey1].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ AcceptedHostnames: map[string][]string{}, FailedCondition: staticConds.NewRouteNoMatchingParent(), @@ -627,11 +824,16 @@ var _ = Describe("ChangeProcessor", func() { AcceptedHostnames: map[string][]string{}, FailedCondition: staticConds.NewRouteNoMatchingParent(), } + expGraph.L4Routes[trKey1].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNoMatchingParent(), + } expGraph.ReferencedSecrets = nil expGraph.ReferencedServices = nil expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} + expRouteTR1.Spec.BackendRef.SvcNsName = types.NamespacedName{} changed, graphCfg := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -647,7 +849,7 @@ var _ = Describe("ChangeProcessor", func() { // No ref grant exists yet for gw1 // so the listener is not valid, but still attachable - listener443 := getListenerByName(expGraph.Gateway, "listener-443-1") + listener443 := getListenerByName(expGraph.Gateway, httpsListenerName) listener443.Valid = false listener443.ResolvedSecret = nil listener443.Conditions = staticConds.NewListenerRefNotPermitted( @@ -656,7 +858,7 @@ var _ = Describe("ChangeProcessor", func() { expAttachment80 := &graph.ParentRefAttachmentStatus{ AcceptedHostnames: map[string][]string{ - "listener-80-1": {"foo.example.com"}, + httpListenerName: {"foo.example.com"}, }, Attached: true, ListenerPort: 80, @@ -664,13 +866,13 @@ var _ = Describe("ChangeProcessor", func() { expAttachment443 := &graph.ParentRefAttachmentStatus{ AcceptedHostnames: map[string][]string{ - "listener-443-1": {"foo.example.com"}, + httpsListenerName: {"foo.example.com"}, }, Attached: true, ListenerPort: 443, } - listener80 := getListenerByName(expGraph.Gateway, "listener-80-1") + listener80 := getListenerByName(expGraph.Gateway, httpListenerName) listener80.Routes[routeKey1].ParentRefs[0].Attachment = expAttachment80 listener443.Routes[routeKey1].ParentRefs[1].Attachment = expAttachment443 @@ -684,10 +886,18 @@ var _ = Describe("ChangeProcessor", func() { expGraph.Routes[routeKey1].ParentRefs[0].Attachment = expAttachment80 expGraph.Routes[routeKey1].ParentRefs[1].Attachment = expAttachment443 + // no ref grant exists yet for tr1 + expGraph.L4Routes[trKey1].Conditions = []conditions.Condition{ + staticConds.NewRouteBackendRefRefNotPermitted( + "Backend ref to Service tls-service-ns/tls-service not permitted by any ReferenceGrant", + ), + } + expGraph.ReferencedSecrets = nil expGraph.ReferencedServices = nil expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} + expRouteTR1.Spec.BackendRef.SvcNsName = types.NamespacedName{} changed, graphCfg := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -697,7 +907,7 @@ var _ = Describe("ChangeProcessor", func() { }) When("the ReferenceGrant allowing the Gateway to reference its Secret is upserted", func() { It("returns updated graph", func() { - processor.CaptureUpsertChange(refGrant1) + processor.CaptureUpsertChange(secretRefGrant) // no ref grant exists yet for hr1 expGraph.Routes[routeKey1].Conditions = []conditions.Condition{ @@ -705,12 +915,21 @@ var _ = Describe("ChangeProcessor", func() { "Backend ref to Service service-ns/service not permitted by any ReferenceGrant", ), } + + // no ref grant exists yet for tr1 + expGraph.L4Routes[trKey1].Conditions = []conditions.Condition{ + staticConds.NewRouteBackendRefRefNotPermitted( + "Backend ref to Service tls-service-ns/tls-service not permitted by any ReferenceGrant", + ), + } + expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ Source: diffNsTLSSecret, } expGraph.ReferencedServices = nil expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} + expRouteTR1.Spec.BackendRef.SvcNsName = types.NamespacedName{} changed, graphCfg := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -720,7 +939,30 @@ var _ = Describe("ChangeProcessor", func() { }) When("the ReferenceGrant allowing the hr1 to reference the Service in different ns is upserted", func() { It("returns updated graph", func() { - processor.CaptureUpsertChange(refGrant2) + processor.CaptureUpsertChange(hrServiceRefGrant) + + // no ref grant exists yet for tr1 + expGraph.L4Routes[trKey1].Conditions = []conditions.Condition{ + staticConds.NewRouteBackendRefRefNotPermitted( + "Backend ref to Service tls-service-ns/tls-service not permitted by any ReferenceGrant", + ), + } + delete(expGraph.ReferencedServices, types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"}) + expRouteTR1.Spec.BackendRef.SvcNsName = types.NamespacedName{} + + expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ + Source: diffNsTLSSecret, + } + + changed, graphCfg := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) + Expect(helpers.Diff(expGraph, processor.GetLatestGraph())).To(BeEmpty()) + }) + }) + When("the ReferenceGrant allowing the tr1 to reference the Service in different ns is upserted", func() { + It("returns updated graph", func() { + processor.CaptureUpsertChange(trServiceRefGrant) expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ Source: diffNsTLSSecret, @@ -789,10 +1031,10 @@ var _ = Describe("ChangeProcessor", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(hr1Updated) - listener443 := getListenerByName(expGraph.Gateway, "listener-443-1") + listener443 := getListenerByName(expGraph.Gateway, httpsListenerName) listener443.Routes[routeKey1].Source.SetGeneration(hr1Updated.Generation) - listener80 := getListenerByName(expGraph.Gateway, "listener-80-1") + listener80 := getListenerByName(expGraph.Gateway, httpListenerName) listener80.Routes[routeKey1].Source.SetGeneration(hr1Updated.Generation) expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ Source: diffNsTLSSecret, @@ -805,6 +1047,24 @@ var _ = Describe("ChangeProcessor", func() { }, ) }) + When("the first TLSRoute update with a generation changed is processed", func() { + It("returns populated graph", func() { + processor.CaptureUpsertChange(tr1Updated) + + tlsListener := getListenerByName(expGraph.Gateway, tlsListenerName) + tlsListener.L4Routes[trKey1].Source.SetGeneration(tr1Updated.Generation) + + expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ + Source: diffNsTLSSecret, + } + + changed, graphCfg := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) + Expect(helpers.Diff(expGraph, processor.GetLatestGraph())).To(BeEmpty()) + }, + ) + }) When("the first Gateway update with a generation changed is processed", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(gw1Updated) @@ -918,6 +1178,39 @@ var _ = Describe("ChangeProcessor", func() { Expect(helpers.Diff(expGraph, processor.GetLatestGraph())).To(BeEmpty()) }) }) + When("the second TLSRoute is upserted", func() { + It("returns populated graph", func() { + processor.CaptureUpsertChange(tr2) + + expGraph.IgnoredGateways = map[types.NamespacedName]*v1.Gateway{ + {Namespace: "test", Name: "gateway-2"}: gw2, + } + expGraph.Routes[routeKey2] = expRouteHR2 + expGraph.Routes[routeKey2].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNotAcceptedGatewayIgnored(), + } + expGraph.Routes[routeKey2].ParentRefs[1].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNotAcceptedGatewayIgnored(), + } + + expGraph.L4Routes[trKey2] = expRouteTR2 + expGraph.L4Routes[trKey2].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNotAcceptedGatewayIgnored(), + } + + expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{ + Source: diffNsTLSSecret, + } + + changed, graphCfg := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) + Expect(helpers.Diff(expGraph, processor.GetLatestGraph())).To(BeEmpty()) + }) + }) When("the first Gateway is deleted", func() { It("returns updated graph", func() { processor.CaptureDeleteChange( @@ -927,26 +1220,37 @@ var _ = Describe("ChangeProcessor", func() { // gateway 2 takes over; // route 1 has been replaced by route 2 - listener80 := getListenerByName(expGraph.Gateway, "listener-80-1") - listener443 := getListenerByName(expGraph.Gateway, "listener-443-1") + listener80 := getListenerByName(expGraph.Gateway, httpListenerName) + listener443 := getListenerByName(expGraph.Gateway, httpsListenerName) + tlsListener := getListenerByName(expGraph.Gateway, tlsListenerName) expGraph.Gateway.Source = gw2 listener80.Source = gw2.Spec.Listeners[0] listener443.Source = gw2.Spec.Listeners[1] + tlsListener.Source = gw2.Spec.Listeners[2] + delete(listener80.Routes, routeKey1) delete(listener443.Routes, routeKey1) + delete(tlsListener.L4Routes, trKey1) + listener80.Routes[routeKey2] = expRouteHR2 listener443.Routes[routeKey2] = expRouteHR2 + tlsListener.L4Routes[trKey2] = expRouteTR2 + delete(expGraph.Routes, routeKey1) + delete(expGraph.L4Routes, trKey1) + expGraph.Routes[routeKey2] = expRouteHR2 + expGraph.L4Routes[trKey2] = expRouteTR2 + sameNsTLSSecretRef := helpers.GetPointer(client.ObjectKeyFromObject(sameNsTLSSecret)) listener443.ResolvedSecret = sameNsTLSSecretRef expGraph.ReferencedSecrets[client.ObjectKeyFromObject(sameNsTLSSecret)] = &graph.Secret{ Source: sameNsTLSSecret, } + delete(expGraph.ReferencedServices, expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName) expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} - expGraph.ReferencedServices = nil changed, graphCfg := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -962,16 +1266,67 @@ var _ = Describe("ChangeProcessor", func() { ) // gateway 2 still in charge; - // no routes remain - listener80 := getListenerByName(expGraph.Gateway, "listener-80-1") - listener443 := getListenerByName(expGraph.Gateway, "listener-443-1") + // no HTTP routes remain + // TLSRoute 2 still exists + listener80 := getListenerByName(expGraph.Gateway, httpListenerName) + listener443 := getListenerByName(expGraph.Gateway, httpsListenerName) + tlsListener := getListenerByName(expGraph.Gateway, tlsListenerName) + + expGraph.Gateway.Source = gw2 + listener80.Source = gw2.Spec.Listeners[0] + listener443.Source = gw2.Spec.Listeners[1] + tlsListener.Source = gw2.Spec.Listeners[2] + + delete(listener80.Routes, routeKey1) + delete(listener443.Routes, routeKey1) + delete(tlsListener.L4Routes, trKey1) + + tlsListener.L4Routes[trKey2] = expRouteTR2 + + expGraph.Routes = map[graph.RouteKey]*graph.L7Route{} + delete(expGraph.L4Routes, trKey1) + expGraph.L4Routes[trKey2] = expRouteTR2 + + sameNsTLSSecretRef := helpers.GetPointer(client.ObjectKeyFromObject(sameNsTLSSecret)) + listener443.ResolvedSecret = sameNsTLSSecretRef + expGraph.ReferencedSecrets[client.ObjectKeyFromObject(sameNsTLSSecret)] = &graph.Secret{ + Source: sameNsTLSSecret, + } + + delete(expGraph.ReferencedServices, expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName) + expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} + + changed, graphCfg := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) + Expect(helpers.Diff(expGraph, processor.GetLatestGraph())).To(BeEmpty()) + }) + }) + When("the second TLSRoute is deleted", func() { + It("returns updated graph", func() { + processor.CaptureDeleteChange( + &v1alpha2.TLSRoute{}, + types.NamespacedName{Namespace: "test", Name: "tr-2"}, + ) + + // gateway 2 still in charge; + // no HTTP or TLS routes remain + listener80 := getListenerByName(expGraph.Gateway, httpListenerName) + listener443 := getListenerByName(expGraph.Gateway, httpsListenerName) + tlsListener := getListenerByName(expGraph.Gateway, tlsListenerName) expGraph.Gateway.Source = gw2 listener80.Source = gw2.Spec.Listeners[0] listener443.Source = gw2.Spec.Listeners[1] + tlsListener.Source = gw2.Spec.Listeners[2] + delete(listener80.Routes, routeKey1) delete(listener443.Routes, routeKey1) + delete(tlsListener.L4Routes, trKey1) + expGraph.Routes = map[graph.RouteKey]*graph.L7Route{} + expGraph.L4Routes = map[graph.L4RouteKey]*graph.L4Route{} + sameNsTLSSecretRef := helpers.GetPointer(client.ObjectKeyFromObject(sameNsTLSSecret)) listener443.ResolvedSecret = sameNsTLSSecretRef expGraph.ReferencedSecrets[client.ObjectKeyFromObject(sameNsTLSSecret)] = &graph.Secret{ @@ -1000,6 +1355,7 @@ var _ = Describe("ChangeProcessor", func() { Conditions: staticConds.NewGatewayInvalid("GatewayClass doesn't exist"), } expGraph.Routes = map[graph.RouteKey]*graph.L7Route{} + expGraph.L4Routes = map[graph.L4RouteKey]*graph.L4Route{} expGraph.ReferencedSecrets = nil expRouteHR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} @@ -1098,12 +1454,12 @@ var _ = Describe("ChangeProcessor", func() { kindInvalid := v1.Kind("Invalid") // backend Refs - fooRef := createBackendRef(&kindService, "foo-svc", &testNamespace) - baz1NilNamespace := createBackendRef(&kindService, "baz-svc-v1", &testNamespace) - barRef := createBackendRef(&kindService, "bar-svc", nil) - baz2Ref := createBackendRef(&kindService, "baz-svc-v2", &testNamespace) - baz3Ref := createBackendRef(&kindService, "baz-svc-v3", &testNamespace) - invalidKindRef := createBackendRef(&kindInvalid, "bar-svc", &testNamespace) + fooRef := createHTTPBackendRef(&kindService, "foo-svc", &testNamespace) + baz1NilNamespace := createHTTPBackendRef(&kindService, "baz-svc-v1", &testNamespace) + barRef := createHTTPBackendRef(&kindService, "bar-svc", nil) + baz2Ref := createHTTPBackendRef(&kindService, "baz-svc-v2", &testNamespace) + baz3Ref := createHTTPBackendRef(&kindService, "baz-svc-v3", &testNamespace) + invalidKindRef := createHTTPBackendRef(&kindInvalid, "bar-svc", &testNamespace) // httproutes hr1 = createRoute("hr1", "gw", "foo.example.com", fooRef) @@ -1140,7 +1496,7 @@ var _ = Describe("ChangeProcessor", func() { // backendTLSPolicy btls = createBackendTLSPolicy("btls", "foo-svc") - gw = createGateway("gw") + gw = createGateway("gw", createHTTPListener()) processor.CaptureUpsertChange(gc) processor.CaptureUpsertChange(gw) changed, _ := processor.Process() @@ -1648,7 +2004,7 @@ var _ = Describe("ChangeProcessor", func() { Expect(newGraph.GatewayClass.Source).To(Equal(gc)) Expect(newGraph.NGFPolicies).To(BeEmpty()) - gw = createGateway("gw") + gw = createGateway("gw", createHTTPListener()) route = createRoute("hr-1", "gw", "foo.example.com", v1.HTTPBackendRef{}) csp = &ngfAPI.ClientSettingsPolicy{ @@ -1866,7 +2222,7 @@ var _ = Describe("ChangeProcessor", func() { GatewayClassName: gcName, Listeners: []v1.Listener{ { - Name: "listener-80-1", + Name: httpListenerName, Hostname: nil, Port: 80, Protocol: v1.HTTPProtocolType, @@ -1882,7 +2238,7 @@ var _ = Describe("ChangeProcessor", func() { }, }, { - Name: "listener-443-1", + Name: httpsListenerName, Hostname: nil, Port: 443, Protocol: v1.HTTPSProtocolType, @@ -1925,8 +2281,8 @@ var _ = Describe("ChangeProcessor", func() { testNamespace := v1.Namespace("test") kindService := v1.Kind("Service") - fooRef := createBackendRef(&kindService, "foo-svc", &testNamespace) - barRef := createBackendRef(&kindService, "bar-svc", &testNamespace) + fooRef := createHTTPBackendRef(&kindService, "foo-svc", &testNamespace) + barRef := createHTTPBackendRef(&kindService, "bar-svc", &testNamespace) hrNsName = types.NamespacedName{Namespace: "test", Name: "hr-1"} diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 229af562e4..326abbccea 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -228,6 +228,7 @@ func BuildGraph( processedGws.GetAllNsNames(), state.Services, npCfg, + refGrantResolver, ) bindRoutesToListeners(routes, l4routes, gw, state.Namespaces) diff --git a/internal/mode/static/state/graph/reference_grant.go b/internal/mode/static/state/graph/reference_grant.go index bdda74aae8..96999b209f 100644 --- a/internal/mode/static/state/graph/reference_grant.go +++ b/internal/mode/static/state/graph/reference_grant.go @@ -81,6 +81,14 @@ func fromGRPCRoute(namespace string) fromResource { } } +func fromTLSRoute(namespace string) fromResource { + return fromResource{ + group: v1.GroupName, + kind: kinds.TLSRoute, + namespace: namespace, + } +} + // newReferenceGrantResolver creates a new referenceGrantResolver. func newReferenceGrantResolver(refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant) *referenceGrantResolver { allowed := make(map[allowedReference]struct{}) diff --git a/internal/mode/static/state/graph/reference_grant_test.go b/internal/mode/static/state/graph/reference_grant_test.go index 4551499891..382ea7286a 100644 --- a/internal/mode/static/state/graph/reference_grant_test.go +++ b/internal/mode/static/state/graph/reference_grant_test.go @@ -233,10 +233,24 @@ func TestFromGRPCRoute(t *testing.T) { g.Expect(ref).To(Equal(exp)) } +func TestFromTLSRoute(t *testing.T) { + ref := fromTLSRoute("ns") + + exp := fromResource{ + group: v1beta1.GroupName, + kind: kinds.TLSRoute, + namespace: "ns", + } + + g := NewWithT(t) + g.Expect(ref).To(Equal(exp)) +} + func TestRefAllowedFrom(t *testing.T) { gwNs := "gw-ns" hrNs := "hr-ns" grNs := "gr-ns" + trNs := "tr-ns" allowedHTTPRouteNs := "hr-allowed-ns" allowedHTTPRouteNsName := types.NamespacedName{Namespace: allowedHTTPRouteNs, Name: "all-allowed-in-ns"} @@ -244,6 +258,9 @@ func TestRefAllowedFrom(t *testing.T) { allowedGRPCRouteNs := "gr-allowed-ns" allowedGRPCRouteNsName := types.NamespacedName{Namespace: allowedGRPCRouteNs, Name: "all-allowed-in-ns"} + allowedTLSRouteNs := "tr-allowed-ns" + allowedTLSRouteNsName := types.NamespacedName{Namespace: allowedTLSRouteNs, Name: "all-allowed-in-ns"} + allowedGatewayNs := "gw-allowed-ns" allowedGatewayNsName := types.NamespacedName{Namespace: allowedGatewayNs, Name: "all-allowed-in-ns"} @@ -298,11 +315,28 @@ func TestRefAllowedFrom(t *testing.T) { }, }, }, + {Namespace: allowedTLSRouteNs, Name: "tr-2-svc"}: { + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1beta1.GroupName, + Kind: kinds.TLSRoute, + Namespace: v1beta1.Namespace(trNs), + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Kind: "Service", + }, + }, + }, + }, } resolver := newReferenceGrantResolver(refGrants) refAllowedFromGRPCRoute := resolver.refAllowedFrom(fromGRPCRoute(grNs)) refAllowedFromHTTPRoute := resolver.refAllowedFrom(fromHTTPRoute(hrNs)) + refAllowedFromTLSRoute := resolver.refAllowedFrom(fromTLSRoute(trNs)) refAllowedFromGateway := resolver.refAllowedFrom(fromGateway(gwNs)) tests := []struct { @@ -347,6 +381,18 @@ func TestRefAllowedFrom(t *testing.T) { toResource: toService(notAllowedNsName), expAllowed: false, }, + { + name: "ref allowed from tlsroute to service", + refAllowedFrom: refAllowedFromTLSRoute, + toResource: toService(allowedTLSRouteNsName), + expAllowed: true, + }, + { + name: "ref not allowed from tlsroute to service", + refAllowedFrom: refAllowedFromTLSRoute, + toResource: toService(notAllowedNsName), + expAllowed: false, + }, } for _, test := range tests { diff --git a/internal/mode/static/state/graph/route_common.go b/internal/mode/static/state/graph/route_common.go index b8e9eee4fa..4db7807d71 100644 --- a/internal/mode/static/state/graph/route_common.go +++ b/internal/mode/static/state/graph/route_common.go @@ -178,6 +178,7 @@ func buildL4RoutesForGateways( gatewayNsNames []types.NamespacedName, services map[types.NamespacedName]*apiv1.Service, npCfg *NginxProxy, + resolver *referenceGrantResolver, ) map[L4RouteKey]*L4Route { if len(gatewayNsNames) == 0 { return nil @@ -185,7 +186,13 @@ func buildL4RoutesForGateways( routes := make(map[L4RouteKey]*L4Route) for _, route := range tlsRoutes { - r := buildTLSRoute(route, gatewayNsNames, services, npCfg) + r := buildTLSRoute( + route, + gatewayNsNames, + services, + npCfg, + resolver.refAllowedFrom(fromTLSRoute(route.Namespace)), + ) if r != nil { routes[CreateRouteKeyL4(route)] = r } diff --git a/internal/mode/static/state/graph/route_common_test.go b/internal/mode/static/state/graph/route_common_test.go index 4d4dee7dc0..0eef2ffe7f 100644 --- a/internal/mode/static/state/graph/route_common_test.go +++ b/internal/mode/static/state/graph/route_common_test.go @@ -2436,11 +2436,14 @@ func TestBuildL4RoutesForGateways_NoGateways(t *testing.T) { }, } + refGrantResolver := newReferenceGrantResolver(nil) + g.Expect(buildL4RoutesForGateways( tlsRoutes, nil, services, nil, + refGrantResolver, )).To(BeNil()) } diff --git a/internal/mode/static/state/graph/tlsroute.go b/internal/mode/static/state/graph/tlsroute.go index f4ca1d1e68..03a8ca8a36 100644 --- a/internal/mode/static/state/graph/tlsroute.go +++ b/internal/mode/static/state/graph/tlsroute.go @@ -16,6 +16,7 @@ func buildTLSRoute( gatewayNsNames []types.NamespacedName, services map[types.NamespacedName]*apiv1.Service, npCfg *NginxProxy, + refGrantResolver func(resource toResource) bool, ) *L4Route { r := &L4Route{ Source: gtr, @@ -53,7 +54,7 @@ func buildTLSRoute( return r } - cond, br := validateBackendRefTLSRoute(gtr, services, npCfg) + br, cond := validateBackendRefTLSRoute(gtr, services, npCfg, refGrantResolver) r.Spec.BackendRef = br r.Valid = true @@ -61,7 +62,6 @@ func buildTLSRoute( if cond != nil { r.Conditions = append(r.Conditions, *cond) - r.Valid = false } return r @@ -70,12 +70,26 @@ func buildTLSRoute( func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute, services map[types.NamespacedName]*apiv1.Service, npCfg *NginxProxy, -) (*conditions.Condition, BackendRef) { + refGrantResolver func(resource toResource) bool, +) (BackendRef, *conditions.Condition) { // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildTLSRoute refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) ref := gtr.Spec.Rules[0].BackendRefs[0] + if valid, cond := validateBackendRef( + ref, + gtr.Namespace, + refGrantResolver, + refPath, + ); !valid { + backendRef := BackendRef{ + Valid: false, + } + + return backendRef, &cond + } + ns := gtr.Namespace if ref.Namespace != nil { ns = string(*ref.Namespace) @@ -86,19 +100,6 @@ func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute, Name: string(gtr.Spec.Rules[0].BackendRefs[0].Name), } - backendRef := BackendRef{ - Valid: true, - } - var cond *conditions.Condition - - if ref.Port == nil { - valErr := field.Required(refPath.Child("port"), "port cannot be nil") - backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(valErr.Error())) - - return cond, backendRef - } - svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( ref, svcNsName, @@ -106,28 +107,23 @@ func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute, refPath, ) - backendRef.ServicePort = svcPort - backendRef.SvcNsName = svcNsName + backendRef := BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Valid: true, + } if err != nil { backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteBackendRefRefBackendNotFound(err.Error())) - } else if err := verifyIPFamily(npCfg, svcIPFamily); err != nil { - backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteInvalidIPFamily(err.Error())) - } else if ref.Group != nil && !(*ref.Group == "core" || *ref.Group == "") { - valErr := field.NotSupported(refPath.Child("group"), *ref.Group, []string{"core", ""}) - backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error())) - } else if ref.Kind != nil && *ref.Kind != "Service" { - valErr := field.NotSupported(refPath.Child("kind"), *ref.Kind, []string{"Service"}) - backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error())) - } else if ref.Namespace != nil && string(*ref.Namespace) != gtr.Namespace { - msg := "Cross-namespace routing is not supported" + + return backendRef, helpers.GetPointer(staticConds.NewRouteBackendRefRefBackendNotFound(err.Error())) + } + + if err := verifyIPFamily(npCfg, svcIPFamily); err != nil { backendRef.Valid = false - cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(msg)) + + return backendRef, helpers.GetPointer(staticConds.NewRouteInvalidIPFamily(err.Error())) } - // FIXME(sarthyparty): Add check for invalid weights, we removed checks to pass the conformance test - return cond, backendRef + + return backendRef, nil } diff --git a/internal/mode/static/state/graph/tlsroute_test.go b/internal/mode/static/state/graph/tlsroute_test.go index da33e1a5bd..8a7df6341a 100644 --- a/internal/mode/static/state/graph/tlsroute_test.go +++ b/internal/mode/static/state/graph/tlsroute_test.go @@ -136,7 +136,7 @@ func TestBuildTLSRoute(t *testing.T) { }, ) - wrongBackendRefNamespaceGtr8 := createTLSRoute("app.example.com", + diffNsBackendRef := createTLSRoute("app.example.com", []v1alpha2.TLSRouteRule{ { BackendRefs: []gatewayv1.BackendRef{ @@ -144,7 +144,7 @@ func TestBuildTLSRoute(t *testing.T) { BackendObjectReference: gatewayv1.BackendObjectReference{ Name: "hi", Port: helpers.GetPointer[gatewayv1.PortNumber](80), - Namespace: helpers.GetPointer[gatewayv1.Namespace]("wrong"), + Namespace: helpers.GetPointer[gatewayv1.Namespace]("diff"), }, }, }, @@ -190,13 +190,33 @@ func TestBuildTLSRoute(t *testing.T) { parentRef, }, ) + + validRefSameNs := createTLSRoute("app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + svcNsName := types.NamespacedName{ Namespace: "test", Name: "hi", } - svcNsNameWrong := types.NamespacedName{ - Namespace: "wrong", + diffSvcNsName := types.NamespacedName{ + Namespace: "diff", Name: "hi", } @@ -214,9 +234,9 @@ func TestBuildTLSRoute(t *testing.T) { } } - badNsSvc := &apiv1.Service{ + diffNsSvc := &apiv1.Service{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "wrong", + Namespace: "diff", Name: "hi", }, Spec: apiv1.ServiceSpec{ @@ -241,19 +261,27 @@ func TestBuildTLSRoute(t *testing.T) { }, } + alwaysTrueRefGrantResolver := func(_ toResource) bool { return true } + alwaysFalseRefGrantResolver := func(_ toResource) bool { return false } + tests := []struct { expected *L4Route gtr *v1alpha2.TLSRoute services map[types.NamespacedName]*apiv1.Service + resolver func(resource toResource) bool name string gatewayNsNames []types.NamespacedName npCfg NginxProxy }{ { - gtr: duplicateParentRefsGtr, - expected: &L4Route{Source: duplicateParentRefsGtr}, + gtr: duplicateParentRefsGtr, + expected: &L4Route{ + Source: duplicateParentRefsGtr, + Valid: false, + }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{}, + resolver: alwaysTrueRefGrantResolver, name: "duplicate parent refs", }, { @@ -261,6 +289,7 @@ func TestBuildTLSRoute(t *testing.T) { expected: nil, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{}, + resolver: alwaysTrueRefGrantResolver, name: "no parent refs", }, { @@ -275,9 +304,11 @@ func TestBuildTLSRoute(t *testing.T) { "ter (e.g. 'example.com', regex used for validation is '[a-z0-9](" + "[-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')", )}, + Valid: false, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{}, + resolver: alwaysTrueRefGrantResolver, name: "invalid hostname", }, { @@ -293,9 +324,11 @@ func TestBuildTLSRoute(t *testing.T) { Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( "Must have exactly one Rule and BackendRef", )}, + Valid: false, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{}, + resolver: alwaysTrueRefGrantResolver, name: "invalid rule", }, { @@ -312,15 +345,18 @@ func TestBuildTLSRoute(t *testing.T) { Namespace: "test", Name: "hi", }, + Valid: false, }, }, Conditions: []conditions.Condition{staticConds.NewRouteBackendRefRefBackendNotFound( "spec.rules[0].backendRefs[0].name: Not found: \"hi\"", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{}, + resolver: alwaysTrueRefGrantResolver, name: "BackendRef not found", }, { @@ -333,8 +369,7 @@ func TestBuildTLSRoute(t *testing.T) { "app.example.com", }, BackendRef: BackendRef{ - SvcNsName: svcNsName, - ServicePort: apiv1.ServicePort{Port: 80}, + Valid: false, }, }, Conditions: []conditions.Condition{staticConds.NewRouteBackendRefInvalidKind( @@ -342,12 +377,14 @@ func TestBuildTLSRoute(t *testing.T) { " Unsupported value: \"wrong\": supported values: \"core\", \"\"", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{ svcNsName: createSvc("hi", 80), }, - name: "BackendRef group wrong", + resolver: alwaysTrueRefGrantResolver, + name: "BackendRef group wrong", }, { gtr: wrongBackendRefKindGtr, @@ -359,8 +396,7 @@ func TestBuildTLSRoute(t *testing.T) { "app.example.com", }, BackendRef: BackendRef{ - SvcNsName: svcNsName, - ServicePort: apiv1.ServicePort{Port: 80}, + Valid: false, }, }, Conditions: []conditions.Condition{staticConds.NewRouteBackendRefInvalidKind( @@ -368,37 +404,40 @@ func TestBuildTLSRoute(t *testing.T) { " Unsupported value: \"not service\": supported values: \"Service\"", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{ svcNsName: createSvc("hi", 80), }, - name: "BackendRef kind wrong", + resolver: alwaysTrueRefGrantResolver, + name: "BackendRef kind wrong", }, { - gtr: wrongBackendRefNamespaceGtr8, + gtr: diffNsBackendRef, expected: &L4Route{ - Source: wrongBackendRefNamespaceGtr8, + Source: diffNsBackendRef, ParentRefs: []ParentRef{parentRefGraph}, Spec: L4RouteSpec{ Hostnames: []gatewayv1.Hostname{ "app.example.com", }, BackendRef: BackendRef{ - SvcNsName: svcNsNameWrong, - ServicePort: apiv1.ServicePort{Port: 80}, + Valid: false, }, }, - Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( - "Cross-namespace routing is not supported", + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefRefNotPermitted( + "Backend ref to Service diff/hi not permitted by any ReferenceGrant", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{ - svcNsNameWrong: badNsSvc, + diffSvcNsName: diffNsSvc, }, - name: "BackendRef namespace wrong", + resolver: alwaysFalseRefGrantResolver, + name: "BackendRef in diff namespace not permitted by any reference grant", }, { gtr: portNilBackendRefGtr, @@ -409,18 +448,22 @@ func TestBuildTLSRoute(t *testing.T) { Hostnames: []gatewayv1.Hostname{ "app.example.com", }, - BackendRef: BackendRef{}, + BackendRef: BackendRef{ + Valid: false, + }, }, Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( "spec.rules[0].backendRefs[0].port: Required value: port cannot be nil", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{ - svcNsNameWrong: createSvc("hi", 80), + diffSvcNsName: createSvc("hi", 80), }, - name: "BackendRef port nil", + resolver: alwaysTrueRefGrantResolver, + name: "BackendRef port nil", }, { gtr: ipFamilyMismatchGtr, @@ -440,16 +483,68 @@ func TestBuildTLSRoute(t *testing.T) { "Service configured with IPv4 family but NginxProxy is configured with IPv6", )}, Attachable: true, + Valid: true, }, gatewayNsNames: []types.NamespacedName{gatewayNsName}, services: map[types.NamespacedName]*apiv1.Service{ svcNsName: ipv4Svc, }, - name: "service and npcfg ip family mismatch", npCfg: NginxProxy{ Source: &ngfAPI.NginxProxy{Spec: ngfAPI.NginxProxySpec{IPFamily: helpers.GetPointer(ngfAPI.IPv6)}}, Valid: true, }, + resolver: alwaysTrueRefGrantResolver, + name: "service and npcfg ip family mismatch", + }, + { + gtr: diffNsBackendRef, + expected: &L4Route{ + Source: diffNsBackendRef, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: diffSvcNsName, + ServicePort: apiv1.ServicePort{Port: 80}, + Valid: true, + }, + }, + Attachable: true, + Valid: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + diffSvcNsName: diffNsSvc, + }, + resolver: alwaysTrueRefGrantResolver, + name: "valid; backendRef in diff namespace permitted by a reference grant", + }, + { + gtr: validRefSameNs, + expected: &L4Route{ + Source: validRefSameNs, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: svcNsName, + ServicePort: apiv1.ServicePort{Port: 80}, + Valid: true, + }, + }, + Attachable: true, + Valid: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsName: ipv4Svc, + }, + resolver: alwaysTrueRefGrantResolver, + name: "valid; same namespace", }, } @@ -461,6 +556,7 @@ func TestBuildTLSRoute(t *testing.T) { test.gatewayNsNames, test.services, &test.npCfg, + test.resolver, ) g.Expect(helpers.Diff(test.expected, r)).To(BeEmpty()) }) diff --git a/tests/Makefile b/tests/Makefile index 9b6905e98a..37d41e5d65 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -17,7 +17,7 @@ SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatchin STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. -SKIP_TESTS = TLSRouteInvalidReferenceGrant +SKIP_TESTS = # Check if ENABLE_EXPERIMENTAL is true ifeq ($(ENABLE_EXPERIMENTAL),true) From cd418985fd0ac1340a166572d1c80e52e7183aa6 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Tue, 13 Aug 2024 14:38:10 -0600 Subject: [PATCH 2/3] Add t.parallel --- .../state/graph/reference_grant_test.go | 20 +++++++++++++++++++ .../mode/static/state/graph/tlsroute_test.go | 4 ++++ 2 files changed, 24 insertions(+) diff --git a/internal/mode/static/state/graph/reference_grant_test.go b/internal/mode/static/state/graph/reference_grant_test.go index 382ea7286a..41d2615ec9 100644 --- a/internal/mode/static/state/graph/reference_grant_test.go +++ b/internal/mode/static/state/graph/reference_grant_test.go @@ -12,6 +12,8 @@ import ( ) func TestReferenceGrantResolver(t *testing.T) { + t.Parallel() + gwNs := "gw-ns" secretNsName := types.NamespacedName{Namespace: "test", Name: "certificate"} @@ -161,6 +163,8 @@ func TestReferenceGrantResolver(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) g.Expect(resolver.refAllowed(test.to, test.from)).To(Equal(test.allowed)) @@ -169,6 +173,8 @@ func TestReferenceGrantResolver(t *testing.T) { } func TestToSecret(t *testing.T) { + t.Parallel() + ref := toSecret(types.NamespacedName{Namespace: "ns", Name: "secret"}) exp := toResource{ @@ -182,6 +188,8 @@ func TestToSecret(t *testing.T) { } func TestToService(t *testing.T) { + t.Parallel() + ref := toService(types.NamespacedName{Namespace: "ns", Name: "service"}) exp := toResource{ @@ -195,6 +203,8 @@ func TestToService(t *testing.T) { } func TestFromGateway(t *testing.T) { + t.Parallel() + ref := fromGateway("ns") exp := fromResource{ @@ -208,6 +218,8 @@ func TestFromGateway(t *testing.T) { } func TestFromHTTPRoute(t *testing.T) { + t.Parallel() + ref := fromHTTPRoute("ns") exp := fromResource{ @@ -221,6 +233,8 @@ func TestFromHTTPRoute(t *testing.T) { } func TestFromGRPCRoute(t *testing.T) { + t.Parallel() + ref := fromGRPCRoute("ns") exp := fromResource{ @@ -234,6 +248,8 @@ func TestFromGRPCRoute(t *testing.T) { } func TestFromTLSRoute(t *testing.T) { + t.Parallel() + ref := fromTLSRoute("ns") exp := fromResource{ @@ -247,6 +263,8 @@ func TestFromTLSRoute(t *testing.T) { } func TestRefAllowedFrom(t *testing.T) { + t.Parallel() + gwNs := "gw-ns" hrNs := "hr-ns" grNs := "gr-ns" @@ -397,6 +415,8 @@ func TestRefAllowedFrom(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) g.Expect(test.refAllowedFrom(test.toResource)).To(Equal(test.expAllowed)) }) diff --git a/internal/mode/static/state/graph/tlsroute_test.go b/internal/mode/static/state/graph/tlsroute_test.go index 8a7df6341a..edd6536ace 100644 --- a/internal/mode/static/state/graph/tlsroute_test.go +++ b/internal/mode/static/state/graph/tlsroute_test.go @@ -37,6 +37,8 @@ func createTLSRoute( } func TestBuildTLSRoute(t *testing.T) { + t.Parallel() + parentRef := gatewayv1.ParentReference{ Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"), Name: "gateway", @@ -551,6 +553,8 @@ func TestBuildTLSRoute(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) + t.Parallel() + r := buildTLSRoute( test.gtr, test.gatewayNsNames, From 3792ed1c0238f6c96a2cc4ab197064813f83b15e Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Tue, 13 Aug 2024 15:40:24 -0600 Subject: [PATCH 3/3] Capture range variable in subtests --- internal/mode/static/state/graph/reference_grant_test.go | 2 ++ internal/mode/static/state/graph/tlsroute_test.go | 1 + 2 files changed, 3 insertions(+) diff --git a/internal/mode/static/state/graph/reference_grant_test.go b/internal/mode/static/state/graph/reference_grant_test.go index 41d2615ec9..c8ac49f82e 100644 --- a/internal/mode/static/state/graph/reference_grant_test.go +++ b/internal/mode/static/state/graph/reference_grant_test.go @@ -162,6 +162,7 @@ func TestReferenceGrantResolver(t *testing.T) { resolver := newReferenceGrantResolver(refGrants) for _, test := range tests { + test := test t.Run(test.msg, func(t *testing.T) { t.Parallel() @@ -414,6 +415,7 @@ func TestRefAllowedFrom(t *testing.T) { } for _, test := range tests { + test := test t.Run(test.name, func(t *testing.T) { t.Parallel() diff --git a/internal/mode/static/state/graph/tlsroute_test.go b/internal/mode/static/state/graph/tlsroute_test.go index edd6536ace..d67d638bf1 100644 --- a/internal/mode/static/state/graph/tlsroute_test.go +++ b/internal/mode/static/state/graph/tlsroute_test.go @@ -551,6 +551,7 @@ func TestBuildTLSRoute(t *testing.T) { } for _, test := range tests { + test := test t.Run(test.name, func(t *testing.T) { g := NewWithT(t) t.Parallel()