diff --git a/controller/konnect/ops/kongkeyset.go b/controller/konnect/ops/kongkeyset.go index 0d3f16381..0ec19c83f 100644 --- a/controller/konnect/ops/kongkeyset.go +++ b/controller/konnect/ops/kongkeyset.go @@ -12,4 +12,5 @@ type KeySetsSDK interface { CreateKeySet(ctx context.Context, controlPlaneID string, keySet sdkkonnectcomp.KeySetInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateKeySetResponse, error) UpsertKeySet(ctx context.Context, request sdkkonnectops.UpsertKeySetRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertKeySetResponse, error) DeleteKeySet(ctx context.Context, controlPlaneID string, keySetID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteKeySetResponse, error) + ListKeySet(ctx context.Context, request sdkkonnectops.ListKeySetRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListKeySetResponse, error) } diff --git a/controller/konnect/ops/kongkeyset_mock.go b/controller/konnect/ops/kongkeyset_mock.go index 636462bea..72ddb0e2e 100644 --- a/controller/konnect/ops/kongkeyset_mock.go +++ b/controller/konnect/ops/kongkeyset_mock.go @@ -175,6 +175,80 @@ func (_c *MockKeySetsSDK_DeleteKeySet_Call) RunAndReturn(run func(context.Contex return _c } +// ListKeySet provides a mock function with given fields: ctx, request, opts +func (_m *MockKeySetsSDK) ListKeySet(ctx context.Context, request operations.ListKeySetRequest, opts ...operations.Option) (*operations.ListKeySetResponse, 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 ListKeySet") + } + + var r0 *operations.ListKeySetResponse + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, operations.ListKeySetRequest, ...operations.Option) (*operations.ListKeySetResponse, error)); ok { + return rf(ctx, request, opts...) + } + if rf, ok := ret.Get(0).(func(context.Context, operations.ListKeySetRequest, ...operations.Option) *operations.ListKeySetResponse); ok { + r0 = rf(ctx, request, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*operations.ListKeySetResponse) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, operations.ListKeySetRequest, ...operations.Option) error); ok { + r1 = rf(ctx, request, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockKeySetsSDK_ListKeySet_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListKeySet' +type MockKeySetsSDK_ListKeySet_Call struct { + *mock.Call +} + +// ListKeySet is a helper method to define mock.On call +// - ctx context.Context +// - request operations.ListKeySetRequest +// - opts ...operations.Option +func (_e *MockKeySetsSDK_Expecter) ListKeySet(ctx interface{}, request interface{}, opts ...interface{}) *MockKeySetsSDK_ListKeySet_Call { + return &MockKeySetsSDK_ListKeySet_Call{Call: _e.mock.On("ListKeySet", + append([]interface{}{ctx, request}, opts...)...)} +} + +func (_c *MockKeySetsSDK_ListKeySet_Call) Run(run func(ctx context.Context, request operations.ListKeySetRequest, opts ...operations.Option)) *MockKeySetsSDK_ListKeySet_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.ListKeySetRequest), variadicArgs...) + }) + return _c +} + +func (_c *MockKeySetsSDK_ListKeySet_Call) Return(_a0 *operations.ListKeySetResponse, _a1 error) *MockKeySetsSDK_ListKeySet_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockKeySetsSDK_ListKeySet_Call) RunAndReturn(run func(context.Context, operations.ListKeySetRequest, ...operations.Option) (*operations.ListKeySetResponse, error)) *MockKeySetsSDK_ListKeySet_Call { + _c.Call.Return(run) + return _c +} + // UpsertKeySet provides a mock function with given fields: ctx, request, opts func (_m *MockKeySetsSDK) UpsertKeySet(ctx context.Context, request operations.UpsertKeySetRequest, opts ...operations.Option) (*operations.UpsertKeySetResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/controller/konnect/ops/ops.go b/controller/konnect/ops/ops.go index f6566b4e1..1e6a2c36d 100644 --- a/controller/konnect/ops/ops.go +++ b/controller/konnect/ops/ops.go @@ -120,6 +120,8 @@ func Create[ id, err = getConsumerForUID(ctx, sdk.GetConsumersSDK(), ent) case *configurationv1beta1.KongConsumerGroup: id, err = getConsumerGroupForUID(ctx, sdk.GetConsumerGroupsSDK(), ent) + case *configurationv1alpha1.KongKeySet: + id, err = getKongKeySetForUID(ctx, sdk.GetKeySetsSDK(), ent) // --------------------------------------------------------------------- // TODO: add other Konnect types default: diff --git a/controller/konnect/ops/ops_kongkeyset.go b/controller/konnect/ops/ops_kongkeyset.go index fbf568948..2bd408800 100644 --- a/controller/konnect/ops/ops_kongkeyset.go +++ b/controller/konnect/ops/ops_kongkeyset.go @@ -36,12 +36,15 @@ func createKeySet( // Can't adopt it as it will cause conflicts between the controller // that created that entity and already manages it, hm if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, keySet); errWrap != nil { - SetKonnectEntityProgrammedConditionFalse(keySet, "FailedToCreate", errWrap.Error()) return errWrap } - keySet.Status.Konnect.SetKonnectID(*resp.KeySet.ID) - SetKonnectEntityProgrammedCondition(keySet) + if resp == nil || resp.KeySet == nil || resp.KeySet.ID == nil || *resp.KeySet.ID == "" { + return fmt.Errorf("failed creating %s: %w", keySet.GetTypeName(), ErrNilResponse) + } + + // At this point, the KeySet has been created successfully. + keySet.SetKonnectID(*resp.KeySet.ID) return nil } @@ -87,12 +90,9 @@ func updateKeySet( Err: sdkError, } } - SetKonnectEntityProgrammedConditionFalse(keySet, "FailedToUpdate", errWrap.Error()) return errWrap } - SetKonnectEntityProgrammedCondition(keySet) - return nil } @@ -136,3 +136,23 @@ func kongKeySetToKeySetInput(keySet *configurationv1alpha1.KongKeySet) sdkkonnec Tags: GenerateTagsForObject(keySet, keySet.Spec.Tags...), } } + +func getKongKeySetForUID( + ctx context.Context, + sdk KeySetsSDK, + keySet *configurationv1alpha1.KongKeySet, +) (string, error) { + resp, err := sdk.ListKeySet(ctx, sdkkonnectops.ListKeySetRequest{ + ControlPlaneID: keySet.GetControlPlaneID(), + Tags: lo.ToPtr(UIDLabelForObject(keySet)), + }) + if err != nil { + return "", fmt.Errorf("failed to list KongKeySets: %w", err) + } + + if resp == nil || resp.Object == nil { + return "", fmt.Errorf("failed listing %s: %w", keySet.GetTypeName(), ErrNilResponse) + } + + return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.Object.Data), keySet) +} diff --git a/test/envtest/konnect_entities_kongkeyset_test.go b/test/envtest/konnect_entities_kongkeyset_test.go index f83d2a95f..b50b46e90 100644 --- a/test/envtest/konnect_entities_kongkeyset_test.go +++ b/test/envtest/konnect_entities_kongkeyset_test.go @@ -2,10 +2,12 @@ package envtest import ( "context" + "strings" "testing" 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" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -115,4 +117,55 @@ func TestKongKeySet(t *testing.T) { assert.EventuallyWithT(t, func(c *assert.CollectT) { assert.True(c, factory.SDK.KeySetsSDK.AssertExpectations(t)) }, waitTime, tickTime) + + t.Run("should handle conflict in creation correctly", func(t *testing.T) { + const ( + keySetID = "keyset-id-conflict" + keySetName = "keyset-name-conflict" + ) + t.Log("Setup mock SDK for creating KeySet and listing KeySets by UID") + cpID := cp.GetKonnectStatus().GetKonnectID() + sdk.KeySetsSDK.EXPECT().CreateKeySet( + mock.Anything, + cpID, + mock.MatchedBy(func(input sdkkonnectcomp.KeySetInput) bool { + return *input.Name == keySetName + }), + ).Return(&sdkkonnectops.CreateKeySetResponse{}, &sdkkonnecterrs.ConflictError{}) + + sdk.KeySetsSDK.EXPECT().ListKeySet( + mock.Anything, + mock.MatchedBy(func(req sdkkonnectops.ListKeySetRequest) bool { + return req.ControlPlaneID == cpID && + req.Tags != nil && strings.HasPrefix(*req.Tags, "k8s-uid") + }), + ).Return(&sdkkonnectops.ListKeySetResponse{ + Object: &sdkkonnectops.ListKeySetResponseBody{ + Data: []sdkkonnectcomp.KeySet{ + { + ID: lo.ToPtr(keySetID), + }, + }, + }, + }, nil) + + t.Log("Creating a KeySet") + createdKeySet := deploy.KongKeySetAttachedToCP(t, ctx, clientNamespaced, keySetName, cp) + + t.Log("Watching for KeySet to verify the created KeySet programmed") + watchFor(t, ctx, w, watch.Modified, func(c *configurationv1alpha1.KongKeySet) bool { + if c.GetName() != createdKeySet.GetName() { + return false + } + return c.GetKonnectID() == keySetID && lo.ContainsBy(c.Status.Conditions, func(condition metav1.Condition) bool { + return condition.Type == konnectv1alpha1.KonnectEntityProgrammedConditionType && + condition.Status == metav1.ConditionTrue + }) + }, "KeySet should be programmed and have ID in status after handling conflict") + + t.Log("Ensuring that the SDK's create and list methods are called") + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.True(c, factory.SDK.ConsumersSDK.AssertExpectations(t)) + }, waitTime, tickTime) + }) }