feat: add uniqueness check of spec.prefix of KongVault

CHANGELOG.md

@@ -145,6 +145,8 @@ Adding a new version? You'll need three changes:
   [#5354](https://github.com/Kong/kubernetes-ingress-controller/pull/5354)
   [#5384](https://github.com/Kong/kubernetes-ingress-controller/pull/5384)
   [#5435](https://github.com/Kong/kubernetes-ingress-controller/pull/5435)
+  [#5412](https://github.com/Kong/kubernetes-ingress-controller/pull/5412)
+
 
 ### Fixed
 

internal/admission/validator.go

@@ -532,14 +532,25 @@ func (validator KongHTTPValidator) ValidateVault(ctx context.Context, k8sKongVau
 	if err != nil {
 		return false, fmt.Sprintf(ErrTextVaultConfigUnmarshalFailed, err), nil
 	}
+
+	// list existing KongVaults and reject if the spec.prefix is duplicate with another `KongVault`.
+	existingKongVaults := validator.Storer.ListKongVaults()
+	dupeVault, hasDupe := lo.Find(existingKongVaults, func(v *kongv1alpha1.KongVault) bool {
+		return v.Spec.Prefix == k8sKongVault.Spec.Prefix && v.Name != k8sKongVault.Name
+	})
+	if hasDupe {
+		return false, fmt.Sprintf("spec.prefix %q is duplicate with existing KongVault %q",
+			k8sKongVault.Spec.Prefix, dupeVault.Name), nil
+	}
+
 	kongVault := kong.Vault{
-		Name:        kong.String(k8sKongVault.Spec.Backend),
-		Prefix:      kong.String(k8sKongVault.Spec.Prefix),
-		Description: kong.String(k8sKongVault.Spec.Description),
-		Config:      config,
+		Name:   kong.String(k8sKongVault.Spec.Backend),
+		Prefix: kong.String(k8sKongVault.Spec.Prefix),
+		Config: config,
+	}
+	if len(k8sKongVault.Spec.Description) > 0 {
+		kongVault.Description = kong.String(k8sKongVault.Spec.Description)
 	}
-	// TODO: /schemas/vaults/test does not check "unique" restraint on `prefix` field:
-	// https://github.com/Kong/kubernetes-ingress-controller/issues/5395
 	errText, err := validator.validateVaultAgainstGatewaySchema(ctx, kongVault)
 	if err != nil || errText != "" {
 		return false, errText, err

internal/admission/validator_test.go

@@ -1241,6 +1241,7 @@ func TestValidator_ValidateVault(t *testing.T) {
 	testCases := []struct {
 		name            string
 		kongVault       kongv1alpha1.KongVault
+		storerObjects   store.FakeObjects
 		validateSvcFail bool
 		expectedOK      bool
 		expectedMessage string
@@ -1276,6 +1277,37 @@ func TestValidator_ValidateVault(t *testing.T) {
 			expectedOK:      false,
 			expectedMessage: "failed to unmarshal vault configuration",
 		},
+		{
+			name: "vault with duplicate prefix",
+			kongVault: kongv1alpha1.KongVault{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vault-1",
+				},
+				Spec: kongv1alpha1.KongVaultSpec{
+					Backend: "env",
+					Prefix:  "env-dupe",
+				},
+			},
+			storerObjects: store.FakeObjects{
+				KongVaults: []*kongv1alpha1.KongVault{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "vault-0",
+							Annotations: map[string]string{
+								annotations.IngressClassKey: annotations.DefaultIngressClass,
+							},
+						},
+						Spec: kongv1alpha1.KongVaultSpec{
+							Backend: "env",
+							Prefix:  "env-dupe",
+						},
+					},
+				},
+			},
+			validateSvcFail: false,
+			expectedOK:      false,
+			expectedMessage: "spec.prefix \"env-dupe\" is duplicate with existing KongVault",
+		},
 		{
 			name: "vault with failure in validating service",
 			kongVault: kongv1alpha1.KongVault{
@@ -1295,12 +1327,14 @@ func TestValidator_ValidateVault(t *testing.T) {
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
+			storer := lo.Must(store.NewFakeStore(tc.storerObjects))
 			validator := KongHTTPValidator{
 				AdminAPIServicesProvider: fakeServicesProvider{
 					vaultSvc: &fakeVaultSvc{
 						shouldFail: tc.validateSvcFail,
 					},
 				},
+				Storer:              storer,
 				ingressClassMatcher: fakeClassMatcher,
 				Logger:              logr.Discard(),
 			}

internal/dataplane/kongstate/kongstate.go

@@ -314,14 +314,17 @@ func (ks *KongState) FillVaults(
 			)
 			continue
 		}
+		kongVault := kong.Vault{
+			Name:   kong.String(vault.Spec.Backend),
+			Prefix: kong.String(vault.Spec.Prefix),
+			Config: config,
+			Tags:   util.GenerateTagsForObject(vault),
+		}
+		if len(vault.Spec.Description) > 0 {
+			kongVault.Description = kong.String(vault.Spec.Description)
+		}
 		ks.Vaults = append(ks.Vaults, Vault{
-			Vault: kong.Vault{
-				Name:        kong.String(vault.Spec.Backend),
-				Description: kong.String(vault.Spec.Description),
-				Prefix:      kong.String(vault.Spec.Prefix),
-				Config:      config,
-				Tags:        util.GenerateTagsForObject(vault),
-			},
+			Vault:        kongVault,
 			K8sKongVault: vault.DeepCopy(),
 		})
 	}

test/integration/webhook_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util/builder"
 	testutils "github.com/kong/kubernetes-ingress-controller/v3/internal/util/test"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
+	kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
 	"github.com/kong/kubernetes-ingress-controller/v3/pkg/clientset"
 	"github.com/kong/kubernetes-ingress-controller/v3/test"
 	"github.com/kong/kubernetes-ingress-controller/v3/test/consts"
@@ -69,7 +70,7 @@ func TestValidationWebhook(t *testing.T) {
 		ctx,
 		t,
 		ns.Name,
-		"kong-validations-consumer",
+		"kong-validations",
 		[]admregv1.RuleWithOperations{
 			{
 				Rule: admregv1.Rule{
@@ -87,13 +88,39 @@ func TestValidationWebhook(t *testing.T) {
 				},
 				Operations: []admregv1.OperationType{admregv1.Create, admregv1.Update},
 			},
+			{
+				Rule: admregv1.Rule{
+					APIGroups:   []string{"configuration.konghq.com"},
+					APIVersions: []string{"v1alpha1"},
+					Resources:   []string{"kongvaults"},
+				},
+				Operations: []admregv1.OperationType{admregv1.Create, admregv1.Update},
+			},
 		},
 	)
 	t.Log("waiting for webhook service to be connective")
-	ensureWebhookServiceIsConnective(ctx, t, "kong-validations-consumer")
+	ensureWebhookServiceIsConnective(ctx, t, "kong-validations")
 
-	t.Log("creating a large number of consumers on the cluster to verify the performance of the cached client during validation")
+	t.Log("creating a background KongVault for duplicate prefix test")
 	kongClient, err := clientset.NewForConfig(env.Cluster().Config())
+	require.NoError(t, err)
+	backgroundKongVault := &kongv1alpha1.KongVault{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "vault-background",
+			Annotations: map[string]string{
+				annotations.IngressClassKey: consts.IngressClass,
+			},
+		},
+		Spec: kongv1alpha1.KongVaultSpec{
+			Backend:     "env",
+			Prefix:      "env-background",
+			Description: "background vault for dupe prefix test",
+		},
+	}
+	_, err = kongClient.ConfigurationV1alpha1().KongVaults().Create(ctx, backgroundKongVault, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	t.Log("creating a large number of consumers on the cluster to verify the performance of the cached client during validation")
 	for i := 0; i < highEndConsumerUsageCount; i++ {
 		consumerName := fmt.Sprintf("background-noise-consumer-%d", i)
 		// create 5 credentials for each consumer
@@ -895,6 +922,125 @@ func TestValidationWebhook(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("verify validation webhook on creating KongVaults", func(t *testing.T) {
+		// Only Kong EE have the API `POST /schemas/vaults/validate`,
+		// so we create a subtest scope and only run it with Kong enterprise.
+		RunWhenKongEnterprise(t) //nolint: contextcheck
+
+		testCases := []struct {
+			name          string
+			kongVault     *kongv1alpha1.KongVault
+			errorOnCreate bool
+		}{
+			{
+				name: "should pass the validation if the configuration is correct",
+				kongVault: &kongv1alpha1.KongVault{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-valid",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: consts.IngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend:     "env",
+						Prefix:      "env-test",
+						Description: "test env vault",
+						Config: apiextensionsv1.JSON{
+							Raw: []byte(`{"prefix":"kong_vault_test_"}`),
+						},
+					},
+				},
+			},
+			{
+				name: "should also pass the validation if the description is empty",
+				kongVault: &kongv1alpha1.KongVault{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-empty-description",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: consts.IngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend: "env",
+						Prefix:  "env-empty-desc",
+						Config: apiextensionsv1.JSON{
+							Raw: []byte(`{"prefix":"kong_vault_test_"}`),
+						},
+					},
+				},
+			},
+			{
+				name: "should fail the validation if the backend is not supported by Kong gateway",
+				kongVault: &kongv1alpha1.KongVault{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-unsupported-backend",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: consts.IngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend:     "env1",
+						Prefix:      "unsupported-backend",
+						Description: "test env vault",
+						Config: apiextensionsv1.JSON{
+							Raw: []byte(`{"prefix":"kong-env-test"}`),
+						},
+					},
+				},
+				errorOnCreate: true,
+			},
+			{
+				name: "should fail the validation if the spec.config does not pass the schema check of Kong gateway",
+				kongVault: &kongv1alpha1.KongVault{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-invalid-config",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: consts.IngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend:     "env",
+						Prefix:      "invalid-config",
+						Description: "test env vault",
+						Config: apiextensionsv1.JSON{
+							Raw: []byte(`{"prefix":"kong-env-test","foo":"bar"}`),
+						},
+					},
+				},
+				errorOnCreate: true,
+			},
+			{
+				name: "should fail the validation if spec.prefix is duplicate",
+				kongVault: &kongv1alpha1.KongVault{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-dupe",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: consts.IngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend:     "env",
+						Prefix:      "env-background",
+						Description: "test env vault",
+					},
+				},
+				errorOnCreate: true,
+			},
+		}
+		t.Log("running test cases for validation on creating KongVault")
+		for _, tc := range testCases {
+			tc := tc
+			t.Run(tc.name, func(t *testing.T) {
+				_, err := kongClient.ConfigurationV1alpha1().KongVaults().Create(ctx, tc.kongVault, metav1.CreateOptions{})
+				if tc.errorOnCreate {
+					require.Error(t, err)
+				} else {
+					require.NoError(t, err)
+				}
+			})
+		}
+	})
 }
 
 func ensureWebhookService(ctx context.Context, t *testing.T, name string) {