[PoC] feat(admission): validate secrets referred by KongPlugins

internal/admission/handler.go

@@ -11,8 +11,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	netv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/reference"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"
 	kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1beta1"
 )
@@ -24,6 +26,8 @@ type RequestHandler struct {
 	// it the server to validate.
 	Validator KongValidator
 
+	ReferenceIndexers reference.CacheIndexers
+
 	Logger logrus.FieldLogger
 }
 
@@ -58,6 +62,19 @@ func (h RequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+var (
+	gvkKongPlugin = schema.GroupVersionKind{
+		Group:   kongv1.GroupVersion.Group,
+		Version: kongv1.GroupVersion.Version,
+		Kind:    "KongPlugin",
+	}
+	gvkKongClusterPlugin = schema.GroupVersionKind{
+		Group:   kongv1.GroupVersion.Group,
+		Version: kongv1.GroupVersion.Version,
+		Kind:    "KongClusterPlugin",
+	}
+)
+
 var (
 	consumerGVResource = metav1.GroupVersionResource{
 		Group:    kongv1.SchemeGroupVersion.Group,
@@ -204,7 +221,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
 	}
@@ -241,6 +258,35 @@ func (h RequestHandler) handleSecret(
 	if err != nil {
 		return nil, err
 	}
+
+	referrers, err := h.ReferenceIndexers.ListObjectsReferredBy(&secret)
+	if err != nil {
+		return nil, err
+	}
+	for _, obj := range referrers {
+		gvk := obj.GetObjectKind().GroupVersionKind()
+		if gvk == gvkKongPlugin {
+			h.Logger.Debug("secret %s/%s referred by KongPlugin %s/%s",
+				secret.Namespace, secret.Name, obj.GetNamespace(), obj.GetName(),
+			)
+
+			plugin, ok := obj.(*kongv1.KongPlugin)
+			if !ok {
+				continue
+			}
+			ok, msg, err := h.Validator.ValidatePlugin(ctx, *plugin, &secret)
+			if err != nil {
+				return nil, err
+			}
+			if !ok {
+				return responseBuilder.Allowed(false).WithMessage(
+					fmt.Sprintf("secret used in KongPlugin %s/%s will generate invalid configuration: %s",
+						plugin.Namespace, plugin.Name, msg),
+				).Build(), nil
+			}
+		}
+	}
+
 	if _, ok := secret.Data["kongCredType"]; !ok {
 		// secret does not look like a credential resource in Kong
 		return responseBuilder.Allowed(true).Build(), nil

internal/admission/server_test.go

@@ -47,6 +47,7 @@ func (v KongFakeValidator) ValidateConsumerGroup(
 func (v KongFakeValidator) ValidatePlugin(
 	_ context.Context,
 	_ kongv1.KongPlugin,
+	_ *corev1.Secret,
 ) (bool, string, error) {
 	return v.Result, v.Message, v.Error
 }

internal/admission/validator.go

@@ -30,7 +30,7 @@ import (
 type KongValidator interface {
 	ValidateConsumer(ctx context.Context, consumer kongv1.KongConsumer) (bool, string, error)
 	ValidateConsumerGroup(ctx context.Context, consumerGroup kongv1beta1.KongConsumerGroup) (bool, string, error)
-	ValidatePlugin(ctx context.Context, plugin kongv1.KongPlugin) (bool, string, error)
+	ValidatePlugin(ctx context.Context, plugin kongv1.KongPlugin, secret *corev1.Secret) (bool, string, error)
 	ValidateClusterPlugin(ctx context.Context, plugin kongv1.KongClusterPlugin) (bool, string, error)
 	ValidateCredential(ctx context.Context, secret corev1.Secret) (bool, string, error)
 	ValidateGateway(ctx context.Context, gateway gatewaycontroller.Gateway) (bool, string, error)
@@ -281,6 +281,17 @@ func (validator KongHTTPValidator) ValidateCredential(
 	return true, "", nil
 }
 
+type SingleSecretGetter struct {
+	secret *corev1.Secret
+}
+
+func (s SingleSecretGetter) GetSecret(namespace, name string) (*corev1.Secret, error) {
+	if namespace == s.secret.Namespace && name == s.secret.Name {
+		return s.secret, nil
+	}
+	return nil, fmt.Errorf("not found")
+}
+
 // ValidatePlugin checks if k8sPlugin is valid. It does so by performing
 // an HTTP request to Kong's Admin API entity validation endpoints.
 // If an error occurs during validation, it is returned as the last argument.
@@ -289,6 +300,7 @@ func (validator KongHTTPValidator) ValidateCredential(
 func (validator KongHTTPValidator) ValidatePlugin(
 	ctx context.Context,
 	k8sPlugin kongv1.KongPlugin,
+	secret *corev1.Secret,
 ) (bool, string, error) {
 	if k8sPlugin.PluginName == "" {
 		return false, ErrTextPluginNameEmpty, nil
@@ -304,7 +316,12 @@ func (validator KongHTTPValidator) ValidatePlugin(
 		if len(plugin.Config) > 0 {
 			return false, ErrTextPluginUsesBothConfigTypes, nil
 		}
-		config, err := kongstate.SecretToConfiguration(validator.SecretGetter, (*k8sPlugin.ConfigFrom).SecretValue, k8sPlugin.Namespace)
+
+		secretGetter := validator.SecretGetter
+		if secret != nil {
+			secretGetter = SingleSecretGetter{secret: secret}
+		}
+		config, err := kongstate.SecretToConfiguration(secretGetter, (*k8sPlugin.ConfigFrom).SecretValue, k8sPlugin.Namespace)
 		if err != nil {
 			return false, ErrTextPluginSecretConfigUnretrievable, err
 		}
@@ -355,7 +372,7 @@ func (validator KongHTTPValidator) ValidateClusterPlugin(
 	} else {
 		derived.ObjectMeta.Namespace = "default"
 	}
-	return validator.ValidatePlugin(ctx, derived)
+	return validator.ValidatePlugin(ctx, derived, nil)
 }
 
 func (validator KongHTTPValidator) ValidateGateway(

internal/admission/validator_test.go

@@ -203,7 +203,7 @@ func TestKongHTTPValidator_ValidatePlugin(t *testing.T) {
 				},
 				ingressClassMatcher: fakeClassMatcher,
 			}
-			got, got1, err := validator.ValidatePlugin(context.Background(), tt.args.plugin)
+			got, got1, err := validator.ValidatePlugin(context.Background(), tt.args.plugin, nil)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("KongHTTPValidator.ValidatePlugin() error = %v, wantErr %v", err, tt.wantErr)
 				return

internal/controllers/reference/indexer.go

@@ -203,3 +203,21 @@ func (c CacheIndexers) ListReferredObjects(referrer client.Object) ([]client.Obj
 	}
 	return objs, nil
 }
+
+// ListObjectsReferredBy lists all objects that refers to the referent in reference cache.
+func (c CacheIndexers) ListObjectsReferredBy(referent client.Object) ([]client.Object, error) {
+	refs, err := c.indexer.ByIndex(IndexNameReferent, objectKeyFunc(referent))
+	if err != nil {
+		return nil, err
+	}
+
+	objs := []client.Object{}
+	for _, ref := range refs {
+		r, ok := ref.(*ObjectReference)
+		if !ok {
+			return nil, ErrTypeNotObjectReference
+		}
+		objs = append(objs, r.Referrer)
+	}
+	return objs, nil
+}

internal/manager/controllerdef.go

@@ -59,6 +59,7 @@ func setupControllers(
 	ctx context.Context,
 	mgr manager.Manager,
 	dataplaneClient controllers.DataPlane,
+	referenceIndexers ctrlref.CacheIndexers,
 	dataplaneAddressFinder *dataplane.AddressFinder,
 	udpDataplaneAddressFinder *dataplane.AddressFinder,
 	kubernetesStatusQueue *status.Queue,
@@ -67,8 +68,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

internal/manager/run.go

@@ -23,6 +23,7 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/clients"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/gateway"
+	ctrlref "github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/configfetcher"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser"
@@ -163,12 +164,23 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
 		c.UpdateStatus,
 	)
 
+	referenceIndexers := ctrlref.NewCacheIndexers(ctrl.LoggerFrom(ctx).WithName("controllers").WithName("reference-indexers"))
+	cache := store.NewCacheStores()
 	setupLog.Info("Starting Admission Server")
-	if err := setupAdmissionServer(ctx, c, clientsManager, mgr.GetClient(), deprecatedLogger, parserFeatureFlags, kongSemVersion); err != nil {
+	if err := setupAdmissionServer(
+		ctx,
+		c,
+		clientsManager,
+		mgr.GetClient(),
+		referenceIndexers,
+		cache,
+		deprecatedLogger,
+		parserFeatureFlags,
+		kongSemVersion,
+	); err != nil {
 		return err
 	}
 
-	cache := store.NewCacheStores()
 	configParser, err := parser.NewParser(
 		deprecatedLogger,
 		store.New(cache, c.IngressClassName, deprecatedLogger),
@@ -227,6 +239,7 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
 		ctx,
 		mgr,
 		dataplaneClient,
+		referenceIndexers,
 		dataplaneAddressFinder,
 		udpDataplaneAddressFinder,
 		kubernetesStatusQueue,

internal/manager/setup.go

@@ -26,9 +26,11 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/admission"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/clients"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/reference"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/manager/scheme"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
 	dataplaneutil "github.com/kong/kubernetes-ingress-controller/v2/internal/util/dataplane"
 )
@@ -172,6 +174,8 @@ func setupAdmissionServer(
 	managerConfig *Config,
 	clientsManager *clients.AdminAPIClientsManager,
 	managerClient client.Client,
+	referenceIndexers reference.CacheIndexers,
+	cache store.CacheStores,
 	deprecatedLogger logrus.FieldLogger,
 	parserFeatures parser.FeatureFlags,
 	kongVersion semver.Version,
@@ -193,7 +197,8 @@ func setupAdmissionServer(
 			parserFeatures,
 			kongVersion,
 		),
-		Logger: logger,
+		ReferenceIndexers: referenceIndexers,
+		Logger:            logger,
 	}, logger)
 	if err != nil {
 		return err

test/integration/webhook_test.go

@@ -615,6 +615,87 @@ func TestValidationWebhook(t *testing.T) {
 	require.Contains(t, err.Error(), "some fields were invalid due to missing data: rsa_public_key, key, secret")
 }
 
+func TestValidationWebhookValidateReferredObject(t *testing.T) {
+	ctx := context.Background()
+
+	t.Parallel()
+	ns := helpers.Namespace(ctx, t, env)
+
+	if env.Cluster().Type() != kind.KindClusterType {
+		t.Skip("webhook tests are only available on KIND clusters currently")
+	}
+
+	t.Log("creating an extra namespace for testing global consumer credentials validation")
+	require.NoError(t, clusters.CreateNamespace(ctx, env.Cluster(), extraWebhookNamespace))
+	defer func() {
+		if err := env.Cluster().Client().CoreV1().Namespaces().Delete(ctx, extraWebhookNamespace, metav1.DeleteOptions{}); err != nil {
+			if !apierrors.IsNotFound(err) {
+				assert.NoError(t, err)
+			}
+		}
+	}()
+
+	ensureAdmissionRegistration(
+		ctx,
+		t,
+		ns.Name,
+		"kong-validations-reference",
+		[]admregv1.RuleWithOperations{
+			{
+				Rule: admregv1.Rule{
+					APIGroups:   []string{""},
+					APIVersions: []string{"v1"},
+					Resources:   []string{"secrets"},
+				},
+				Operations: []admregv1.OperationType{admregv1.Update},
+			},
+		},
+	)
+	t.Log("waiting for webhook service to be connective")
+	ensureWebhookServiceIsConnective(ctx, t, "kong-validations-reference")
+
+	ratelimitSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "ratelimit-config-1",
+		},
+		StringData: map[string]string{
+			"config.yaml": `limit_by: consumer
+policy: local
+minute: 5`,
+		},
+	}
+	ratelimitSecret, err := env.Cluster().Client().CoreV1().Secrets(ns.Name).Create(ctx, ratelimitSecret, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	kongClient, err := clientset.NewForConfig(env.Cluster().Config())
+	ratelimitPlugin := &kongv1.KongPlugin{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "ratelimit-1",
+		},
+		PluginName: "rate-limiting",
+		ConfigFrom: &kongv1.ConfigSource{
+			SecretValue: kongv1.SecretValueFromSource{
+				Secret: "ratelimit-config-1",
+				Key:    "config.yaml",
+			},
+		},
+	}
+	ratelimitPlugin, err = kongClient.ConfigurationV1().KongPlugins(ns.Name).Create(ctx, ratelimitPlugin, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	t.Log("update secret to an invalid configuration of rate-limiting plugin")
+	ratelimitSecret.StringData = map[string]string{
+		"config.yaml": `limit_by: consumer
+policy: local`,
+	}
+	_, err = env.Cluster().Client().CoreV1().Secrets(ns.Name).Update(ctx, ratelimitSecret, metav1.UpdateOptions{})
+	require.Error(t, err)
+	require.Contains(t, err.Error(),
+		fmt.Sprintf("secret used in KongPlugin %s/ratelimit-1 will generate invalid configuration", ns.Name),
+	)
+
+}
+
 func ensureWebhookService(ctx context.Context, t *testing.T, name string) {
 	t.Logf("creating webhook service: %q in namespace: %q", name, consts.ControllerNamespace)
 	validationsService, err := env.Cluster().Client().CoreV1().Services(consts.ControllerNamespace).Create(ctx, &corev1.Service{