From 9c6420bd8e8fdf7699bb37438eb4e74015b065d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Mon, 30 Sep 2024 16:33:29 +0200 Subject: [PATCH] feat(konnect): add support for serviceless `KongRoute`s (#669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(konnect): allow serviceless KongRoutes * tests: add serviceref tests * Update controller/konnect/ops/ops_kongroute.go Co-authored-by: Grzegorz Burzyński * chore: fix build --------- Co-authored-by: Grzegorz Burzyński --- CHANGELOG.md | 4 +- .../konnect_kongroute_controlplane_ref.yaml | 39 +++ controller/konnect/ops/ops_kongroute.go | 9 +- controller/konnect/reconciler_generic.go | 197 +---------- controller/konnect/reconciler_serviceref.go | 190 +++++++++++ .../konnect/reconciler_serviceref_test.go | 321 ++++++++++++++++++ controller/konnect/watch.go | 13 +- controller/konnect/watch_credentialacl.go | 3 +- controller/konnect/watch_credentialapikey.go | 3 +- .../konnect/watch_credentialbasicauth.go | 3 +- controller/konnect/watch_kongkeyset.go | 18 +- controller/konnect/watch_kongroute.go | 10 +- controller/konnect/watch_kongtarget.go | 3 +- 13 files changed, 596 insertions(+), 217 deletions(-) create mode 100644 config/samples/konnect_kongroute_controlplane_ref.yaml create mode 100644 controller/konnect/reconciler_serviceref.go create mode 100644 controller/konnect/reconciler_serviceref_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d3ced2a9f..dc54bf531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,8 +82,6 @@ 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` [#652](https://github.com/Kong/gateway-operator/pull/652) - - `KongConsumerGroup` [#654](https://github.com/Kong/gateway-operator/pull/654) 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 @@ -93,6 +91,8 @@ - basic-auth [#625](https://github.com/Kong/gateway-operator/pull/625) - API key [#635](https://github.com/Kong/gateway-operator/pull/635) - ACL [#661](https://github.com/Kong/gateway-operator/pull/661) +- Add support for `KongRoute`s bound directly to `KonnectGatewayControlPlane`s (serviceless rotues). + [#669](https://github.com/Kong/gateway-operator/pull/669) ### Fixed diff --git a/config/samples/konnect_kongroute_controlplane_ref.yaml b/config/samples/konnect_kongroute_controlplane_ref.yaml new file mode 100644 index 000000000..82c9b9cd6 --- /dev/null +++ b/config/samples/konnect_kongroute_controlplane_ref.yaml @@ -0,0 +1,39 @@ +kind: KonnectAPIAuthConfiguration +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: konnect-api-auth-dev-1 + namespace: default +spec: + type: token + token: kpat_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + serverURL: us.api.konghq.com +--- +kind: KonnectGatewayControlPlane +apiVersion: konnect.konghq.com/v1alpha1 +metadata: + name: test1 + namespace: default +spec: + name: test1 + labels: + app: test1 + key1: test1 + konnect: + authRef: + name: konnect-api-auth-dev-1 +--- +kind: KongRoute +apiVersion: configuration.konghq.com/v1alpha1 +metadata: + name: service-1 + namespace: default +spec: + name: route-1 + protocols: + - http + hosts: + - example.com + controlPlaneRef: + type: konnectNamespacedRef + konnectNamespacedRef: + name: test1 diff --git a/controller/konnect/ops/ops_kongroute.go b/controller/konnect/ops/ops_kongroute.go index bd8b0fef9..dd20a652c 100644 --- a/controller/konnect/ops/ops_kongroute.go +++ b/controller/konnect/ops/ops_kongroute.go @@ -122,7 +122,7 @@ func kongRouteToSDKRouteInput( // Deduplicate tags to avoid rejection by Konnect. tags := lo.Uniq(slices.Concat(specTags, annotationTags, k8sTags)) - return sdkkonnectcomp.RouteInput{ + r := sdkkonnectcomp.RouteInput{ Destinations: route.Spec.KongRouteAPISpec.Destinations, Headers: route.Spec.KongRouteAPISpec.Headers, Hosts: route.Spec.KongRouteAPISpec.Hosts, @@ -140,8 +140,11 @@ func kongRouteToSDKRouteInput( Sources: route.Spec.KongRouteAPISpec.Sources, StripPath: route.Spec.KongRouteAPISpec.StripPath, Tags: tags, - Service: &sdkkonnectcomp.RouteService{ + } + if route.Status.Konnect != nil && route.Status.Konnect.ServiceID != "" { + r.Service = &sdkkonnectcomp.RouteService{ ID: sdkkonnectgo.String(route.Status.Konnect.ServiceID), - }, + } } + return r } diff --git a/controller/konnect/reconciler_generic.go b/controller/konnect/reconciler_generic.go index 02a172ebe..bee185b8c 100644 --- a/controller/konnect/reconciler_generic.go +++ b/controller/konnect/reconciler_generic.go @@ -667,175 +667,6 @@ func getConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constraints.E } } -func getServiceRef[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]]( - e TEnt, -) mo.Option[configurationv1alpha1.ServiceRef] { - switch e := any(e).(type) { - case *configurationv1alpha1.KongRoute: - if e.Spec.ServiceRef == nil { - return mo.None[configurationv1alpha1.ServiceRef]() - } - return mo.Some(*e.Spec.ServiceRef) - default: - return mo.None[configurationv1alpha1.ServiceRef]() - } -} - -// handleKongServiceRef handles the ServiceRef for the given entity. -// It sets the owner reference to the referenced KongService and updates the -// status of the entity based on the referenced KongService status. -func handleKongServiceRef[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]]( - ctx context.Context, - cl client.Client, - ent TEnt, -) (ctrl.Result, error) { - kongServiceRef, ok := getServiceRef(ent).Get() - if !ok { - return ctrl.Result{}, nil - } - switch kongServiceRef.Type { - case configurationv1alpha1.ServiceRefNamespacedRef: - svc := configurationv1alpha1.KongService{} - nn := types.NamespacedName{ - Name: kongServiceRef.NamespacedRef.Name, - // TODO: handle cross namespace refs - Namespace: ent.GetNamespace(), - } - - if err := cl.Get(ctx, nn, &svc); err != nil { - if res, errStatus := updateStatusWithCondition( - ctx, cl, ent, - conditions.KongServiceRefValidConditionType, - metav1.ConditionFalse, - conditions.KongServiceRefReasonInvalid, - err.Error(), - ); errStatus != nil || !res.IsZero() { - return res, errStatus - } - - return ctrl.Result{}, fmt.Errorf("can't get the referenced KongService %s: %w", nn, err) - } - - // If referenced KongService is being deleted, return an error so that we - // can remove the entity from Konnect first. - if delTimestamp := svc.GetDeletionTimestamp(); !delTimestamp.IsZero() { - return ctrl.Result{}, ReferencedKongServiceIsBeingDeleted{ - Reference: nn, - } - } - - cond, ok := k8sutils.GetCondition(conditions.KonnectEntityProgrammedConditionType, &svc) - if !ok || cond.Status != metav1.ConditionTrue { - ent.SetKonnectID("") - if res, err := updateStatusWithCondition( - ctx, cl, ent, - conditions.KongServiceRefValidConditionType, - metav1.ConditionFalse, - conditions.KongServiceRefReasonInvalid, - fmt.Sprintf("Referenced KongService %s is not programmed yet", nn), - ); err != nil || !res.IsZero() { - return ctrl.Result{}, err - } - return ctrl.Result{Requeue: true}, nil - } - - old := ent.DeepCopyObject().(TEnt) - if err := controllerutil.SetOwnerReference(&svc, ent, cl.Scheme(), controllerutil.WithBlockOwnerDeletion(true)); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set owner reference: %w", err) - } - if err := cl.Patch(ctx, ent, client.MergeFrom(old)); err != nil { - if k8serrors.IsConflict(err) { - return ctrl.Result{Requeue: true}, nil - } - return ctrl.Result{}, fmt.Errorf("failed to update status: %w", err) - } - - // TODO(pmalek): make this generic. - // Service ID is not stored in KonnectEntityStatus because not all entities - // have a ServiceRef, hence the type constraints in the reconciler can't be used. - if route, ok := any(ent).(*configurationv1alpha1.KongRoute); ok { - if route.Status.Konnect == nil { - route.Status.Konnect = &konnectv1alpha1.KonnectEntityStatusWithControlPlaneAndServiceRefs{} - } - route.Status.Konnect.ServiceID = svc.Status.Konnect.GetKonnectID() - } - - if res, errStatus := updateStatusWithCondition( - ctx, cl, ent, - conditions.KongServiceRefValidConditionType, - metav1.ConditionTrue, - conditions.KongServiceRefReasonValid, - fmt.Sprintf("Referenced KongService %s programmed", nn), - ); errStatus != nil || !res.IsZero() { - return res, errStatus - } - - cpRef, ok := getControlPlaneRef(&svc).Get() - if !ok { - return ctrl.Result{}, fmt.Errorf( - "KongRoute references a KongService %s which does not have a ControlPlane ref", - client.ObjectKeyFromObject(&svc), - ) - } - cp, err := getCPForRef(ctx, cl, cpRef, ent.GetNamespace()) - if err != nil { - if res, errStatus := updateStatusWithCondition( - ctx, cl, ent, - conditions.ControlPlaneRefValidConditionType, - metav1.ConditionFalse, - conditions.ControlPlaneRefReasonInvalid, - err.Error(), - ); errStatus != nil || !res.IsZero() { - return res, errStatus - } - if k8serrors.IsNotFound(err) { - return ctrl.Result{}, ReferencedControlPlaneDoesNotExistError{ - Reference: nn, - Err: err, - } - } - return ctrl.Result{}, err - } - - cond, ok = k8sutils.GetCondition(conditions.KonnectEntityProgrammedConditionType, cp) - if !ok || cond.Status != metav1.ConditionTrue || cond.ObservedGeneration != cp.GetGeneration() { - if res, errStatus := updateStatusWithCondition( - ctx, cl, ent, - conditions.ControlPlaneRefValidConditionType, - metav1.ConditionFalse, - conditions.ControlPlaneRefReasonInvalid, - fmt.Sprintf("Referenced ControlPlane %s is not programmed yet", nn), - ); errStatus != nil || !res.IsZero() { - return res, errStatus - } - - return ctrl.Result{Requeue: true}, nil - } - - // TODO(pmalek): make this generic. - // CP ID is not stored in KonnectEntityStatus because not all entities - // have a ControlPlaneRef, hence the type constraints in the reconciler can't be used. - if resource, ok := any(ent).(EntityWithControlPlaneRef); ok { - resource.SetControlPlaneID(cp.Status.ID) - } - - if res, errStatus := updateStatusWithCondition( - ctx, cl, ent, - conditions.ControlPlaneRefValidConditionType, - metav1.ConditionTrue, - conditions.ControlPlaneRefReasonValid, - fmt.Sprintf("Referenced ControlPlane %s is programmed", nn), - ); errStatus != nil || !res.IsZero() { - return res, errStatus - } - - default: - return ctrl.Result{}, fmt.Errorf("unimplemented KongService ref type %q", kongServiceRef.Type) - } - - return ctrl.Result{}, nil -} - // handleKongConsumerRef handles the ConsumerRef for the given entity. // It sets the owner reference to the referenced KongConsumer and updates the // status of the entity based on the referenced KongConsumer status. @@ -1000,59 +831,65 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr func getControlPlaneRef[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]]( e TEnt, ) mo.Option[configurationv1alpha1.ControlPlaneRef] { + none := mo.None[configurationv1alpha1.ControlPlaneRef]() switch e := any(e).(type) { case *configurationv1.KongConsumer: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1beta1.KongConsumerGroup: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none + } + return mo.Some(*e.Spec.ControlPlaneRef) + case *configurationv1alpha1.KongRoute: + if e.Spec.ControlPlaneRef == nil { + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongService: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongPluginBinding: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongUpstream: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongCACertificate: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongCertificate: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongVault: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongKey: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) case *configurationv1alpha1.KongKeySet: if e.Spec.ControlPlaneRef == nil { - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } return mo.Some(*e.Spec.ControlPlaneRef) default: - return mo.None[configurationv1alpha1.ControlPlaneRef]() + return none } } diff --git a/controller/konnect/reconciler_serviceref.go b/controller/konnect/reconciler_serviceref.go new file mode 100644 index 000000000..8487d70af --- /dev/null +++ b/controller/konnect/reconciler_serviceref.go @@ -0,0 +1,190 @@ +package konnect + +import ( + "context" + "fmt" + + "github.com/samber/mo" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/kong/gateway-operator/controller/konnect/conditions" + "github.com/kong/gateway-operator/controller/konnect/constraints" + k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" + konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1" +) + +func getServiceRef[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]]( + e TEnt, +) mo.Option[configurationv1alpha1.ServiceRef] { + switch e := any(e).(type) { + case *configurationv1alpha1.KongRoute: + if e.Spec.ServiceRef == nil { + return mo.None[configurationv1alpha1.ServiceRef]() + } + return mo.Some(*e.Spec.ServiceRef) + default: + return mo.None[configurationv1alpha1.ServiceRef]() + } +} + +// handleKongServiceRef handles the ServiceRef for the given entity. +// It sets the owner reference to the referenced KongService and updates the +// status of the entity based on the referenced KongService status. +func handleKongServiceRef[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]]( + ctx context.Context, + cl client.Client, + ent TEnt, +) (ctrl.Result, error) { + kongServiceRef, ok := getServiceRef(ent).Get() + if !ok { + return ctrl.Result{}, nil + } + switch kongServiceRef.Type { + case configurationv1alpha1.ServiceRefNamespacedRef: + svc := configurationv1alpha1.KongService{} + nn := types.NamespacedName{ + Name: kongServiceRef.NamespacedRef.Name, + // TODO: handle cross namespace refs + Namespace: ent.GetNamespace(), + } + + if err := cl.Get(ctx, nn, &svc); err != nil { + if res, errStatus := updateStatusWithCondition( + ctx, cl, ent, + conditions.KongServiceRefValidConditionType, + metav1.ConditionFalse, + conditions.KongServiceRefReasonInvalid, + err.Error(), + ); errStatus != nil || !res.IsZero() { + return res, errStatus + } + + return ctrl.Result{}, fmt.Errorf("can't get the referenced KongService %s: %w", nn, err) + } + + // If referenced KongService is being deleted, return an error so that we + // can remove the entity from Konnect first. + if delTimestamp := svc.GetDeletionTimestamp(); !delTimestamp.IsZero() { + return ctrl.Result{}, ReferencedKongServiceIsBeingDeleted{ + Reference: nn, + } + } + + cond, ok := k8sutils.GetCondition(conditions.KonnectEntityProgrammedConditionType, &svc) + if !ok || cond.Status != metav1.ConditionTrue { + ent.SetKonnectID("") + if res, err := updateStatusWithCondition( + ctx, cl, ent, + conditions.KongServiceRefValidConditionType, + metav1.ConditionFalse, + conditions.KongServiceRefReasonInvalid, + fmt.Sprintf("Referenced KongService %s is not programmed yet", nn), + ); err != nil || !res.IsZero() { + return ctrl.Result{}, err + } + return ctrl.Result{Requeue: true}, nil + } + + old := ent.DeepCopyObject().(TEnt) + if err := controllerutil.SetOwnerReference(&svc, ent, cl.Scheme(), controllerutil.WithBlockOwnerDeletion(true)); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to set owner reference: %w", err) + } + if err := cl.Patch(ctx, ent, client.MergeFrom(old)); err != nil { + if k8serrors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to update status: %w", err) + } + + // TODO(pmalek): make this generic. + // Service ID is not stored in KonnectEntityStatus because not all entities + // have a ServiceRef, hence the type constraints in the reconciler can't be used. + if route, ok := any(ent).(*configurationv1alpha1.KongRoute); ok { + if route.Status.Konnect == nil { + route.Status.Konnect = &konnectv1alpha1.KonnectEntityStatusWithControlPlaneAndServiceRefs{} + } + route.Status.Konnect.ServiceID = svc.Status.Konnect.GetKonnectID() + } + + if res, errStatus := updateStatusWithCondition( + ctx, cl, ent, + conditions.KongServiceRefValidConditionType, + metav1.ConditionTrue, + conditions.KongServiceRefReasonValid, + fmt.Sprintf("Referenced KongService %s programmed", nn), + ); errStatus != nil || !res.IsZero() { + return res, errStatus + } + + cpRef, ok := getControlPlaneRef(&svc).Get() + if !ok { + return ctrl.Result{}, fmt.Errorf( + "KongRoute references a KongService %s which does not have a ControlPlane ref", + client.ObjectKeyFromObject(&svc), + ) + } + cp, err := getCPForRef(ctx, cl, cpRef, ent.GetNamespace()) + if err != nil { + if res, errStatus := updateStatusWithCondition( + ctx, cl, ent, + conditions.ControlPlaneRefValidConditionType, + metav1.ConditionFalse, + conditions.ControlPlaneRefReasonInvalid, + err.Error(), + ); errStatus != nil || !res.IsZero() { + return res, errStatus + } + if k8serrors.IsNotFound(err) { + return ctrl.Result{}, ReferencedControlPlaneDoesNotExistError{ + Reference: nn, + Err: err, + } + } + return ctrl.Result{}, err + } + + cond, ok = k8sutils.GetCondition(conditions.KonnectEntityProgrammedConditionType, cp) + if !ok || cond.Status != metav1.ConditionTrue || cond.ObservedGeneration != cp.GetGeneration() { + if res, errStatus := updateStatusWithCondition( + ctx, cl, ent, + conditions.ControlPlaneRefValidConditionType, + metav1.ConditionFalse, + conditions.ControlPlaneRefReasonInvalid, + fmt.Sprintf("Referenced ControlPlane %s is not programmed yet", nn), + ); errStatus != nil || !res.IsZero() { + return res, errStatus + } + + return ctrl.Result{Requeue: true}, nil + } + + // TODO(pmalek): make this generic. + // CP ID is not stored in KonnectEntityStatus because not all entities + // have a ControlPlaneRef, hence the type constraints in the reconciler can't be used. + if resource, ok := any(ent).(EntityWithControlPlaneRef); ok { + resource.SetControlPlaneID(cp.Status.ID) + } + + if res, errStatus := updateStatusWithCondition( + ctx, cl, ent, + conditions.ControlPlaneRefValidConditionType, + metav1.ConditionTrue, + conditions.ControlPlaneRefReasonValid, + fmt.Sprintf("Referenced ControlPlane %s is programmed", client.ObjectKeyFromObject(cp)), + ); errStatus != nil || !res.IsZero() { + return res, errStatus + } + + default: + return ctrl.Result{}, fmt.Errorf("unimplemented KongService ref type %q", kongServiceRef.Type) + } + + return ctrl.Result{}, nil +} diff --git a/controller/konnect/reconciler_serviceref_test.go b/controller/konnect/reconciler_serviceref_test.go new file mode 100644 index 000000000..2c4fead1f --- /dev/null +++ b/controller/konnect/reconciler_serviceref_test.go @@ -0,0 +1,321 @@ +package konnect + +import ( + "context" + "testing" + "time" + + "github.com/samber/lo" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/kong/gateway-operator/controller/konnect/conditions" + "github.com/kong/gateway-operator/controller/konnect/constraints" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" + konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1" +) + +type handleServiceRefTestCase[T constraints.SupportedKonnectEntityType, TEnt constraints.EntityType[T]] struct { + name string + ent TEnt + objects []client.Object + expectResult ctrl.Result + expectError bool + expectErrorContains string + // Returns true if the updated entity satisfy the assertion. + // Returns false and error message if entity fails to satisfy it. + updatedEntAssertions []func(TEnt) (ok bool, message string) +} + +var testKongServiceOK = &configurationv1alpha1.KongService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-ok", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp-ok", + }, + }, + }, + Status: configurationv1alpha1.KongServiceStatus{ + Konnect: &konnectv1alpha1.KonnectEntityStatusWithControlPlaneRef{ + KonnectEntityStatus: konnectv1alpha1.KonnectEntityStatus{ + ID: "12345", + }, + ControlPlaneID: "123456789", + }, + Conditions: []metav1.Condition{ + { + Type: conditions.KonnectEntityProgrammedConditionType, + Status: metav1.ConditionTrue, + }, + }, + }, +} + +var testKongServiceWithCPRefUnprogrammed = &configurationv1alpha1.KongService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-with-cp-ref-unprogrammed", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongServiceSpec{ + ControlPlaneRef: &configurationv1alpha1.ControlPlaneRef{ + Type: configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef, + KonnectNamespacedRef: &configurationv1alpha1.KonnectNamespacedRef{ + Name: "cp-not-programmed", + }, + }, + }, + Status: configurationv1alpha1.KongServiceStatus{ + Konnect: &konnectv1alpha1.KonnectEntityStatusWithControlPlaneRef{ + KonnectEntityStatus: konnectv1alpha1.KonnectEntityStatus{ + ID: "12345", + }, + ControlPlaneID: "123456789", + }, + Conditions: []metav1.Condition{ + { + Type: conditions.KonnectEntityProgrammedConditionType, + Status: metav1.ConditionTrue, + }, + }, + }, +} + +var testKongServiceNotProgrammed = &configurationv1alpha1.KongService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-not-programmed", + Namespace: "default", + }, + Status: configurationv1alpha1.KongServiceStatus{ + Conditions: []metav1.Condition{ + { + Type: conditions.KonnectEntityProgrammedConditionType, + Status: metav1.ConditionFalse, + }, + }, + }, +} + +var testKongServiceBeingDeleted = &configurationv1alpha1.KongService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-being-deleted", + Namespace: "default", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{KonnectCleanupFinalizer}, + }, +} + +func TestHandleServiceRef(t *testing.T) { + testCases := []handleServiceRefTestCase[configurationv1alpha1.KongRoute, *configurationv1alpha1.KongRoute]{ + { + name: "has service ref", + ent: &configurationv1alpha1.KongRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "svc-ok", + }, + }, + }, + }, + objects: []client.Object{ + testKongServiceOK, + testControlPlaneOK, + }, + expectResult: ctrl.Result{}, + expectError: false, + updatedEntAssertions: []func(*configurationv1alpha1.KongRoute) (bool, string){ + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.ControlPlaneRefValidConditionType && c.Status == metav1.ConditionTrue + }), "KongRoute does not have ControlPlaneRefValid condition set to True" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.KongServiceRefValidConditionType && c.Status == metav1.ConditionTrue + }), "KongRoute does not have KongServiceRefValid condition set to True" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.OwnerReferences, func(o metav1.OwnerReference) bool { + return o.Kind == "KongService" && o.Name == "svc-ok" + }), "OwnerReference of KongRoute is not set" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return ks.Status.Konnect.ServiceID == "12345", + "KongRoute does not have Konnect Service ID set" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return ks.Status.Konnect.ControlPlaneID == "123456789", + "KongRoute does not have Konnect ControlPlane ID set (should be inherited from ControlPlane)" + }, + }, + }, + { + name: "with service ref to a service that is being deleted", + ent: &configurationv1alpha1.KongRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-with-service-ref-being-deleted", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "svc-ok", + }, + }, + }, + }, + objects: []client.Object{ + testKongServiceBeingDeleted, + testControlPlaneOK, + }, + expectResult: ctrl.Result{}, + expectError: true, + }, + { + name: "has service ref to an unprogrammed service", + ent: &configurationv1alpha1.KongRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "svc-not-programmed", + }, + }, + }, + }, + objects: []client.Object{ + testKongServiceNotProgrammed, + testControlPlaneOK, + }, + expectResult: ctrl.Result{ + Requeue: true, + }, + expectError: false, + updatedEntAssertions: []func(*configurationv1alpha1.KongRoute) (bool, string){ + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.KongServiceRefValidConditionType && + c.Status == metav1.ConditionFalse && + c.Reason == conditions.KongServiceRefReasonInvalid + }), "KongRoute does not have KongServiceRefValid condition set to False" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.NoneBy(ks.OwnerReferences, func(o metav1.OwnerReference) bool { + return o.Kind == "KongService" && o.Name == "svc-ok" + }), "OwnerReference of KongRoute is set but it shouldn't be" + }, + }, + }, + { + name: "has service ref which has an unprogrammed cp", + ent: &configurationv1alpha1.KongRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-1", + Namespace: "default", + }, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + Name: "svc-with-cp-ref-unprogrammed", + }, + }, + }, + }, + objects: []client.Object{ + testKongServiceWithCPRefUnprogrammed, + testControlPlaneNotProgrammed, + }, + expectResult: ctrl.Result{ + Requeue: true, + }, + expectError: false, + updatedEntAssertions: []func(*configurationv1alpha1.KongRoute) (bool, string){ + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.ControlPlaneRefValidConditionType && + c.Status == metav1.ConditionFalse && + c.Reason == conditions.ControlPlaneRefReasonInvalid + }), "KongRoute does not have ControlPlaneRef condition set to False" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.ContainsBy(ks.Status.Conditions, func(c metav1.Condition) bool { + return c.Type == conditions.KongServiceRefValidConditionType && + c.Status == metav1.ConditionTrue && + c.Reason == conditions.KongServiceRefReasonValid + }), "KongRoute does not have KongServiceRefValid condition set to True" + }, + func(ks *configurationv1alpha1.KongRoute) (bool, string) { + return lo.NoneBy(ks.OwnerReferences, func(o metav1.OwnerReference) bool { + return o.Kind == "KongService" && o.Name == "svc-ok" + }), "OwnerReference of KongRoute is set but it shouldn't be" + }, + }, + }, + } + + testHandleServiceRef(t, testCases) +} + +func testHandleServiceRef[ + T constraints.SupportedKonnectEntityType, + TEnt constraints.EntityType[T], +]( + t *testing.T, testCases []handleServiceRefTestCase[T, TEnt], +) { + t.Helper() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, configurationv1alpha1.AddToScheme(scheme)) + require.NoError(t, konnectv1alpha1.AddToScheme(scheme)) + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tc.ent). + WithObjects(tc.objects...). + // WithStatusSubresource is required for updating status of handled entity. + WithStatusSubresource(tc.ent). + Build() + require.NoError(t, fakeClient.SubResource("status").Update(context.Background(), tc.ent)) + + res, err := handleKongServiceRef(context.Background(), fakeClient, tc.ent) + + var updatedEnt TEnt = tc.ent.DeepCopyObject().(TEnt) + require.NoError(t, fakeClient.Get(context.Background(), client.ObjectKeyFromObject(tc.ent), updatedEnt)) + for _, assertion := range tc.updatedEntAssertions { + ok, msg := assertion(updatedEnt) + require.True(t, ok, msg) + } + + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErrorContains) + return + } + + require.NoError(t, err) + require.Equal(t, tc.expectResult, res) + }) + } +} diff --git a/controller/konnect/watch.go b/controller/konnect/watch.go index 4ffdc5274..e8bfcfd56 100644 --- a/controller/konnect/watch.go +++ b/controller/konnect/watch.go @@ -81,9 +81,14 @@ func objRefersToKonnectGatewayControlPlane[ return false } + return objHasControlPlaneRefKonnectNamespacedRef(ent) +} + +func objHasControlPlaneRefKonnectNamespacedRef[ + T constraints.SupportedKonnectEntityType, + TEnt constraints.EntityType[T], +](ent TEnt) bool { cpRef, ok := getControlPlaneRef(ent).Get() - if !ok { - return false - } - return cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return ok && + cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef } diff --git a/controller/konnect/watch_credentialacl.go b/controller/konnect/watch_credentialacl.go index a8d8a3d33..688bbe09f 100644 --- a/controller/konnect/watch_credentialacl.go +++ b/controller/konnect/watch_credentialacl.go @@ -92,8 +92,7 @@ func kongCredentialACLRefersToKonnectGatewayControlPlane(cl client.Client) func( return true } - cpRef := consumer.Spec.ControlPlaneRef - return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return objHasControlPlaneRefKonnectNamespacedRef(&consumer) } } diff --git a/controller/konnect/watch_credentialapikey.go b/controller/konnect/watch_credentialapikey.go index 202152ab4..32694e446 100644 --- a/controller/konnect/watch_credentialapikey.go +++ b/controller/konnect/watch_credentialapikey.go @@ -92,8 +92,7 @@ func kongCredentialAPIKeyRefersToKonnectGatewayControlPlane(cl client.Client) fu return true } - cpRef := consumer.Spec.ControlPlaneRef - return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return objHasControlPlaneRefKonnectNamespacedRef(&consumer) } } diff --git a/controller/konnect/watch_credentialbasicauth.go b/controller/konnect/watch_credentialbasicauth.go index 3f2a13577..3e1736789 100644 --- a/controller/konnect/watch_credentialbasicauth.go +++ b/controller/konnect/watch_credentialbasicauth.go @@ -92,8 +92,7 @@ func kongCredentialBasicAuthRefersToKonnectGatewayControlPlane(cl client.Client) return true } - cpRef := consumer.Spec.ControlPlaneRef - return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return objHasControlPlaneRefKonnectNamespacedRef(&consumer) } } diff --git a/controller/konnect/watch_kongkeyset.go b/controller/konnect/watch_kongkeyset.go index 9bbede0a1..231bb6063 100644 --- a/controller/konnect/watch_kongkeyset.go +++ b/controller/konnect/watch_kongkeyset.go @@ -3,7 +3,6 @@ package konnect import ( "context" "fmt" - "reflect" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -14,7 +13,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - operatorerrors "github.com/kong/gateway-operator/internal/errors" "github.com/kong/gateway-operator/modules/manager/logging" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" @@ -27,7 +25,7 @@ func KongKeySetReconciliationWatchOptions(cl client.Client) []func(*ctrl.Builder func(b *ctrl.Builder) *ctrl.Builder { return b.For(&configurationv1alpha1.KongKeySet{}, builder.WithPredicates( - predicate.NewPredicateFuncs(kongKeySetRefersToKonnectControlPlane), + predicate.NewPredicateFuncs(objRefersToKonnectGatewayControlPlane[configurationv1alpha1.KongKeySet]), ), ) }, @@ -50,20 +48,6 @@ func KongKeySetReconciliationWatchOptions(cl client.Client) []func(*ctrl.Builder } } -func kongKeySetRefersToKonnectControlPlane(obj client.Object) bool { - kongKeySet, ok := obj.(*configurationv1alpha1.KongKeySet) - if !ok { - ctrllog.FromContext(context.Background()).Error( - operatorerrors.ErrUnexpectedObject, - "failed to run predicate function", - "expected", "KongKeySet", "found", reflect.TypeOf(obj), - ) - return false - } - return kongKeySet.Spec.ControlPlaneRef != nil && - kongKeySet.Spec.ControlPlaneRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef -} - func enqueueKongKeySetForKonnectAPIAuthConfiguration(cl client.Client) handler.MapFunc { return func(ctx context.Context, obj client.Object) []reconcile.Request { auth, ok := obj.(*konnectv1alpha1.KonnectAPIAuthConfiguration) diff --git a/controller/konnect/watch_kongroute.go b/controller/konnect/watch_kongroute.go index d3038079f..7c16a6264 100644 --- a/controller/konnect/watch_kongroute.go +++ b/controller/konnect/watch_kongroute.go @@ -65,6 +65,12 @@ func kongRouteRefersToKonnectGatewayControlPlane(cl client.Client) func(obj clie return false } + // If the KongRoute refers to a KonnectGatewayControlPlane directly (it's a serviceless route), + // enqueue it. + if objHasControlPlaneRefKonnectNamespacedRef(kongRoute) { + return true + } + scvRef := kongRoute.Spec.ServiceRef if scvRef == nil || scvRef.Type != configurationv1alpha1.ServiceRefNamespacedRef { return false @@ -77,9 +83,7 @@ func kongRouteRefersToKonnectGatewayControlPlane(cl client.Client) func(obj clie if err := cl.Get(context.Background(), nn, &kongSvc); client.IgnoreNotFound(err) != nil { return true } - - cpRef := kongSvc.Spec.ControlPlaneRef - return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return objHasControlPlaneRefKonnectNamespacedRef(&kongSvc) } } diff --git a/controller/konnect/watch_kongtarget.go b/controller/konnect/watch_kongtarget.go index e92c5c892..e7d6e1a34 100644 --- a/controller/konnect/watch_kongtarget.go +++ b/controller/konnect/watch_kongtarget.go @@ -62,8 +62,7 @@ func kongTargetRefersToKonnectGatewayControlPlane(cl client.Client) func(obj cli if err := cl.Get(context.Background(), nn, &upstream); client.IgnoreNotFound(err) != nil { return true } - cpRef := upstream.Spec.ControlPlaneRef - return cpRef != nil && cpRef.Type == configurationv1alpha1.ControlPlaneRefKonnectNamespacedRef + return objHasControlPlaneRefKonnectNamespacedRef(&upstream) } }