diff --git a/deploy/manifests/rbac.yaml b/deploy/manifests/rbac.yaml index 068f3acda1..f014f5fa2f 100644 --- a/deploy/manifests/rbac.yaml +++ b/deploy/manifests/rbac.yaml @@ -12,6 +12,7 @@ rules: - apiGroups: - "" resources: + - namespaces - services - secrets verbs: diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 51f12f5066..b90850f927 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -64,7 +64,7 @@ Fields: * `mode` - partially supported. Allowed value: `Terminate`. * `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported. * `options` - not supported. - * `allowedRoutes` - not supported. + * `allowedRoutes` - supported. * `addresses` - not supported. * `status` * `addresses` - Pod IPAddress supported. @@ -122,6 +122,7 @@ Fields: * `Accepted/True/Accepted` * `Accepted/False/NoMatchingListenerHostname` * `Accepted/False/NoMatchingParent` + * `Accepted/False/NotAllowedByListeners` * `Accepted/False/UnsupportedValue`: Custom reason for when the HTTPRoute includes an invalid or unsupported value. * `Accepted/False/InvalidListener`: Custom reason for when the HTTPRoute references an invalid listener. * `Accepted/False/GatewayNotProgrammed`: Custom reason for when the Gateway is not Programmed. HTTPRoute may be valid and configured, but will maintain this status as long as the Gateway is not Programmed. diff --git a/internal/events/handler.go b/internal/events/handler.go index 56eb1e25fd..d7e5e26d34 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -128,6 +128,8 @@ func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) { h.cfg.Processor.CaptureUpsertChange(r) case *apiv1.Service: h.cfg.Processor.CaptureUpsertChange(r) + case *apiv1.Namespace: + h.cfg.Processor.CaptureUpsertChange(r) case *apiv1.Secret: // FIXME(kate-osborn): need to handle certificate rotation // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553 @@ -149,6 +151,8 @@ func (h *EventHandlerImpl) propagateDelete(e *DeleteEvent) { h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName) case *apiv1.Service: h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName) + case *apiv1.Namespace: + h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName) case *apiv1.Secret: // FIXME(kate-osborn): make sure that affected servers are updated // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553 diff --git a/internal/manager/manager.go b/internal/manager/manager.go index b67fc565e0..e534ea9c52 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -112,6 +112,12 @@ func Start(cfg config.Config) error { controller.WithFieldIndices(index.CreateEndpointSliceFieldIndices()), }, }, + { + objectType: &apiv1.Namespace{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.LabelChangedPredicate{}), + }, + }, } ctx := ctlr.SetupSignalHandler() @@ -195,6 +201,7 @@ func prepareFirstEventBatchPreparerArgs( objectLists := []client.ObjectList{ &apiv1.ServiceList{}, &apiv1.SecretList{}, + &apiv1.NamespaceList{}, &discoveryV1.EndpointSliceList{}, &gatewayv1beta1.HTTPRouteList{}, } diff --git a/internal/manager/manager_test.go b/internal/manager/manager_test.go index 7f51051080..9166962ae2 100644 --- a/internal/manager/manager_test.go +++ b/internal/manager/manager_test.go @@ -30,6 +30,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { expectedObjectLists: []client.ObjectList{ &apiv1.ServiceList{}, &apiv1.SecretList{}, + &apiv1.NamespaceList{}, &discoveryV1.EndpointSliceList{}, &gatewayv1beta1.HTTPRouteList{}, &gatewayv1beta1.GatewayList{}, @@ -48,6 +49,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { expectedObjectLists: []client.ObjectList{ &apiv1.ServiceList{}, &apiv1.SecretList{}, + &apiv1.NamespaceList{}, &discoveryV1.EndpointSliceList{}, &gatewayv1beta1.HTTPRouteList{}, }, diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index 07050889cb..2853269dce 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -60,7 +60,7 @@ type ChangeProcessorConfig struct { Logger logr.Logger // EventRecorder records events for Kubernetes resources. EventRecorder record.EventRecorder - // Scheme is the a Kubernetes scheme. + // Scheme is the Kubernetes scheme. Scheme *runtime.Scheme // GatewayCtlrName is the name of the Gateway controller. GatewayCtlrName string @@ -89,6 +89,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { Gateways: make(map[types.NamespacedName]*v1beta1.Gateway), HTTPRoutes: make(map[types.NamespacedName]*v1beta1.HTTPRoute), Services: make(map[types.NamespacedName]*apiv1.Service), + Namespaces: make(map[types.NamespacedName]*apiv1.Namespace), } extractGVK := func(obj client.Object) schema.GroupVersionKind { @@ -118,6 +119,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.HTTPRoutes), trackUpsertDelete: true, }, + { + gvk: extractGVK(&apiv1.Namespace{}), + store: newObjectStoreMapAdapter(clusterStore.Namespaces), + trackUpsertDelete: false, + }, { gvk: extractGVK(&apiv1.Service{}), store: newObjectStoreMapAdapter(clusterStore.Services), diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index f8f97b0cf2..dedf884d78 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -924,6 +924,54 @@ var _ = Describe("ChangeProcessor", func() { }) }) }) + Describe("namespace changes", func() { + When("namespace is linked via label selectors", func() { + It("triggers an update when labels are removed", func() { + ns := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns", + Labels: map[string]string{ + "app": "allowed", + }, + }, + } + gw := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1beta1.GatewaySpec{ + Listeners: []v1beta1.Listener{ + { + AllowedRoutes: &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", + }, + }, + }, + }, + }, + }, + }, + } + + processor.CaptureUpsertChange(gw) + processor.CaptureUpsertChange(ns) + + changed, _ := processor.Process() + Expect(changed).To(BeTrue()) + + newNS := ns.DeepCopy() + newNS.Labels = nil + processor.CaptureUpsertChange(newNS) + + changed, _ = processor.Process() + Expect(changed).To(BeTrue()) + }) + }) + }) }) Describe("Ensuring non-changing changes don't override previously changing changes", func() { diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index ab11ccfda2..a6fcbe19b8 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -109,6 +109,17 @@ func NewDefaultRouteConditions() []Condition { } } +// NewRouteNotAllowedByListeners returns a Condition that indicates that the HTTPRoute is not allowed by +// any listener. +func NewRouteNotAllowedByListeners() Condition { + return Condition{ + Type: string(v1beta1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonNotAllowedByListeners), + Message: "HTTPRoute is not allowed by any listener", + } +} + // NewRouteNoMatchingListenerHostname returns a Condition that indicates that the hostname of the listener // does not match the hostnames of the HTTPRoute. func NewRouteNoMatchingListenerHostname() Condition { diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index a4b3e64be8..cd1cd8ddec 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -3,6 +3,8 @@ package graph import ( "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -19,6 +21,8 @@ type Listener struct { // Routes holds the routes attached to the Listener. // Only valid routes are attached. Routes map[types.NamespacedName]*Route + // AllowedRouteLabelSelector is the label selector for this Listener's allowed routes, if defined. + AllowedRouteLabelSelector labels.Selector // SecretPath is the path to the secret on disk. SecretPath string // Conditions holds the conditions of the Listener. @@ -78,6 +82,8 @@ func newListenerConfiguratorFactory( }, http: &listenerConfigurator{ validators: []listenerValidator{ + validateListenerAllowedRouteKind, + validateListenerLabelSelector, validateListenerHostname, validateHTTPListener, }, @@ -87,6 +93,8 @@ func newListenerConfiguratorFactory( }, https: &listenerConfigurator{ validators: []listenerValidator{ + validateListenerAllowedRouteKind, + validateListenerLabelSelector, validateListenerHostname, createHTTPSListenerValidator(gw.Namespace), }, @@ -135,6 +143,16 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { conds = append(conds, validator(listener)...) } + var allowedRouteSelector labels.Selector + if selector := GetAllowedRouteLabelSelector(listener); selector != nil { + var err error + allowedRouteSelector, err = metav1.LabelSelectorAsSelector(selector) + if err != nil { + msg := fmt.Sprintf("invalid label selector: %s", err.Error()) + conds = append(conds, conditions.NewListenerUnsupportedValue(msg)) + } + } + if len(conds) > 0 { return &Listener{ Source: listener, @@ -144,9 +162,10 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { } l := &Listener{ - Source: listener, - Routes: make(map[types.NamespacedName]*Route), - Valid: true, + Source: listener, + AllowedRouteLabelSelector: allowedRouteSelector, + Routes: make(map[types.NamespacedName]*Route), + Valid: true, } // resolvers might add different conditions to the listener, so we run them all. @@ -182,6 +201,45 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition return nil } +func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition { + validHTTPRouteKind := func(kind v1beta1.RouteGroupKind) bool { + if kind.Kind != v1beta1.Kind("HTTPRoute") { + return false + } + if kind.Group == nil || *kind.Group != v1beta1.GroupName { + return false + } + return true + } + + switch listener.Protocol { + case v1beta1.HTTPProtocolType, v1beta1.HTTPSProtocolType: + if listener.AllowedRoutes != nil { + for _, kind := range listener.AllowedRoutes.Kinds { + if !validHTTPRouteKind(kind) { + msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind) + return []conditions.Condition{conditions.NewListenerUnsupportedValue(msg)} + } + } + } + } + + return nil +} + +func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condition { + if listener.AllowedRoutes != nil && + listener.AllowedRoutes.Namespaces != nil && + listener.AllowedRoutes.Namespaces.From != nil && + *listener.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector && + listener.AllowedRoutes.Namespaces.Selector == nil { + msg := "Listener's AllowedRoutes Selector must be set when From is set to type Selector" + return []conditions.Condition{conditions.NewListenerUnsupportedValue(msg)} + } + + return nil +} + func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition { if listener.Port != 80 { path := field.NewPath("port") @@ -314,3 +372,14 @@ func createExternalReferencesForTLSSecretsResolver( } } } + +// GetAllowedRouteLabelSelector returns a listener's AllowedRoutes label selector if it exists. +func GetAllowedRouteLabelSelector(l v1beta1.Listener) *metav1.LabelSelector { + if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil { + if *l.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector && l.AllowedRoutes.Namespaces.Selector != nil { + return l.AllowedRoutes.Namespaces.Selector + } + } + + return nil +} diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index 553bf5769a..3652ead5ff 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" @@ -244,3 +245,122 @@ func TestValidateListenerHostname(t *testing.T) { }) } } + +func TestValidateListenerAllowedRouteKind(t *testing.T) { + tests := []struct { + protocol v1beta1.ProtocolType + kind v1beta1.Kind + group v1beta1.Group + name string + expectErr bool + }{ + { + protocol: v1beta1.TCPProtocolType, + expectErr: false, + name: "unsupported protocol is ignored", + }, + { + protocol: v1beta1.HTTPProtocolType, + group: "bad-group", + kind: "HTTPRoute", + expectErr: true, + name: "invalid group", + }, + { + protocol: v1beta1.HTTPProtocolType, + group: v1beta1.GroupName, + kind: "TCPRoute", + expectErr: true, + name: "invalid kind", + }, + { + protocol: v1beta1.HTTPProtocolType, + group: v1beta1.GroupName, + kind: "HTTPRoute", + expectErr: false, + name: "valid HTTP", + }, + { + protocol: v1beta1.HTTPSProtocolType, + group: v1beta1.GroupName, + kind: "HTTPRoute", + expectErr: false, + name: "valid HTTPS", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + listener := v1beta1.Listener{ + Protocol: test.protocol, + AllowedRoutes: &v1beta1.AllowedRoutes{ + Kinds: []v1beta1.RouteGroupKind{ + { + Kind: test.kind, + Group: &test.group, + }, + }, + }, + } + + conds := validateListenerAllowedRouteKind(listener) + if test.expectErr { + g.Expect(conds).ToNot(BeEmpty()) + } else { + g.Expect(conds).To(BeEmpty()) + } + }) + } +} + +func TestValidateListenerLabelSelector(t *testing.T) { + tests := []struct { + selector *metav1.LabelSelector + from v1beta1.FromNamespaces + name string + expectErr bool + }{ + { + from: v1beta1.NamespacesFromSelector, + selector: &metav1.LabelSelector{}, + expectErr: false, + name: "valid spec", + }, + { + from: v1beta1.NamespacesFromSelector, + selector: nil, + expectErr: true, + name: "invalid spec", + }, + { + from: v1beta1.NamespacesFromAll, + selector: nil, + expectErr: false, + name: "ignored from type", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + listener := v1beta1.Listener{ + AllowedRoutes: &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: &test.from, + Selector: test.selector, + }, + }, + } + + conds := validateListenerLabelSelector(listener) + if test.expectErr { + g.Expect(conds).ToNot(BeEmpty()) + } else { + g.Expect(conds).To(BeEmpty()) + } + }) + } +} diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index 727ff4627f..f3c59473ed 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -177,6 +178,31 @@ func TestBuildGateway(t *testing.T) { Port: 80, Protocol: v1beta1.HTTPProtocolType, } + labelSet := map[string]string{ + "key": "value", + } + listenerAllowedRoutes := v1beta1.Listener{ + Name: "listener-with-allowed-routes", + Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), + Port: 80, + Protocol: v1beta1.HTTPProtocolType, + AllowedRoutes: &v1beta1.AllowedRoutes{ + Kinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute", Group: helpers.GetPointer(v1beta1.Group(v1beta1.GroupName))}, + }, + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{MatchLabels: labelSet}, + }, + }, + } + listenerInvalidSelector := *listenerAllowedRoutes.DeepCopy() + listenerInvalidSelector.Name = "listener-with-invalid-selector" + listenerInvalidSelector.AllowedRoutes.Namespaces.Selector.MatchExpressions = []metav1.LabelSelectorRequirement{ + { + Operator: "invalid", + }, + } gatewayTLSConfig := &v1beta1.GatewayTLSConfig{ Mode: helpers.GetTLSModePointer(v1beta1.TLSModeTerminate), @@ -324,6 +350,43 @@ func TestBuildGateway(t *testing.T) { }, name: "valid https listener", }, + { + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listenerAllowedRoutes}}), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-with-allowed-routes": { + Source: listenerAllowedRoutes, + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(labels.Set(labelSet)), + Routes: map[types.NamespacedName]*Route{}, + }, + }, + Valid: true, + }, + name: "valid http listener with allowed routes label selector", + }, + { + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listenerInvalidSelector}}), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Listeners: map[string]*Listener{ + "listener-with-invalid-selector": { + Source: listenerInvalidSelector, + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue( + `invalid label selector: "invalid" is not a valid label selector operator`, + ), + }, + }, + }, + Valid: true, + }, + name: "http listener with invalid label selector", + }, { gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener802}}), gatewayClass: validGC, diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index 042c00228a..6fb7557f58 100644 --- a/internal/state/graph/graph.go +++ b/internal/state/graph/graph.go @@ -15,6 +15,7 @@ type ClusterState struct { Gateways map[types.NamespacedName]*v1beta1.Gateway HTTPRoutes map[types.NamespacedName]*v1beta1.HTTPRoute Services map[types.NamespacedName]*v1.Service + Namespaces map[types.NamespacedName]*v1.Namespace } // Graph is a Graph-like representation of Gateway API resources. @@ -52,7 +53,7 @@ func BuildGraph( gw := buildGateway(processedGws.Winner, secretMemoryMgr, gc) routes := buildRoutesForGateways(validators.HTTPFieldsValidator, state.HTTPRoutes, processedGws.GetAllNsNames()) - bindRoutesToListeners(routes, gw) + bindRoutesToListeners(routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, state.Services) g := &Graph{ diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index d00b7878d4..d4691add28 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" + apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -242,17 +244,21 @@ func buildRoute( return r } -func bindRoutesToListeners(routes map[types.NamespacedName]*Route, gw *Gateway) { +func bindRoutesToListeners( + routes map[types.NamespacedName]*Route, + gw *Gateway, + namespaces map[types.NamespacedName]*apiv1.Namespace, +) { if gw == nil { return } for _, r := range routes { - bindRouteToListeners(r, gw) + bindRouteToListeners(r, gw, namespaces) } } -func bindRouteToListeners(r *Route, gw *Gateway) { +func bindRouteToListeners(r *Route, gw *Gateway, namespaces map[types.NamespacedName]*apiv1.Namespace) { if !r.Valid { return } @@ -295,7 +301,7 @@ func bindRouteToListeners(r *Route, gw *Gateway) { // Case 4 - winning Gateway // Try to attach Route to all matching listeners - cond, attached := tryToAttachRouteToListeners(ref.Attachment, routeRef.SectionName, r, gw.Listeners) + cond, attached := tryToAttachRouteToListeners(ref.Attachment, routeRef.SectionName, r, gw, namespaces) if !attached { attachment.FailedCondition = cond continue @@ -312,9 +318,10 @@ func tryToAttachRouteToListeners( refStatus *ParentRefAttachmentStatus, sectionName *v1beta1.SectionName, route *Route, - listeners map[string]*Listener, + gw *Gateway, + namespaces map[types.NamespacedName]*apiv1.Namespace, ) (conditions.Condition, bool) { - validListeners, listenerExists := findValidListeners(getSectionName(sectionName), listeners) + validListeners, listenerExists := findValidListeners(getSectionName(sectionName), gw.Listeners) if !listenerExists { return conditions.NewRouteNoMatchingParent(), false @@ -324,24 +331,33 @@ func tryToAttachRouteToListeners( return conditions.NewRouteInvalidListener(), false } - bind := func(l *Listener) (attached bool) { + bind := func(l *Listener) (allowed, attached bool) { + if !routeAllowedByListener(l, route.Source.Namespace, gw.Source.Namespace, namespaces) { + return false, false + } + hostnames := findAcceptedHostnames(l.Source.Hostname, route.Source.Spec.Hostnames) if len(hostnames) == 0 { - return false + return true, false } refStatus.AcceptedHostnames[string(l.Source.Name)] = hostnames l.Routes[client.ObjectKeyFromObject(route.Source)] = route - return true + return true, true } - attached := false + var allowed, attached bool for _, l := range validListeners { - attached = bind(l) || attached + routeAllowed, routeAttached := bind(l) + allowed = allowed || routeAllowed + attached = attached || routeAttached } if !attached { + if !allowed { + return conditions.NewRouteNotAllowedByListeners(), false + } return conditions.NewRouteNoMatchingListenerHostname(), false } @@ -403,6 +419,33 @@ func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames [] return result } +func routeAllowedByListener( + listener *Listener, + routeNS, + gwNS string, + namespaces map[types.NamespacedName]*apiv1.Namespace, +) bool { + if listener.Source.AllowedRoutes != nil { + switch *listener.Source.AllowedRoutes.Namespaces.From { + case v1beta1.NamespacesFromAll: + return true + case v1beta1.NamespacesFromSame: + return routeNS == gwNS + case v1beta1.NamespacesFromSelector: + if listener.AllowedRouteLabelSelector == nil { + return false + } + + ns, exists := namespaces[types.NamespacedName{Name: routeNS}] + if !exists { + panic(fmt.Errorf("route namespace %q not found in map", routeNS)) + } + return listener.AllowedRouteLabelSelector.Matches(labels.Set(ns.Labels)) + } + } + return true +} + func getHostname(h *v1beta1.Hostname) string { if h == nil { return "" diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 2e62d59232..caea6a9d7b 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -6,7 +6,9 @@ import ( "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -552,6 +554,12 @@ func TestBindRouteToListeners(t *testing.T) { Name: "gateway", }, } + gwDiffNamespace := &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "diff-namespace", + Name: "gateway", + }, + } createHTTPRouteWithSectionNameAndPort := func( sectionName *v1beta1.SectionName, @@ -592,14 +600,14 @@ func TestBindRouteToListeners(t *testing.T) { ) var normalRoute *Route - createNormalRoute := func() *Route { + createNormalRoute := func(gateway *v1beta1.Gateway) *Route { normalRoute = &Route{ Source: hr, Valid: true, ParentRefs: []ParentRef{ { Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), + Gateway: client.ObjectKeyFromObject(gateway), }, }, } @@ -685,7 +693,7 @@ func TestBindRouteToListeners(t *testing.T) { expectedSectionNameRefs []ParentRef }{ { - route: createNormalRoute(), + route: createNormalRoute(gw), gateway: &Gateway{ Source: gw, Valid: true, @@ -862,7 +870,7 @@ func TestBindRouteToListeners(t *testing.T) { name: "listener doesn't exist", }, { - route: createNormalRoute(), + route: createNormalRoute(gw), gateway: &Gateway{ Source: gw, Valid: true, @@ -887,7 +895,7 @@ func TestBindRouteToListeners(t *testing.T) { name: "listener isn't valid", }, { - route: createNormalRoute(), + route: createNormalRoute(gw), gateway: &Gateway{ Source: gw, Valid: true, @@ -958,7 +966,7 @@ func TestBindRouteToListeners(t *testing.T) { name: "route isn't valid", }, { - route: createNormalRoute(), + route: createNormalRoute(gw), gateway: &Gateway{ Source: gw, Valid: false, @@ -982,13 +990,226 @@ 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.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + }, + } + allowedLabels := map[string]string{"app": "not-allowed"} + l.AllowedRouteLabelSelector = labels.SelectorFromSet(allowedLabels) + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteNotAllowedByListeners(), + AcceptedHostnames: map[string][]string{}, + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + }, + } + allowedLabels := map[string]string{"app": "not-allowed"} + l.AllowedRouteLabelSelector = labels.SelectorFromSet(allowedLabels) + }), + }, + name: "route not allowed via labels", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + }, + } + allowedLabels := map[string]string{"app": "allowed"} + l.AllowedRouteLabelSelector = labels.SelectorFromSet(allowedLabels) + }), + }, + }, + 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) { + allowedLabels := map[string]string{"app": "allowed"} + l.AllowedRouteLabelSelector = labels.SelectorFromSet(allowedLabels) + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + }, + } + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): getLastNormalRoute(), + } + }), + }, + name: "route allowed via labels", + }, + { + route: createNormalRoute(gwDiffNamespace), + gateway: &Gateway{ + Source: gwDiffNamespace, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSame), + }, + } + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gwDiffNamespace), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteNotAllowedByListeners(), + AcceptedHostnames: map[string][]string{}, + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSame), + }, + } + }), + }, + name: "route not allowed via same namespace", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSame), + }, + } + }), + }, + }, + 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.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSame), + }, + } + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): getLastNormalRoute(), + } + }), + }, + name: "route allowed via same namespace", + }, + { + route: createNormalRoute(gwDiffNamespace), + gateway: &Gateway{ + Source: gwDiffNamespace, + Valid: true, + Listeners: map[string]*Listener{ + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + l.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromAll), + }, + } + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gwDiffNamespace), + 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.Source.AllowedRoutes = &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromAll), + }, + } + l.Routes = map[types.NamespacedName]*Route{ + client.ObjectKeyFromObject(hr): getLastNormalRoute(), + } + }), + }, + name: "route allowed via all namespaces", + }, } + namespaces := map[types.NamespacedName]*v1.Namespace{ + {Name: "test"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{"app": "allowed"}, + }, + }, + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - bindRouteToListeners(test.route, test.gateway) + bindRouteToListeners(test.route, test.gateway, namespaces) g.Expect(test.route.ParentRefs).To(Equal(test.expectedSectionNameRefs)) g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty()) diff --git a/internal/state/relationship/capturer.go b/internal/state/relationship/capturer.go index 31449ff880..6767e287fc 100644 --- a/internal/state/relationship/capturer.go +++ b/internal/state/relationship/capturer.go @@ -3,11 +3,14 @@ package relationship import ( v1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Capturer @@ -15,12 +18,12 @@ import ( // Capturer captures relationships between Kubernetes objects and can be queried for whether a relationship exists // for a given object. // -// Currently, it only captures relationships between HTTPRoutes and Services and Services and EndpointSlices, -// but it can be extended to capture additional relationships. // The relationships between HTTPRoutes -> Services are many to 1, // so these relationships are tracked using a counter. // A Service relationship exists if at least one HTTPRoute references it. -// An EndpointSlice relationship exists, if its Service owner is referenced by at least one HTTPRoute. +// An EndpointSlice relationship exists if its Service owner is referenced by at least one HTTPRoute. +// +// A Namespace relationship exists if it has labels that match a Gateway listener's label selector. type Capturer interface { Capture(obj client.Object) Remove(resourceType client.Object, nsname types.NamespacedName) @@ -32,21 +35,40 @@ type ( routeToServicesMap map[types.NamespacedName]map[types.NamespacedName]struct{} // serviceRefCountMap maps Service names to the number of HTTPRoutes that reference it. serviceRefCountMap map[types.NamespacedName]int + // gatewayLabelSelectorsMap maps Gateways to the label selectors that their listeners use for allowed routes + gatewayLabelSelectorsMap map[types.NamespacedName][]labels.Selector + // namespaceCfg holds information about a namespace + // - labels that it contains + // - gateways that reference it (if labels match) + namespaceCfg struct { + labelMap map[string]string + gateways map[types.NamespacedName]struct{} + } + // namespaces is a collection of namespaces in the system + namespaces map[types.NamespacedName]namespaceCfg ) +func (n namespaceCfg) match() bool { + return len(n.gateways) > 0 +} + // CapturerImpl implements the Capturer interface. type CapturerImpl struct { - routesToServices routeToServicesMap - serviceRefCount serviceRefCountMap - endpointSliceOwners map[types.NamespacedName]types.NamespacedName + routesToServices routeToServicesMap + serviceRefCount serviceRefCountMap + gatewayLabelSelectors gatewayLabelSelectorsMap + namespaces namespaces + endpointSliceOwners map[types.NamespacedName]types.NamespacedName } // NewCapturerImpl creates a new instance of CapturerImpl. func NewCapturerImpl() *CapturerImpl { return &CapturerImpl{ - routesToServices: make(map[types.NamespacedName]map[types.NamespacedName]struct{}), - serviceRefCount: make(map[types.NamespacedName]int), - endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName), + routesToServices: make(routeToServicesMap), + serviceRefCount: make(serviceRefCountMap), + gatewayLabelSelectors: make(gatewayLabelSelectorsMap), + namespaces: make(namespaces), + endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName), } } @@ -63,6 +85,47 @@ func (c *CapturerImpl) Capture(obj client.Object) { Name: svcName, } } + case *v1beta1.Gateway: + var selectors []labels.Selector + for _, listener := range o.Spec.Listeners { + if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil { + convertedSelector, err := metav1.LabelSelectorAsSelector(selector) + if err == nil { + selectors = append(selectors, convertedSelector) + } + } + } + + gatewayName := client.ObjectKeyFromObject(o) + if len(selectors) > 0 { + c.gatewayLabelSelectors[gatewayName] = selectors + for ns, cfg := range c.namespaces { + var gatewayMatches bool + for _, selector := range selectors { + if selector.Matches(labels.Set(cfg.labelMap)) { + gatewayMatches = true + cfg.gateways[gatewayName] = struct{}{} + break + } + } + if !gatewayMatches { + // if gateway was previously referenced by this namespace, clean it up + delete(cfg.gateways, gatewayName) + } + c.namespaces[ns] = cfg + } + } else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists { + // label selectors existed previously for this gateway, so clean up any references to them + c.removeGatewayLabelSelector(gatewayName) + } + case *v1.Namespace: + nsLabels := o.GetLabels() + gateways := c.matchingGateways(nsLabels) + nsCfg := namespaceCfg{ + labelMap: nsLabels, + gateways: gateways, + } + c.namespaces[client.ObjectKeyFromObject(o)] = nsCfg } } @@ -73,6 +136,10 @@ func (c *CapturerImpl) Remove(resourceType client.Object, nsname types.Namespace c.deleteForRoute(nsname) case *discoveryV1.EndpointSlice: delete(c.endpointSliceOwners, nsname) + case *v1beta1.Gateway: + c.removeGatewayLabelSelector(nsname) + case *v1.Namespace: + delete(c.namespaces, nsname) } } @@ -84,6 +151,9 @@ func (c *CapturerImpl) Exists(resourceType client.Object, nsname types.Namespace case *discoveryV1.EndpointSlice: svcOwner, exists := c.endpointSliceOwners[nsname] return exists && c.serviceRefCount[svcOwner] > 0 + case *v1.Namespace: + cfg, exists := c.namespaces[nsname] + return exists && cfg.match() } return false @@ -155,3 +225,27 @@ func getBackendServiceNamesFromRoute(hr *v1beta1.HTTPRoute) map[types.Namespaced return svcNames } + +// matchingGateways looks through all existing label selectors defined by listeners in a gateway, +// and if any matches are found, returns a map of those gateways +func (c *CapturerImpl) matchingGateways(labelMap map[string]string) map[types.NamespacedName]struct{} { + gateways := make(map[types.NamespacedName]struct{}) + for gw, selectors := range c.gatewayLabelSelectors { + for _, selector := range selectors { + if selector.Matches(labels.Set(labelMap)) { + gateways[gw] = struct{}{} + break + } + } + } + + return gateways +} + +func (c *CapturerImpl) removeGatewayLabelSelector(gatewayName types.NamespacedName) { + delete(c.gatewayLabelSelectors, gatewayName) + for ns, cfg := range c.namespaces { + delete(cfg.gateways, gatewayName) + c.namespaces[ns] = cfg + } +} diff --git a/internal/state/relationship/capturer_test.go b/internal/state/relationship/capturer_test.go index ecb4b32849..eb0b7fce8e 100644 --- a/internal/state/relationship/capturer_test.go +++ b/internal/state/relationship/capturer_test.go @@ -7,6 +7,7 @@ import ( discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" @@ -313,23 +314,149 @@ var _ = Describe("Capturer", func() { }) }) }) - Describe("Edge cases", func() { - BeforeEach(func() { - capturer = relationship.NewCapturerImpl() + }) + Describe("Capture namespace and gateway relationships", func() { + var gw *v1beta1.Gateway + var nsNoLabels, ns *v1.Namespace + + BeforeEach(func() { + capturer = relationship.NewCapturerImpl() + gw = &v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1beta1.GatewaySpec{ + Listeners: []v1beta1.Listener{ + { + AllowedRoutes: &v1beta1.AllowedRoutes{ + Namespaces: &v1beta1.RouteNamespaces{ + From: helpers.GetPointer(v1beta1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "valid", + }, + }, + }, + }, + }, + }, + }, + } + nsNoLabels = &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + }, + } + ns = &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "with-labels", + Labels: map[string]string{ + "app": "valid", + }, + }, + } + }) + + When("a gateway with label selectors is created, but no namespace has been captured", func() { + It("does not report a relationship", func() { + capturer.Capture(gw) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) }) - It("Capture does not panic when passed an unsupported resource type", func() { - Expect(func() { - capturer.Capture(&v1beta1.Gateway{}) - }).ToNot(Panic()) + }) + When("a namespace is created that is not linked to a listener", func() { + It("does not report a relationship", func() { + capturer.Capture(gw) + capturer.Capture(nsNoLabels) + + Expect(capturer.Exists(nsNoLabels, client.ObjectKeyFromObject(nsNoLabels))).To(BeFalse()) }) - It("Remove does not panic when passed an unsupported resource type", func() { - Expect(func() { - capturer.Remove(&v1beta1.Gateway{}, types.NamespacedName{}) - }).ToNot(Panic()) + }) + When("a namespace is created that is linked to a listener", func() { + It("reports a relationship", func() { + capturer.Capture(gw) + capturer.Capture(ns) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) }) - It("Exist returns false if passed an unsupported resource type", func() { - Expect(capturer.Exists(&v1beta1.Gateway{}, types.NamespacedName{})).To(BeFalse()) + }) + When("a gateway with label selectors is created after a linked namespace", func() { + It("reports a relationship", func() { + capturer.Capture(ns) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) + + capturer.Capture(gw) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) }) }) + When("label selectors are removed from gateway", func() { + It("does not report a relationship", func() { + capturer.Capture(gw) + capturer.Capture(ns) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) + + gw.Spec.Listeners[0].AllowedRoutes = nil + capturer.Capture(gw) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) + }) + }) + When("gateway changes its labels", func() { + It("does not report a relationship", func() { + capturer.Capture(gw) + capturer.Capture(ns) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) + + gw.Spec.Listeners[0].AllowedRoutes.Namespaces.Selector.MatchLabels = map[string]string{ + "app": "new-value", + } + capturer.Capture(gw) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) + }) + }) + When("gateway is deleted", func() { + It("does not report a relationship", func() { + capturer.Capture(gw) + capturer.Capture(ns) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) + + capturer.Remove(gw, client.ObjectKeyFromObject(gw)) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) + }) + }) + When("a namespace has its labels removed after being linked", func() { + It("reports that a relationship once existed", func() { + capturer.Capture(gw) + capturer.Capture(ns) + + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) + + ns.Labels = nil + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) + + capturer.Capture(ns) + Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) + }) + }) + }) + Describe("Edge cases", func() { + BeforeEach(func() { + capturer = relationship.NewCapturerImpl() + }) + It("Capture does not panic when passed an unsupported resource type", func() { + Expect(func() { + capturer.Capture(&v1beta1.GatewayClass{}) + }).ToNot(Panic()) + }) + It("Remove does not panic when passed an unsupported resource type", func() { + Expect(func() { + capturer.Remove(&v1beta1.GatewayClass{}, types.NamespacedName{}) + }).ToNot(Panic()) + }) + It("Exist returns false if passed an unsupported resource type", func() { + Expect(capturer.Exists(&v1beta1.GatewayClass{}, types.NamespacedName{})).To(BeFalse()) + }) }) }) diff --git a/internal/state/relationship/relationshipfakes/fake_capturer.go b/internal/state/relationship/relationshipfakes/fake_capturer.go index 4307105edf..cea6fd9e90 100644 --- a/internal/state/relationship/relationshipfakes/fake_capturer.go +++ b/internal/state/relationship/relationshipfakes/fake_capturer.go @@ -27,6 +27,17 @@ type FakeCapturer struct { existsReturnsOnCall map[int]struct { result1 bool } + RelationshipEndedStub func(client.Object) bool + relationshipEndedMutex sync.RWMutex + relationshipEndedArgsForCall []struct { + arg1 client.Object + } + relationshipEndedReturns struct { + result1 bool + } + relationshipEndedReturnsOnCall map[int]struct { + result1 bool + } RemoveStub func(client.Object, types.NamespacedName) removeMutex sync.RWMutex removeArgsForCall []struct { @@ -131,6 +142,67 @@ func (fake *FakeCapturer) ExistsReturnsOnCall(i int, result1 bool) { }{result1} } +func (fake *FakeCapturer) RelationshipEnded(arg1 client.Object) bool { + fake.relationshipEndedMutex.Lock() + ret, specificReturn := fake.relationshipEndedReturnsOnCall[len(fake.relationshipEndedArgsForCall)] + fake.relationshipEndedArgsForCall = append(fake.relationshipEndedArgsForCall, struct { + arg1 client.Object + }{arg1}) + stub := fake.RelationshipEndedStub + fakeReturns := fake.relationshipEndedReturns + fake.recordInvocation("RelationshipEnded", []interface{}{arg1}) + fake.relationshipEndedMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeCapturer) RelationshipEndedCallCount() int { + fake.relationshipEndedMutex.RLock() + defer fake.relationshipEndedMutex.RUnlock() + return len(fake.relationshipEndedArgsForCall) +} + +func (fake *FakeCapturer) RelationshipEndedCalls(stub func(client.Object) bool) { + fake.relationshipEndedMutex.Lock() + defer fake.relationshipEndedMutex.Unlock() + fake.RelationshipEndedStub = stub +} + +func (fake *FakeCapturer) RelationshipEndedArgsForCall(i int) client.Object { + fake.relationshipEndedMutex.RLock() + defer fake.relationshipEndedMutex.RUnlock() + argsForCall := fake.relationshipEndedArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeCapturer) RelationshipEndedReturns(result1 bool) { + fake.relationshipEndedMutex.Lock() + defer fake.relationshipEndedMutex.Unlock() + fake.RelationshipEndedStub = nil + fake.relationshipEndedReturns = struct { + result1 bool + }{result1} +} + +func (fake *FakeCapturer) RelationshipEndedReturnsOnCall(i int, result1 bool) { + fake.relationshipEndedMutex.Lock() + defer fake.relationshipEndedMutex.Unlock() + fake.RelationshipEndedStub = nil + if fake.relationshipEndedReturnsOnCall == nil { + fake.relationshipEndedReturnsOnCall = make(map[int]struct { + result1 bool + }) + } + fake.relationshipEndedReturnsOnCall[i] = struct { + result1 bool + }{result1} +} + func (fake *FakeCapturer) Remove(arg1 client.Object, arg2 types.NamespacedName) { fake.removeMutex.Lock() fake.removeArgsForCall = append(fake.removeArgsForCall, struct { @@ -171,6 +243,8 @@ func (fake *FakeCapturer) Invocations() map[string][][]interface{} { defer fake.captureMutex.RUnlock() fake.existsMutex.RLock() defer fake.existsMutex.RUnlock() + fake.relationshipEndedMutex.RLock() + defer fake.relationshipEndedMutex.RUnlock() fake.removeMutex.RLock() defer fake.removeMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/internal/state/store.go b/internal/state/store.go index 2f2db86acb..0284dd0371 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -128,9 +128,9 @@ type changeTrackingUpdater struct { capturer relationship.Capturer extractGVK extractGVKFunc - supportedGKVs gvkList - trackedUpsertDeleteGKVs gvkList - persistedGKVs gvkList + supportedGVKs gvkList + trackedUpsertDeleteGVKs gvkList + persistedGVKs gvkList changed bool } @@ -141,22 +141,22 @@ func newChangeTrackingUpdater( objectTypeCfgs []changeTrackingUpdaterObjectTypeCfg, ) *changeTrackingUpdater { var ( - supportedGKVs gvkList - trackedUpsertDeleteGKVs gvkList - persistedGKVs gvkList + supportedGVKs gvkList + trackedUpsertDeleteGVKs gvkList + persistedGVKs gvkList stores = make(map[schema.GroupVersionKind]objectStore) ) for _, cfg := range objectTypeCfgs { - supportedGKVs = append(supportedGKVs, cfg.gvk) + supportedGVKs = append(supportedGVKs, cfg.gvk) if cfg.trackUpsertDelete { - trackedUpsertDeleteGKVs = append(trackedUpsertDeleteGKVs, cfg.gvk) + trackedUpsertDeleteGVKs = append(trackedUpsertDeleteGVKs, cfg.gvk) } if cfg.store != nil { - persistedGKVs = append(persistedGKVs, cfg.gvk) + persistedGVKs = append(persistedGVKs, cfg.gvk) stores[cfg.gvk] = cfg.store } } @@ -164,28 +164,28 @@ func newChangeTrackingUpdater( return &changeTrackingUpdater{ store: newMultiObjectStore(stores, extractGVK), extractGVK: extractGVK, - supportedGKVs: supportedGKVs, - trackedUpsertDeleteGKVs: trackedUpsertDeleteGKVs, - persistedGKVs: persistedGKVs, + supportedGVKs: supportedGVKs, + trackedUpsertDeleteGVKs: trackedUpsertDeleteGVKs, + persistedGVKs: persistedGVKs, capturer: capturer, } } func (s *changeTrackingUpdater) assertSupportedGVK(gvk schema.GroupVersionKind) { - if !s.supportedGKVs.contains(gvk) { + if !s.supportedGVKs.contains(gvk) { panic(fmt.Errorf("unsupported GVK %v", gvk)) } } func (s *changeTrackingUpdater) upsert(obj client.Object) (changed bool) { - if !s.persistedGKVs.contains(s.extractGVK(obj)) { + if !s.persistedGVKs.contains(s.extractGVK(obj)) { return false } oldObj, exist := s.store.get(obj, client.ObjectKeyFromObject(obj)) s.store.upsert(obj) - if !s.trackedUpsertDeleteGKVs.contains(s.extractGVK(obj)) { + if !s.trackedUpsertDeleteGVKs.contains(s.extractGVK(obj)) { return false } @@ -196,15 +196,19 @@ func (s *changeTrackingUpdater) Upsert(obj client.Object) { s.assertSupportedGVK(s.extractGVK(obj)) changingUpsert := s.upsert(obj) + relationshipExisted := s.capturer.Exists(obj, client.ObjectKeyFromObject(obj)) + s.capturer.Capture(obj) - s.changed = s.changed || changingUpsert || s.capturer.Exists(obj, client.ObjectKeyFromObject(obj)) + relationshipExists := s.capturer.Exists(obj, client.ObjectKeyFromObject(obj)) + + s.changed = s.changed || changingUpsert || relationshipExisted || relationshipExists } func (s *changeTrackingUpdater) delete(objType client.Object, nsname types.NamespacedName) (changed bool) { objTypeGVK := s.extractGVK(objType) - if !s.persistedGKVs.contains(objTypeGVK) { + if !s.persistedGVKs.contains(objTypeGVK) { return false } @@ -214,7 +218,7 @@ func (s *changeTrackingUpdater) delete(objType client.Object, nsname types.Names } s.store.delete(objType, nsname) - return s.trackedUpsertDeleteGKVs.contains(objTypeGVK) + return s.trackedUpsertDeleteGVKs.contains(objTypeGVK) } func (s *changeTrackingUpdater) Delete(objType client.Object, nsname types.NamespacedName) {