From 0129f25ee9959c34a8fa61ebb2a2cec3093e7062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Mon, 21 Oct 2024 10:59:33 +0200 Subject: [PATCH] fix: handle KongVault creation conflicts (#759) --- controller/konnect/ops/kongvault.go | 1 + controller/konnect/ops/kongvault_mock.go | 74 +++++++++++++++++++ controller/konnect/ops/ops.go | 2 + controller/konnect/ops/ops_kongvault.go | 28 ++++++- controller/konnect/ops/ops_kongvault_test.go | 28 +------ .../konnect_entities_kongvault_test.go | 67 +++++++++++++++++ 6 files changed, 169 insertions(+), 31 deletions(-) diff --git a/controller/konnect/ops/kongvault.go b/controller/konnect/ops/kongvault.go index 848780a04..d3d140977 100644 --- 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 index 6dbb84a75..d305d9d65 100644 --- a/controller/konnect/ops/kongvault_mock.go +++ b/controller/konnect/ops/kongvault_mock.go @@ -175,6 +175,80 @@ func (_c *MockVaultSDK_DeleteVault_Call) RunAndReturn(run func(context.Context, return _c } +// ListVault provides a mock function with given fields: ctx, request, opts +func (_m *MockVaultSDK) ListVault(ctx context.Context, request operations.ListVaultRequest, opts ...operations.Option) (*operations.ListVaultResponse, 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 ListVault") + } + + var r0 *operations.ListVaultResponse + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, operations.ListVaultRequest, ...operations.Option) (*operations.ListVaultResponse, error)); ok { + return rf(ctx, request, opts...) + } + if rf, ok := ret.Get(0).(func(context.Context, operations.ListVaultRequest, ...operations.Option) *operations.ListVaultResponse); ok { + r0 = rf(ctx, request, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*operations.ListVaultResponse) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, operations.ListVaultRequest, ...operations.Option) error); ok { + r1 = rf(ctx, request, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockVaultSDK_ListVault_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListVault' +type MockVaultSDK_ListVault_Call struct { + *mock.Call +} + +// ListVault is a helper method to define mock.On call +// - ctx context.Context +// - request operations.ListVaultRequest +// - opts ...operations.Option +func (_e *MockVaultSDK_Expecter) ListVault(ctx interface{}, request interface{}, opts ...interface{}) *MockVaultSDK_ListVault_Call { + return &MockVaultSDK_ListVault_Call{Call: _e.mock.On("ListVault", + append([]interface{}{ctx, request}, opts...)...)} +} + +func (_c *MockVaultSDK_ListVault_Call) Run(run func(ctx context.Context, request operations.ListVaultRequest, opts ...operations.Option)) *MockVaultSDK_ListVault_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.ListVaultRequest), variadicArgs...) + }) + return _c +} + +func (_c *MockVaultSDK_ListVault_Call) Return(_a0 *operations.ListVaultResponse, _a1 error) *MockVaultSDK_ListVault_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockVaultSDK_ListVault_Call) RunAndReturn(run func(context.Context, operations.ListVaultRequest, ...operations.Option) (*operations.ListVaultResponse, error)) *MockVaultSDK_ListVault_Call { + _c.Call.Return(run) + return _c +} + // UpsertVault provides a mock function with given fields: ctx, request, opts func (_m *MockVaultSDK) UpsertVault(ctx context.Context, request operations.UpsertVaultRequest, opts ...operations.Option) (*operations.UpsertVaultResponse, error) { _va := make([]interface{}, len(opts)) diff --git a/controller/konnect/ops/ops.go b/controller/konnect/ops/ops.go index ec054ca43..94a1102a2 100644 --- 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 index f64da4d08..7a0efd5c2 100644 --- 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 index 2be1cc5d3..bf7535693 100644 --- 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 index 6e238ae67..14d726ed5 100644 --- 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) + }) }