add unit test for FillVault and validateVault

Kong · Dec 29, 2023 · b5effab · b5effab
1 parent 0140e3b
commit b5effab
Show file tree

Hide file tree

Showing 9 changed files with 299 additions and 14 deletions.
diff --git a/go.mod b/go.mod
@@ -30,6 +30,9 @@ require (
 	github.com/google/uuid v1.5.0
 	github.com/jpillora/backoff v1.0.0
 	github.com/kong/go-database-reconciler v1.1.0
+	// TODO: Update to latest release version after the following PR merged:
+	// https://github.com/Kong/go-kong/pull/391
+	// https://github.com/Kong/go-kong/pull/392
 	github.com/kong/go-kong v0.48.1-0.20231228093107-7e35dcb56df9
 	github.com/kong/kubernetes-telemetry v0.1.3
 	github.com/kong/kubernetes-testing-framework v0.43.0

diff --git a/internal/admission/handler_test.go b/internal/admission/handler_test.go
@@ -21,7 +21,6 @@ import (
 	ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
-	kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
 )
 
@@ -38,10 +37,6 @@ var (
 		APIVersion: kongv1.GroupVersion.String(),
 		Kind:       "KongClusterPlugin",
 	}
-	kongVaultMeta = metav1.TypeMeta{
-		APIVersion: kongv1alpha1.GroupVersion.String(),
-		Kind:       "KongVault",
-	}
 )
 
 func TestHandleKongIngress(t *testing.T) {

diff --git a/internal/admission/validator.go b/internal/admission/validator.go
@@ -538,6 +538,8 @@ func (validator KongHTTPValidator) ValidateVault(ctx context.Context, k8sKongVau
 		Description: kong.String(k8sKongVault.Spec.Description),
 		Config:      config,
 	}
+	// TODO: /schemas/vaults/test does not check "unique" restraint on `prefix` field.
+	// We may need implement the uniqueness check of `prefix`.
 	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
@@ -28,6 +28,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util/builder"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
+	kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
 	incubatorv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/incubator/v1alpha1"
 )
@@ -101,7 +102,7 @@ func (f fakeServicesProvider) GetRoutesService() (kong.AbstractRouteService, boo
 }
 
 func (f fakeServicesProvider) GetVaultsService() (kong.AbstractVaultService, bool) {
-	if f.routeSvc != nil {
+	if f.vaultSvc != nil {
 		return f.vaultSvc, true
 	}
 	return nil, false
@@ -1235,3 +1236,90 @@ func (f *fakeRouteSvc) Validate(context.Context, *kong.Route) (bool, string, err
 	}
 	return true, "", nil
 }
+
+func TestValidator_ValidateVault(t *testing.T) {
+	testCases := []struct {
+		name            string
+		kongVault       kongv1alpha1.KongVault
+		validateSvcFail bool
+		expectedOK      bool
+		expectedMessage string
+		expectedError   string
+	}{
+		{
+			name: "valid vault",
+			kongVault: kongv1alpha1.KongVault{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vault-1",
+				},
+				Spec: kongv1alpha1.KongVaultSpec{
+					Backend: "env",
+					Prefix:  "env-1",
+				},
+			},
+			expectedOK: true,
+		},
+		{
+			name: "vault with invalid(malformed) configuration",
+			kongVault: kongv1alpha1.KongVault{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vault-1",
+				},
+				Spec: kongv1alpha1.KongVaultSpec{
+					Backend: "env",
+					Prefix:  "env-1",
+					Config: apiextensionsv1.JSON{
+						Raw: []byte(`{{}`),
+					},
+				},
+			},
+			expectedOK:      false,
+			expectedMessage: "failed to unmarshal validate configurations",
+		},
+		{
+			name: "vault with failure in validating service",
+			kongVault: kongv1alpha1.KongVault{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vault-1",
+				},
+				Spec: kongv1alpha1.KongVaultSpec{
+					Backend: "env",
+					Prefix:  "env-1",
+				},
+			},
+			validateSvcFail: true,
+			expectedOK:      false,
+			expectedMessage: "something is wrong with the vault",
+		},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			validator := KongHTTPValidator{
+				AdminAPIServicesProvider: fakeServicesProvider{
+					vaultSvc: &fakeVaultSvc{
+						shouldFail: tc.validateSvcFail,
+					},
+				},
+				ingressClassMatcher: fakeClassMatcher,
+				Logger:              logr.Discard(),
+			}
+			ok, msg, err := validator.ValidateVault(context.Background(), tc.kongVault)
+			require.NoError(t, err)
+			assert.Equal(t, tc.expectedOK, ok)
+			assert.Contains(t, msg, tc.expectedMessage)
+		})
+	}
+}
+
+type fakeVaultSvc struct {
+	kong.AbstractVaultService
+	shouldFail bool
+}
+
+func (s fakeVaultSvc) Validate(context.Context, *kong.Vault) (bool, string, error) {
+	if s.shouldFail {
+		return false, "something is wrong with the vault", nil
+	}
+	return true, "", nil
+}
diff --git a/internal/dataplane/failures/failures.go b/internal/dataplane/failures/failures.go
@@ -41,9 +41,13 @@ func NewResourceFailure(reason string, causingObjects ...client.Object) (Resourc
 		if obj.GetName() == "" {
 			return ResourceFailure{}, fmt.Errorf("one of causing objects (%s) has no name", gvk.String())
 		}
-		if obj.GetNamespace() == "" {
-			return ResourceFailure{}, fmt.Errorf("one of causing objects (%s) has no namespace", gvk.String())
-		}
+		// REVIEW: should we remove this to allow generating resource failures for cluster scope objects like KongClusterPlugins?
+		// https://github.com/Kong/kubernetes-ingress-controller/issues/5387
+		/*
+			if obj.GetNamespace() == "" {
+				return ResourceFailure{}, fmt.Errorf("one of causing objects (%s) has no namespace", gvk.String())
+			}
+		*/
 	}
 
 	return ResourceFailure{

diff --git a/internal/dataplane/failures/failures_test.go b/internal/dataplane/failures/failures_test.go
@@ -54,11 +54,14 @@ func TestResourceFailure(t *testing.T) {
 		noName.Name = ""
 		_, err = NewResourceFailure(someValidResourceFailureReason, noName)
 		assert.Error(t, err, "expected an empty name object to be rejected")
-
-		noNamespace := validCausingObject()
-		noNamespace.Namespace = ""
-		_, err = NewResourceFailure(someValidResourceFailureReason, noNamespace)
-		assert.Error(t, err, "expected an empty namespace object to be rejected")
+		// REVIEW: should we remove this to allow generating resource failures for cluster scope objects like KongClusterPlugins?
+		// https://github.com/Kong/kubernetes-ingress-controller/issues/5387
+		/*
+			noNamespace := validCausingObject()
+			noNamespace.Namespace = ""
+			_, err = NewResourceFailure(someValidResourceFailureReason, noNamespace)
+			assert.Error(t, err, "expected an empty namespace object to be rejected")
+		*/
 	})
 }
 

diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go
@@ -520,6 +520,7 @@ func (ks *KongState) FillIDs(logger logr.Logger) {
 	}
 
 	// TODO: Add FillID() for vaults in go-kong to fill IDs for vaults.
+	// https://github.com/Kong/go-kong/pull/391
 }
 
 // maybeLogKongIngressDeprecationError iterates over services and logs a deprecation error if a service