diff --git a/controller/konnect/ops/kongroute.go b/controller/konnect/ops/kongroute.go index 809382866..08cb761ae 100644 --- a/controller/konnect/ops/kongroute.go +++ b/controller/konnect/ops/kongroute.go @@ -12,4 +12,5 @@ type RoutesSDK interface { CreateRoute(ctx context.Context, controlPlaneID string, route sdkkonnectcomp.RouteInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateRouteResponse, error) UpsertRoute(ctx context.Context, req sdkkonnectops.UpsertRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertRouteResponse, error) DeleteRoute(ctx context.Context, controlPlaneID, routeID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteRouteResponse, error) + ListRoute(ctx context.Context, request sdkkonnectops.ListRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListRouteResponse, error) } diff --git a/controller/konnect/ops/kongroute_mock.go b/controller/konnect/ops/kongroute_mock.go index 13d6db971..40b235b87 100644 --- a/controller/konnect/ops/kongroute_mock.go +++ b/controller/konnect/ops/kongroute_mock.go @@ -175,6 +175,80 @@ func (_c *MockRoutesSDK_DeleteRoute_Call) RunAndReturn(run func(context.Context, return _c } +// ListRoute provides a mock function with given fields: ctx, request, opts +func (_m *MockRoutesSDK) ListRoute(ctx context.Context, request operations.ListRouteRequest, opts ...operations.Option) (*operations.ListRouteResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, request) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for ListRoute") + } + + var r0 *operations.ListRouteResponse + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, operations.ListRouteRequest, ...operations.Option) (*operations.ListRouteResponse, error)); ok { + return rf(ctx, request, opts...) + } + if rf, ok := ret.Get(0).(func(context.Context, operations.ListRouteRequest, ...operations.Option) *operations.ListRouteResponse); ok { + r0 = rf(ctx, request, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*operations.ListRouteResponse) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, operations.ListRouteRequest, ...operations.Option) error); ok { + r1 = rf(ctx, request, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockRoutesSDK_ListRoute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListRoute' +type MockRoutesSDK_ListRoute_Call struct { + *mock.Call +} + +// ListRoute is a helper method to define mock.On call +// - ctx context.Context +// - request operations.ListRouteRequest +// - opts ...operations.Option +func (_e *MockRoutesSDK_Expecter) ListRoute(ctx interface{}, request interface{}, opts ...interface{}) *MockRoutesSDK_ListRoute_Call { + return &MockRoutesSDK_ListRoute_Call{Call: _e.mock.On("ListRoute", + append([]interface{}{ctx, request}, opts...)...)} +} + +func (_c *MockRoutesSDK_ListRoute_Call) Run(run func(ctx context.Context, request operations.ListRouteRequest, opts ...operations.Option)) *MockRoutesSDK_ListRoute_Call { + _c.Call.Run(func(args mock.Arguments) { + variadicArgs := make([]operations.Option, len(args)-2) + for i, a := range args[2:] { + if a != nil { + variadicArgs[i] = a.(operations.Option) + } + } + run(args[0].(context.Context), args[1].(operations.ListRouteRequest), variadicArgs...) + }) + return _c +} + +func (_c *MockRoutesSDK_ListRoute_Call) Return(_a0 *operations.ListRouteResponse, _a1 error) *MockRoutesSDK_ListRoute_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockRoutesSDK_ListRoute_Call) RunAndReturn(run func(context.Context, operations.ListRouteRequest, ...operations.Option) (*operations.ListRouteResponse, error)) *MockRoutesSDK_ListRoute_Call { + _c.Call.Return(run) + return _c +} + // UpsertRoute provides a mock function with given fields: ctx, req, opts func (_m *MockRoutesSDK) UpsertRoute(ctx context.Context, req operations.UpsertRouteRequest, opts ...operations.Option) (*operations.UpsertRouteResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/controller/konnect/ops/ops.go b/controller/konnect/ops/ops.go index 457f269a8..34d5029eb 100644 --- a/controller/konnect/ops/ops.go +++ b/controller/konnect/ops/ops.go @@ -60,14 +60,14 @@ func Create[ err = createControlPlane(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent) case *configurationv1alpha1.KongService: err = createService(ctx, sdk.GetServicesSDK(), ent) - - // TODO: modify the create* operation wrappers to not set Programmed conditions and return - // a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed. - case *configurationv1alpha1.KongRoute: err = createRoute(ctx, sdk.GetRoutesSDK(), ent) case *configurationv1.KongConsumer: err = createConsumer(ctx, sdk.GetConsumersSDK(), sdk.GetConsumerGroupsSDK(), cl, ent) + + // TODO: modify the create* operation wrappers to not set Programmed conditions and return + // a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed. + case *configurationv1beta1.KongConsumerGroup: err = createConsumerGroup(ctx, sdk.GetConsumerGroupsSDK(), ent) case *configurationv1alpha1.KongPluginBinding: @@ -122,7 +122,7 @@ func Create[ switch { case errors.As(err, &errConflict) || - // Service API returns 400 for conflicts + // ControlPlane resource APIs return 400 for conflicts errSdkBody.Code == 3 && errSdkBody.Message == "data constraint error": // If there was a conflict on the create request, we can assume the entity already exists. // We'll get its Konnect ID by listing all entities of its type filtered by the Kubernetes object UID. @@ -132,6 +132,8 @@ func Create[ id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent) case *configurationv1alpha1.KongService: id, err = getKongServiceForUID(ctx, sdk.GetServicesSDK(), ent) + case *configurationv1alpha1.KongRoute: + id, err = getKongRouteForUID(ctx, sdk.GetRoutesSDK(), ent) // --------------------------------------------------------------------- // TODO: add other Konnect types default: diff --git a/controller/konnect/ops/ops_kongroute.go b/controller/konnect/ops/ops_kongroute.go index a7f951218..4e1250a3e 100644 --- a/controller/konnect/ops/ops_kongroute.go +++ b/controller/konnect/ops/ops_kongroute.go @@ -9,12 +9,54 @@ import ( sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components" sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations" sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors" + "github.com/samber/lo" "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" ) +// getKongRouteForUID returns the Konnect ID of the KongRoute +// that matches the UID of the provided KongRoute. +func getKongRouteForUID( + ctx context.Context, + sdk RoutesSDK, + r *configurationv1alpha1.KongRoute, +) (string, error) { + reqList := sdkkonnectops.ListRouteRequest{ + // NOTE: only filter on object's UID. + // Other fields like name might have changed in the meantime but that's OK. + // Those will be enforced via subsequent updates. + Tags: lo.ToPtr(UIDLabelForObject(r)), + ControlPlaneID: r.GetControlPlaneID(), + } + + respList, err := sdk.ListRoute(ctx, reqList) + if err != nil { + return "", err + } + + if respList == nil || respList.Object == nil { + return "", fmt.Errorf("failed listing %s: %w", r.GetTypeName(), ErrNilResponse) + } + + var id string + for _, entry := range respList.Object.Data { + if entry.ID != nil && *entry.ID != "" { + id = *entry.ID + break + } + } + + if id == "" { + return "", EntityWithMatchingUIDNotFoundError{ + Entity: r, + } + } + + return id, nil +} + func createRoute( ctx context.Context, sdk RoutesSDK, @@ -25,16 +67,18 @@ func createRoute( } resp, err := sdk.CreateRoute(ctx, route.Status.Konnect.ControlPlaneID, kongRouteToSDKRouteInput(route)) - // TODO: handle already exists // Can't adopt it as it will cause conflicts between the controller - // that created that entity and already manages it, hm + // that created that entity and already manages it. + // TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460 if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, route); errWrap != nil { - SetKonnectEntityProgrammedConditionFalse(route, "FailedToCreate", errWrap.Error()) return errWrap } - route.Status.Konnect.SetKonnectID(*resp.Route.ID) - SetKonnectEntityProgrammedCondition(route) + if resp == nil || resp.Route == nil || resp.Route.ID == nil { + return fmt.Errorf("failed creating %s: %w", route.GetTypeName(), ErrNilResponse) + } + + route.SetKonnectID(*resp.Route.ID) return nil } @@ -53,22 +97,42 @@ func updateRoute( return fmt.Errorf("can't update %T %s without a Konnect ControlPlane ID", route, client.ObjectKeyFromObject(route)) } + id := route.GetKonnectStatus().GetKonnectID() _, err := sdk.UpsertRoute(ctx, sdkkonnectops.UpsertRouteRequest{ ControlPlaneID: cpID, - RouteID: route.Status.Konnect.ID, + RouteID: id, Route: kongRouteToSDKRouteInput(route), }) - // TODO: handle already exists // Can't adopt it as it will cause conflicts between the controller - // that created that entity and already manages it, hm + // that created that entity and already manages it. + // TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460 if errWrap := wrapErrIfKonnectOpFailed(err, UpdateOp, route); errWrap != nil { - SetKonnectEntityProgrammedConditionFalse(route, "FailedToUpdate", errWrap.Error()) + // Route update operation returns an SDKError instead of a NotFoundError. + var sdkError *sdkkonnecterrs.SDKError + if errors.As(errWrap, &sdkError) { + switch sdkError.StatusCode { + case 404: + logEntityNotFoundRecreating(ctx, route, id) + if err := createRoute(ctx, sdk, route); err != nil { + return FailedKonnectOpError[configurationv1alpha1.KongRoute]{ + Op: UpdateOp, + Err: err, + } + } + // Create succeeded, createService sets the status so no need to do this here. + return nil + default: + return FailedKonnectOpError[configurationv1alpha1.KongRoute]{ + Op: UpdateOp, + Err: sdkError, + } + } + } + return errWrap } - SetKonnectEntityProgrammedCondition(route) - return nil } diff --git a/controller/konnect/ops/ops_kongservice.go b/controller/konnect/ops/ops_kongservice.go index 68364e7c7..63015647b 100644 --- a/controller/konnect/ops/ops_kongservice.go +++ b/controller/konnect/ops/ops_kongservice.go @@ -36,7 +36,7 @@ func getKongServiceForUID( } if respList == nil || respList.Object == nil { - return "", fmt.Errorf("failed listing KongServices: %w", ErrNilResponse) + return "", fmt.Errorf("failed listing %s: %w", svc.GetTypeName(), ErrNilResponse) } var id string @@ -115,9 +115,9 @@ func updateService( }, ) - // TODO: handle already exists // Can't adopt it as it will cause conflicts between the controller - // that created that entity and already manages it, hm + // that created that entity and already manages it. + // TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460 if errWrap := wrapErrIfKonnectOpFailed(err, UpdateOp, svc); errWrap != nil { // Service update operation returns an SDKError instead of a NotFoundError. var sdkError *sdkkonnecterrs.SDKError diff --git a/test/envtest/konnect_entities_kongroute_test.go b/test/envtest/konnect_entities_kongroute_test.go new file mode 100644 index 000000000..0361d2c31 --- /dev/null +++ b/test/envtest/konnect_entities_kongroute_test.go @@ -0,0 +1,144 @@ +package envtest + +import ( + "context" + "slices" + "testing" + + sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components" + sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/watch" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kong/gateway-operator/controller/konnect" + "github.com/kong/gateway-operator/controller/konnect/ops" + "github.com/kong/gateway-operator/modules/manager/scheme" + k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes" + "github.com/kong/gateway-operator/test/helpers/deploy" + + configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1" +) + +func TestKongRoute(t *testing.T) { + t.Parallel() + ctx, cancel := Context(t, context.Background()) + defer cancel() + cfg, ns := Setup(t, ctx, scheme.Get()) + + t.Log("Setting up the manager with reconcilers") + mgr, logs := NewManager(t, ctx, cfg, scheme.Get()) + factory := ops.NewMockSDKFactory(t) + sdk := factory.SDK + StartReconcilers(ctx, t, mgr, logs, + konnect.NewKonnectEntityReconciler(factory, false, mgr.GetClient(), + konnect.WithKonnectEntitySyncPeriod[configurationv1alpha1.KongRoute](konnectInfiniteSyncTime), + ), + ) + + t.Log("Setting up clients") + cl, err := client.NewWithWatch(mgr.GetConfig(), client.Options{ + Scheme: scheme.Get(), + }) + require.NoError(t, err) + clientNamespaced := client.NewNamespacedClient(mgr.GetClient(), ns.Name) + + t.Log("Creating KonnectAPIAuthConfiguration and KonnectGatewayControlPlane") + apiAuth := deploy.KonnectAPIAuthConfigurationWithProgrammed(t, ctx, clientNamespaced) + cp := deploy.KonnectGatewayControlPlaneWithID(t, ctx, clientNamespaced, apiAuth) + svc := deploy.KongServiceAttachedToCPWithID(t, ctx, clientNamespaced, cp) + + t.Run("adding, patching and deleting KongRoute", func(t *testing.T) { + const ( + routeID = "route-12345" + ) + + t.Log("Setting up a watch for KongRoute events") + w := setupWatch[configurationv1alpha1.KongRouteList](t, ctx, cl, client.InNamespace(ns.Name)) + + t.Log("Setting up SDK expectations on Route creation") + sdk.RoutesSDK.EXPECT(). + CreateRoute( + mock.Anything, + cp.GetKonnectID(), + mock.MatchedBy(func(req sdkkonnectcomp.RouteInput) bool { + return slices.Equal(req.Paths, []string{"/path"}) + }), + ). + Return( + &sdkkonnectops.CreateRouteResponse{ + Route: &sdkkonnectcomp.Route{ + ID: lo.ToPtr(routeID), + }, + }, + nil, + ) + + t.Log("Creating a KongRoute") + createdRoute := deploy.KongRouteAttachedToService(t, ctx, clientNamespaced, svc, + func(obj client.Object) { + s := obj.(*configurationv1alpha1.KongRoute) + s.Spec.KongRouteAPISpec.Paths = []string{"/path"} + }, + ) + + t.Log("Checking SDK KongRoute operations") + require.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, factory.SDK.RoutesSDK.AssertExpectations(t)) + }, waitTime, tickTime) + + t.Log("Waiting for Route to be programmed and get Konnect ID") + watchFor(t, ctx, w, watch.Modified, func(r *configurationv1alpha1.KongRoute) bool { + return r.GetKonnectID() == routeID && k8sutils.IsProgrammed(r) + }, "KongRoute didn't get Programmed status condition or didn't get the correct (route-12345) Konnect ID assigned") + + t.Log("Setting up SDK expectations on Route update") + sdk.RoutesSDK.EXPECT(). + UpsertRoute( + mock.Anything, + mock.MatchedBy(func(req sdkkonnectops.UpsertRouteRequest) bool { + return req.RouteID == routeID && + slices.Equal(req.Route.Paths, []string{"/path"}) && + req.Route.PreserveHost != nil && *req.Route.PreserveHost == true + }), + ). + Return(&sdkkonnectops.UpsertRouteResponse{}, nil) + + t.Log("Patching KongRoute") + routeToPatch := createdRoute.DeepCopy() + routeToPatch.Spec.PreserveHost = lo.ToPtr(true) + require.NoError(t, clientNamespaced.Patch(ctx, routeToPatch, client.MergeFrom(createdRoute))) + + t.Log("Waiting for Route to be updated in the SDK") + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, factory.SDK.RoutesSDK.AssertExpectations(t)) + }, waitTime, tickTime) + + t.Log("Setting up SDK expectations on Route deletion") + sdk.RoutesSDK.EXPECT(). + DeleteRoute( + mock.Anything, + cp.GetKonnectID(), + routeID, + ). + Return(&sdkkonnectops.DeleteRouteResponse{}, nil) + + t.Log("Deleting KongRoute") + require.NoError(t, clientNamespaced.Delete(ctx, createdRoute)) + + t.Log("Waiting for KongRoute to disappear") + assert.EventuallyWithT(t, func(c *assert.CollectT) { + err := clientNamespaced.Get(ctx, client.ObjectKeyFromObject(createdRoute), createdRoute) + assert.True(c, err != nil && k8serrors.IsNotFound(err)) + }, waitTime, tickTime) + + t.Log("Waiting for Route to be deleted in the SDK") + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, factory.SDK.RoutesSDK.AssertExpectations(t)) + }, waitTime, tickTime) + }) +} diff --git a/test/helpers/deploy/deploy_resources.go b/test/helpers/deploy/deploy_resources.go index 055af50cd..8e0e50e4d 100644 --- a/test/helpers/deploy/deploy_resources.go +++ b/test/helpers/deploy/deploy_resources.go @@ -151,7 +151,7 @@ func KonnectGatewayControlPlane( return cp } -// deploy.KonnectGatewayControlPlaneWithID deploys a KonnectGatewayControlPlane resource and returns the resource. +// KonnectGatewayControlPlaneWithID deploys a KonnectGatewayControlPlane resource and returns the resource. // The Status ID and Programmed condition are set on the CP using status Update() call. // It can be useful where the reconciler for KonnectGatewayControlPlane is not started // and hence the status has to be filled manually. @@ -179,6 +179,34 @@ func KonnectGatewayControlPlaneWithID( return cp } +// KongServiceAttachedToCPWithID deploys a KongService resource and returns the resource. +// The Status ID and Programmed condition are set on the Service using status Update() call. +// It can be useful where the reconciler for KonnectGatewayControlPlane is not started +// and hence the status has to be filled manually. +func KongServiceAttachedToCPWithID( + t *testing.T, + ctx context.Context, + cl client.Client, + cp *konnectv1alpha1.KonnectGatewayControlPlane, + opts ...objOption, +) *configurationv1alpha1.KongService { + t.Helper() + + svc := KongServiceAttachedToCP(t, ctx, cl, cp, opts...) + svc.Status.Conditions = []metav1.Condition{ + { + Type: konnectv1alpha1.KonnectEntityProgrammedConditionType, + Status: metav1.ConditionTrue, + Reason: konnectv1alpha1.KonnectEntityProgrammedReasonProgrammed, + ObservedGeneration: svc.GetGeneration(), + LastTransitionTime: metav1.Now(), + }, + } + svc.SetKonnectID(uuid.NewString()[:8]) + require.NoError(t, cl.Status().Update(ctx, svc)) + return svc +} + // KongServiceAttachedToCP deploys a KongService resource and returns the resource. func KongServiceAttachedToCP( t *testing.T,