fix: handle KongVault creation conflicts (#759)

Kong · Oct 21, 2024 · 0129f25 · 0129f25
1 parent e358cb1
commit 0129f25
Show file tree

Hide file tree

Showing 6 changed files with 169 additions and 31 deletions.
diff --git a/controller/konnect/ops/kongvault.go b/controller/konnect/ops/kongvault.go
@@ -12,4 +12,5 @@ type VaultSDK interface {
 	CreateVault(ctx context.Context, controlPlaneID string, vault sdkkonnectcomp.VaultInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateVaultResponse, error)
 	UpsertVault(ctx context.Context, request sdkkonnectops.UpsertVaultRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertVaultResponse, error)
 	DeleteVault(ctx context.Context, controlPlaneID string, vaultID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteVaultResponse, error)
+	ListVault(ctx context.Context, request sdkkonnectops.ListVaultRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListVaultResponse, error)
 }
diff --git a/controller/konnect/ops/kongvault_mock.go b/controller/konnect/ops/kongvault_mock.go
diff --git a/controller/konnect/ops/ops.go b/controller/konnect/ops/ops.go
@@ -130,6 +130,8 @@ func Create[
 			id, err = getKongTargetForUID(ctx, sdk.GetTargetsSDK(), ent)
 		case *configurationv1alpha1.KongPluginBinding:
 			id, err = getPluginForUID(ctx, sdk.GetPluginSDK(), ent)
+		case *configurationv1alpha1.KongVault:
+			id, err = getKongVaultForUID(ctx, sdk.GetVaultSDK(), ent)
 			// ---------------------------------------------------------------------
 			// TODO: add other Konnect types
 		default:

diff --git a/controller/konnect/ops/ops_kongvault.go b/controller/konnect/ops/ops_kongvault.go
@@ -29,12 +29,14 @@ func createVault(ctx context.Context, sdk VaultSDK, vault *configurationv1alpha1
 	resp, err := sdk.CreateVault(ctx, cpID, vaultInput)
 
 	if errWrapped := wrapErrIfKonnectOpFailed(err, CreateOp, vault); errWrapped != nil {
-		SetKonnectEntityProgrammedConditionFalse(vault, "FailedToCreate", errWrapped.Error())
 		return errWrapped
 	}
 
+	if resp == nil || resp.Vault == nil || resp.Vault.ID == nil || *resp.Vault.ID == "" {
+		return fmt.Errorf("failed creating %s: %w", vault.GetTypeName(), ErrNilResponse)
+	}
+
 	vault.SetKonnectID(*resp.Vault.ID)
-	SetKonnectEntityProgrammedCondition(vault)
 	return nil
 }
 
@@ -78,11 +80,9 @@ func updateVault(ctx context.Context, sdk VaultSDK, vault *configurationv1alpha1
 			}
 		}
 
-		SetKonnectEntityProgrammedConditionFalse(vault, "FailedToUpdate", errWrapped.Error())
 		return errWrapped
 	}
 
-	SetKonnectEntityProgrammedCondition(vault)
 	return nil
 }
 
@@ -140,3 +140,23 @@ func kongVaultToVaultInput(vault *configurationv1alpha1.KongVault) (sdkkonnectco
 	}
 	return input, nil
 }
+
+func getKongVaultForUID(
+	ctx context.Context,
+	sdk VaultSDK,
+	vault *configurationv1alpha1.KongVault,
+) (string, error) {
+	resp, err := sdk.ListVault(ctx, sdkkonnectops.ListVaultRequest{
+		ControlPlaneID: vault.GetControlPlaneID(),
+		Tags:           lo.ToPtr(UIDLabelForObject(vault)),
+	})
+	if err != nil {
+		return "", fmt.Errorf("failed to list KongVaults: %w", err)
+	}
+
+	if resp == nil || resp.Object == nil {
+		return "", fmt.Errorf("failed to list KongVaults: %w", ErrNilResponse)
+	}
+
+	return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.Object.Data), vault)
+}
diff --git a/controller/konnect/ops/ops_kongvault_test.go b/controller/konnect/ops/ops_kongvault_test.go
@@ -15,8 +15,6 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	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"
 )
@@ -71,11 +69,6 @@ func TestCreateKongVault(t *testing.T) {
 			},
 			assertions: func(t *testing.T, vault *configurationv1alpha1.KongVault) {
 				assert.Equal(t, "12345", vault.GetKonnectStatus().GetKonnectID())
-				cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, vault)
-				require.True(t, ok, "Programmed condition not set on KongVault")
-				assert.Equal(t, metav1.ConditionTrue, cond.Status)
-				assert.Equal(t, konnectv1alpha1.KonnectEntityProgrammedReasonProgrammed, cond.Reason)
-				assert.Equal(t, vault.GetGeneration(), cond.ObservedGeneration)
 			},
 		},
 		{
@@ -137,11 +130,6 @@ func TestCreateKongVault(t *testing.T) {
 			expectedErr: true,
 			assertions: func(t *testing.T, vault *configurationv1alpha1.KongVault) {
 				assert.Equal(t, "", vault.GetKonnectStatus().GetKonnectID())
-				cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, vault)
-				require.True(t, ok, "Programmed condition not set on KongVault")
-				assert.Equal(t, metav1.ConditionFalse, cond.Status)
-				assert.Equal(t, "FailedToCreate", cond.Reason)
-				assert.Equal(t, vault.GetGeneration(), cond.ObservedGeneration)
 			},
 		},
 	}
@@ -213,11 +201,6 @@ func TestUpdateKongVault(t *testing.T) {
 			},
 			assertions: func(t *testing.T, vault *configurationv1alpha1.KongVault) {
 				assert.Equal(t, "12345", vault.GetKonnectStatus().GetKonnectID())
-				cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, vault)
-				require.True(t, ok, "Programmed condition not set on KongVault")
-				assert.Equal(t, metav1.ConditionTrue, cond.Status)
-				assert.Equal(t, konnectv1alpha1.KonnectEntityProgrammedReasonProgrammed, cond.Reason)
-				assert.Equal(t, vault.GetGeneration(), cond.ObservedGeneration)
 			},
 		},
 		{
@@ -258,11 +241,7 @@ func TestUpdateKongVault(t *testing.T) {
 			},
 			expectedErr: true,
 			assertions: func(t *testing.T, vault *configurationv1alpha1.KongVault) {
-				cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, vault)
-				require.True(t, ok, "Programmed condition not set on KongVault")
-				assert.Equal(t, metav1.ConditionFalse, cond.Status)
-				assert.Equal(t, "FailedToUpdate", cond.Reason)
-				assert.Equal(t, vault.GetGeneration(), cond.ObservedGeneration)
+				assert.Equal(t, "12345", vault.GetKonnectID())
 			},
 		},
 		{
@@ -313,11 +292,6 @@ func TestUpdateKongVault(t *testing.T) {
 			},
 			assertions: func(t *testing.T, vault *configurationv1alpha1.KongVault) {
 				assert.Equal(t, "12345", vault.GetKonnectStatus().GetKonnectID())
-				cond, ok := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, vault)
-				require.True(t, ok, "Programmed condition not set on KongVault")
-				assert.Equal(t, metav1.ConditionTrue, cond.Status)
-				assert.Equal(t, konnectv1alpha1.KonnectEntityProgrammedReasonProgrammed, cond.Reason)
-				assert.Equal(t, vault.GetGeneration(), cond.ObservedGeneration)
 			},
 		},
 	}

diff --git a/test/envtest/konnect_entities_kongvault_test.go b/test/envtest/konnect_entities_kongvault_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"
@@ -121,4 +123,69 @@ func TestKongVault(t *testing.T) {
 			assert.True(c, factory.SDK.VaultSDK.AssertExpectations(t))
 		}, waitTime, tickTime)
 	})
+
+	t.Run("Should correctly handle conflict on create", func(t *testing.T) {
+		const (
+			vaultBackend   = "env-conflict"
+			vaultPrefix    = "env-vault-conflict"
+			vaultRawConfig = `{"prefix":"env_vault_conflict"}`
+			vaultID        = "vault-conflict-id"
+		)
+
+		t.Log("Setting up mock SDK for vault creation with conflict")
+		sdk.VaultSDK.EXPECT().CreateVault(mock.Anything, cp.GetKonnectStatus().GetKonnectID(), mock.MatchedBy(func(input sdkkonnectcomp.VaultInput) bool {
+			return input.Name == vaultBackend && input.Prefix == vaultPrefix
+		})).Return(nil, &sdkkonnecterrs.SDKError{
+			StatusCode: 400,
+			Body: `{
+					"code": 3,
+					"message": "data constraint error",
+					"details": [
+						{
+							"@type": "type.googleapis.com/kong.admin.model.v1.ErrorDetail",
+							"type": "ERROR_TYPE_REFERENCE",
+							"field": "name",
+							"messages": [
+								"name (type: unique) constraint failed"
+							]
+						}
+					]
+				}`,
+		})
+
+		sdk.VaultSDK.EXPECT().ListVault(
+			mock.Anything,
+			mock.MatchedBy(func(r sdkkonnectops.ListVaultRequest) bool {
+				return r.ControlPlaneID == cp.GetKonnectStatus().GetKonnectID() &&
+					r.Tags != nil && strings.Contains(*r.Tags, "k8s-uid")
+			},
+			)).Return(&sdkkonnectops.ListVaultResponse{
+			Object: &sdkkonnectops.ListVaultResponseBody{
+				Data: []sdkkonnectcomp.Vault{
+					{
+						ID: lo.ToPtr(vaultID),
+					},
+				},
+			},
+		}, nil)
+
+		vault := deploy.KongVaultAttachedToCP(t, ctx, cl, vaultBackend, vaultPrefix, []byte(vaultRawConfig), cp)
+
+		t.Log("Waiting for KongVault to be programmed")
+		watchFor(t, ctx, vaultWatch, watch.Modified, func(v *configurationv1alpha1.KongVault) bool {
+			if v.GetName() != vault.GetName() {
+				return false
+			}
+
+			return lo.ContainsBy(v.Status.Conditions, func(condition metav1.Condition) bool {
+				return condition.Type == konnectv1alpha1.KonnectEntityProgrammedConditionType &&
+					condition.Status == metav1.ConditionTrue
+			})
+		}, "KongVault's Programmed condition should be true eventually")
+
+		t.Log("Waiting for KongVault to be created in the SDK")
+		require.EventuallyWithT(t, func(c *assert.CollectT) {
+			assert.True(c, factory.SDK.VaultSDK.AssertExpectations(t))
+		}, waitTime, tickTime)
+	})
 }