From 3c32a9ec7510dda53c9a240049c2f0f865baacba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Tue, 1 Oct 2024 11:36:06 +0200 Subject: [PATCH] feat(konnect): add support for konghq.com/plugins annotation on KongConsumers --- CHANGELOG.md | 1 + ...ngroute-kongconsumer-plugin-annotated.yaml | 85 ++++++ controller/konnect/index_kongconsumer.go | 17 ++ controller/konnect/reconciler_kongplugin.go | 163 ++++++++++-- .../reconciler_kongplugin_combinations.go | 38 ++- ...reconciler_kongplugin_combinations_test.go | 60 ++++- controller/konnect/watch.go | 14 +- controller/konnect/watch_credentialacl.go | 3 +- controller/konnect/watch_kongtarget.go | 4 +- pkg/utils/kubernetes/reduce/filters.go | 44 +++ pkg/utils/kubernetes/reduce/filters_test.go | 109 ++++++++ pkg/utils/kubernetes/reduce/reduce.go | 15 ++ .../envtest/kongpluginbinding_managed_test.go | 251 ++++++++++++++++-- 13 files changed, 743 insertions(+), 61 deletions(-) create mode 100644 config/samples/konnect-kongservice-kongroute-kongconsumer-plugin-annotated.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index dc54bf531..40731ab7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ the creation of a managed `KongPluginBinding` resource: - `KongService` [#550](https://github.com/Kong/gateway-operator/pull/550) - `KongRoute` [#644](https://github.com/Kong/gateway-operator/pull/644) + - `KongConsumer` [#676](https://github.com/Kong/gateway-operator/pull/676) These `KongPluginBinding`s are taken by the `KongPluginBinding` reconciler to create the corresponding plugin objects in Konnect. - `KongConsumer` associated with `ConsumerGroups` is now reconciled in Konnect by removing/adding diff --git a/config/samples/konnect-kongservice-kongroute-kongconsumer-plugin-annotated.yaml b/config/samples/konnect-kongservice-kongroute-kongconsumer-plugin-annotated.yaml new file mode 100644 index 000000000..ae968dca7 --- /dev/null +++ b/config/samples/konnect-kongservice-kongroute-kongconsumer-plugin-annotated.yaml @@ -0,0 +1,85 @@ +kind: KonnectAPIAuthConfiguration +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: demo-auth + namespace: default +spec: + type: token + token: kpat_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + serverURL: eu.api.konghq.tech +--- +kind: KonnectGatewayControlPlane +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: demo-cp + namespace: default +spec: + name: demo-cp + labels: + app: demo-cp + key1: demo-cp + konnect: + authRef: + name: demo-auth + # namespace not required if APIAuthConfiguration is in the same namespace +--- +# This KongPlugin is bound to both the KongService, KongRoute and KongCon +# hence it will create 2 KongPluginBinding with the following targets: +# - KongService and KongConsumer +# - KongRoute and KongConsumer +apiVersion: configuration.konghq.com/v1 +kind: KongPlugin +metadata: + name: rate-limit-5-min + namespace: default +config: + minute: 5 + policy: local +plugin: rate-limiting +--- +kind: KongService +apiVersion: configuration.konghq.com/v1alpha1 +metadata: + name: service-1 + namespace: default + annotations: + konghq.com/plugins: rate-limit-5-min +spec: + name: service-1 + host: example.com + controlPlaneRef: + type: konnectNamespacedRef + konnectNamespacedRef: + name: demo-cp +--- +kind: KongRoute +apiVersion: configuration.konghq.com/v1alpha1 +metadata: + name: route-1 + namespace: default + annotations: + konghq.com/plugins: rate-limit-5-min +spec: + name: route-1 + protocols: + - http + hosts: + - example.com + serviceRef: + type: namespacedRef + namespacedRef: + name: service-1 +--- +kind: KongConsumer +apiVersion: configuration.konghq.com/v1 +metadata: + name: consumer-api-key-1 + namespace: default + annotations: + konghq.com/plugins: rate-limit-5-min +username: consumer1 +spec: + controlPlaneRef: + type: konnectNamespacedRef + konnectNamespacedRef: + name: demo-cp diff --git a/controller/konnect/index_kongconsumer.go b/controller/konnect/index_kongconsumer.go index cc6976f9b..80c3a9683 100644 --- a/controller/konnect/index_kongconsumer.go +++ b/controller/konnect/index_kongconsumer.go @@ -3,12 +3,16 @@ package konnect import ( "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/kong/gateway-operator/pkg/annotations" + configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" ) const ( // IndexFieldKongConsumerOnKongConsumerGroup is the index field for KongConsumer -> KongConsumerGroup. IndexFieldKongConsumerOnKongConsumerGroup = "consumerGroupRef" + // IndexFieldKongConsumerOnPlugin is the index field for KongConsumer -> KongPlugin. + IndexFieldKongConsumerOnPlugin = "consumerPluginRef" ) // IndexOptionsForKongConsumer returns required Index options for KongConsumer reconciler. @@ -19,6 +23,11 @@ func IndexOptionsForKongConsumer() []ReconciliationIndexOption { IndexField: IndexFieldKongConsumerOnKongConsumerGroup, ExtractValue: kongConsumerReferencesFromKongConsumerGroup, }, + { + IndexObject: &configurationv1.KongConsumer{}, + IndexField: IndexFieldKongConsumerOnPlugin, + ExtractValue: kongConsumerReferencesKongPluginsViaAnnotation, + }, } } @@ -29,3 +38,11 @@ func kongConsumerReferencesFromKongConsumerGroup(object client.Object) []string } return consumer.ConsumerGroups } + +func kongConsumerReferencesKongPluginsViaAnnotation(object client.Object) []string { + consumer, ok := object.(*configurationv1.KongConsumer) + if !ok { + return nil + } + return annotations.ExtractPluginsWithNamespaces(consumer) +} diff --git a/controller/konnect/reconciler_kongplugin.go b/controller/konnect/reconciler_kongplugin.go index 82d3cd94b..22eb9e5f4 100644 --- a/controller/konnect/reconciler_kongplugin.go +++ b/controller/konnect/reconciler_kongplugin.go @@ -20,6 +20,7 @@ import ( "github.com/kong/gateway-operator/controller/pkg/log" "github.com/kong/gateway-operator/pkg/consts" + k8sreduce "github.com/kong/gateway-operator/pkg/utils/kubernetes/reduce" configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" @@ -65,6 +66,13 @@ func (r *KongPluginReconciler) SetupWithManager(_ context.Context, mgr ctrl.Mana kongPluginsAnnotationChangedPredicate, ), ). + Watches( + &configurationv1.KongConsumer{}, + handler.EnqueueRequestsFromMapFunc(mapPluginsFromAnnotation[configurationv1.KongConsumer](r.developmentMode)), + builder.WithPredicates( + kongPluginsAnnotationChangedPredicate, + ), + ). Complete(r) } @@ -99,10 +107,6 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - // TODO(mlavacca): So far we are supporting only KongService targets here. We need to implement - // the same logic for KongRoute, KongConsumer, and KongConsumerGroup as well. - // https://github.com/Kong/gateway-operator/issues/583 - kongServiceList := configurationv1alpha1.KongServiceList{} err = clientWithNamespace.List(ctx, &kongServiceList, client.MatchingFields{ @@ -123,6 +127,19 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("failed listing KongRoutes referencing %s KongPlugin: %w", client.ObjectKeyFromObject(&kongPlugin), err) } + kongConsumerList := configurationv1.KongConsumerList{} + err = clientWithNamespace.List(ctx, &kongConsumerList, + client.MatchingFields{ + IndexFieldKongConsumerOnPlugin: kongPlugin.Namespace + "/" + kongPlugin.Name, + }, + ) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed listing KongRoutes referencing %s KongPlugin: %w", client.ObjectKeyFromObject(&kongPlugin), err) + } + + // TODO: https://github.com/Kong/gateway-operator/issues/583 + // add support for consumer groups + foreignRelations := ForeignRelations{ Service: lo.Filter(kongServiceList.Items, func(s configurationv1alpha1.KongService, _ int) bool { return s.DeletionTimestamp.IsZero() }, @@ -130,7 +147,11 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) Route: lo.Filter(kongRouteList.Items, func(s configurationv1alpha1.KongRoute, _ int) bool { return s.DeletionTimestamp.IsZero() }, ), - // TODO consumers and consumer groups + Consumer: lo.Filter(kongConsumerList.Items, + func(c configurationv1.KongConsumer, _ int) bool { return c.DeletionTimestamp.IsZero() }, + ), + // TODO: https://github.com/Kong/gateway-operator/issues/583 + // add support for consumer groups } grouped, err := foreignRelations.GroupByControlPlane(ctx, r.client) if err != nil { @@ -178,8 +199,16 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) }) builder.WithRouteTarget(rel.Route) } + if rel.Consumer != "" { + kpbList = lo.Filter(kpbList, func(pb configurationv1alpha1.KongPluginBinding, _ int) bool { + return pb.Spec.Targets.ConsumerReference != nil && + pb.Spec.Targets.ConsumerReference.Name == rel.Consumer + }) + builder.WithConsumerTarget(rel.Consumer) + } - // TODO consumers and consumer groups + // TODO: https://github.com/Kong/gateway-operator/issues/583 + // add support for consumer groups builder, err = builder.WithOwnerReference(&kongPlugin, clientWithNamespace.Scheme()) if err != nil { @@ -198,8 +227,11 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) case 1: existing := kpbList[0] if reflect.DeepEqual(existing.Spec.Targets.ServiceReference, kongPluginBinding.Spec.Targets.ServiceReference) && - reflect.DeepEqual(existing.Spec.Targets.RouteReference, kongPluginBinding.Spec.Targets.RouteReference) { + reflect.DeepEqual(existing.Spec.Targets.RouteReference, kongPluginBinding.Spec.Targets.RouteReference) && + reflect.DeepEqual(existing.Spec.Targets.ConsumerReference, kongPluginBinding.Spec.Targets.ConsumerReference) { // TODO consumers and consumer groups checks + // TODO: https://github.com/Kong/gateway-operator/issues/583 + // add consumer groups checks continue } @@ -214,7 +246,9 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) log.Debug(logger, "Managed KongPluginBinding updated", kongPluginBinding) default: - // TODO: remove surplus KongPluginBindings + if err := k8sreduce.ReduceKongPluginBindings(ctx, clientWithNamespace, kpbList); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to reduce KongPluginBindings: %w", err) + } } } @@ -304,13 +338,12 @@ func deleteUnusedKongPluginBindings( continue } - cpRef := pb.Spec.ControlPlaneRef - if cpRef == nil || - cpRef.KonnectNamespacedRef == nil || - cpRef.Type != configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef { + cpRef, ok := controlPlaneIsRefKonnectNamespacedRef(&pb) + if !ok { continue } + // If a ControlPlane this KongPluginBinding references, is not found, delete the it. combinations, ok := groupedCombinations[types.NamespacedName{ // TODO: implement cross namespace references Namespace: pb.Namespace, @@ -327,12 +360,94 @@ func deleteUnusedKongPluginBindings( serviceRef := pb.Spec.Targets.ServiceReference routeRef := pb.Spec.Targets.RouteReference + consumerRef := pb.Spec.Targets.ConsumerReference switch { + case consumerRef != nil && serviceRef != nil && routeRef != nil: + combinationFound := false + for _, rel := range combinations { + if rel.Consumer != consumerRef.Name && + rel.Service != serviceRef.Name && + rel.Route != routeRef.Name { + continue + } + combinationFound = true + break + } + + if !combinationFound { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + s, serviceExists := getIfRefNotNil[*configurationv1alpha1.KongService](ctx, clientWithNamespace, serviceRef) + r, routeExists := getIfRefNotNil[*configurationv1alpha1.KongRoute](ctx, clientWithNamespace, routeRef) + c, consumerExists := getIfRefNotNil[*configurationv1.KongConsumer](ctx, clientWithNamespace, consumerRef) + if !consumerExists || !serviceExists || !routeExists || + !objHasPluginConfigured(c, kongPlugin.Name) || !c.DeletionTimestamp.IsZero() || + !objHasPluginConfigured(s, kongPlugin.Name) || !s.DeletionTimestamp.IsZero() || + !objHasPluginConfigured(r, kongPlugin.Name) || !r.DeletionTimestamp.IsZero() { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + case consumerRef != nil && routeRef != nil: + combinationFound := false + for _, rel := range combinations { + if rel.Consumer != consumerRef.Name || + rel.Service != "" || + rel.Route != routeRef.Name { + continue + } + combinationFound = true + break + } + + if !combinationFound { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + r, routeExists := getIfRefNotNil[*configurationv1alpha1.KongRoute](ctx, clientWithNamespace, routeRef) + c, consumerExists := getIfRefNotNil[*configurationv1.KongConsumer](ctx, clientWithNamespace, consumerRef) + if !consumerExists || !routeExists || + !objHasPluginConfigured(c, kongPlugin.Name) || !c.DeletionTimestamp.IsZero() || + !objHasPluginConfigured(r, kongPlugin.Name) || !r.DeletionTimestamp.IsZero() { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + case consumerRef != nil && serviceRef != nil: + combinationFound := false + for _, rel := range combinations { + if rel.Consumer != consumerRef.Name || + rel.Service != serviceRef.Name || + rel.Route != "" { + continue + } + combinationFound = true + break + } + + if !combinationFound { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + s, serviceExists := getIfRefNotNil[*configurationv1alpha1.KongService](ctx, clientWithNamespace, serviceRef) + c, consumerExists := getIfRefNotNil[*configurationv1.KongConsumer](ctx, clientWithNamespace, consumerRef) + if !consumerExists || !serviceExists || + !objHasPluginConfigured(c, kongPlugin.Name) || !c.DeletionTimestamp.IsZero() || + !objHasPluginConfigured(s, kongPlugin.Name) || !s.DeletionTimestamp.IsZero() { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + case serviceRef != nil && routeRef != nil: combinationFound := false for _, rel := range combinations { - if rel.Service != serviceRef.Name && + if rel.Service != serviceRef.Name || + rel.Consumer != "" || rel.Route != routeRef.Name { continue } @@ -358,6 +473,7 @@ func deleteUnusedKongPluginBindings( combinationFound := false for _, rel := range combinations { if rel.Service != serviceRef.Name || + rel.Consumer != "" || rel.Route != "" { continue } @@ -381,6 +497,7 @@ func deleteUnusedKongPluginBindings( combinationFound := false for _, rel := range combinations { if rel.Route != routeRef.Name || + rel.Consumer != "" || rel.Service != "" { continue } @@ -423,17 +540,29 @@ func getIfRefNotNil[ *T client.Object }, + TRef interface { + *configurationv1alpha1.TargetRefWithGroupKind | *configurationv1alpha1.TargetRef + }, T any, ]( ctx context.Context, c client.Client, - ref *configurationv1alpha1.TargetRefWithGroupKind, + ref TRef, ) (TPtr, bool) { + if ref == nil { + return nil, false + } + var t T var obj TPtr = &t - if ref == nil { - return obj, false + var name string + switch ref := any(ref).(type) { + case *configurationv1alpha1.TargetRefWithGroupKind: + name = ref.Name + case *configurationv1alpha1.TargetRef: + name = ref.Name } - err := c.Get(ctx, client.ObjectKey{Name: ref.Name}, obj) + + err := c.Get(ctx, client.ObjectKey{Name: name}, obj) return obj, err == nil } diff --git a/controller/konnect/reconciler_kongplugin_combinations.go b/controller/konnect/reconciler_kongplugin_combinations.go index ac690e503..07e208b71 100644 --- a/controller/konnect/reconciler_kongplugin_combinations.go +++ b/controller/konnect/reconciler_kongplugin_combinations.go @@ -6,14 +6,16 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) // ForeignRelations contains all the relations between Kong entities and KongPlugin. type ForeignRelations struct { - Consumer, ConsumerGroup []string - Route []configurationv1alpha1.KongRoute - Service []configurationv1alpha1.KongService + Consumer []configurationv1.KongConsumer + ConsumerGroup []string + Route []configurationv1alpha1.KongRoute + Service []configurationv1alpha1.KongService } // ForeignRelationsGroupedByControlPlane is a map of ForeignRelations grouped by ControlPlane. @@ -37,10 +39,8 @@ func (relations *ForeignRelations) GroupByControlPlane( ) (ForeignRelationsGroupedByControlPlane, error) { ret := make(map[types.NamespacedName]ForeignRelations) for _, service := range relations.Service { - cpRef := service.Spec.ControlPlaneRef - if cpRef == nil || - cpRef.KonnectNamespacedRef == nil || - cpRef.Type != configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef { + cpRef, ok := controlPlaneIsRefKonnectNamespacedRef(&service) + if !ok { continue } nn := types.NamespacedName{ @@ -69,8 +69,8 @@ func (relations *ForeignRelations) GroupByControlPlane( return nil, err } - cpRef := svc.Spec.ControlPlaneRef - if cpRef == nil || cpRef.KonnectNamespacedRef == nil { + cpRef, ok := controlPlaneIsRefKonnectNamespacedRef(&svc) + if !ok { continue } @@ -83,6 +83,20 @@ func (relations *ForeignRelations) GroupByControlPlane( fr.Route = append(fr.Route, route) ret[nn] = fr } + for _, consumer := range relations.Consumer { + cpRef, ok := controlPlaneIsRefKonnectNamespacedRef(&consumer) + if !ok { + continue + } + nn := types.NamespacedName{ + // TODO: implement cross namespace references + Namespace: consumer.Namespace, + Name: cpRef.KonnectNamespacedRef.Name, + } + fr := ret[nn] + fr.Consumer = append(fr.Consumer, consumer) + ret[nn] = fr + } // TODO consumers and consumer groups @@ -123,20 +137,20 @@ func (relations *ForeignRelations) GetCombinations() []Rel { for _, route := range relations.Route { cartesianProduct = append(cartesianProduct, Rel{ Route: route.Name, - Consumer: consumer, + Consumer: consumer.Name, }) } for _, service := range relations.Service { cartesianProduct = append(cartesianProduct, Rel{ Service: service.Name, - Consumer: consumer, + Consumer: consumer.Name, }) } } } else { cartesianProduct = make([]Rel, 0, len(relations.Consumer)) for _, consumer := range relations.Consumer { - cartesianProduct = append(cartesianProduct, Rel{Consumer: consumer}) + cartesianProduct = append(cartesianProduct, Rel{Consumer: consumer.Name}) } } } else if lConsumerGroup > 0 { diff --git a/controller/konnect/reconciler_kongplugin_combinations_test.go b/controller/konnect/reconciler_kongplugin_combinations_test.go index 81a6f485b..5f7f68ade 100644 --- a/controller/konnect/reconciler_kongplugin_combinations_test.go +++ b/controller/konnect/reconciler_kongplugin_combinations_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) @@ -29,7 +30,18 @@ func TestGetCombinations(t *testing.T) { name: "plugins on consumer only", args: args{ relations: ForeignRelations{ - Consumer: []string{"foo", "bar"}, + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, }, }, want: []Rel{ @@ -151,7 +163,13 @@ func TestGetCombinations(t *testing.T) { }, }, }, - Consumer: []string{"c1"}, + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + }, + }, + }, }, }, want: []Rel{ @@ -179,7 +197,18 @@ func TestGetCombinations(t *testing.T) { }, }, }, - Consumer: []string{"c1", "c2"}, + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c2", + }, + }, + }, }, }, want: []Rel{ @@ -233,7 +262,7 @@ func TestGetCombinations(t *testing.T) { }, }, { - name: "plugins on combination of service,route and consumer", + name: "plugins on combination of service, route and consumer", args: args{ relations: ForeignRelations{ Route: []configurationv1alpha1.KongRoute{ @@ -258,7 +287,13 @@ func TestGetCombinations(t *testing.T) { }, }, }, - Consumer: []string{"c1"}, + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + }, + }, + }, }, }, want: []Rel{ @@ -273,7 +308,7 @@ func TestGetCombinations(t *testing.T) { }, }, { - name: "plugins on combination of service,route and consumers", + name: "plugins on combination of service, route and consumers", args: args{ relations: ForeignRelations{ Route: []configurationv1alpha1.KongRoute{ @@ -298,7 +333,18 @@ func TestGetCombinations(t *testing.T) { }, }, }, - Consumer: []string{"c1", "c2"}, + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c2", + }, + }, + }, }, }, want: []Rel{ diff --git a/controller/konnect/watch.go b/controller/konnect/watch.go index 3e9b9ed61..33c20ee59 100644 --- a/controller/konnect/watch.go +++ b/controller/konnect/watch.go @@ -90,7 +90,17 @@ func objHasControlPlaneRefKonnectNamespacedRef[ T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T], ](ent TEnt) bool { + _, ok := controlPlaneIsRefKonnectNamespacedRef(ent) + return ok +} + +func controlPlaneIsRefKonnectNamespacedRef[ + T constraints.SupportedKonnectEntityType, + TEnt constraints.EntityType[T], +](ent TEnt) (configurationv1alpha1.ControlPlaneRef, bool) { cpRef, ok := getControlPlaneRef(ent).Get() - return ok && - cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + if !ok { + return configurationv1alpha1.ControlPlaneRef{}, false + } + return cpRef, cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef } diff --git a/controller/konnect/watch_credentialacl.go b/controller/konnect/watch_credentialacl.go index 688bbe09f..a8d8a3d33 100644 --- a/controller/konnect/watch_credentialacl.go +++ b/controller/konnect/watch_credentialacl.go @@ -92,7 +92,8 @@ func kongCredentialACLRefersToKonnectGatewayControlPlane(cl client.Client) func( return true } - return objHasControlPlaneRefKonnectNamespacedRef(&consumer) + cpRef := consumer.Spec.ControlPlaneRef + return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef } } diff --git a/controller/konnect/watch_kongtarget.go b/controller/konnect/watch_kongtarget.go index e7d6e1a34..f4b600606 100644 --- a/controller/konnect/watch_kongtarget.go +++ b/controller/konnect/watch_kongtarget.go @@ -73,10 +73,10 @@ func enqueueKongTargetForKongUpstream(cl client.Client, if !ok { return nil } - cpRef := kongUpstream.Spec.ControlPlaneRef - if cpRef == nil || cpRef.Type != configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef { + if !objHasControlPlaneRefKonnectNamespacedRef(kongUpstream) { return nil } + var targetList configurationv1alpha1.KongTargetList if err := cl.List(ctx, &targetList, &client.ListOptions{ // TODO: change this when cross namespace refs are allowed. diff --git a/pkg/utils/kubernetes/reduce/filters.go b/pkg/utils/kubernetes/reduce/filters.go index 9b6fdb0ef..38751bc28 100644 --- a/pkg/utils/kubernetes/reduce/filters.go +++ b/pkg/utils/kubernetes/reduce/filters.go @@ -10,9 +10,13 @@ import ( networkingv1 "k8s.io/api/networking/v1" policyv1 "k8s.io/api/policy/v1" rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1" + "github.com/kong/gateway-operator/controller/konnect/conditions" "github.com/kong/gateway-operator/pkg/consts" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) // FiltenNone filter nothing, that is it returns the same slice as provided. @@ -463,3 +467,43 @@ func filterDataPlanes(dataplanes []operatorv1beta1.DataPlane) []operatorv1beta1. return append(dataplanes[:best], dataplanes[best+1:]...) } + +// ----------------------------------------------------------------------------- +// Filter functions - KongPluginBindings +// ----------------------------------------------------------------------------- + +// filterKongPluginBindings filters out the KongPluginBindings to be kept and returns all the KongPluginBindings +// to be deleted. +// The KongPluginBinding with Programmed status condition is kept. +// If no such binding is found the oldest is kept. +func filterKongPluginBindings(kpbs []configurationv1alpha1.KongPluginBinding) []configurationv1alpha1.KongPluginBinding { + if len(kpbs) < 2 { + return []configurationv1alpha1.KongPluginBinding{} + } + + programmed := -1 + best := 0 + for i, kpb := range kpbs { + if lo.ContainsBy(kpb.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.KonnectEntityProgrammedConditionType && + c.Status == metav1.ConditionTrue + }) { + + if programmed != -1 && kpb.CreationTimestamp.Before(&kpbs[programmed].CreationTimestamp) { + best = i + programmed = i + } else if programmed == -1 { + best = i + programmed = i + } + + continue + } + + if kpb.CreationTimestamp.Before(&kpbs[best].CreationTimestamp) && programmed == -1 { + best = i + } + } + + return append(kpbs[:best], kpbs[best+1:]...) +} diff --git a/pkg/utils/kubernetes/reduce/filters_test.go b/pkg/utils/kubernetes/reduce/filters_test.go index 5cc93d4f0..23e924306 100644 --- a/pkg/utils/kubernetes/reduce/filters_test.go +++ b/pkg/utils/kubernetes/reduce/filters_test.go @@ -18,6 +18,8 @@ import ( operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1" "github.com/kong/gateway-operator/pkg/consts" k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) func TestFilterSecrets(t *testing.T) { @@ -1074,3 +1076,110 @@ func TestFilterPodDisruptionBudgets(t *testing.T) { }) } } + +func TestFilterKongPluginBindings(t *testing.T) { + testCases := []struct { + name string + kpbs []configurationv1alpha1.KongPluginBinding + filteredKpbs []configurationv1alpha1.KongPluginBinding + }{ + { + name: "the Programmed binding must be filtered out regardless of the creation timestamp", + kpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1/1/2000", + CreationTimestamp: metav1.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + Status: configurationv1alpha1.KongPluginBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "12/31/1995", + CreationTimestamp: metav1.Date(1995, time.December, 31, 0, 0, 0, 0, time.UTC), + }, + }, + }, + filteredKpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "12/31/1995", + CreationTimestamp: metav1.Date(1995, time.December, 31, 0, 0, 0, 0, time.UTC), + }, + }, + }, + }, + { + name: "the Programmed binding must be filtered out", + kpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1/1/2000", + CreationTimestamp: metav1.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "12/31/1995", + CreationTimestamp: metav1.Date(1995, time.December, 31, 0, 0, 0, 0, time.UTC), + }, + Status: configurationv1alpha1.KongPluginBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: "True", + }, + }, + }, + }, + }, + filteredKpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1/1/2000", + CreationTimestamp: metav1.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + }, + }, + }, + { + name: "the oldest binding must be filtered out if it's not Programmed", + kpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1/1/2000", + CreationTimestamp: metav1.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "12/31/1995", + CreationTimestamp: metav1.Date(1995, time.December, 31, 0, 0, 0, 0, time.UTC), + }, + }, + }, + filteredKpbs: []configurationv1alpha1.KongPluginBinding{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1/1/2000", + CreationTimestamp: metav1.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC), + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + filteredDeployments := filterKongPluginBindings(tc.kpbs) + require.Equal(t, tc.filteredKpbs, filteredDeployments) + }) + } +} diff --git a/pkg/utils/kubernetes/reduce/reduce.go b/pkg/utils/kubernetes/reduce/reduce.go index 2aae02917..adfdc0146 100644 --- a/pkg/utils/kubernetes/reduce/reduce.go +++ b/pkg/utils/kubernetes/reduce/reduce.go @@ -15,6 +15,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) // PreDeleteHook is a function that can be executed before deleting an object. @@ -198,3 +200,16 @@ func ReduceDataPlanes(ctx context.Context, k8sClient client.Client, dataplanes [ } return nil } + +// +kubebuilder:rbac:groups=configuration.konghq.com,resources=kongpluginbindings,verbs=delete + +// ReduceKongPluginBindings detects the best KongPluginBinding in the set and deletes all the others. +func ReduceKongPluginBindings(ctx context.Context, k8sClient client.Client, kpbs []configurationv1alpha1.KongPluginBinding) error { + filteredKongPluginBindings := filterKongPluginBindings(kpbs) + for _, kpb := range filteredKongPluginBindings { + if err := k8sClient.Delete(ctx, &kpb); client.IgnoreNotFound(err) != nil { + return err + } + } + return nil +} diff --git a/test/envtest/kongpluginbinding_managed_test.go b/test/envtest/kongpluginbinding_managed_test.go index e8d8b2cdd..454b80859 100644 --- a/test/envtest/kongpluginbinding_managed_test.go +++ b/test/envtest/kongpluginbinding_managed_test.go @@ -108,8 +108,9 @@ func TestKongPluginBindingManaged(t *testing.T) { t.Logf("waiting for KongPluginBinding to be created") kongPluginBinding := watchFor(t, ctx, wKongPluginBinding, watch.Added, func(kpb *configurationv1alpha1.KongPluginBinding) bool { - return kpb.Spec.Targets.ServiceReference != nil && - kpb.Spec.Targets.ServiceReference.Name == kongService.Name && + targets := kpb.Spec.Targets + return targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name && kpb.Spec.PluginReference.Name == rateLimitingkongPlugin.Name }, "KongPluginBinding wasn't created", @@ -130,8 +131,9 @@ func TestKongPluginBindingManaged(t *testing.T) { require.NoError(t, clientNamespaced.Delete(ctx, kongPluginBinding)) kongPluginBinding = watchFor(t, ctx, wKongPluginBinding, watch.Added, func(kpb *configurationv1alpha1.KongPluginBinding) bool { - return kpb.Spec.Targets.ServiceReference != nil && - kpb.Spec.Targets.ServiceReference.Name == kongService.Name && + svcRef := kpb.Spec.Targets.ServiceReference + return svcRef != nil && + svcRef.Name == kongService.Name && kpb.Spec.PluginReference.Name == rateLimitingkongPlugin.Name }, "KongPluginBinding wasn't recreated", @@ -213,8 +215,9 @@ func TestKongPluginBindingManaged(t *testing.T) { t.Logf("waiting for KongPluginBinding to be created") kongPluginBinding := watchFor(t, ctx, wKongPluginBinding, watch.Added, func(kpb *configurationv1alpha1.KongPluginBinding) bool { - return kpb.Spec.Targets.RouteReference != nil && - kpb.Spec.Targets.RouteReference.Name == kongRoute.Name && + rRef := kpb.Spec.Targets.RouteReference + return rRef != nil && + rRef.Name == kongRoute.Name && kpb.Spec.PluginReference.Name == rateLimitingkongPlugin.Name }, "KongPluginBinding wasn't created", @@ -235,8 +238,9 @@ func TestKongPluginBindingManaged(t *testing.T) { require.NoError(t, clientNamespaced.Delete(ctx, kongPluginBinding)) kongPluginBinding = watchFor(t, ctx, wKongPluginBinding, watch.Added, func(kpb *configurationv1alpha1.KongPluginBinding) bool { - return kpb.Spec.Targets.RouteReference != nil && - kpb.Spec.Targets.RouteReference.Name == kongRoute.Name && + rRef := kpb.Spec.Targets.RouteReference + return rRef != nil && + rRef.Name == kongRoute.Name && kpb.Spec.PluginReference.Name == rateLimitingkongPlugin.Name }, "KongPluginBinding wasn't recreated", @@ -325,13 +329,14 @@ func TestKongPluginBindingManaged(t *testing.T) { return false } - if kpb.Spec.Targets.RouteReference != nil && - kpb.Spec.Targets.RouteReference.Name == kongRoute.Name && - kpb.Spec.Targets.ServiceReference == nil { + targets := kpb.Spec.Targets + if targets.RouteReference != nil && + targets.RouteReference.Name == kongRoute.Name && + targets.ServiceReference == nil { kpbRouteFound = true - } else if kpb.Spec.Targets.RouteReference == nil && - kpb.Spec.Targets.ServiceReference != nil && - kpb.Spec.Targets.ServiceReference.Name == kongService.Name { + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name { kpbServiceFound = true } return kpbRouteFound && kpbServiceFound @@ -364,13 +369,14 @@ func TestKongPluginBindingManaged(t *testing.T) { return false } - if kpb.Spec.Targets.RouteReference != nil && - kpb.Spec.Targets.RouteReference.Name == kongRoute.Name && - kpb.Spec.Targets.ServiceReference == nil { + targets := kpb.Spec.Targets + if targets.RouteReference != nil && + targets.RouteReference.Name == kongRoute.Name && + targets.ServiceReference == nil { kpbRoute = kpb - } else if kpb.Spec.Targets.RouteReference == nil && - kpb.Spec.Targets.ServiceReference != nil && - kpb.Spec.Targets.ServiceReference.Name == kongService.Name { + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name { kpbService = kpb } return kpbRoute != nil && kpbService != nil @@ -425,4 +431,209 @@ func TestKongPluginBindingManaged(t *testing.T) { assert.True(c, sdk.PluginSDK.AssertExpectations(t)) }, waitTime, tickTime) }) + + t.Run("binding to KongConsumer, KongService and KongRoute", func(t *testing.T) { + serviceID := uuid.NewString() + routeID := uuid.NewString() + consumerID := uuid.NewString() + + wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name)) + wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name)) + kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp, + deploy.WithAnnotation(consts.PluginsAnnotationKey, rateLimitingkongPlugin.Name), + ) + t.Cleanup(func() { + require.NoError(t, clientNamespaced.Delete(ctx, kongService)) + }) + updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID()) + kongRoute := deploy.KongRouteAttachedToService(t, ctx, clientNamespaced, kongService, + deploy.WithAnnotation(consts.PluginsAnnotationKey, rateLimitingkongPlugin.Name), + ) + t.Cleanup(func() { + require.NoError(t, clientNamespaced.Delete(ctx, kongRoute)) + }) + updateKongRouteStatusWithProgrammed(t, ctx, clientNamespaced, kongRoute, routeID, cp.GetKonnectStatus().GetKonnectID(), serviceID) + kongConsumer := deploy.KongConsumerAttachedToCP(t, ctx, clientNamespaced, "username-1", cp, + deploy.WithAnnotation(consts.PluginsAnnotationKey, rateLimitingkongPlugin.Name), + ) + t.Cleanup(func() { + require.NoError(t, clientNamespaced.Delete(ctx, kongConsumer)) + }) + updateKongConsumerStatusWithKonnectID(t, ctx, clientNamespaced, kongConsumer, consumerID, cp.GetKonnectStatus().GetKonnectID()) + + t.Logf("waiting for 2 KongPluginBindings to be created") + kpbRouteFound := false + kpbServiceFound := false + watchFor(t, ctx, wKongPluginBinding, watch.Added, + func(kpb *configurationv1alpha1.KongPluginBinding) bool { + if kpb.Spec.PluginReference.Name != rateLimitingkongPlugin.Name { + return false + } + + targets := kpb.Spec.Targets + if targets.RouteReference != nil && + targets.RouteReference.Name == kongRoute.Name && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name && + targets.ServiceReference == nil { + kpbRouteFound = true + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name { + kpbServiceFound = true + } + return kpbRouteFound && kpbServiceFound + }, + "2 KongPluginBindings were not created", + ) + t.Logf( + "checking that managed KongPlugin %s gets plugin-in-use finalizer added", + client.ObjectKeyFromObject(rateLimitingkongPlugin), + ) + _ = watchFor(t, ctx, wKongPlugin, watch.Modified, + func(kp *configurationv1.KongPlugin) bool { + return kp.Name == rateLimitingkongPlugin.Name && + controllerutil.ContainsFinalizer(kp, consts.PluginInUseFinalizer) + }, + "KongPlugin wasn't updated to get plugin-in-use finalizer added", + ) + + var l configurationv1alpha1.KongPluginBindingList + require.NoError(t, clientNamespaced.List(ctx, &l)) + for _, kpb := range l.Items { + t.Logf("delete managed kongPluginBinding %s, then check it gets recreated", client.ObjectKeyFromObject(&kpb)) + require.NoError(t, client.IgnoreNotFound(clientNamespaced.Delete(ctx, &kpb))) + } + + var kpbRoute, kpbService *configurationv1alpha1.KongPluginBinding + watchFor(t, ctx, wKongPluginBinding, watch.Added, + func(kpb *configurationv1alpha1.KongPluginBinding) bool { + if kpb.Spec.PluginReference.Name != rateLimitingkongPlugin.Name { + return false + } + + targets := kpb.Spec.Targets + if targets.RouteReference != nil && + targets.RouteReference.Name == kongRoute.Name && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name && + targets.ServiceReference == nil { + kpbRoute = kpb + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name { + kpbService = kpb + } + return kpbRoute != nil && kpbService != nil + }, + "2 KongPluginBindings were not recreated", + ) + + t.Logf( + "remove annotation from KongRoute %s and check that there exists "+ + "a managed KongPluginBinding with KongService and KongConsumer in its targets", + client.ObjectKeyFromObject(kongRoute), + ) + sdk.PluginSDK.EXPECT(). + DeletePlugin(mock.Anything, cp.GetKonnectStatus().GetKonnectID(), mock.Anything). + Return( + &sdkkonnectops.DeletePluginResponse{ + StatusCode: 200, + }, + nil, + ) + + delete(kongRoute.Annotations, consts.PluginsAnnotationKey) + require.NoError(t, clientNamespaced.Update(ctx, kongRoute)) + + assert.EventuallyWithT(t, + func(t *assert.CollectT) { + var l configurationv1alpha1.KongPluginBindingList + if !assert.NoError(t, clientNamespaced.List(ctx, &l, + client.MatchingFields{ + konnect.IndexFieldKongPluginBindingKongPluginReference: rateLimitingkongPlugin.Namespace + "/" + rateLimitingkongPlugin.Name, + konnect.IndexFieldKongPluginBindingKongServiceReference: kongService.Name, + konnect.IndexFieldKongPluginBindingKongConsumerReference: kongConsumer.Name, + }, + )) { + return + } + assert.Len(t, l.Items, 1) + }, waitTime, tickTime, + "KongPluginBinding bound to Consumer and Service doesn't exist but it should", + ) + + assert.EventuallyWithT(t, + func(c *assert.CollectT) { + assert.True(c, k8serrors.IsNotFound( + clientNamespaced.Get(ctx, client.ObjectKeyFromObject(kpbRoute), kpbRoute), + )) + }, waitTime, tickTime, + "KongPluginBinding bound to Route wasn't deleted after removing annotation from KongRoute", + ) + + t.Logf( + "remove annotation from KongService %s and check that there exists (is created)"+ + "a managed KongPluginBinding with only KongConsumer in its targets", + client.ObjectKeyFromObject(kongService), + ) + + delete(kongService.Annotations, consts.PluginsAnnotationKey) + require.NoError(t, clientNamespaced.Update(ctx, kongService)) + + watchFor(t, ctx, wKongPluginBinding, watch.Added, + func(kpb *configurationv1alpha1.KongPluginBinding) bool { + if kpb.Spec.PluginReference.Name != rateLimitingkongPlugin.Name { + return false + } + + targets := kpb.Spec.Targets + return targets.RouteReference == nil && + targets.ServiceReference == nil && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name + }, + "KongConsumer bound KongPluginBinding wasn't created", + ) + + t.Logf( + "remove annotation from KongConsumer %s and check that no KongPluginBinding exists that binds to it", + client.ObjectKeyFromObject(kongConsumer), + ) + + sdk.PluginSDK.EXPECT(). + DeletePlugin(mock.Anything, cp.GetKonnectStatus().GetKonnectID(), mock.Anything). + Return( + &sdkkonnectops.DeletePluginResponse{ + StatusCode: 200, + }, + nil, + ) + + delete(kongConsumer.Annotations, consts.PluginsAnnotationKey) + require.NoError(t, clientNamespaced.Update(ctx, kongConsumer)) + + watchFor(t, ctx, wKongPluginBinding, watch.Deleted, + func(kpb *configurationv1alpha1.KongPluginBinding) bool { + if kpb.Spec.PluginReference.Name != rateLimitingkongPlugin.Name { + return false + } + + targets := kpb.Spec.Targets + return targets.RouteReference == nil && + targets.ServiceReference == nil && + targets.ConsumerReference != nil && + targets.ConsumerReference.Name == kongConsumer.Name + }, + "KongConsumer bound KongPluginBinding wasn't deleted", + ) + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, sdk.PluginSDK.AssertExpectations(t)) + }, waitTime, tickTime) + }) }