Reconcile trigger on OIDC service account changes only, if SA referen…

…ces a trigger for correct broker class (knative#7849)

* Reconcile trigger on OIDC service account changes only, if SA references a trigger for correct broker class

* Run goimports and gofmt

* Remove deprecated use of pointer.Bool(v) and switch to prt.Bool(v)

pkg/reconciler/broker/trigger/controller.go

@@ -19,6 +19,9 @@ package mttrigger
 import (
 	"context"
 
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"knative.dev/eventing/pkg/auth"
 
 	"go.uber.org/zap"
@@ -116,13 +119,41 @@ func NewController(
 
 	// Reconciler Trigger when the OIDC service account changes
 	oidcServiceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
-		FilterFunc: controller.FilterController(&eventing.Trigger{}),
+		FilterFunc: filterOIDCServiceAccounts(triggerInformer.Lister(), brokerInformer.Lister()),
 		Handler:    controller.HandleAll(impl.EnqueueControllerOf),
 	})
 
 	return impl
 }
 
+// filterOIDCServiceAccounts returns a function that returns true if the resource passed
+// is a service account, which is owned by a trigger pointing to a MTChannelBased broker.
+func filterOIDCServiceAccounts(triggerLister eventinglisters.TriggerLister, brokerLister eventinglisters.BrokerLister) func(interface{}) bool {
+	return func(obj interface{}) bool {
+		controlledByTrigger := controller.FilterController(&eventing.Trigger{})(obj)
+		if !controlledByTrigger {
+			return false
+		}
+
+		sa, ok := obj.(*corev1.ServiceAccount)
+		if !ok {
+			return false
+		}
+
+		owner := metav1.GetControllerOf(sa)
+		if owner == nil {
+			return false
+		}
+
+		trigger, err := triggerLister.Triggers(sa.Namespace).Get(owner.Name)
+		if err != nil {
+			return false
+		}
+
+		return filterTriggers(brokerLister)(trigger)
+	}
+}
+
 // filterTriggers returns a function that returns true if the resource passed
 // is a trigger pointing to a MTChannelBroker.
 func filterTriggers(lister eventinglisters.BrokerLister) func(interface{}) bool {

pkg/reconciler/broker/trigger/controller_test.go

@@ -21,6 +21,10 @@ import (
 	"fmt"
 	"testing"
 
+	"knative.dev/pkg/ptr"
+
+	triggerinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger"
+
 	"knative.dev/eventing/pkg/auth"
 	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
 
@@ -67,6 +71,134 @@ func SetUpInformerSelector(ctx context.Context) context.Context {
 	return ctx
 }
 
+func TestFilterOIDCServiceAccounts(t *testing.T) {
+	ctx, _ := SetupFakeContext(t, SetUpInformerSelector)
+
+	tt := []struct {
+		name    string
+		sa      *corev1.ServiceAccount
+		trigger *eventing.Trigger
+		brokers []*eventing.Broker
+		pass    bool
+	}{{
+		name: "matching owner reference",
+		sa: &corev1.ServiceAccount{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "sa",
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						APIVersion: eventing.SchemeGroupVersion.String(),
+						Kind:       "Trigger",
+						Name:       "tr",
+						Controller: ptr.Bool(true),
+					},
+				},
+			},
+		},
+		trigger: &eventing.Trigger{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "tr",
+			},
+			Spec: eventing.TriggerSpec{
+				Broker: "br",
+			},
+		},
+		brokers: []*eventing.Broker{{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "br",
+				Annotations: map[string]string{
+					eventing.BrokerClassAnnotationKey: apiseventing.MTChannelBrokerClassValue,
+				},
+			},
+		}},
+		pass: true,
+	}, {
+		name: "references trigger for wrong broker class",
+		sa: &corev1.ServiceAccount{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "sa",
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						APIVersion: eventing.SchemeGroupVersion.String(),
+						Kind:       "Trigger",
+						Name:       "tr",
+						Controller: ptr.Bool(true),
+					},
+				},
+			},
+		},
+		trigger: &eventing.Trigger{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "tr",
+			},
+			Spec: eventing.TriggerSpec{
+				Broker: "br",
+			},
+		},
+		brokers: []*eventing.Broker{{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "br",
+				Annotations: map[string]string{
+					eventing.BrokerClassAnnotationKey: "another-broker-class",
+				},
+			},
+		}},
+		pass: false,
+	}, {
+		name: "no owner reference",
+		sa: &corev1.ServiceAccount{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "sa",
+			},
+		},
+		trigger: &eventing.Trigger{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "tr",
+			},
+			Spec: eventing.TriggerSpec{
+				Broker: "br",
+			},
+		},
+		brokers: []*eventing.Broker{{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "br",
+				Annotations: map[string]string{
+					eventing.BrokerClassAnnotationKey: apiseventing.MTChannelBrokerClassValue,
+				},
+			},
+		}},
+		pass: false,
+	}}
+
+	for _, tc := range tt {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			brokerInformer := brokerinformer.Get(ctx)
+			for _, obj := range tc.brokers {
+				err := brokerInformer.Informer().GetStore().Add(obj)
+				assert.NoError(t, err)
+			}
+
+			triggerInformer := triggerinformer.Get(ctx)
+			err := triggerInformer.Informer().GetStore().Add(tc.trigger)
+			assert.NoError(t, err)
+
+			filter := filterOIDCServiceAccounts(triggerInformer.Lister(), brokerInformer.Lister())
+			pass := filter(tc.sa)
+			assert.Equal(t, tc.pass, pass)
+		})
+	}
+}
+
 func TestFilterTriggers(t *testing.T) {
 	ctx, _ := SetupFakeContext(t, SetUpInformerSelector)