Skip to content

Commit

Permalink
Reconcile trigger on OIDC service account changes only, if SA referen…
Browse files Browse the repository at this point in the history
…ces a trigger for correct broker class (#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)
  • Loading branch information
creydr committed Apr 23, 2024
1 parent ea14296 commit e23ebab
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 1 deletion.
33 changes: 32 additions & 1 deletion pkg/reconciler/broker/trigger/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
132 changes: 132 additions & 0 deletions pkg/reconciler/broker/trigger/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit e23ebab

Please sign in to comment.