add integration test on webhook and fix setup

Kong · Dec 1, 2023 · 0093f43 · 0093f43
1 parent 8c3c38a
commit 0093f43
Show file tree

Hide file tree

Showing 7 changed files with 297 additions and 28 deletions.
diff --git a/internal/admission/handler.go b/internal/admission/handler.go
@@ -20,6 +20,11 @@ import (
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
 )
 
+const (
+	KindKongPlugin        = "KongPlugin"
+	KindKongClusterPlugin = "KongClusterPlugin"
+)
+
 // RequestHandler is an HTTP server that can validate Kong Ingress Controllers'
 // Custom Resources using Kubernetes Admission Webhooks.
 type RequestHandler struct {
@@ -264,7 +269,7 @@ func (h RequestHandler) handleSecret(
 		for _, obj := range referrers {
 			gvk := obj.GetObjectKind().GroupVersionKind()
 			// REVIEW: Should we check version here? Seems that we do not need to support KongPlugin and KongClusterPlugin in other versions.
-			if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == "KongPlugin" {
+			if gvk.Group == kongv1.GroupVersion.Group && gvk.Kind == KindKongPlugin {
 				// REVIEW: run type check here to avoid panic, although it should be unlikely to happen after checking the GVK?
 				plugin := obj.(*kongv1.KongPlugin)
 				ok, message, err := h.Validator.ValidatePlugin(ctx, *plugin, []*corev1.Secret{&secret})
@@ -280,7 +285,7 @@ func (h RequestHandler) handleSecret(
 						)).Build(), nil
 				}
 			}
-			if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == "KongClusterPlugin" {
+			if gvk.Group == kongv1.GroupVersion.Group && gvk.Kind == KindKongClusterPlugin {
 				plugin := obj.(*kongv1.KongClusterPlugin)
 				ok, message, err := h.Validator.ValidateClusterPlugin(ctx, *plugin, []*corev1.Secret{&secret})
 				if err != nil {

diff --git a/internal/admission/validator.go b/internal/admission/validator.go
@@ -56,14 +56,14 @@ type ConsumerGetter interface {
 	ListAllConsumers(ctx context.Context) ([]kongv1.KongConsumer, error)
 }
 
-var _ kongstate.SecretGetter = SecretGetterWithOverride{}
+var _ kongstate.SecretGetter = &SecretGetterWithOverride{}
 
 type SecretGetterWithOverride struct {
 	overrideSecrets map[k8stypes.NamespacedName]*corev1.Secret
 	secretGetter    kongstate.SecretGetter
 }
 
-func (s SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Secret, error) {
+func (s *SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Secret, error) {
 	nsName := k8stypes.NamespacedName{
 		Namespace: namespace,
 		Name:      name,
@@ -76,15 +76,16 @@ func (s SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Sec
 	return s.secretGetter.GetSecret(namespace, name)
 }
 
-func NewSecretGetterWithOverride(s kongstate.SecretGetter, overrideSecrets []*corev1.Secret) SecretGetterWithOverride {
-	return SecretGetterWithOverride{
-		overrideSecrets: lo.SliceToMap(overrideSecrets, func(secret *corev1.Secret) (k8stypes.NamespacedName, *corev1.Secret) {
-			return k8stypes.NamespacedName{
-				Namespace: secret.Namespace,
-				Name:      secret.Name,
-			}, secret
-		}),
-		secretGetter: s,
+func NewSecretGetterWithOverride(s kongstate.SecretGetter, overrideSecrets []*corev1.Secret) *SecretGetterWithOverride {
+	overrideSecretMap := lo.SliceToMap(overrideSecrets, func(secret *corev1.Secret) (k8stypes.NamespacedName, *corev1.Secret) {
+		return k8stypes.NamespacedName{
+			Namespace: secret.Namespace,
+			Name:      secret.Name,
+		}, secret.DeepCopy()
+	})
+	return &SecretGetterWithOverride{
+		overrideSecrets: overrideSecretMap,
+		secretGetter:    s,
 	}
 }
 
@@ -336,7 +337,7 @@ func (validator KongHTTPValidator) ValidatePlugin(
 		return false, fmt.Sprintf("%s: %s", ErrTextPluginConfigInvalid, err), nil
 	}
 	if k8sPlugin.ConfigFrom != nil {
-		config, err := kongstate.SecretToConfiguration(validator.SecretGetter, (*k8sPlugin.ConfigFrom).SecretValue, k8sPlugin.Namespace)
+		config, err := kongstate.SecretToConfiguration(secretGetter, (*k8sPlugin.ConfigFrom).SecretValue, k8sPlugin.Namespace)
 		if err != nil {
 			return false, fmt.Sprintf("%s: %s", ErrTextPluginSecretConfigUnretrievable, err), nil
 		}

diff --git a/internal/admission/validator_test.go b/internal/admission/validator_test.go
@@ -114,7 +114,8 @@ func TestKongHTTPValidator_ValidatePlugin(t *testing.T) {
 		},
 	})
 	type args struct {
-		plugin kongv1.KongPlugin
+		plugin          kongv1.KongPlugin
+		overrideSecrets []*corev1.Secret
 	}
 	tests := []struct {
 		name        string
@@ -239,6 +240,34 @@ func TestKongHTTPValidator_ValidatePlugin(t *testing.T) {
 			wantMessage: ErrTextPluginConfigValidationFailed,
 			wantErr:     true,
 		},
+		{
+			name:      "validate from override secret which generates valid configuration",
+			PluginSvc: &fakePluginSvc{valid: true},
+			args: args{
+				plugin: kongv1.KongPlugin{
+					PluginName: "key-auth",
+					ConfigFrom: &kongv1.ConfigSource{
+						SecretValue: kongv1.SecretValueFromSource{
+							Key:    "valid-conf",
+							Secret: "another-conf-secret",
+						},
+					},
+				},
+				overrideSecrets: []*corev1.Secret{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "",
+							Name:      "another-conf-secret",
+						},
+						Data: map[string][]byte{
+							"valid-conf":   []byte(`{"foo":"bar"}`),
+							"invalid-conf": []byte(`{"foo":"baz}`),
+						},
+					},
+				},
+			},
+			wantOK: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -249,7 +278,7 @@ func TestKongHTTPValidator_ValidatePlugin(t *testing.T) {
 				},
 				ingressClassMatcher: fakeClassMatcher,
 			}
-			gotOK, gotMessage, err := validator.ValidatePlugin(context.Background(), tt.args.plugin, nil)
+			gotOK, gotMessage, err := validator.ValidatePlugin(context.Background(), tt.args.plugin, tt.args.overrideSecrets)
 			assert.Equalf(t, tt.wantOK, gotOK,
 				"KongHTTPValidator.ValidatePlugin() want OK: %v, got OK: %v",
 				tt.wantOK, gotOK,
@@ -284,7 +313,8 @@ func TestKongHTTPValidator_ValidateClusterPlugin(t *testing.T) {
 		},
 	})
 	type args struct {
-		plugin kongv1.KongClusterPlugin
+		plugin          kongv1.KongClusterPlugin
+		overrideSecrets []*corev1.Secret
 	}
 	tests := []struct {
 		name        string
@@ -412,6 +442,35 @@ func TestKongHTTPValidator_ValidateClusterPlugin(t *testing.T) {
 			wantMessage: ErrTextPluginConfigValidationFailed,
 			wantErr:     true,
 		},
+		{
+			name:      "validate from override secret which generates valid configuration",
+			PluginSvc: &fakePluginSvc{valid: true},
+			args: args{
+				plugin: kongv1.KongClusterPlugin{
+					PluginName: "key-auth",
+					ConfigFrom: &kongv1.NamespacedConfigSource{
+						SecretValue: kongv1.NamespacedSecretValueFromSource{
+							Namespace: "default",
+							Key:       "valid-conf",
+							Secret:    "another-conf-secret",
+						},
+					},
+				},
+				overrideSecrets: []*corev1.Secret{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "default",
+							Name:      "another-conf-secret",
+						},
+						Data: map[string][]byte{
+							"valid-conf":   []byte(`{"foo":"bar"}`),
+							"invalid-conf": []byte(`{"foo":"baz}`),
+						},
+					},
+				},
+			},
+			wantOK: true,
+		},
 		{
 			name:      "no gateway was available at the time of validation",
 			PluginSvc: nil, // no plugin service is available as there's no gateways
@@ -433,7 +492,7 @@ func TestKongHTTPValidator_ValidateClusterPlugin(t *testing.T) {
 				ingressClassMatcher: fakeClassMatcher,
 			}
 
-			gotOK, gotMessage, err := validator.ValidateClusterPlugin(context.Background(), tt.args.plugin, nil)
+			gotOK, gotMessage, err := validator.ValidateClusterPlugin(context.Background(), tt.args.plugin, tt.args.overrideSecrets)
 			assert.Equalf(t, tt.wantOK, gotOK,
 				"KongHTTPValidator.ValidateClusterPlugin() want OK: %v, got OK: %v",
 				tt.wantOK, gotOK,

diff --git a/internal/manager/controllerdef.go b/internal/manager/controllerdef.go
@@ -58,6 +58,7 @@ func setupControllers(
 	ctx context.Context,
 	mgr manager.Manager,
 	dataplaneClient controllers.DataPlane,
+	referenceIndexers ctrlref.CacheIndexers,
 	dataplaneAddressFinder *dataplane.AddressFinder,
 	udpDataplaneAddressFinder *dataplane.AddressFinder,
 	kubernetesStatusQueue *status.Queue,
@@ -66,8 +67,6 @@ func setupControllers(
 	kongAdminAPIEndpointsNotifier configuration.EndpointsNotifier,
 	adminAPIsDiscoverer configuration.AdminAPIsDiscoverer,
 ) []ControllerDef {
-	referenceIndexers := ctrlref.NewCacheIndexers(ctrl.LoggerFrom(ctx).WithName("controllers").WithName("reference-indexers"))
-
 	controllers := []ControllerDef{
 		// ---------------------------------------------------------------------------
 		// Kong Gateway Admin API Service discovery

diff --git a/internal/manager/run.go b/internal/manager/run.go
@@ -21,6 +21,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/clients"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/gateway"
+	ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane"
 	dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/configfetcher"
@@ -165,6 +166,13 @@ func Run(
 		c.UpdateStatus,
 	)
 
+	referenceIndexers := ctrlref.NewCacheIndexers(setupLog)
+
+	setupLog.Info("Starting Admission Server")
+	if err := setupAdmissionServer(ctx, c, clientsManager, referenceIndexers, mgr.GetClient(), logger, translatorFeatureFlags); err != nil {
+		return err
+	}
+
 	cache := store.NewCacheStores()
 	storer := store.New(cache, c.IngressClassName, logger)
 	configTranslator, err := translator.NewTranslator(
@@ -228,6 +236,7 @@ func Run(
 		ctx,
 		mgr,
 		dataplaneClient,
+		referenceIndexers,
 		dataplaneAddressFinder,
 		udpDataplaneAddressFinder,
 		kubernetesStatusQueue,

diff --git a/internal/manager/setup.go b/internal/manager/setup.go
@@ -25,6 +25,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/admission"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/clients"
+	ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane"
 	dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
@@ -170,6 +171,7 @@ func setupAdmissionServer(
 	ctx context.Context,
 	managerConfig *Config,
 	clientsManager *clients.AdminAPIClientsManager,
+	referenceIndexers ctrlref.CacheIndexers,
 	managerClient client.Client,
 	logger logr.Logger,
 	translatorFeatures translator.FeatureFlags,
@@ -192,7 +194,8 @@ func setupAdmissionServer(
 			translatorFeatures,
 			storer,
 		),
-		Logger: admissionLogger,
+		ReferenceIndexers: referenceIndexers,
+		Logger:            admissionLogger,
 	}, admissionLogger)
 	if err != nil {
 		return err