feat: add validation on secrets on generated plugin cofigs

CHANGELOG.md

@@ -104,12 +104,19 @@ Adding a new version? You'll need three changes:
 - Added support for GRPC over HTTP (without TLS) in Gateway API.
   [#5128](https://github.com/Kong/kubernetes-ingress-controller/pull/5128)
   [#5283](https://github.com/Kong/kubernetes-ingress-controller/pull/5283)
-- Added `-init-cache-sync-duration` CLI flag. This flag configures how long the controller waits for Kubernetes resources to populate at startup before generating the initial Kong configuration. It also fixes a bug that removed the default 5 second wait period.
+- Added `--init-cache-sync-duration` CLI flag. This flag configures how long the
+  controller waits for Kubernetes resources to populate at startup before
+  generating the initial Kong configuration. It also fixes a bug that removed
+  the default 5 second wait period.
   [#5238](https://github.com/Kong/kubernetes-ingress-controller/pull/5238)
 - Added `--emit-kubernetes-events` CLI flag to disable the creation of events
   in translating and applying configurations to Kong.
   [#5296](https://github.com/Kong/kubernetes-ingress-controller/pull/5296)
   [#5299](https://github.com/Kong/kubernetes-ingress-controller/pull/5299)
+ - Added validation on `Secret`s to reject the change if it will generate
+   invalid confiugration of plugins for `KongPlugin`s or `KongClusterPlugin`s
+   referencing to the secret.
+   [#5203](https://github.com/Kong/kubernetes-ingress-controller/pull/5203)
 
 ### Fixed
 

internal/admission/handler.go

@@ -13,18 +13,28 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
+	ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
 	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 {
 	// Validator validates the entities that the k8s API-server asks
 	// it the server to validate.
 	Validator KongValidator
+	// ReferenceIndexers gets the resources (KongPlugin and KongClusterPlugin)
+	// referring the validated resource (Secret) to check the changes on
+	// referred Secret will produce invalid configuration of the plugins.
+	ReferenceIndexers ctrlref.CacheIndexers
 
 	Logger logr.Logger
 }
@@ -203,7 +213,7 @@ func (h RequestHandler) handleKongPlugin(
 		return nil, err
 	}
 
-	ok, message, err := h.Validator.ValidatePlugin(ctx, plugin)
+	ok, message, err := h.Validator.ValidatePlugin(ctx, plugin, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -222,7 +232,7 @@ func (h RequestHandler) handleKongClusterPlugin(
 		return nil, err
 	}
 
-	ok, message, err := h.Validator.ValidateClusterPlugin(ctx, plugin)
+	ok, message, err := h.Validator.ValidateClusterPlugin(ctx, plugin, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -240,24 +250,77 @@ func (h RequestHandler) handleSecret(
 	if err != nil {
 		return nil, err
 	}
-	// TODO so long as we still handle the deprecated field, this has to remain
-	// once the deprecated field is removed, we must replace this with a label filter in the webhook itself
-	// https://github.com/Kong/kubernetes-ingress-controller/issues/4853
-
-	if _, credentialTypeSource := util.ExtractKongCredentialType(&secret); credentialTypeSource == util.CredentialTypeAbsent {
-		// secret does not look like a credential resource in Kong
-		return responseBuilder.Allowed(true).Build(), nil
-	}
 
 	switch request.Operation {
 	case admissionv1.Update, admissionv1.Create:
-		ok, message := h.Validator.ValidateCredential(ctx, secret)
-		return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
+		// TODO so long as we still handle the deprecated field, this has to remain
+		// once the deprecated field is removed, we must replace this with a label filter in the webhook itself
+		// https://github.com/Kong/kubernetes-ingress-controller/issues/4853
+		if _, credentialTypeSource := util.ExtractKongCredentialType(&secret); credentialTypeSource != util.CredentialTypeAbsent {
+			ok, message := h.Validator.ValidateCredential(ctx, secret)
+			if !ok {
+				return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
+			}
+		}
+
+		ok, message, err := h.checkReferrersOfSecret(ctx, &secret)
+		if err != nil {
+			return responseBuilder.Allowed(false).WithMessage(fmt.Sprintf("failed to validate other objects referencing the secret: %v", err)).Build(), err
+		}
+		if !ok {
+			return responseBuilder.Allowed(false).WithMessage(message).Build(), nil
+		}
+
+		return responseBuilder.Allowed(true).Build(), nil
+
 	default:
 		return nil, fmt.Errorf("unknown operation %q", string(request.Operation))
 	}
 }
 
+// checkReferrersOfSecret validates all referrers (KongPlugins and KongClusterPlugins) of the secret
+// and rejects the secret if it generates invalid configurations for any of the referrers.
+func (h RequestHandler) checkReferrersOfSecret(ctx context.Context, secret *corev1.Secret) (bool, string, error) {
+	referrers, err := h.ReferenceIndexers.ListReferrerObjectsByReferent(secret)
+	if err != nil {
+		return false, "", fmt.Errorf("failed to list referrers of secret: %w", err)
+	}
+
+	for _, obj := range referrers {
+		gvk := obj.GetObjectKind().GroupVersionKind()
+		if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongPlugin {
+			plugin := obj.(*kongv1.KongPlugin)
+			ok, message, err := h.Validator.ValidatePlugin(ctx, *plugin, []*corev1.Secret{secret})
+			if err != nil {
+				return false, "", fmt.Errorf("failed to run validation on KongPlugin %s/%s: %w",
+					plugin.Namespace, plugin.Name, err,
+				)
+			}
+			if !ok {
+				return false,
+					fmt.Sprintf("Change on secret will generate invalid configuration for KongPlugin %s/%s: %s",
+						plugin.Namespace, plugin.Name, message,
+					), nil
+			}
+		}
+		if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongClusterPlugin {
+			plugin := obj.(*kongv1.KongClusterPlugin)
+			ok, message, err := h.Validator.ValidateClusterPlugin(ctx, *plugin, []*corev1.Secret{secret})
+			if err != nil {
+				return false, "", fmt.Errorf("failed to run validation on KongClusterPlugin %s: %w",
+					plugin.Name, err,
+				)
+			}
+			if !ok {
+				return false, fmt.Sprintf("Change on secret will generate invalid configuration for KongClusterPlugin %s: %s",
+					plugin.Name, message,
+				), nil
+			}
+		}
+	}
+	return true, "", nil
+}
+
 func (h RequestHandler) handleGateway(
 	ctx context.Context,
 	request admissionv1.AdmissionRequest,

internal/admission/handler_test.go

@@ -4,21 +4,41 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
 	"testing"
 
 	"github.com/go-logr/logr"
+	"github.com/go-logr/logr/testr"
 	"github.com/stretchr/testify/require"
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	k8stypes "k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
+	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"
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
 )
 
+var (
+	secretTypeMeta = metav1.TypeMeta{
+		APIVersion: "v1",
+		Kind:       "Secret",
+	}
+	kongPluginTypeMeta = metav1.TypeMeta{
+		APIVersion: kongv1.GroupVersion.String(),
+		Kind:       "KongPlugin",
+	}
+	kongClusterPluginTypeMeta = metav1.TypeMeta{
+		APIVersion: kongv1.GroupVersion.String(),
+		Kind:       "KongClusterPlugin",
+	}
+)
+
 func TestHandleKongIngress(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -130,7 +150,6 @@ func TestHandleService(t *testing.T) {
 					Raw:    raw,
 				},
 			}
-
 			handler := RequestHandler{
 				Validator: validator,
 				Logger:    logr.Discard(),
@@ -145,3 +164,178 @@ func TestHandleService(t *testing.T) {
 		})
 	}
 }
+
+func TestHandleSecret(t *testing.T) {
+	testCases := []struct {
+		name             string
+		secret           *corev1.Secret
+		referrers        []client.Object
+		validatorOK      bool
+		validatorMessage string
+		validatorError   error
+		expectAllowed    bool
+		expectStatusCode int32
+		expectMessage    string
+		expectError      bool
+	}{
+		{
+			name: "secret used as a credential and passes the validation of credential",
+			secret: &corev1.Secret{
+				TypeMeta: secretTypeMeta,
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "credential-0",
+					Labels: map[string]string{
+						"konghq.com/credential": "true",
+					},
+				},
+				Data: map[string][]byte{},
+			},
+			validatorOK:   true,
+			expectAllowed: true,
+		},
+		{
+			name: "secret used as credential and fails the validation of credential",
+			secret: &corev1.Secret{
+				TypeMeta: secretTypeMeta,
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "credential-1",
+					Labels: map[string]string{
+						"konghq.com/credential": "true",
+					},
+				},
+				Data: map[string][]byte{},
+			},
+			validatorOK:      false,
+			validatorMessage: "invalid credential",
+			expectAllowed:    false,
+			expectStatusCode: http.StatusBadRequest,
+			expectMessage:    "invalid credential",
+		},
+		{
+			name: "secret used as KongPlugin config and KongClusterPlugin and passes validation of both CRDs",
+			secret: &corev1.Secret{
+				TypeMeta: secretTypeMeta,
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "plugin-conf",
+				},
+			},
+			referrers: []client.Object{
+				&kongv1.KongPlugin{
+					TypeMeta: kongPluginTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "default",
+						Name:      "plugin-0",
+					},
+					PluginName: "test-plugin",
+				},
+				&kongv1.KongClusterPlugin{
+					TypeMeta: kongClusterPluginTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "cluster-plugin-0",
+					},
+					PluginName: "test-plugin",
+				},
+			},
+			validatorOK:   true,
+			expectAllowed: true,
+		},
+		{
+			name: "secret used as KongPlugin config and fails validation of KongPlugin",
+			secret: &corev1.Secret{
+				TypeMeta: secretTypeMeta,
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "plugin-conf",
+				},
+			},
+			referrers: []client.Object{
+				&kongv1.KongPlugin{
+					TypeMeta: kongPluginTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "default",
+						Name:      "plugin-0",
+					},
+					PluginName: "test-plugin",
+				},
+			},
+			validatorOK:      false,
+			validatorMessage: "invalid KongPlugin",
+			expectAllowed:    false,
+			expectStatusCode: http.StatusBadRequest,
+			expectMessage:    "Change on secret will generate invalid configuration for KongPlugin default/plugin-0: invalid KongPlugin",
+		},
+		{
+			name: "secret used as KongClusterPlugin config and fails validation of KongClusterPlugin",
+			secret: &corev1.Secret{
+				TypeMeta: secretTypeMeta,
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "plugin-conf",
+				},
+			},
+			referrers: []client.Object{
+				&kongv1.KongClusterPlugin{
+					TypeMeta: kongClusterPluginTypeMeta,
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "cluster-plugin-0",
+					},
+					PluginName: "test-plugin",
+				},
+			},
+			validatorOK:      false,
+			validatorMessage: "invalid KongClusterPlugin",
+			expectAllowed:    false,
+			expectStatusCode: http.StatusBadRequest,
+			expectMessage:    "Change on secret will generate invalid configuration for KongClusterPlugin cluster-plugin-0: invalid KongClusterPlugin",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			validator := KongFakeValidator{
+				Result:  tc.validatorOK,
+				Message: tc.validatorMessage,
+				Error:   tc.validatorError,
+			}
+			raw, err := json.Marshal(tc.secret)
+			require.NoError(t, err)
+			request := admissionv1.AdmissionRequest{
+				Object: runtime.RawExtension{
+					Object: tc.secret,
+					Raw:    raw,
+				},
+				Operation: admissionv1.Update,
+			}
+
+			logger := testr.NewWithOptions(t, testr.Options{Verbosity: util.DebugLevel})
+			referenceIndexer := ctrlref.NewCacheIndexers(logger)
+
+			handler := RequestHandler{
+				Validator:         validator,
+				Logger:            logger,
+				ReferenceIndexers: referenceIndexer,
+			}
+			for _, obj := range tc.referrers {
+				err := handler.ReferenceIndexers.SetObjectReference(obj, tc.secret)
+				require.NoError(t, err)
+			}
+
+			responseBuilder := NewResponseBuilder(k8stypes.UID(""))
+			got, err := handler.handleSecret(context.Background(), request, responseBuilder)
+			if tc.expectError {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tc.expectAllowed, got.Allowed, "should return expected result of allowed")
+			require.Equal(t, tc.expectStatusCode, got.Result.Code)
+			if len(tc.expectMessage) > 0 {
+				require.Contains(t, got.Result.Message, tc.expectMessage)
+			}
+		})
+	}
+}