From 45dac7520bbbf9ffdd92d2e3eb039b45d645e797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 22 Apr 2024 14:13:42 +0200 Subject: [PATCH 1/3] Reconcile trigger on OIDC service account changes only, if SA references a trigger for correct broker class --- pkg/reconciler/broker/trigger/controller.go | 32 ++++- .../broker/trigger/controller_test.go | 130 ++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/broker/trigger/controller.go b/pkg/reconciler/broker/trigger/controller.go index a575df9fc82..932ff402125 100644 --- a/pkg/reconciler/broker/trigger/controller.go +++ b/pkg/reconciler/broker/trigger/controller.go @@ -18,6 +18,8 @@ package mttrigger import ( "context" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/eventing/pkg/auth" @@ -116,13 +118,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 { diff --git a/pkg/reconciler/broker/trigger/controller_test.go b/pkg/reconciler/broker/trigger/controller_test.go index 772dec3bd9c..24b7b78dda2 100644 --- a/pkg/reconciler/broker/trigger/controller_test.go +++ b/pkg/reconciler/broker/trigger/controller_test.go @@ -19,6 +19,8 @@ package mttrigger import ( "context" "fmt" + "k8s.io/utils/pointer" + triggerinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger" "testing" "knative.dev/eventing/pkg/auth" @@ -67,6 +69,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: pointer.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: pointer.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) From 54e3b69459abb79ea5f57bfd91a48b084ddc6710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 22 Apr 2024 14:24:04 +0200 Subject: [PATCH 2/3] Run goimports and gofmt --- pkg/reconciler/broker/trigger/controller.go | 1 + pkg/reconciler/broker/trigger/controller_test.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/broker/trigger/controller.go b/pkg/reconciler/broker/trigger/controller.go index 932ff402125..8e92136d93a 100644 --- a/pkg/reconciler/broker/trigger/controller.go +++ b/pkg/reconciler/broker/trigger/controller.go @@ -18,6 +18,7 @@ package mttrigger import ( "context" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/reconciler/broker/trigger/controller_test.go b/pkg/reconciler/broker/trigger/controller_test.go index 24b7b78dda2..dc70408e208 100644 --- a/pkg/reconciler/broker/trigger/controller_test.go +++ b/pkg/reconciler/broker/trigger/controller_test.go @@ -19,9 +19,10 @@ package mttrigger import ( "context" "fmt" + "testing" + "k8s.io/utils/pointer" triggerinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger" - "testing" "knative.dev/eventing/pkg/auth" filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered" From 0c252163c2d3ea4959af41cf593dfa3cfc1ef9d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 22 Apr 2024 17:53:36 +0200 Subject: [PATCH 3/3] Remove deprecated use of pointer.Bool(v) and switch to prt.Bool(v) --- pkg/reconciler/broker/trigger/controller_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/reconciler/broker/trigger/controller_test.go b/pkg/reconciler/broker/trigger/controller_test.go index dc70408e208..f9f090e5ba6 100644 --- a/pkg/reconciler/broker/trigger/controller_test.go +++ b/pkg/reconciler/broker/trigger/controller_test.go @@ -21,7 +21,8 @@ import ( "fmt" "testing" - "k8s.io/utils/pointer" + "knative.dev/pkg/ptr" + triggerinformer "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger" "knative.dev/eventing/pkg/auth" @@ -90,7 +91,7 @@ func TestFilterOIDCServiceAccounts(t *testing.T) { APIVersion: eventing.SchemeGroupVersion.String(), Kind: "Trigger", Name: "tr", - Controller: pointer.Bool(true), + Controller: ptr.Bool(true), }, }, }, @@ -125,7 +126,7 @@ func TestFilterOIDCServiceAccounts(t *testing.T) { APIVersion: eventing.SchemeGroupVersion.String(), Kind: "Trigger", Name: "tr", - Controller: pointer.Bool(true), + Controller: ptr.Bool(true), }, }, },