feat(KongPlugin): generate K8s events for missing grant

internal/dataplane/kongstate/customentity.go

@@ -178,7 +178,7 @@ func (ks *KongState) FillCustomEntities(
 	}
 	// Fetch relations between plugins and services/routes/consumers and store the pointer to translated Kong entities.
 	// Used for fetching entity referred by a custom entity and fill the ID of referred entity.
-	pluginRels := ks.getPluginRelatedEntitiesRef(s, logger)
+	pluginRels := ks.getPluginRelatedEntitiesRef(s, logger, failuresCollector)
 
 	for _, entity := range entities {
 		// reject the custom entity if its type is in "known" entity types that are already processed.
@@ -333,7 +333,7 @@ func findCustomEntityRelatedPlugin(logger logr.Logger, cacheStore store.Storer,
 			Namespace: parentRefNamespace,
 			Name:      parentRef.Name,
 		}
-		paretRefNamespace, err := extractReferredPluginNamespace(logger, cacheStore, k8sEntity, nn)
+		paretRefNamespace, err := extractReferredPluginNamespaceIfAllowed(logger, cacheStore, k8sEntity, nn)
 		if err != nil {
 			return "", false, err
 		}

internal/dataplane/kongstate/kongstate.go

@@ -3,6 +3,7 @@ package kongstate
 import (
 	"bytes"
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -378,7 +379,9 @@ type NamespacedKongPlugin struct {
 	Name      string
 }
 
-func (ks *KongState) getPluginRelations(cacheStore store.Storer, log logr.Logger) map[string]util.ForeignRelations {
+func (ks *KongState) getPluginRelations(
+	cacheStore store.Storer, log logr.Logger, failuresCollector *failures.ResourceFailuresCollector,
+) map[string]util.ForeignRelations {
 	// KongPlugin key (KongPlugin's name:namespace) to corresponding associations
 	pluginRels := map[string]util.ForeignRelations{}
 
@@ -400,9 +403,13 @@ func (ks *KongState) getPluginRelations(cacheStore store.Storer, log logr.Logger
 		//
 		// Code in buildPlugins() will combine plugin associations into
 		// multi-entity plugins within the local namespace
-		namespace, err := extractReferredPluginNamespace(log, cacheStore, referrer, plugin)
+		namespace, err := extractReferredPluginNamespaceIfAllowed(log, cacheStore, referrer, plugin)
 		if err != nil {
-			log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace)
+			if errors.As(err, &referredPluginNotAllowedError{}) {
+				failuresCollector.PushResourceFailure(err.Error(), referrer)
+			} else {
+				log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace)
+			}
 			return
 		}
 
@@ -489,58 +496,6 @@ type pluginReference struct {
 	Name      string
 }
 
-func isRemotePluginReferenceAllowed(log logr.Logger, s store.Storer, r pluginReference) error {
-	// remote plugin. considered part of this namespace if a suitable ReferenceGrant exists
-	grants, err := s.ListReferenceGrants()
-	if err != nil {
-		return fmt.Errorf("could not retrieve ReferenceGrants from store when building plugin relations map: %w", err)
-	}
-	allowed := gatewayapi.GetPermittedForReferenceGrantFrom(
-		log,
-		gatewayapi.ReferenceGrantFrom{
-			Group:     gatewayapi.Group(r.Referrer.GetObjectKind().GroupVersionKind().Group),
-			Kind:      gatewayapi.Kind(r.Referrer.GetObjectKind().GroupVersionKind().Kind),
-			Namespace: gatewayapi.Namespace(r.Referrer.GetNamespace()),
-		},
-		grants,
-	)
-
-	// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/6000
-	// Our reference checking code wasn't originally designed to handle multiple object types and relationships and
-	// wasn't designed with much guidance for future usage. It expects something akin to the BackendRef section of an
-	// HTTPRoute or similar, since that's what it was originally designed for. A future simplified model should probably
-	// work with source and target client.Object resources derived from the particular resources' reference specs, as
-	// those reference formats aren't necessarily consistent. We should possibly push for a standard upstream utility as
-	// part of https://github.com/kubernetes/enhancements/issues/3766 if it proceeds further, as this is can be difficult
-	// to wrap your head around when deep in the code that's checking an individual use case. A standard model for "these
-	// are the inputs and outputs for a reference grant check, derive them from your spec" should help avoid inconsistency
-	// in check implementation.
-
-	// we don't have a full plugin resource here for the grant checker, so we build a fake one with the correct
-	// name and namespace.
-	pluginReference := gatewayapi.PluginLabelReference{
-		Namespace: &r.Namespace,
-		Name:      r.Name,
-	}
-
-	// Because we should check whether it is allowed to refer FROM the referrer TO the plugin here,
-	// we should put the referrer on the "target" and the plugin on the "backendRef".
-
-	log.V(logging.TraceLevel).Info("requested grant to plugins",
-		"from-namespace", r.Referrer.GetNamespace(),
-		"from-group", r.Referrer.GetObjectKind().GroupVersionKind().Group,
-		"from-kind", r.Referrer.GetObjectKind().GroupVersionKind().Kind,
-		"to-namespace", r.Namespace,
-		"to-name", r.Name,
-	)
-
-	if !gatewayapi.NewRefCheckerForKongPlugin(log, r.Referrer, pluginReference).IsRefAllowedByGrant(allowed) {
-		return fmt.Errorf("no grant found for %s in %s to plugin %s in %s requested remote KongPlugin bind",
-			r.Referrer.GetObjectKind().GroupVersionKind().Kind, r.Referrer.GetNamespace(), r.Name, r.Namespace)
-	}
-	return nil
-}
-
 func buildPlugins(
 	logger logr.Logger,
 	s store.Storer,
@@ -691,7 +646,7 @@ func (ks *KongState) FillPlugins(
 	s store.Storer,
 	failuresCollector *failures.ResourceFailuresCollector,
 ) {
-	ks.Plugins = buildPlugins(log, s, failuresCollector, ks.getPluginRelations(s, log))
+	ks.Plugins = buildPlugins(log, s, failuresCollector, ks.getPluginRelations(s, log, failuresCollector))
 }
 
 // FillIDs iterates over the KongState and fills in the ID field for each entity
@@ -819,15 +774,21 @@ func getServiceIDFromPluginRels(
 //
 // TODO: refactor the building of plugin related entities and share the result between here and building plugins:
 // https://github.com/Kong/kubernetes-ingress-controller/issues/6115
-func (ks *KongState) getPluginRelatedEntitiesRef(cacheStore store.Storer, log logr.Logger) PluginRelatedEntitiesRefs {
+func (ks *KongState) getPluginRelatedEntitiesRef(
+	cacheStore store.Storer, log logr.Logger, failuresCollector *failures.ResourceFailuresCollector,
+) PluginRelatedEntitiesRefs {
 	pluginRels := PluginRelatedEntitiesRefs{
 		RelatedEntities:      map[string]RelatedEntitiesRef{},
 		RouteAttachedService: map[string]*Service{},
 	}
 	addRelation := func(referrer client.Object, plugin k8stypes.NamespacedName, entity any) {
-		namespace, err := extractReferredPluginNamespace(log, cacheStore, referrer, plugin)
+		namespace, err := extractReferredPluginNamespaceIfAllowed(log, cacheStore, referrer, plugin)
 		if err != nil {
-			log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace)
+			if errors.As(err, &referredPluginNotAllowedError{}) {
+				failuresCollector.PushResourceFailure(err.Error(), referrer)
+			} else {
+				log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace)
+			}
 			return
 		}
 		pluginKey := namespace + ":" + plugin.Name
@@ -882,7 +843,15 @@ func (ks *KongState) getPluginRelatedEntitiesRef(cacheStore store.Storer, log lo
 	return pluginRels
 }
 
-func extractReferredPluginNamespace(
+type referredPluginNotAllowedError struct {
+	pluginReference gatewayapi.PluginLabelReference
+}
+
+func (e referredPluginNotAllowedError) Error() string {
+	return fmt.Sprintf("no grant found to referenced %q plugin in the requested remote KongPlugin bind", e.pluginReference)
+}
+
+func extractReferredPluginNamespaceIfAllowed(
 	log logr.Logger, cacheStore store.Storer, referrer client.Object, plugin k8stypes.NamespacedName,
 ) (string, error) {
 	// There are 2 types of KongPlugin references: local and remote.
@@ -911,3 +880,56 @@ func extractReferredPluginNamespace(
 	}
 	return plugin.Namespace, nil
 }
+
+func isRemotePluginReferenceAllowed(log logr.Logger, s store.Storer, r pluginReference) error {
+	// remote plugin. considered part of this namespace if a suitable ReferenceGrant exists
+	grants, err := s.ListReferenceGrants()
+	if err != nil {
+		return fmt.Errorf("could not retrieve ReferenceGrants from store when building plugin relations map: %w", err)
+	}
+	allowed := gatewayapi.GetPermittedForReferenceGrantFrom(
+		log,
+		gatewayapi.ReferenceGrantFrom{
+			Group:     gatewayapi.Group(r.Referrer.GetObjectKind().GroupVersionKind().Group),
+			Kind:      gatewayapi.Kind(r.Referrer.GetObjectKind().GroupVersionKind().Kind),
+			Namespace: gatewayapi.Namespace(r.Referrer.GetNamespace()),
+		},
+		grants,
+	)
+
+	// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/6000
+	// Our reference checking code wasn't originally designed to handle multiple object types and relationships and
+	// wasn't designed with much guidance for future usage. It expects something akin to the BackendRef section of an
+	// HTTPRoute or similar, since that's what it was originally designed for. A future simplified model should probably
+	// work with source and target client.Object resources derived from the particular resources' reference specs, as
+	// those reference formats aren't necessarily consistent. We should possibly push for a standard upstream utility as
+	// part of https://github.com/kubernetes/enhancements/issues/3766 if it proceeds further, as this is can be difficult
+	// to wrap your head around when deep in the code that's checking an individual use case. A standard model for "these
+	// are the inputs and outputs for a reference grant check, derive them from your spec" should help avoid inconsistency
+	// in check implementation.
+
+	// we don't have a full plugin resource here for the grant checker, so we build a fake one with the correct
+	// name and namespace.
+	pluginReference := gatewayapi.PluginLabelReference{
+		Namespace: &r.Namespace,
+		Name:      r.Name,
+	}
+
+	// Because we should check whether it is allowed to refer FROM the referrer TO the plugin here,
+	// we should put the referrer on the "target" and the plugin on the "backendRef".
+
+	log.V(logging.TraceLevel).Info("requested grant to plugins",
+		"from-namespace", r.Referrer.GetNamespace(),
+		"from-group", r.Referrer.GetObjectKind().GroupVersionKind().Group,
+		"from-kind", r.Referrer.GetObjectKind().GroupVersionKind().Kind,
+		"to-namespace", r.Namespace,
+		"to-name", r.Name,
+	)
+
+	if !gatewayapi.NewRefCheckerForKongPlugin(log, r.Referrer, pluginReference).IsRefAllowedByGrant(allowed) {
+		return referredPluginNotAllowedError{
+			pluginReference: pluginReference,
+		}
+	}
+	return nil
+}

internal/dataplane/kongstate/kongstate_test.go

@@ -677,7 +677,7 @@ func TestGetPluginRelations(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			store, err := store.NewFakeStore(store.FakeObjects{})
 			require.NoError(t, err)
-			computedPluginRelations := tt.data.inputState.getPluginRelations(store, logr.Discard())
+			computedPluginRelations := tt.data.inputState.getPluginRelations(store, logr.Discard(), nil)
 			if diff := cmp.Diff(tt.data.expectedPluginRelations, computedPluginRelations); diff != "" {
 				t.Fatal("expected value differs from actual, see the human-readable diff:", diff)
 			}
@@ -819,7 +819,7 @@ func BenchmarkGetPluginRelations(b *testing.B) {
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
-		fr := ks.getPluginRelations(store, logr.Discard())
+		fr := ks.getPluginRelations(store, logr.Discard(), nil)
 		_ = fr
 	}
 }

internal/gatewayapi/contraints.go

@@ -1,6 +1,10 @@
 package gatewayapi
 
-import "sigs.k8s.io/controller-runtime/pkg/client"
+import (
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
 
 type HostnameT interface {
 	Hostname | string
@@ -33,3 +37,10 @@ type PluginLabelReference struct {
 	Namespace *string
 	Name      string
 }
+
+func (plr PluginLabelReference) String() string {
+	if plr.Namespace == nil {
+		return plr.Name
+	}
+	return fmt.Sprintf("%s:%s", *plr.Namespace, plr.Name)
+}

test/envtest/configerrorevent_envtest_test.go

@@ -66,7 +66,7 @@ func TestConfigErrorEventGenerationInMemoryMode(t *testing.T) {
 		"konghq.com/protocol": "tcp",
 		"konghq.com/path":     "/aitmatov",
 		// Referencing non-existent KongPlugins.
-		"konghq.com/plugins": "foo,bar",
+		"konghq.com/plugins": "foo,bar,n1:p1",
 	}
 	service.Namespace = ns.Name
 	require.NoError(t, ctrlClient.Create(ctx, service))
@@ -102,7 +102,7 @@ func TestConfigErrorEventGenerationInMemoryMode(t *testing.T) {
 		}
 		t.Logf("got %d events", len(events.Items))
 
-		const numberOfExpectedEvents = 7
+		const numberOfExpectedEvents = 8
 		matches := make([]bool, numberOfExpectedEvents)
 		matches[0] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
 			return e.Type == corev1.EventTypeWarning &&
@@ -154,6 +154,13 @@ func TestConfigErrorEventGenerationInMemoryMode(t *testing.T) {
 				e.InvolvedObject.Name == ingress.Name &&
 				e.Message == `referenced KongPlugin or KongClusterPlugin "baz" does not exist`
 		})
+		matches[7] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
+			return e.Type == corev1.EventTypeWarning &&
+				e.Reason == dataplane.KongConfigurationTranslationFailedEventReason &&
+				e.InvolvedObject.Kind == "Service" &&
+				e.InvolvedObject.Name == service.Name &&
+				e.Message == `no grant found to referenced "n1:p1" plugin in the requested remote KongPlugin bind`
+		})
 		if lo.Count(matches, true) != numberOfExpectedEvents {
 			t.Logf("not all events matched: %+v", matches)
 			return false
@@ -311,7 +318,7 @@ func TestConfigErrorEventGenerationDBMode(t *testing.T) {
 			Annotations: map[string]string{
 				annotations.IngressClassKey: ingressClassName,
 				// Referencing non-existent KongPlugin.
-				"konghq.com/plugins": "foo",
+				"konghq.com/plugins": "foo, n1:p1",
 			},
 		},
 		Username: "donenbai",
@@ -342,7 +349,7 @@ func TestConfigErrorEventGenerationDBMode(t *testing.T) {
 		}
 		t.Logf("got %d events", len(events.Items))
 
-		const numberOfExpectedEvents = 2
+		const numberOfExpectedEvents = 3
 		matches := make([]bool, numberOfExpectedEvents)
 		matches[0] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
 			return e.Type == corev1.EventTypeWarning &&
@@ -358,6 +365,13 @@ func TestConfigErrorEventGenerationDBMode(t *testing.T) {
 				e.InvolvedObject.Name == consumer.Name &&
 				e.Message == `referenced KongPlugin or KongClusterPlugin "foo" does not exist`
 		})
+		matches[2] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
+			return e.Type == corev1.EventTypeWarning &&
+				e.Reason == dataplane.KongConfigurationTranslationFailedEventReason &&
+				e.InvolvedObject.Kind == "KongConsumer" &&
+				e.InvolvedObject.Name == consumer.Name &&
+				e.Message == `no grant found to referenced "n1:p1" plugin in the requested remote KongPlugin bind`
+		})
 		if lo.Count(matches, true) != numberOfExpectedEvents {
 			t.Logf("not all events matched: %+v", matches)
 			return false