fix: handle KeySet creation conflicts (#755)

* fix: handle KeySet creation conflicts * Update controller/konnect/ops/ops_kongkeyset.go Co-authored-by: Patryk Małek <[email protected]> --------- Co-authored-by: Patryk Małek <[email protected]>
Kong · Oct 18, 2024 · 3fc5ce0 · 3fc5ce0
1 parent a44e9a9
commit 3fc5ce0
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 6 deletions.
diff --git 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
diff --git 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
@@ -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
@@ -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)
+	})
 }