diff --git a/CHANGELOG.md b/CHANGELOG.md index 6890fe3788..80630858a3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/internal/admission/validator.go b/internal/admission/validator.go index 80c62443d5..a7a4329ef1 100644 --- a/internal/admission/validator.go +++ b/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 diff --git a/internal/admission/validator_test.go b/internal/admission/validator_test.go index 4bb6b4bda8..c6f5c32093 100644 --- a/internal/admission/validator_test.go +++ b/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(), } diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index 26eb9a69d3..33825fe32a 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/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(), }) } diff --git a/test/integration/webhook_test.go b/test/integration/webhook_test.go index b2767a3681..5e237c50da 100644 --- a/test/integration/webhook_test.go +++ b/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) {