diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 8d1a397990..1e50371b2b 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -407,7 +407,8 @@ var _ = Describe("ChangeProcessor", func() { ValidFilters: true, }, }, - Valid: true, + Valid: true, + Attachable: true, Conditions: []conditions.Condition{ staticConds.NewRouteBackendRefRefBackendNotFound( "spec.rules[0].backendRefs[0].name: Not found: \"service\"", @@ -434,8 +435,9 @@ var _ = Describe("ChangeProcessor", func() { Idx: 1, }, }, - Rules: []graph.Rule{{ValidMatches: true, ValidFilters: true}}, - Valid: true, + Rules: []graph.Rule{{ValidMatches: true, ValidFilters: true}}, + Valid: true, + Attachable: true, } // This is the base case expected graph. Tests will manipulate this to add or remove elements @@ -449,16 +451,18 @@ var _ = Describe("ChangeProcessor", func() { Source: gw1, Listeners: map[string]*graph.Listener{ "listener-80-1": { - Source: gw1.Spec.Listeners[0], - Valid: true, + Source: gw1.Spec.Listeners[0], + Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: expRouteHR1, }, SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, "listener-443-1": { - Source: gw1.Spec.Listeners[1], - Valid: true, + Source: gw1.Spec.Listeners[1], + Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: expRouteHR1, }, @@ -541,32 +545,40 @@ var _ = Describe("ChangeProcessor", func() { It("returns updated graph", func() { processor.CaptureUpsertChange(gc) - // no ref grant exists yet for gw1 - expGraph.Gateway.Listeners["listener-443-1"] = &graph.Listener{ - Source: gw1.Spec.Listeners[1], - Valid: false, - Routes: map[types.NamespacedName]*graph.Route{}, - Conditions: staticConds.NewListenerRefNotPermitted( - "Certificate ref to secret cert-ns/different-ns-tls-secret not permitted by any ReferenceGrant", - ), - SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}}, + // No ref grant exists yet for gw1 + // so the listener is not valid, but still attachable + expGraph.Gateway.Listeners["listener-443-1"].Valid = false + expGraph.Gateway.Listeners["listener-443-1"].ResolvedSecret = nil + expGraph.Gateway.Listeners["listener-443-1"].Conditions = staticConds.NewListenerRefNotPermitted( + "Certificate ref to secret cert-ns/different-ns-tls-secret not permitted by any ReferenceGrant", + ) + + expAttachment80 := &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-80-1": {"foo.example.com"}, + }, + Attached: true, } - expAttachment := &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{}, - FailedCondition: staticConds.NewRouteInvalidListener(), - Attached: false, + expAttachment443 := &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-443-1": {"foo.example.com"}, + }, + Attached: true, } - expGraph.Gateway.Listeners["listener-80-1"].Routes[hr1Name].ParentRefs[1].Attachment = expAttachment + expGraph.Gateway.Listeners["listener-80-1"].Routes[hr1Name].ParentRefs[0].Attachment = expAttachment80 + expGraph.Gateway.Listeners["listener-443-1"].Routes[hr1Name].ParentRefs[1].Attachment = expAttachment443 // no ref grant exists yet for hr1 - expGraph.Routes[hr1Name].ParentRefs[1].Attachment = expAttachment expGraph.Routes[hr1Name].Conditions = []conditions.Condition{ + staticConds.NewRouteInvalidListener(), staticConds.NewRouteBackendRefRefNotPermitted( "Backend ref to Service service-ns/service not permitted by any ReferenceGrant", ), } + expGraph.Routes[hr1Name].ParentRefs[0].Attachment = expAttachment80 + expGraph.Routes[hr1Name].ParentRefs[1].Attachment = expAttachment443 expGraph.ReferencedSecrets = nil diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 887467b13c..8c4eb4f0b6 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -7,6 +7,7 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" @@ -202,70 +203,80 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { hpr.httpsListeners = append(hpr.httpsListeners, l) } - for routeNsName, r := range l.Routes { - var hostnames []string - for _, p := range r.ParentRefs { - if val, exist := p.Attachment.AcceptedHostnames[string(l.Source.Name)]; exist { - hostnames = val - } + for _, r := range l.Routes { + if !r.Valid { + continue } - for _, h := range hostnames { - if prevListener, exists := hpr.listenersForHost[h]; exists { - // override the previous listener if the new one has a more specific hostname - if listenerHostnameMoreSpecific(l.Source.Hostname, prevListener.Source.Hostname) { - hpr.listenersForHost[h] = l - } - } else { - hpr.listenersForHost[h] = l - } + hpr.upsertRoute(r, l) + } +} - if _, exist := hpr.rulesPerHost[h]; !exist { - hpr.rulesPerHost[h] = make(map[pathAndType]PathRule) - } +func (hpr *hostPathRules) upsertRoute(route *graph.Route, listener *graph.Listener) { + var hostnames []string + for _, p := range route.ParentRefs { + if val, exist := p.Attachment.AcceptedHostnames[string(listener.Source.Name)]; exist { + hostnames = val } + } - for i, rule := range r.Source.Spec.Rules { - if !r.Rules[i].ValidMatches { - continue + for _, h := range hostnames { + if prevListener, exists := hpr.listenersForHost[h]; exists { + // override the previous listener if the new one has a more specific hostname + if listenerHostnameMoreSpecific(listener.Source.Hostname, prevListener.Source.Hostname) { + hpr.listenersForHost[h] = listener } + } else { + hpr.listenersForHost[h] = listener + } - var filters HTTPFilters - if r.Rules[i].ValidFilters { - filters = createHTTPFilters(rule.Filters) - } else { - filters = HTTPFilters{ - InvalidFilter: &InvalidHTTPFilter{}, - } + if _, exist := hpr.rulesPerHost[h]; !exist { + hpr.rulesPerHost[h] = make(map[pathAndType]PathRule) + } + } + + for i, rule := range route.Source.Spec.Rules { + if !route.Rules[i].ValidMatches { + continue + } + + var filters HTTPFilters + if route.Rules[i].ValidFilters { + filters = createHTTPFilters(rule.Filters) + } else { + filters = HTTPFilters{ + InvalidFilter: &InvalidHTTPFilter{}, } + } - for _, h := range hostnames { - for _, m := range rule.Matches { - path := getPath(m.Path) + for _, h := range hostnames { + for _, m := range rule.Matches { + path := getPath(m.Path) - key := pathAndType{ - path: path, - pathType: *m.Path.Type, - } + key := pathAndType{ + path: path, + pathType: *m.Path.Type, + } - rule, exist := hpr.rulesPerHost[h][key] - if !exist { - rule.Path = path - rule.PathType = convertPathType(*m.Path.Type) - } + rule, exist := hpr.rulesPerHost[h][key] + if !exist { + rule.Path = path + rule.PathType = convertPathType(*m.Path.Type) + } - // create iteration variable inside the loop to fix implicit memory aliasing - om := r.Source.ObjectMeta + // create iteration variable inside the loop to fix implicit memory aliasing + om := route.Source.ObjectMeta - rule.MatchRules = append(rule.MatchRules, MatchRule{ - Source: &om, - BackendGroup: newBackendGroup(r.Rules[i].BackendRefs, routeNsName, i), - Filters: filters, - Match: convertMatch(m), - }) + routeNsName := client.ObjectKeyFromObject(route.Source) - hpr.rulesPerHost[h][key] = rule - } + rule.MatchRules = append(rule.MatchRules, MatchRule{ + Source: &om, + BackendGroup: newBackendGroup(route.Rules[i].BackendRefs, routeNsName, i), + Filters: filters, + Match: convertMatch(m), + }) + + hpr.rulesPerHost[h][key] = rule } } } @@ -371,6 +382,10 @@ func buildUpstreams( } for _, route := range l.Routes { + if !route.Valid { + continue + } + for _, rule := range route.Rules { if !rule.ValidMatches || !rule.ValidFilters { // don't generate upstreams for rules that have invalid matches or filters diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index be671119ca..3759e2ad7c 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -138,6 +138,7 @@ func TestBuildConfiguration(t *testing.T) { r := &graph.Route{ Source: source, Rules: createRules(source, paths), + Valid: true, ParentRefs: []graph.ParentRef{ { Attachment: &graph.ParentRefAttachmentStatus{ @@ -187,6 +188,14 @@ func TestBuildConfiguration(t *testing.T) { "listener-80-1", pathAndType{path: "/", pathType: prefix}, ) + hr1Invalid, _, routeHR1Invalid := createTestResources( + "hr-1", + "foo.example.com", + "listener-80-1", + pathAndType{path: "/", pathType: prefix}, + ) + routeHR1Invalid.Valid = false + hr2, expHR2Groups, routeHR2 := createTestResources( "hr-2", "bar.example.com", @@ -254,6 +263,13 @@ func TestBuildConfiguration(t *testing.T) { "listener-443-1", pathAndType{path: "/", pathType: prefix}, ) + httpsHR1Invalid, _, httpsRouteHR1Invalid := createTestResources( + "https-hr-1", + "foo.example.com", + "listener-443-1", + pathAndType{path: "/", pathType: prefix}, + ) + httpsRouteHR1Invalid.Valid = false httpsHR2, expHTTPSHR2Groups, httpsRouteHR2 := createTestResources( "https-hr-2", @@ -465,6 +481,66 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "http listener with no routes", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1.Gateway{}, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + client.ObjectKeyFromObject(hr1Invalid): routeHR1Invalid, + }, + }, + "listener-443-1": { + Source: listener443, // nil hostname + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + client.ObjectKeyFromObject(httpsHR1Invalid): httpsRouteHR1Invalid, + }, + ResolvedSecret: &secret1NsName, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{ + client.ObjectKeyFromObject(hr1Invalid): routeHR1Invalid, + }, + ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + secret1NsName: secret1, + }, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + Port: 80, + }, + }, + SSLServers: []VirtualServer{ + { + IsDefault: true, + Port: 443, + }, + { + Hostname: wildcardHostname, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, + }, + }, + SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ + "ssl_keypair_test_secret-1": { + Cert: []byte("cert-1"), + Key: []byte("privateKey-1"), + }, + }, + }, + msg: "http and https listeners with no valid routes", + }, { graph: &graph.Graph{ GatewayClass: &graph.GatewayClass{ @@ -1749,6 +1825,13 @@ func TestBuildUpstreams(t *testing.T) { }, } + abcEndpoints := []resolver.Endpoint{ + { + Address: "14.0.0.0", + Port: 80, + }, + } + createBackendRefs := func(serviceNames ...string) []graph.BackendRef { var backends []graph.BackendRef for _, name := range serviceNames { @@ -1775,36 +1858,50 @@ func TestBuildUpstreams(t *testing.T) { hr4Refs1 := createBackendRefs("baz2") - invalidRefs := createBackendRefs("invalid") + nonExistingRefs := createBackendRefs("non-existing") + + invalidHRRefs := createBackendRefs("abc") routes := map[types.NamespacedName]*graph.Route{ {Name: "hr1", Namespace: "test"}: { + Valid: true, Rules: refsToValidRules(hr1Refs0, hr1Refs1), }, {Name: "hr2", Namespace: "test"}: { + Valid: true, Rules: refsToValidRules(hr2Refs0, hr2Refs1), }, {Name: "hr3", Namespace: "test"}: { + Valid: true, Rules: refsToValidRules(hr3Refs0), }, } routes2 := map[types.NamespacedName]*graph.Route{ {Name: "hr4", Namespace: "test"}: { + Valid: true, Rules: refsToValidRules(hr4Refs0, hr4Refs1), }, } + routesWithNonExistingRefs := map[types.NamespacedName]*graph.Route{ + {Name: "non-existing", Namespace: "test"}: { + Valid: true, + Rules: refsToValidRules(nonExistingRefs), + }, + } + invalidRoutes := map[types.NamespacedName]*graph.Route{ {Name: "invalid", Namespace: "test"}: { - Rules: refsToValidRules(invalidRefs), + Valid: false, + Rules: refsToValidRules(invalidHRRefs), }, } listeners := map[string]*graph.Listener{ "invalid-listener": { Valid: false, - Routes: invalidRoutes, // shouldn't be included since listener is invalid + Routes: routesWithNonExistingRefs, // shouldn't be included since listener is invalid }, "listener-1": { Valid: true, @@ -1814,6 +1911,10 @@ func TestBuildUpstreams(t *testing.T) { Valid: true, Routes: routes2, }, + "listener-3": { + Valid: true, + Routes: invalidRoutes, // shouldn't be included since routes are invalid + }, } emptyEndpointsErrMsg := "empty endpoints error" @@ -1863,6 +1964,8 @@ func TestBuildUpstreams(t *testing.T) { return fooEndpoints, nil case "nil-endpoints": return nil, errors.New(nilEndpointsErrMsg) + case "abc": + return abcEndpoints, nil default: return nil, fmt.Errorf("unexpected service %s", svc.Name) } diff --git a/internal/mode/static/state/graph/gateway_listener.go b/internal/mode/static/state/graph/gateway_listener.go index 8927cbf677..4e6eb02eaa 100644 --- a/internal/mode/static/state/graph/gateway_listener.go +++ b/internal/mode/static/state/graph/gateway_listener.go @@ -34,6 +34,9 @@ type Listener struct { // Valid shows whether the Listener is valid. // A Listener is considered valid if NGF can generate valid NGINX configuration for it. Valid bool + // Attachable shows whether Routes can attach to the Listener. + // Listener can be invalid but still attachable. + Attachable bool } func buildListeners( @@ -80,13 +83,13 @@ func newListenerConfiguratorFactory( return &listenerConfiguratorFactory{ unsupportedProtocol: &listenerConfigurator{ validators: []listenerValidator{ - func(listener v1.Listener) []conditions.Condition { + func(listener v1.Listener) ([]conditions.Condition, bool) { valErr := field.NotSupported( field.NewPath("protocol"), listener.Protocol, []string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType)}, ) - return staticConds.NewListenerUnsupportedProtocol(valErr.Error()) + return staticConds.NewListenerUnsupportedProtocol(valErr.Error()), false /* not attachable */ }, }, }, @@ -119,7 +122,8 @@ func newListenerConfiguratorFactory( } // listenerValidator validates a listener. If the listener is invalid, the validator will return appropriate conditions. -type listenerValidator func(v1.Listener) []conditions.Condition +// It also returns whether the listener is attachable, which is independent of whether the listener is valid. +type listenerValidator func(v1.Listener) (conds []conditions.Condition, attachable bool) // listenerConflictResolver resolves conflicts between listeners. In case of a conflict, the resolver will make // the conflicting listeners invalid - i.e. it will modify the passed listener and the previously processed conflicting @@ -136,9 +140,7 @@ type listenerExternalReferenceResolver func(listener *Listener) // don't need to include the full path to the field (e.g. "spec.listeners[0].hostname"). They will include // a path starting from the field of a listener (e.g. "hostname", "tls.options"). type listenerConfigurator struct { - // validators must not depend on the order of execution. validators []listenerValidator - // conflictResolvers can depend on validators - they will only be executed if all validators pass. conflictResolvers []listenerConflictResolver // externalReferenceResolvers can depend on validators - they will only be executed if all validators pass. @@ -148,11 +150,18 @@ type listenerConfigurator struct { func (c *listenerConfigurator) configure(listener v1.Listener) *Listener { var conds []conditions.Condition + attachable := true + // validators might return different conditions, so we run them all. for _, validator := range c.validators { - conds = append(conds, validator(listener)...) + currConds, currAttachable := validator(listener) + conds = append(conds, currConds...) + + attachable = attachable && currAttachable } + valid := len(conds) == 0 + var allowedRouteSelector labels.Selector if selector := GetAllowedRouteLabelSelector(listener); selector != nil { var err error @@ -160,28 +169,26 @@ func (c *listenerConfigurator) configure(listener v1.Listener) *Listener { if err != nil { msg := fmt.Sprintf("invalid label selector: %s", err.Error()) conds = append(conds, staticConds.NewListenerUnsupportedValue(msg)...) + valid = false } } supportedKinds := getListenerSupportedKinds(listener) - if len(conds) > 0 { - return &Listener{ - Source: listener, - Conditions: conds, - Valid: false, - SupportedKinds: supportedKinds, - } - } - l := &Listener{ Source: listener, + Conditions: conds, AllowedRouteLabelSelector: allowedRouteSelector, Routes: make(map[types.NamespacedName]*Route), - Valid: true, + Valid: valid, + Attachable: attachable, SupportedKinds: supportedKinds, } + if !l.Valid { + return l + } + // resolvers might add different conditions to the listener, so we run them all. for _, resolver := range c.conflictResolvers { @@ -195,23 +202,23 @@ func (c *listenerConfigurator) configure(listener v1.Listener) *Listener { return l } -func validateListenerHostname(listener v1.Listener) []conditions.Condition { +func validateListenerHostname(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if listener.Hostname == nil { - return nil + return nil, true } h := string(*listener.Hostname) if h == "" { - return nil + return nil, true } if err := validateHostname(h); err != nil { path := field.NewPath("hostname") valErr := field.Invalid(path, listener.Hostname, err.Error()) - return staticConds.NewListenerUnsupportedValue(valErr.Error()) + return staticConds.NewListenerUnsupportedValue(valErr.Error()), false } - return nil + return nil, true } func getAndValidateListenerSupportedKinds(listener v1.Listener) ( @@ -253,9 +260,9 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) ( return conds, supportedKinds } -func validateListenerAllowedRouteKind(listener v1.Listener) []conditions.Condition { - conds, _ := getAndValidateListenerSupportedKinds(listener) - return conds +func validateListenerAllowedRouteKind(listener v1.Listener) (conds []conditions.Condition, attachable bool) { + conds, _ = getAndValidateListenerSupportedKinds(listener) + return conds, len(conds) == 0 } func getListenerSupportedKinds(listener v1.Listener) []v1.RouteGroupKind { @@ -263,23 +270,21 @@ func getListenerSupportedKinds(listener v1.Listener) []v1.RouteGroupKind { return kinds } -func validateListenerLabelSelector(listener v1.Listener) []conditions.Condition { +func validateListenerLabelSelector(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil && listener.AllowedRoutes.Namespaces.From != nil && *listener.AllowedRoutes.Namespaces.From == v1.NamespacesFromSelector && listener.AllowedRoutes.Namespaces.Selector == nil { msg := "Listener's AllowedRoutes Selector must be set when From is set to type Selector" - return staticConds.NewListenerUnsupportedValue(msg) + return staticConds.NewListenerUnsupportedValue(msg), false } - return nil + return nil, true } func createHTTPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { - return func(listener v1.Listener) []conditions.Condition { - var conds []conditions.Condition - + return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if err := validateListenerPort(listener.Port, protectedPorts); err != nil { path := field.NewPath("port") valErr := field.Invalid(path, listener.Port, err.Error()) @@ -290,7 +295,7 @@ func createHTTPListenerValidator(protectedPorts ProtectedPorts) listenerValidato panicForBrokenWebhookAssumption(fmt.Errorf("tls is not nil for HTTP listener %q", listener.Name)) } - return conds + return conds, true } } @@ -307,9 +312,7 @@ func validateListenerPort(port v1.PortNumber, protectedPorts ProtectedPorts) err } func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidator { - return func(listener v1.Listener) []conditions.Condition { - var conds []conditions.Condition - + return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if err := validateListenerPort(listener.Port, protectedPorts); err != nil { path := field.NewPath("port") valErr := field.Invalid(path, listener.Port, err.Error()) @@ -364,7 +367,7 @@ func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidat conds = append(conds, staticConds.NewListenerUnsupportedValue(valErr.Error())...) } - return conds + return conds, true } } diff --git a/internal/mode/static/state/graph/gateway_listener_test.go b/internal/mode/static/state/graph/gateway_listener_test.go index 7adb935900..4e54ade569 100644 --- a/internal/mode/static/state/graph/gateway_listener_test.go +++ b/internal/mode/static/state/graph/gateway_listener_test.go @@ -52,9 +52,10 @@ func TestValidateHTTPListener(t *testing.T) { v := createHTTPListenerValidator(protectedPorts) - result := v(test.l) + result, attachable := v(test.l) g.Expect(result).To(Equal(test.expected)) + g.Expect(attachable).To(BeTrue()) }) } } @@ -195,8 +196,9 @@ func TestValidateHTTPSListener(t *testing.T) { v := createHTTPSListenerValidator(protectedPorts) - result := v(test.l) + result, attachable := v(test.l) g.Expect(result).To(Equal(test.expected)) + g.Expect(attachable).To(BeTrue()) }) } } @@ -238,12 +240,14 @@ func TestValidateListenerHostname(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - conds := validateListenerHostname(v1.Listener{Hostname: test.hostname}) + conds, attachable := validateListenerHostname(v1.Listener{Hostname: test.hostname}) if test.expectErr { g.Expect(conds).ToNot(BeEmpty()) + g.Expect(attachable).To(BeFalse()) } else { g.Expect(conds).To(BeEmpty()) + g.Expect(attachable).To(BeTrue()) } }) } @@ -405,11 +409,13 @@ func TestValidateListenerLabelSelector(t *testing.T) { }, } - conds := validateListenerLabelSelector(listener) + conds, attachable := validateListenerLabelSelector(listener) if test.expectErr { g.Expect(conds).ToNot(BeEmpty()) + g.Expect(attachable).To(BeFalse()) } else { g.Expect(conds).To(BeEmpty()) + g.Expect(attachable).To(BeTrue()) } }) } diff --git a/internal/mode/static/state/graph/gateway_test.go b/internal/mode/static/state/graph/gateway_test.go index fa550c0856..c378d71180 100644 --- a/internal/mode/static/state/graph/gateway_test.go +++ b/internal/mode/static/state/graph/gateway_test.go @@ -297,7 +297,7 @@ func TestBuildGateway(t *testing.T) { 443, tlsConfigInvalidSecret, ) - invalidHTTPSPortListener := createHTTPSListener("invalid-https-port", "foo.example.com", 0, gatewayTLSConfigSameNs) + invalidHTTPSPortListener := createHTTPSListener("invalid-https-port", "foo.example.com", 65536, gatewayTLSConfigSameNs) const ( invalidHostnameMsg = `hostname: Invalid value: "$example.com": a lowercase RFC 1123 subdomain ` + @@ -356,17 +356,19 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "foo-80-1": { - Source: foo80Listener1, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "foo-8080": { - Source: foo8080Listener, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: foo8080Listener, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -387,6 +389,7 @@ func TestBuildGateway(t *testing.T) { "foo-443-https-1": { Source: foo443HTTPSListener1, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -396,6 +399,7 @@ func TestBuildGateway(t *testing.T) { "foo-8443-https": { Source: foo8443HTTPSListener, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -416,6 +420,7 @@ func TestBuildGateway(t *testing.T) { "listener-with-allowed-routes": { Source: listenerAllowedRoutes, Valid: true, + Attachable: true, AllowedRouteLabelSelector: labels.SelectorFromSet(labels.Set(labelSet)), Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ @@ -460,6 +465,7 @@ func TestBuildGateway(t *testing.T) { "listener-cross-ns-secret": { Source: crossNamespaceSecretListener, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretDiffNamespace)), SupportedKinds: []v1.RouteGroupKind{ @@ -478,8 +484,9 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-cross-ns-secret": { - Source: crossNamespaceSecretListener, - Valid: false, + Source: crossNamespaceSecretListener, + Valid: false, + Attachable: true, Conditions: staticConds.NewListenerRefNotPermitted( `Certificate ref to secret diff-ns/secret not permitted by any ReferenceGrant`, ), @@ -491,7 +498,7 @@ func TestBuildGateway(t *testing.T) { }, Valid: true, }, - name: "invalid https listener with cross-namespace secret; no reference grant", + name: "invalid attachable https listener with cross-namespace secret; no reference grant", }, { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{listenerInvalidSelector}}), @@ -500,11 +507,13 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-with-invalid-selector": { - Source: listenerInvalidSelector, - Valid: false, + Source: listenerInvalidSelector, + Valid: false, + Attachable: true, Conditions: staticConds.NewListenerUnsupportedValue( `invalid label selector: "invalid" is not a valid label selector operator`, ), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute", Group: helpers.GetPointer[v1.Group](v1.GroupName)}, }, @@ -512,7 +521,7 @@ func TestBuildGateway(t *testing.T) { }, Valid: true, }, - name: "http listener with invalid label selector", + name: "attachable http listener with invalid label selector", }, { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{invalidProtocolListener}}), @@ -521,11 +530,13 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "invalid-protocol": { - Source: invalidProtocolListener, - Valid: false, + Source: invalidProtocolListener, + Valid: false, + Attachable: false, Conditions: staticConds.NewListenerUnsupportedProtocol( `protocol: Unsupported value: "TCP": supported values: "HTTP", "HTTPS"`, ), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -550,31 +561,37 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "invalid-port": { - Source: invalidPortListener, - Valid: false, + Source: invalidPortListener, + Valid: false, + Attachable: true, Conditions: staticConds.NewListenerUnsupportedValue( `port: Invalid value: 0: port must be between 1-65535`, ), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "invalid-https-port": { - Source: invalidHTTPSPortListener, - Valid: false, + Source: invalidHTTPSPortListener, + Valid: false, + Attachable: true, Conditions: staticConds.NewListenerUnsupportedValue( - `port: Invalid value: 0: port must be between 1-65535`, + `port: Invalid value: 65536: port must be between 1-65535`, ), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "invalid-protected-port": { - Source: invalidProtectedPortListener, - Valid: false, + Source: invalidProtectedPortListener, + Valid: false, + Attachable: true, Conditions: staticConds.NewListenerUnsupportedValue( `port: Invalid value: 9113: port is already in use as MetricsPort`, ), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -596,6 +613,7 @@ func TestBuildGateway(t *testing.T) { Source: invalidHostnameListener, Valid: false, Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -604,6 +622,7 @@ func TestBuildGateway(t *testing.T) { Source: invalidHTTPSHostnameListener, Valid: false, Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -620,9 +639,10 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "invalid-tls-config": { - Source: invalidTLSConfigListener, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, + Source: invalidTLSConfigListener, + Valid: false, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerInvalidCertificateRef( `tls.certificateRefs[0]: Invalid value: test/does-not-exist: secret does not exist`, ), @@ -655,33 +675,37 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "foo-80-1": { - Source: foo80Listener1, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "foo-8080": { - Source: foo8080Listener, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: foo8080Listener, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "foo-8081": { - Source: foo8081Listener, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: foo8081Listener, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, }, "bar-80": { - Source: bar80Listener, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Source: bar80Listener, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: "HTTPRoute"}, }, @@ -689,6 +713,7 @@ func TestBuildGateway(t *testing.T) { "foo-443-https-1": { Source: foo443HTTPSListener1, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -698,6 +723,7 @@ func TestBuildGateway(t *testing.T) { "foo-8443-https": { Source: foo8443HTTPSListener, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -707,6 +733,7 @@ func TestBuildGateway(t *testing.T) { "bar-443-https": { Source: bar443HTTPSListener, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -716,6 +743,7 @@ func TestBuildGateway(t *testing.T) { "bar-8443-https": { Source: bar8443HTTPSListener, Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: []v1.RouteGroupKind{ @@ -747,6 +775,7 @@ func TestBuildGateway(t *testing.T) { "foo-80-1": { Source: foo80Listener1, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: []v1.RouteGroupKind{ @@ -756,6 +785,7 @@ func TestBuildGateway(t *testing.T) { "bar-80": { Source: bar80Listener, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: []v1.RouteGroupKind{ @@ -765,6 +795,7 @@ func TestBuildGateway(t *testing.T) { "foo-443": { Source: foo443Listener, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), SupportedKinds: []v1.RouteGroupKind{ @@ -774,6 +805,7 @@ func TestBuildGateway(t *testing.T) { "foo-80-https": { Source: foo80HTTPSListener, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), @@ -784,6 +816,7 @@ func TestBuildGateway(t *testing.T) { "foo-443-https-1": { Source: foo443HTTPSListener1, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), @@ -794,6 +827,7 @@ func TestBuildGateway(t *testing.T) { "bar-443-https": { Source: bar443HTTPSListener, Valid: false, + Attachable: true, Routes: map[types.NamespacedName]*Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 84091377e9..a1ec837134 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -231,8 +231,9 @@ func TestBuildGraph(t *testing.T) { } routeHR1 := &Route{ - Valid: true, - Source: hr1, + Valid: true, + Attachable: true, + Source: hr1, ParentRefs: []ParentRef{ { Idx: 0, @@ -247,8 +248,9 @@ func TestBuildGraph(t *testing.T) { } routeHR3 := &Route{ - Valid: true, - Source: hr3, + Valid: true, + Attachable: true, + Source: hr3, ParentRefs: []ParentRef{ { Idx: 0, @@ -272,16 +274,18 @@ func TestBuildGraph(t *testing.T) { Source: gw1, Listeners: map[string]*Listener{ "listener-80-1": { - Source: gw1.Spec.Listeners[0], - Valid: true, + Source: gw1.Spec.Listeners[0], + Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, SupportedKinds: []gatewayv1.RouteGroupKind{{Kind: "HTTPRoute"}}, }, "listener-443-1": { - Source: gw1.Spec.Listeners[1], - Valid: true, + Source: gw1.Spec.Listeners[1], + Valid: true, + Attachable: true, Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-3"}: routeHR3, }, diff --git a/internal/mode/static/state/graph/httproute.go b/internal/mode/static/state/graph/httproute.go index 4baa67e589..4f9da0fbc3 100644 --- a/internal/mode/static/state/graph/httproute.go +++ b/internal/mode/static/state/graph/httproute.go @@ -68,6 +68,9 @@ type Route struct { // Valid tells if the Route is valid. // If it is invalid, NGF should not generate any configuration for it. Valid bool + // Attachable tells if the Route can be attached to any of the Gateways. + // Route can be invalid but still attachable. + Attachable bool } // buildRoutesForGateways builds routes from HTTPRoutes that reference any of the specified Gateways. @@ -191,6 +194,7 @@ func buildRoute( } r.Valid = true + r.Attachable = true r.Rules = make([]Rule, len(ghr.Spec.Rules)) @@ -260,7 +264,7 @@ func bindRoutesToListeners( } func bindRouteToListeners(r *Route, gw *Gateway, namespaces map[types.NamespacedName]*apiv1.Namespace) { - if !r.Valid { + if !r.Attachable { return } @@ -302,19 +306,25 @@ func bindRouteToListeners(r *Route, gw *Gateway, namespaces map[types.Namespaced // Case 4 - winning Gateway // Try to attach Route to all matching listeners + cond, attached := tryToAttachRouteToListeners(ref.Attachment, routeRef.SectionName, r, gw, namespaces) if !attached { attachment.FailedCondition = cond continue } + if cond != (conditions.Condition{}) { + r.Conditions = append(r.Conditions, cond) + } attachment.Attached = true } } // tryToAttachRouteToListeners tries to attach the route to the listeners that match the parentRef and the hostnames. -// If it succeeds in attaching at least one listener it will return true and the condition will be empty. -// If it fails to attach the route, it will return false and the failure condition. +// There are two cases: +// (1) If it succeeds in attaching at least one listener it will return true. The returned condition will be empty if +// at least one of the listeners is valid. Otherwise, it will return the failure condition. +// (2) If it fails to attach the route, it will return false and the failure condition. func tryToAttachRouteToListeners( refStatus *ParentRefAttachmentStatus, sectionName *v1.SectionName, @@ -322,13 +332,13 @@ func tryToAttachRouteToListeners( gw *Gateway, namespaces map[types.NamespacedName]*apiv1.Namespace, ) (conditions.Condition, bool) { - validListeners, listenerExists := findValidListeners(getSectionName(sectionName), gw.Listeners) + attachableListeners, listenerExists := findAttachableListeners(getSectionName(sectionName), gw.Listeners) if !listenerExists { return staticConds.NewRouteNoMatchingParent(), false } - if len(validListeners) == 0 { + if len(attachableListeners) == 0 { return staticConds.NewRouteInvalidListener(), false } @@ -348,11 +358,14 @@ func tryToAttachRouteToListeners( return true, true } + var attachedToAtLeastOneValidListener bool + var allowed, attached bool - for _, l := range validListeners { + for _, l := range attachableListeners { routeAllowed, routeAttached := bind(l) allowed = allowed || routeAllowed attached = attached || routeAttached + attachedToAtLeastOneValidListener = attachedToAtLeastOneValidListener || (routeAttached && l.Valid) } if !attached { @@ -362,34 +375,39 @@ func tryToAttachRouteToListeners( return staticConds.NewRouteNoMatchingListenerHostname(), false } + if !attachedToAtLeastOneValidListener { + return staticConds.NewRouteInvalidListener(), true + } + return conditions.Condition{}, true } -// findValidListeners returns a list of valid listeners and whether the listener exists for a non-empty sectionName. -func findValidListeners(sectionName string, listeners map[string]*Listener) ([]*Listener, bool) { +// findAttachableListeners returns a list of attachable listeners and whether the listener exists for a non-empty +// sectionName. +func findAttachableListeners(sectionName string, listeners map[string]*Listener) ([]*Listener, bool) { if sectionName != "" { l, exists := listeners[sectionName] if !exists { return nil, false } - if l.Valid { + if l.Attachable { return []*Listener{l}, true } return nil, true } - validListeners := make([]*Listener, 0, len(listeners)) + attachableListeners := make([]*Listener, 0, len(listeners)) for _, l := range listeners { - if !l.Valid { + if !l.Attachable { continue } - validListeners = append(validListeners, l) + attachableListeners = append(attachableListeners, l) } - return validListeners, true + return attachableListeners, true } func findAcceptedHostnames(listenerHostname *v1.Hostname, routeHostnames []v1.Hostname) []string { diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go index fc4222f2e1..b0a87f7e47 100644 --- a/internal/mode/static/state/graph/httproute_test.go +++ b/internal/mode/static/state/graph/httproute_test.go @@ -105,7 +105,8 @@ func TestBuildRoutes(t *testing.T) { Gateway: gwNsName, }, }, - Valid: true, + Valid: true, + Attachable: true, Rules: []Rule{ { ValidMatches: true, @@ -399,7 +400,8 @@ func TestBuildRoute(t *testing.T) { Gateway: gatewayNsName, }, }, - Valid: true, + Valid: true, + Attachable: true, Rules: []Rule{ { ValidMatches: true, @@ -423,8 +425,9 @@ func TestBuildRoute(t *testing.T) { validator: &validationfakes.FakeHTTPFieldsValidator{}, hr: hrInvalidHostname, expected: &Route{ - Source: hrInvalidHostname, - Valid: false, + Source: hrInvalidHostname, + Valid: false, + Attachable: false, ParentRefs: []ParentRef{ { Idx: 0, @@ -443,8 +446,9 @@ func TestBuildRoute(t *testing.T) { validator: validatorInvalidFieldsInRule, hr: hrInvalidMatches, expected: &Route{ - Source: hrInvalidMatches, - Valid: false, + Source: hrInvalidMatches, + Valid: false, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -469,8 +473,9 @@ func TestBuildRoute(t *testing.T) { validator: validatorInvalidFieldsInRule, hr: hrInvalidFilters, expected: &Route{ - Source: hrInvalidFilters, - Valid: false, + Source: hrInvalidFilters, + Valid: false, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -496,8 +501,9 @@ func TestBuildRoute(t *testing.T) { validator: validatorInvalidFieldsInRule, hr: hrDroppedInvalidMatches, expected: &Route{ - Source: hrDroppedInvalidMatches, - Valid: true, + Source: hrDroppedInvalidMatches, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -527,8 +533,9 @@ func TestBuildRoute(t *testing.T) { validator: validatorInvalidFieldsInRule, hr: hrDroppedInvalidMatchesAndInvalidFilters, expected: &Route{ - Source: hrDroppedInvalidMatchesAndInvalidFilters, - Valid: true, + Source: hrDroppedInvalidMatchesAndInvalidFilters, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -563,8 +570,9 @@ func TestBuildRoute(t *testing.T) { validator: validatorInvalidFieldsInRule, hr: hrDroppedInvalidFilters, expected: &Route{ - Source: hrDroppedInvalidFilters, - Valid: true, + Source: hrDroppedInvalidFilters, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -612,8 +620,9 @@ func TestBindRouteToListeners(t *testing.T) { Name: gatewayv1.SectionName(name), Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("foo.example.com")), }, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, + Valid: true, + Attachable: true, + Routes: map[types.NamespacedName]*Route{}, } } createModifiedListener := func(name string, m func(*Listener)) *Listener { @@ -676,8 +685,9 @@ func TestBindRouteToListeners(t *testing.T) { var normalRoute *Route createNormalRoute := func(gateway *gatewayv1.Gateway) *Route { normalRoute = &Route{ - Source: hr, - Valid: true, + Source: hr, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -691,9 +701,33 @@ func TestBindRouteToListeners(t *testing.T) { return normalRoute } + invalidAttachableRoute1 := &Route{ + Source: hr, + Valid: false, + Attachable: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + }, + }, + } + invalidAttachableRoute2 := &Route{ + Source: hr, + Valid: false, + Attachable: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + }, + }, + } + routeWithMissingSectionName := &Route{ - Source: hrWithNilSectionName, - Valid: true, + Source: hrWithNilSectionName, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -702,8 +736,9 @@ func TestBindRouteToListeners(t *testing.T) { }, } routeWithEmptySectionName := &Route{ - Source: hrWithEmptySectionName, - Valid: true, + Source: hrWithEmptySectionName, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -712,8 +747,9 @@ func TestBindRouteToListeners(t *testing.T) { }, } routeWithNonExistingListener := &Route{ - Source: hrWithNonExistingListener, - Valid: true, + Source: hrWithNonExistingListener, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -722,8 +758,9 @@ func TestBindRouteToListeners(t *testing.T) { }, } routeWithPort := &Route{ - Source: hrWithPort, - Valid: true, + Source: hrWithPort, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -733,8 +770,9 @@ func TestBindRouteToListeners(t *testing.T) { } ignoredGwNsName := types.NamespacedName{Namespace: "test", Name: "ignored-gateway"} routeWithIgnoredGateway := &Route{ - Source: hr, - Valid: true, + Source: hr, + Valid: true, + Attachable: true, ParentRefs: []ParentRef{ { Idx: 0, @@ -742,7 +780,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, } - notValidRoute := &Route{ + invalidRoute := &Route{ Valid: false, ParentRefs: []ParentRef{ { @@ -752,8 +790,9 @@ func TestBindRouteToListeners(t *testing.T) { }, } - notValidListener := createModifiedListener("", func(l *Listener) { + invalidNotAttachableListener := createModifiedListener("", func(l *Listener) { l.Valid = false + l.Attachable = false }) nonMatchingHostnameListener := createModifiedListener("", func(l *Listener) { l.Source.Hostname = helpers.GetPointer[gatewayv1.Hostname]("bar.example.com") @@ -765,6 +804,7 @@ func TestBindRouteToListeners(t *testing.T) { expectedGatewayListeners map[string]*Listener name string expectedSectionNameRefs []ParentRef + expectedConditions []conditions.Condition }{ { route: createNormalRoute(gw), @@ -869,7 +909,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": notValidListener, + "listener-80-1": invalidNotAttachableListener, }, }, expectedSectionNameRefs: []ParentRef{ @@ -884,9 +924,9 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": notValidListener, + "listener-80-1": invalidNotAttachableListener, }, - name: "empty section name with no valid listeners", + name: "empty section name with no valid and attachable listeners", }, { route: routeWithPort, @@ -946,7 +986,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": notValidListener, + "listener-80-1": invalidNotAttachableListener, }, }, expectedSectionNameRefs: []ParentRef{ @@ -961,9 +1001,9 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": notValidListener, + "listener-80-1": invalidNotAttachableListener, }, - name: "listener isn't valid", + name: "listener isn't valid and attachable", }, { route: createNormalRoute(gw), @@ -1016,7 +1056,7 @@ func TestBindRouteToListeners(t *testing.T) { name: "gateway is ignored", }, { - route: notValidRoute, + route: invalidRoute, gateway: &Gateway{ Source: gw, Valid: true, @@ -1061,6 +1101,104 @@ func TestBindRouteToListeners(t *testing.T) { }, name: "invalid gateway", }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Valid = false + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-80-1": {"foo.example.com"}, + }, + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Valid = false + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): getLastNormalRoute(), + } + }), + }, + expectedConditions: []conditions.Condition{staticConds.NewRouteInvalidListener()}, + name: "invalid attachable listener", + }, + { + route: invalidAttachableRoute1, + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createListener("listener-80-1"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-80-1": {"foo.example.com"}, + }, + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): invalidAttachableRoute1, + } + }), + }, + name: "invalid attachable route", + }, + { + route: invalidAttachableRoute2, + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Valid = false + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-80-1": {"foo.example.com"}, + }, + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Valid = false + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): invalidAttachableRoute2, + } + }), + }, + expectedConditions: []conditions.Condition{staticConds.NewRouteInvalidListener()}, + name: "invalid attachable listener with invalid attachable route", + }, { route: createNormalRoute(gw), gateway: &Gateway{ @@ -1284,6 +1422,7 @@ func TestBindRouteToListeners(t *testing.T) { g.Expect(test.route.ParentRefs).To(Equal(test.expectedSectionNameRefs)) g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty()) + g.Expect(helpers.Diff(test.route.Conditions, test.expectedConditions)).To(BeEmpty()) }) } }