From f189f2d3972fe08122bb27ead06152132ee791a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Thu, 3 Oct 2024 09:59:20 +0200 Subject: [PATCH] feat(konnect): add support for konghq.com/plugins annotation on KongConsumerGroups (#684) --- CHANGELOG.md | 1 + ...te-kongconsumergroup-plugin-annotated.yaml | 85 ++++ controller/konnect/index_kongconsumergroup.go | 33 ++ controller/konnect/reconciler_kongplugin.go | 232 ++++++--- .../reconciler_kongplugin_combinations.go | 39 +- ...reconciler_kongplugin_combinations_test.go | 441 +++++++++++++++++- modules/manager/controller_setup.go | 4 + .../envtest/kongpluginbinding_managed_test.go | 205 ++++++++ 8 files changed, 977 insertions(+), 63 deletions(-) create mode 100644 config/samples/konnect-kongservice-kongroute-kongconsumergroup-plugin-annotated.yaml create mode 100644 controller/konnect/index_kongconsumergroup.go diff --git a/CHANGELOG.md b/CHANGELOG.md index eb8c12ac6..e9127c8cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,7 @@ - `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) + - `KongConsumerGroup` [#684](https://github.com/Kong/gateway-operator/pull/684) 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-kongconsumergroup-plugin-annotated.yaml b/config/samples/konnect-kongservice-kongroute-kongconsumergroup-plugin-annotated.yaml new file mode 100644 index 000000000..3dad9f14a --- /dev/null +++ b/config/samples/konnect-kongservice-kongroute-kongconsumergroup-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 KongConsumerGroup +# hence it will create 2 KongPluginBinding with the following targets: +# - KongService and KongConsumerGroup +# - KongRoute and KongConsumerGroup +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: KongConsumerGroup +apiVersion: configuration.konghq.com/v1beta1 +metadata: + name: consumer-group-1 + namespace: default + annotations: + konghq.com/plugins: rate-limit-5-min +spec: + name: consumer-group-1 + controlPlaneRef: + type: konnectNamespacedRef + konnectNamespacedRef: + name: test1 diff --git a/controller/konnect/index_kongconsumergroup.go b/controller/konnect/index_kongconsumergroup.go new file mode 100644 index 000000000..717482ae6 --- /dev/null +++ b/controller/konnect/index_kongconsumergroup.go @@ -0,0 +1,33 @@ +package konnect + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kong/gateway-operator/pkg/annotations" + + configurationv1beta1 "github.com/kong/kubernetes-configuration/api/configuration/v1beta1" +) + +const ( + // IndexFieldKongConsumerGroupOnPlugin is the index field for KongConsumerGroup -> KongPlugin. + IndexFieldKongConsumerGroupOnPlugin = "consumerGroupPluginRef" +) + +// IndexOptionsForKongConsumerGroup returns required Index options for KongConsumerGroup reconciler. +func IndexOptionsForKongConsumerGroup() []ReconciliationIndexOption { + return []ReconciliationIndexOption{ + { + IndexObject: &configurationv1beta1.KongConsumerGroup{}, + IndexField: IndexFieldKongConsumerGroupOnPlugin, + ExtractValue: kongConsumerGroupReferencesKongPluginsViaAnnotation, + }, + } +} + +func kongConsumerGroupReferencesKongPluginsViaAnnotation(object client.Object) []string { + consumerGroup, ok := object.(*configurationv1beta1.KongConsumerGroup) + if !ok { + return nil + } + return annotations.ExtractPluginsWithNamespaces(consumerGroup) +} diff --git a/controller/konnect/reconciler_kongplugin.go b/controller/konnect/reconciler_kongplugin.go index b41cdaadc..f89083761 100644 --- a/controller/konnect/reconciler_kongplugin.go +++ b/controller/konnect/reconciler_kongplugin.go @@ -24,6 +24,7 @@ import ( configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" + configurationv1beta1 "github.com/kong/kubernetes-configuration/api/configuration/v1beta1" ) // KongPluginReconciler reconciles a KongPlugin object. @@ -73,6 +74,13 @@ func (r *KongPluginReconciler) SetupWithManager(_ context.Context, mgr ctrl.Mana kongPluginsAnnotationChangedPredicate, ), ). + Watches( + &configurationv1beta1.KongConsumerGroup{}, + handler.EnqueueRequestsFromMapFunc(mapPluginsFromAnnotation[configurationv1beta1.KongConsumerGroup](r.developmentMode)), + builder.WithPredicates( + kongPluginsAnnotationChangedPredicate, + ), + ). Complete(r) } @@ -93,6 +101,7 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) log.Debug(logger, "reconciling", kongPlugin) clientWithNamespace := client.NewNamespacedClient(r.client, kongPlugin.Namespace) + kongPluginNN := client.ObjectKeyFromObject(&kongPlugin) // Get the pluginBindings that use this KongPlugin kongPluginBindingList := configurationv1alpha1.KongPluginBindingList{} @@ -100,59 +109,18 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) ctx, &kongPluginBindingList, client.MatchingFields{ - IndexFieldKongPluginBindingKongPluginReference: kongPlugin.Namespace + "/" + kongPlugin.Name, + IndexFieldKongPluginBindingKongPluginReference: kongPluginNN.String(), }, ) if err != nil { return ctrl.Result{}, err } - kongServiceList := configurationv1alpha1.KongServiceList{} - err = clientWithNamespace.List(ctx, &kongServiceList, - client.MatchingFields{ - IndexFieldKongServiceOnReferencedPluginNames: kongPlugin.Namespace + "/" + kongPlugin.Name, - }, - ) + foreignRelations, err := listAllEntitiesReferencingPluginIntoRelations(ctx, clientWithNamespace, kongPluginNN) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed listing KongServices referencing %s KongPlugin: %w", client.ObjectKeyFromObject(&kongPlugin), err) - } - - kongRouteList := configurationv1alpha1.KongRouteList{} - err = clientWithNamespace.List(ctx, &kongRouteList, - client.MatchingFields{ - IndexFieldKongRouteOnReferencedPluginNames: kongPlugin.Namespace + "/" + kongPlugin.Name, - }, - ) - if err != nil { - 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) + return ctrl.Result{}, 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() }, - ), - Route: lo.Filter(kongRouteList.Items, - func(s configurationv1alpha1.KongRoute, _ int) bool { return s.DeletionTimestamp.IsZero() }, - ), - 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 { return ctrl.Result{}, err @@ -206,9 +174,13 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) }) builder.WithConsumerTarget(rel.Consumer) } - - // TODO: https://github.com/Kong/gateway-operator/issues/583 - // add support for consumer groups + if rel.ConsumerGroup != "" { + kpbList = lo.Filter(kpbList, func(pb configurationv1alpha1.KongPluginBinding, _ int) bool { + return pb.Spec.Targets.ConsumerGroupReference != nil && + pb.Spec.Targets.ConsumerGroupReference.Name == rel.ConsumerGroup + }) + builder.WithConsumerGroupTarget(rel.ConsumerGroup) + } builder, err = builder.WithOwnerReference(&kongPlugin, clientWithNamespace.Scheme()) if err != nil { @@ -228,10 +200,8 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) 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.ConsumerReference, kongPluginBinding.Spec.Targets.ConsumerReference) { - // TODO consumers and consumer groups checks - // TODO: https://github.com/Kong/gateway-operator/issues/583 - // add consumer groups checks + reflect.DeepEqual(existing.Spec.Targets.ConsumerReference, kongPluginBinding.Spec.Targets.ConsumerReference) && + reflect.DeepEqual(existing.Spec.Targets.ConsumerGroupReference, kongPluginBinding.Spec.Targets.ConsumerGroupReference) { continue } @@ -287,6 +257,67 @@ func (r *KongPluginReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } +func listAllEntitiesReferencingPluginIntoRelations( + ctx context.Context, + clientWithNamespace client.Client, + kongPluginNN types.NamespacedName, +) (ForeignRelations, error) { + var kongServiceList configurationv1alpha1.KongServiceList + err := clientWithNamespace.List(ctx, &kongServiceList, + client.MatchingFields{ + IndexFieldKongServiceOnReferencedPluginNames: kongPluginNN.String(), + }, + ) + if err != nil { + return ForeignRelations{}, fmt.Errorf("failed listing KongServices referencing %s KongPlugin: %w", kongPluginNN, err) + } + + var kongRouteList configurationv1alpha1.KongRouteList + err = clientWithNamespace.List(ctx, &kongRouteList, + client.MatchingFields{ + IndexFieldKongRouteOnReferencedPluginNames: kongPluginNN.String(), + }, + ) + if err != nil { + return ForeignRelations{}, fmt.Errorf("failed listing KongRoutes referencing %s KongPlugin: %w", kongPluginNN, err) + } + + var kongConsumerList configurationv1.KongConsumerList + err = clientWithNamespace.List(ctx, &kongConsumerList, + client.MatchingFields{ + IndexFieldKongConsumerOnPlugin: kongPluginNN.String(), + }, + ) + if err != nil { + return ForeignRelations{}, fmt.Errorf("failed listing KongConsumers referencing %s KongPlugin: %w", kongPluginNN, err) + } + + var kongConsumerGroupList configurationv1beta1.KongConsumerGroupList + err = clientWithNamespace.List(ctx, &kongConsumerGroupList, + client.MatchingFields{ + IndexFieldKongConsumerGroupOnPlugin: kongPluginNN.String(), + }, + ) + if err != nil { + return ForeignRelations{}, fmt.Errorf("failed listing KongConsumerGroups referencing %s KongPlugin: %w", kongPluginNN, err) + } + + return ForeignRelations{ + Service: lo.Filter(kongServiceList.Items, + func(s configurationv1alpha1.KongService, _ int) bool { return s.DeletionTimestamp.IsZero() }, + ), + Route: lo.Filter(kongRouteList.Items, + func(s configurationv1alpha1.KongRoute, _ int) bool { return s.DeletionTimestamp.IsZero() }, + ), + Consumer: lo.Filter(kongConsumerList.Items, + func(c configurationv1.KongConsumer, _ int) bool { return c.DeletionTimestamp.IsZero() }, + ), + ConsumerGroup: lo.Filter(kongConsumerGroupList.Items, + func(c configurationv1beta1.KongConsumerGroup, _ int) bool { return c.DeletionTimestamp.IsZero() }, + ), + }, nil +} + func deleteKongPluginBindings( ctx context.Context, logger logr.Logger, @@ -361,14 +392,17 @@ func deleteUnusedKongPluginBindings( serviceRef := pb.Spec.Targets.ServiceReference routeRef := pb.Spec.Targets.RouteReference consumerRef := pb.Spec.Targets.ConsumerReference + consumerGroupRef := pb.Spec.Targets.ConsumerGroupReference + switch { - case consumerRef != nil && serviceRef != nil && routeRef != nil: + case consumerRef != nil && serviceRef != nil && routeRef != nil && consumerGroupRef == nil: combinationFound := false for _, rel := range combinations { - if rel.Consumer != consumerRef.Name && - rel.Service != serviceRef.Name && - rel.Route != routeRef.Name { + if rel.Consumer != consumerRef.Name || + rel.Service != serviceRef.Name || + rel.Route != routeRef.Name || + rel.ConsumerGroup != "" { continue } combinationFound = true @@ -391,11 +425,41 @@ func deleteUnusedKongPluginBindings( continue } + case consumerGroupRef != nil && serviceRef != nil && routeRef != nil && consumerRef == nil: + combinationFound := false + for _, rel := range combinations { + if rel.ConsumerGroup != consumerGroupRef.Name || + rel.Service != serviceRef.Name || + rel.Route != routeRef.Name || + rel.Consumer != "" { + 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) + cg, consumerGroupExists := getIfRefNotNil[*configurationv1beta1.KongConsumerGroup](ctx, clientWithNamespace, consumerGroupRef) + if !consumerGroupExists || !serviceExists || !routeExists || + !objHasPluginConfigured(cg, kongPlugin.Name) || !cg.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.ConsumerGroup != "" || rel.Route != routeRef.Name { continue } @@ -422,6 +486,7 @@ func deleteUnusedKongPluginBindings( for _, rel := range combinations { if rel.Consumer != consumerRef.Name || rel.Service != serviceRef.Name || + rel.ConsumerGroup != "" || rel.Route != "" { continue } @@ -443,11 +508,66 @@ func deleteUnusedKongPluginBindings( continue } + case consumerGroupRef != nil && routeRef != nil: + combinationFound := false + for _, rel := range combinations { + if rel.ConsumerGroup != consumerGroupRef.Name || + rel.Service != "" || + rel.Consumer != "" || + rel.Route != routeRef.Name { + continue + } + combinationFound = true + break + } + + if !combinationFound { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + r, routeExists := getIfRefNotNil[*configurationv1alpha1.KongRoute](ctx, clientWithNamespace, routeRef) + cg, consumerGroupExists := getIfRefNotNil[*configurationv1beta1.KongConsumerGroup](ctx, clientWithNamespace, consumerGroupRef) + if !consumerGroupExists || !routeExists || + !objHasPluginConfigured(cg, kongPlugin.Name) || !cg.DeletionTimestamp.IsZero() || + !objHasPluginConfigured(r, kongPlugin.Name) || !r.DeletionTimestamp.IsZero() { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + case consumerGroupRef != nil && serviceRef != nil: + combinationFound := false + for _, rel := range combinations { + if rel.ConsumerGroup != consumerGroupRef.Name || + rel.Service != serviceRef.Name || + rel.Consumer != "" || + rel.Route != "" { + continue + } + combinationFound = true + break + } + + if !combinationFound { + pluginBindingsToDelete[client.ObjectKeyFromObject(&pb)] = pb + continue + } + + s, serviceExists := getIfRefNotNil[*configurationv1alpha1.KongService](ctx, clientWithNamespace, serviceRef) + cg, consumerGroupExists := getIfRefNotNil[*configurationv1beta1.KongConsumerGroup](ctx, clientWithNamespace, consumerGroupRef) + if !consumerGroupExists || !serviceExists || + !objHasPluginConfigured(cg, kongPlugin.Name) || !cg.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 || rel.Consumer != "" || + rel.ConsumerGroup != "" || rel.Route != routeRef.Name { continue } @@ -474,6 +594,7 @@ func deleteUnusedKongPluginBindings( for _, rel := range combinations { if rel.Service != serviceRef.Name || rel.Consumer != "" || + rel.ConsumerGroup != "" || rel.Route != "" { continue } @@ -498,6 +619,7 @@ func deleteUnusedKongPluginBindings( for _, rel := range combinations { if rel.Route != routeRef.Name || rel.Consumer != "" || + rel.ConsumerGroup != "" || rel.Service != "" { continue } diff --git a/controller/konnect/reconciler_kongplugin_combinations.go b/controller/konnect/reconciler_kongplugin_combinations.go index 919b46a59..588ceda59 100644 --- a/controller/konnect/reconciler_kongplugin_combinations.go +++ b/controller/konnect/reconciler_kongplugin_combinations.go @@ -8,12 +8,13 @@ import ( configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" + configurationv1beta1 "github.com/kong/kubernetes-configuration/api/configuration/v1beta1" ) // ForeignRelations contains all the relations between Kong entities and KongPlugin. type ForeignRelations struct { Consumer []configurationv1.KongConsumer - ConsumerGroup []string + ConsumerGroup []configurationv1beta1.KongConsumerGroup Route []configurationv1alpha1.KongRoute Service []configurationv1alpha1.KongService } @@ -55,6 +56,20 @@ func (relations *ForeignRelations) GroupByControlPlane( for _, route := range relations.Route { svcRef := route.Spec.ServiceRef if svcRef == nil || svcRef.NamespacedRef == nil { + + cpRef, ok := controlPlaneRefIsKonnectNamespacedRef(&route) + if !ok { + continue + } + + nn := types.NamespacedName{ + Namespace: route.Namespace, + Name: cpRef.KonnectNamespacedRef.Name, + } + fr := ret[nn] + fr.Route = append(fr.Route, route) + ret[nn] = fr + continue } @@ -97,8 +112,20 @@ func (relations *ForeignRelations) GroupByControlPlane( fr.Consumer = append(fr.Consumer, consumer) ret[nn] = fr } - - // TODO consumers and consumer groups + for _, group := range relations.ConsumerGroup { + cpRef, ok := controlPlaneRefIsKonnectNamespacedRef(&group) + if !ok { + continue + } + nn := types.NamespacedName{ + // TODO: implement cross namespace references + Namespace: group.Namespace, + Name: cpRef.KonnectNamespacedRef.Name, + } + fr := ret[nn] + fr.ConsumerGroup = append(fr.ConsumerGroup, group) + ret[nn] = fr + } return ret, nil } @@ -161,13 +188,13 @@ func (relations *ForeignRelations) GetCombinations() []Rel { for _, route := range relations.Route { cartesianProduct = append(cartesianProduct, Rel{ Route: route.Name, - ConsumerGroup: group, + ConsumerGroup: group.Name, }) } for _, service := range relations.Service { cartesianProduct = append(cartesianProduct, Rel{ Service: service.Name, - ConsumerGroup: group, + ConsumerGroup: group.Name, }) } } @@ -176,7 +203,7 @@ func (relations *ForeignRelations) GetCombinations() []Rel { cartesianProduct = make([]Rel, 0, lConsumerGroup) for _, group := range relations.ConsumerGroup { cartesianProduct = append(cartesianProduct, Rel{ - ConsumerGroup: group, + ConsumerGroup: group.Name, }) } } diff --git a/controller/konnect/reconciler_kongplugin_combinations_test.go b/controller/konnect/reconciler_kongplugin_combinations_test.go index 5f7f68ade..c70be2039 100644 --- a/controller/konnect/reconciler_kongplugin_combinations_test.go +++ b/controller/konnect/reconciler_kongplugin_combinations_test.go @@ -1,13 +1,20 @@ package konnect import ( + "context" "testing" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/kong/gateway-operator/modules/manager/scheme" configurationv1 "github.com/kong/kubernetes-configuration/api/configuration/v1" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" + configurationv1beta1 "github.com/kong/kubernetes-configuration/api/configuration/v1beta1" ) func TestGetCombinations(t *testing.T) { @@ -57,7 +64,18 @@ func TestGetCombinations(t *testing.T) { name: "plugins on consumer group only", args: args{ relations: ForeignRelations{ - ConsumerGroup: []string{"foo", "bar"}, + ConsumerGroup: []configurationv1beta1.KongConsumerGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, }, }, want: []Rel{ @@ -240,7 +258,18 @@ func TestGetCombinations(t *testing.T) { }, }, }, - ConsumerGroup: []string{"cg1", "cg2"}, + ConsumerGroup: []configurationv1beta1.KongConsumerGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg2", + }, + }, + }, }, }, want: []Rel{ @@ -261,6 +290,75 @@ func TestGetCombinations(t *testing.T) { // }, }, }, + { + name: "plugins on combination of service, route and consumer groups", + args: args{ + relations: ForeignRelations{ + Route: []configurationv1alpha1.KongRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "r1", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "s1", + }, + }, + }, + }, + }, + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + }, + }, + }, + ConsumerGroup: []configurationv1beta1.KongConsumerGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg2", + }, + }, + }, + }, + }, + want: []Rel{ + { + ConsumerGroup: "cg1", + Route: "r1", + }, + { + ConsumerGroup: "cg1", + Service: "s1", + }, + { + ConsumerGroup: "cg2", + Route: "r1", + }, + { + ConsumerGroup: "cg2", + Service: "s1", + }, + // NOTE: https://github.com/Kong/gateway-operator/issues/660 + // is related to the following combination not being present. + // Currently we do not generate combination for Service and Route + // on their own. + // { + // Service: "s1", + // }, + // { + // Route: "r1", + // }, + }, + }, { name: "plugins on combination of service, route and consumer", args: args{ @@ -373,3 +471,342 @@ func TestGetCombinations(t *testing.T) { }) } } + +func TestGroupByControlPlane(t *testing.T) { + type args struct { + relations ForeignRelations + } + tests := []struct { + name string + args args + objects []client.Object + want ForeignRelationsGroupedByControlPlane + wantErr bool + }{ + { + name: "empty", + args: args{ + relations: ForeignRelations{}, + }, + want: ForeignRelationsGroupedByControlPlane{}, + }, + { + name: "single service with control plane ref", + args: args{ + relations: ForeignRelations{ + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + want: ForeignRelationsGroupedByControlPlane{ + types.NamespacedName{Namespace: "default", Name: "cp1"}: { + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "multiple services with different control plane refs", + args: args{ + relations: ForeignRelations{ + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s2", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp2", + }, + }, + }, + }, + }, + }, + }, + want: ForeignRelationsGroupedByControlPlane{ + types.NamespacedName{Namespace: "default", Name: "cp1"}: { + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + types.NamespacedName{Namespace: "default", Name: "cp2"}: { + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s2", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp2", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "service and route with same control plane ref", + objects: []client.Object{ + &configurationv1alpha1.KongService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + args: args{ + relations: ForeignRelations{ + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + Route: []configurationv1alpha1.KongRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "r1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "s1", + }, + }, + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + want: ForeignRelationsGroupedByControlPlane{ + types.NamespacedName{Namespace: "default", Name: "cp1"}: { + Service: []configurationv1alpha1.KongService{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + Route: []configurationv1alpha1.KongRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "r1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "s1", + }, + }, + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "consumer with control plane ref", + args: args{ + relations: ForeignRelations{ + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + Namespace: "default", + }, + Spec: configurationv1.KongConsumerSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + want: ForeignRelationsGroupedByControlPlane{ + types.NamespacedName{Namespace: "default", Name: "cp1"}: { + Consumer: []configurationv1.KongConsumer{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + Namespace: "default", + }, + Spec: configurationv1.KongConsumerSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "consumer group with control plane ref", + args: args{ + relations: ForeignRelations{ + ConsumerGroup: []configurationv1beta1.KongConsumerGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg1", + Namespace: "default", + }, + Spec: configurationv1beta1.KongConsumerGroupSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + want: ForeignRelationsGroupedByControlPlane{ + types.NamespacedName{Namespace: "default", Name: "cp1"}: { + ConsumerGroup: []configurationv1beta1.KongConsumerGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cg1", + Namespace: "default", + }, + Spec: configurationv1beta1.KongConsumerGroupSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp1", + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(scheme.Get()). + WithObjects(tt.objects...). + Build() + got, err := tt.args.relations.GroupByControlPlane(context.Background(), cl) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/modules/manager/controller_setup.go b/modules/manager/controller_setup.go index 8905beb99..7fdbe4e8a 100644 --- a/modules/manager/controller_setup.go +++ b/modules/manager/controller_setup.go @@ -596,6 +596,10 @@ func SetupCacheIndicesForKonnectTypes(ctx context.Context, mgr manager.Manager, Object: &configurationv1.KongConsumer{}, IndexOptions: konnect.IndexOptionsForKongConsumer(), }, + { + Object: &configurationv1beta1.KongConsumerGroup{}, + IndexOptions: konnect.IndexOptionsForKongConsumerGroup(), + }, { Object: &configurationv1alpha1.KongService{}, IndexOptions: konnect.IndexOptionsForKongService(), diff --git a/test/envtest/kongpluginbinding_managed_test.go b/test/envtest/kongpluginbinding_managed_test.go index e98e4d7b5..3aeaaa732 100644 --- a/test/envtest/kongpluginbinding_managed_test.go +++ b/test/envtest/kongpluginbinding_managed_test.go @@ -636,4 +636,209 @@ func TestKongPluginBindingManaged(t *testing.T) { assert.True(c, sdk.PluginSDK.AssertExpectations(t)) }, waitTime, tickTime) }) + + t.Run("binding to KongConsumerGroup, KongService and KongRoute", func(t *testing.T) { + serviceID := uuid.NewString() + routeID := uuid.NewString() + consumerGroupID := 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) + kongConsumerGroup := deploy.KongConsumerGroupAttachedToCP(t, ctx, clientNamespaced, cp, + deploy.WithAnnotation(consts.PluginsAnnotationKey, rateLimitingkongPlugin.Name), + ) + t.Cleanup(func() { + require.NoError(t, clientNamespaced.Delete(ctx, kongConsumerGroup)) + }) + updateKongConsumerGroupStatusWithKonnectID(t, ctx, clientNamespaced, kongConsumerGroup, consumerGroupID, 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.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.Name && + targets.ServiceReference == nil { + kpbRouteFound = true + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name && + targets.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.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.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.Name && + targets.ServiceReference == nil { + kpbRoute = kpb + } else if targets.RouteReference == nil && + targets.ServiceReference != nil && + targets.ServiceReference.Name == kongService.Name && + targets.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.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 KongConsumerGroup 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.IndexFieldKongPluginBindingKongConsumerGroupReference: kongConsumerGroup.Name, + }, + )) { + return + } + assert.Len(t, l.Items, 1) + }, waitTime, tickTime, + "KongPluginBinding bound to ConsumerGroup 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 KongConsumerGroup 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.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.Name + }, + "KongConsumerGroup bound KongPluginBinding wasn't created", + ) + + t.Logf( + "remove annotation from KongConsumerGroup %s and check that no KongPluginBinding exists that binds to it", + client.ObjectKeyFromObject(kongConsumerGroup), + ) + + sdk.PluginSDK.EXPECT(). + DeletePlugin(mock.Anything, cp.GetKonnectStatus().GetKonnectID(), mock.Anything). + Return( + &sdkkonnectops.DeletePluginResponse{ + StatusCode: 200, + }, + nil, + ) + + delete(kongConsumerGroup.Annotations, consts.PluginsAnnotationKey) + require.NoError(t, clientNamespaced.Update(ctx, kongConsumerGroup)) + + 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.ConsumerGroupReference != nil && + targets.ConsumerGroupReference.Name == kongConsumerGroup.Name + }, + "KongConsumerGroup bound KongPluginBinding wasn't deleted", + ) + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, sdk.PluginSDK.AssertExpectations(t)) + }, waitTime, tickTime) + }) }