feat: translate only one if multiple KongVaults with the same spec.pr…

…efix (#5435)

* change spec.prefix of KongVault to be immutable

* translate only one of KongVaults with same spec.prefix

* fix linter and add CHANGELOG

* add envtest for validation of KongVault

* update manifests

CHANGELOG.md

@@ -137,9 +137,14 @@ Adding a new version? You'll need three changes:
 - New CRD `KongVault` to reperesent a custom Kong vault for storing senstive
   data used in plugin configurations. Now users can create a `KongVault` to
   create a custom Kong vault and reference the values in configurations of
-  plugins. Reference of using Kong vaults: [Kong vault]
+  plugins. Reference of using Kong vaults: [Kong vault]. Since the prefix of
+  Kong vault is restrained unique, the `spec.prefix` field is set to immutable,
+  and only one of multiple `KongVault`s with the same `spec.prefix` will get
+  translated. Translation failiure events will be recorded for others with
+  duplicating `spec.prefix`.  
   [#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)
 
 ### Fixed
 

config/crd/bases/configuration.konghq.com_kongvaults.yaml

@@ -82,8 +82,9 @@ spec:
                 description: Description is the additional information about the vault.
                 type: string
               prefix:
-                description: Prefix is the prefix of vault URI for referencing values
-                  in the vault.
+                description: |-
+                  Prefix is the prefix of vault URI for referencing values in the vault.
+                  It is immutable after created.
                 minLength: 1
                 type: string
             required:
@@ -187,6 +188,9 @@ spec:
         required:
         - spec
         type: object
+        x-kubernetes-validations:
+        - message: The spec.prefix field is immutable
+          rule: self.spec.prefix == oldSelf.spec.prefix
     served: true
     storage: true
     subresources:

internal/dataplane/kongstate/kongstate.go

@@ -18,6 +18,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
+	kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
 )
 
@@ -257,17 +258,58 @@ func (ks *KongState) FillUpstreamOverrides(
 	}
 }
 
+// compareKongVault compares two `KongVault`s when they have the same `spec.prefix`.
+// When 2 or more KongVaults have the same prefix, only one of them is translated.
+// It returns true when v1 has higher priority then v2, by the following order:
+// - The one created earlier (earlier `creationTimestamp`) takes precedence.
+// - If the creationTimestamp equals, the one with smaller lexical order (`<` for strings) takes precedence.
+func compareKongVault(v1, v2 *kongv1alpha1.KongVault) bool {
+	if v1.CreationTimestamp.Before(&v2.CreationTimestamp) {
+		return true
+	}
+	if v2.CreationTimestamp.Before(&v1.CreationTimestamp) {
+		return false
+	}
+	// None of them can be seen created before the other (equal or not comparable), compare by lexical order of name.
+	return v1.Name < v2.Name
+}
+
 func (ks *KongState) FillVaults(
 	logger logr.Logger,
 	s store.Storer,
 	failuresCollector *failures.ResourceFailuresCollector,
 ) {
-	for _, vault := range s.ListKongVaults() {
+	// List all vaults and reject the KongVaults with duplicate prefix to prevent invalid Kong configuration generated.
+	allKongVaults := s.ListKongVaults()
+	prefixToKongVault := map[string]*kongv1alpha1.KongVault{}
+	for _, vault := range allKongVaults {
+		prefix := vault.Spec.Prefix
+		existingVault, ok := prefixToKongVault[prefix]
+		if !ok {
+			prefixToKongVault[prefix] = vault
+			continue
+		}
+		// ok == true, which means we have KongVaults with same spec.prefix
+		if compareKongVault(existingVault, vault) {
+			// the one already in the map has higher priority, the current KongVault is rejected.
+			failuresCollector.PushResourceFailure(
+				fmt.Sprintf("spec.prefix %q is duplicate", prefix), vault,
+			)
+		} else {
+			// the current vault has the higher priority, the existing one is rejected and taken out of the map.
+			prefixToKongVault[prefix] = vault
+			failuresCollector.PushResourceFailure(
+				fmt.Sprintf("spec.prefix %q is duplicate", prefix), existingVault,
+			)
+		}
+	}
+
+	for _, vault := range prefixToKongVault {
 		config, err := RawConfigToConfiguration(vault.Spec.Config.Raw)
 		if err != nil {
 			logger.Error(err, "failed to parse configuration of vault to JSON", "name", vault.Name)
 			failuresCollector.PushResourceFailure(
-				fmt.Sprintf("failed to parse configuration of vault %s to JSON: %v", vault.Name, err),
+				fmt.Sprintf("failed to parse configuration of vault %q to JSON: %v", vault.Name, err),
 				vault,
 			)
 			continue

internal/dataplane/kongstate/kongstate_test.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/logr/testr"
@@ -16,9 +17,11 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures"
@@ -1198,6 +1201,7 @@ func TestFillVaults(t *testing.T) {
 		APIVersion: kongv1alpha1.GroupVersion.String(),
 		Kind:       "KongVault",
 	}
+	now := time.Now()
 	testCases := []struct {
 		name                     string
 		kongVaults               []*kongv1alpha1.KongVault
@@ -1281,6 +1285,94 @@ func TestFillVaults(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "KongVault with invalid configuration is rejected",
+			kongVaults: []*kongv1alpha1.KongVault{
+				{
+					TypeMeta: kongVaultTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vault-invalid",
+						Annotations: map[string]string{
+							annotations.IngressClassKey: annotations.DefaultIngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend: "env",
+						Prefix:  "env-1",
+						Config: apiextensionsv1.JSON{
+							Raw: []byte(`{{}`),
+						},
+					},
+				},
+			},
+			expectedTranslationFailures: map[string]string{
+				"vault-invalid": `failed to parse configuration of vault "vault-invalid" to JSON`,
+			},
+		},
+		{
+			name: "multiple KongVaults with same spec.prefix, only one translated and translation failure for the other",
+			kongVaults: []*kongv1alpha1.KongVault{
+				{
+					TypeMeta: kongVaultTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "vault-0-newer",
+						CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Second)),
+						Annotations: map[string]string{
+							annotations.IngressClassKey: annotations.DefaultIngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend: "env",
+						Prefix:  "env-1",
+					},
+				},
+				{
+					TypeMeta: kongVaultTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "vault-1",
+						CreationTimestamp: metav1.NewTime(now.Add(-10 * time.Second)),
+						Annotations: map[string]string{
+							annotations.IngressClassKey: annotations.DefaultIngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend: "env",
+						Prefix:  "env-1",
+					},
+				},
+				{
+					TypeMeta: kongVaultTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "vault-2",
+						CreationTimestamp: metav1.NewTime(now.Add(-10 * time.Second)),
+						Annotations: map[string]string{
+							annotations.IngressClassKey: annotations.DefaultIngressClass,
+						},
+					},
+					Spec: kongv1alpha1.KongVaultSpec{
+						Backend: "env",
+						Prefix:  "env-1",
+					},
+				},
+			},
+			expectedTranslatedVaults: []Vault{
+				{
+					Vault: kong.Vault{
+						Name:   kong.String("env"),
+						Prefix: kong.String("env-1"),
+					},
+					K8sKongVault: &kongv1alpha1.KongVault{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "vault-1",
+						},
+					},
+				},
+			},
+			expectedTranslationFailures: map[string]string{
+				"vault-0-newer": `spec.prefix "env-1" is duplicate`,
+				"vault-2":       `spec.prefix "env-1" is duplicate`,
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1307,8 +1399,17 @@ func TestFillVaults(t *testing.T) {
 				)
 			}
 
-			// TODO: check translation failures after we implement translation failure events for cluster scoped objects:
-			// https://github.com/Kong/kubernetes-ingress-controller/issues/5387
+			translationFailures := f.PopResourceFailures()
+			for name, message := range tc.expectedTranslationFailures {
+				assert.Truef(t, lo.ContainsBy(translationFailures, func(failure failures.ResourceFailure) bool {
+					return strings.Contains(failure.Message(), message) &&
+						lo.ContainsBy(failure.CausingObjects(), func(obj client.Object) bool {
+							return obj.GetName() == name
+						})
+				}),
+					"cannot find expected translation failure for KongVault %s", name,
+				)
+			}
 		})
 	}
 }

pkg/apis/configuration/v1alpha1/kong_vault_types.go

@@ -37,6 +37,7 @@ const (
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="Age"
 // +kubebuilder:printcolumn:name="Description",type=string,JSONPath=`.spec.description`,description="Description",priority=1
 // +kubebuilder:printcolumn:name="Programmed",type=string,JSONPath=`.status.conditions[?(@.type=="Programmed")].status`
+// +kubebuilder:validation:XValidation:rule="self.spec.prefix == oldSelf.spec.prefix", message="The spec.prefix field is immutable"
 
 // KongVault is the schema for kongvaults API which defines a custom Kong vault.
 // A Kong vault is a storage to store sensitive data, where the values can be referenced in configuration of plugins.
@@ -56,6 +57,7 @@ type KongVaultSpec struct {
 	// +kubebuilder:validation:MinLength=1
 	Backend string `json:"backend"`
 	// Prefix is the prefix of vault URI for referencing values in the vault.
+	// It is immutable after created.
 	// +kubebuilder:validation:MinLength=1
 	Prefix string `json:"prefix"`
 	// Description is the additional information about the vault.