Use filtered informer to watch OIDC service accounts (#7527)

* controller.go changed * #7320 WIP * WIP: Testing filtered informer (#7341) * unit test passed * Revert "Merge remote-tracking branch 'otherfork/main' into main" This reverts commit 94cd51b, reversing changes made to 0bf2982. * Removed comments * Changed to filtered informer for Subscription identity service account * Changed to filtered informer for Sequence service accounts * Changed to filtered informer for Parallel identity service accounts * Changed to filtered informer for APIServerSource identity service account * fixed unit tests * added label selector for mtchannel_broker * added filtered informer for sinkbinding identity service accounts * added OIDC label selector in webhook * added filtered informer for containersource service accounts * added filtered informer for pingsource service accounts * added OIDC label selector in apiserver ctx * added OIDC label selector in broker/filter * added OIDC label selector in broker/ingress * added OIDC label selector in in_memory/channel_dispatcher * added OIDC label selector in mtping * fixed unit test issues for pingsource * fixed unit test for container source * formatted files * updated service account informer in apiserversource * updated service account informers in other places * small typo fix * added actual value for OIDC label * added a valid value for OIDClabelkey * changed references of OIDCLabelKey * fixed import path problem * changed OIDCLabelSelector in all main.go files * changed instances of OIDCLabelSelector in controller and controller test files * deleted OIDC related labels from register.go * fixed formatting issues * Added value for OIDCLabelKey --------- Co-authored-by: Scott <scottprotoss@gmail.com>
knative · Jan 30, 2024 · ff52881 · ff52881
1 parent 54f3952
commit ff52881
Show file tree

Hide file tree

Showing 31 changed files with 272 additions and 66 deletions.
diff --git a/cmd/apiserver_receive_adapter/main.go b/cmd/apiserver_receive_adapter/main.go
@@ -22,6 +22,7 @@ import (
 
 	"knative.dev/eventing/pkg/adapter/apiserver"
 	"knative.dev/eventing/pkg/adapter/v2"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 )
 
@@ -34,6 +35,7 @@ func main() {
 	ctx = adapter.WithInjectorEnabled(ctx)
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/broker/filter/main.go b/cmd/broker/filter/main.go
@@ -81,6 +81,7 @@ func main() {
 	log.Printf("Registering %d informers", len(injection.Default.GetInformers()))
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/broker/ingress/main.go b/cmd/broker/ingress/main.go
@@ -103,6 +103,7 @@ func main() {
 	log.Printf("Registering %d informers", len(injection.Default.GetInformers()))
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -28,7 +28,7 @@ import (
 
 	"knative.dev/pkg/injection/sharedmain"
 
-	"knative.dev/eventing/pkg/apis/sources"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 
 	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
@@ -79,7 +79,7 @@ func main() {
 	}()
 
 	ctx = filteredFactory.WithSelectors(ctx,
-		sources.OIDCTokenRoleLabelSelector,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/in_memory/channel_dispatcher/main.go b/cmd/in_memory/channel_dispatcher/main.go
@@ -27,6 +27,7 @@ import (
 	"knative.dev/pkg/injection/sharedmain"
 	"knative.dev/pkg/signals"
 
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 	inmemorychannel "knative.dev/eventing/pkg/reconciler/inmemorychannel/dispatcher"
 )
@@ -39,6 +40,7 @@ func main() {
 	}
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/mtchannel_broker/main.go b/cmd/mtchannel_broker/main.go
@@ -22,8 +22,12 @@ import (
 
 	"context"
 
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/pkg/injection/sharedmain"
 
+	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
+	"knative.dev/pkg/signals"
+
 	"knative.dev/eventing/pkg/reconciler/broker"
 	mttrigger "knative.dev/eventing/pkg/reconciler/broker/trigger"
 )
@@ -33,7 +37,12 @@ const (
 )
 
 func main() {
-	sharedmain.Main(
+	ctx := signals.NewContext()
+
+	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector)
+
+	sharedmain.MainWithContext(ctx,
 		component,
 
 		broker.NewController,

diff --git a/cmd/mtping/main.go b/cmd/mtping/main.go
@@ -22,6 +22,7 @@ import (
 
 	"knative.dev/eventing/pkg/adapter/mtping"
 	"knative.dev/eventing/pkg/adapter/v2"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 )
 
@@ -57,6 +58,7 @@ func main() {
 	})
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -26,6 +26,7 @@ import (
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/filtered"
 
 	"knative.dev/eventing/pkg/apis/feature"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 
 	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
@@ -287,6 +288,7 @@ func main() {
 	})
 
 	ctx = filteredFactory.WithSelectors(ctx,
+		auth.OIDCLabelSelector,
 		eventingtls.TrustBundleLabelSelector,
 	)
 

diff --git a/pkg/apis/sources/register.go b/pkg/apis/sources/register.go
@@ -32,12 +32,6 @@ const (
 	// SourceDuckLabelValue is the label value to indicate
 	// the CRD is a Source duck type.
 	SourceDuckLabelValue = "true"
-
-	//OIDCLabelKey is used to filter out all the informers that related to OIDC work
-	OIDCLabelKey = "oidc"
-
-	// OIDCTokenRoleLabelSelector is the label selector for the OIDC token creator role and rolebinding informers
-	OIDCTokenRoleLabelSelector = OIDCLabelKey
 )
 
 var (

diff --git a/pkg/auth/serviceaccount.go b/pkg/auth/serviceaccount.go
@@ -37,6 +37,14 @@ import (
 	"knative.dev/pkg/ptr"
 )
 
+const (
+	//OIDCLabelKey is used to filter out all the informers that related to OIDC work
+	OIDCLabelKey = "oidc"
+
+	// OIDCTokenRoleLabelSelector is the label selector for the OIDC token creator role and rolebinding informers
+	OIDCLabelSelector = OIDCLabelKey
+)
+
 // GetOIDCServiceAccountNameForResource returns the service account name to use
 // for OIDC authentication for the given resource.
 func GetOIDCServiceAccountNameForResource(gvk schema.GroupVersionKind, objectMeta metav1.ObjectMeta) string {
@@ -66,6 +74,9 @@ func GetOIDCServiceAccountForResource(gvk schema.GroupVersionKind, objectMeta me
 			Annotations: map[string]string{
 				"description": fmt.Sprintf("Service Account for OIDC Authentication for %s %q", gvk.GroupKind().Kind, objectMeta.Name),
 			},
+			Labels: map[string]string{
+				OIDCLabelKey: "enabled",
+			},
 		},
 	}
 }

diff --git a/pkg/auth/serviceaccount_test.go b/pkg/auth/serviceaccount_test.go
@@ -108,6 +108,9 @@ func TestGetOIDCServiceAccountForResource(t *testing.T) {
 			Annotations: map[string]string{
 				"description": "Service Account for OIDC Authentication for Broker \"my-broker\"",
 			},
+			Labels: map[string]string{
+				OIDCLabelKey: "enabled",
+			},
 		},
 	}
 

diff --git a/pkg/reconciler/apiserversource/apiserversource_test.go b/pkg/reconciler/apiserversource/apiserversource_test.go
@@ -21,8 +21,6 @@ import (
 	"fmt"
 	"testing"
 
-	"knative.dev/eventing/pkg/apis/sources"
-
 	"knative.dev/pkg/kmeta"
 
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -1425,7 +1423,7 @@ func makeOIDCRole() *rbacv1.Role {
 				"description": fmt.Sprintf("Role for OIDC Authentication for ApiServerSource %q", sourceName),
 			},
 			Labels: map[string]string{
-				sources.OIDCLabelKey: "",
+				auth.OIDCLabelKey: "enabled",
 			},
 			OwnerReferences: []metav1.OwnerReference{
 				*kmeta.NewControllerRef(src),
@@ -1455,7 +1453,7 @@ func makeOIDCRoleBinding() *rbacv1.RoleBinding {
 				"description": fmt.Sprintf("Role Binding for OIDC Authentication for ApiServerSource %q", sourceName),
 			},
 			Labels: map[string]string{
-				sources.OIDCLabelKey: "",
+				auth.OIDCLabelKey: "enabled",
 			},
 			OwnerReferences: []metav1.OwnerReference{
 				*kmeta.NewControllerRef(src),

diff --git a/pkg/reconciler/apiserversource/controller.go b/pkg/reconciler/apiserversource/controller.go
@@ -22,7 +22,7 @@ import (
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/filtered"
 	"knative.dev/pkg/system"
 
-	"knative.dev/eventing/pkg/apis/sources"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 	eventingreconciler "knative.dev/eventing/pkg/reconciler"
 
@@ -42,7 +42,8 @@ import (
 	deploymentinformer "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment"
 	"knative.dev/pkg/client/injection/kube/informers/core/v1/namespace"
 
-	serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
+	serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered"
+
 	roleinformer "knative.dev/pkg/client/injection/kube/informers/rbac/v1/role/filtered"
 	rolebindinginformer "knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/filtered"
 
@@ -67,11 +68,11 @@ func NewController(
 	deploymentInformer := deploymentinformer.Get(ctx)
 	apiServerSourceInformer := apiserversourceinformer.Get(ctx)
 	namespaceInformer := namespace.Get(ctx)
-	serviceaccountInformer := serviceaccountinformer.Get(ctx)
+	oidcServiceaccountInformer := serviceaccountinformer.Get(ctx, auth.OIDCLabelSelector)
 
 	// Create a selector string
-	roleInformer := roleinformer.Get(ctx, sources.OIDCTokenRoleLabelSelector)
-	rolebindingInformer := rolebindinginformer.Get(ctx, sources.OIDCTokenRoleLabelSelector)
+	roleInformer := roleinformer.Get(ctx, auth.OIDCLabelSelector)
+	rolebindingInformer := rolebindinginformer.Get(ctx, auth.OIDCLabelSelector)
 
 	trustBundleConfigMapInformer := configmapinformer.Get(ctx, eventingtls.TrustBundleLabelSelector)
 
@@ -89,7 +90,7 @@ func NewController(
 		ceSource:                   GetCfgHost(ctx),
 		configs:                    reconcilersource.WatchConfigurations(ctx, component, cmw),
 		namespaceLister:            namespaceInformer.Lister(),
-		serviceAccountLister:       serviceaccountInformer.Lister(),
+		serviceAccountLister:       oidcServiceaccountInformer.Lister(),
 		roleLister:                 roleInformer.Lister(),
 		roleBindingLister:          rolebindingInformer.Lister(),
 		trustBundleConfigMapLister: trustBundleConfigMapInformer.Lister(),
@@ -142,7 +143,7 @@ func NewController(
 	})
 
 	// Reconciler ApiServerSource when the OIDC service account changes
-	serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
+	oidcServiceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
 		FilterFunc: controller.FilterController(&v1.ApiServerSource{}),
 		Handler:    controller.HandleAll(impl.EnqueueControllerOf),
 	})

diff --git a/pkg/reconciler/apiserversource/controller_test.go b/pkg/reconciler/apiserversource/controller_test.go
@@ -23,7 +23,7 @@ import (
 
 	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
 
-	"knative.dev/eventing/pkg/apis/sources"
+	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
 
 	"knative.dev/eventing/pkg/apis/feature"
@@ -42,7 +42,7 @@ import (
 	// Fake injection informers
 	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/filtered/fake"
 	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace/fake"
-	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake"
+	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/fake"
 	_ "knative.dev/pkg/client/injection/kube/informers/factory/filtered/fake"
 	_ "knative.dev/pkg/client/injection/kube/informers/rbac/v1/role/filtered/fake"
 	_ "knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/filtered/fake"
@@ -98,6 +98,6 @@ func TestNew(t *testing.T) {
 }
 
 func SetUpInformerSelector(ctx context.Context) context.Context {
-	ctx = filteredFactory.WithSelectors(ctx, eventingtls.TrustBundleLabelSelector, sources.OIDCTokenRoleLabelSelector)
+	ctx = filteredFactory.WithSelectors(ctx, eventingtls.TrustBundleLabelSelector, auth.OIDCLabelSelector)
 	return ctx
 }
diff --git a/pkg/reconciler/apiserversource/resources/oidc_rolebinding.go b/pkg/reconciler/apiserversource/resources/oidc_rolebinding.go
@@ -19,7 +19,7 @@ package resources
 import (
 	"fmt"
 
-	"knative.dev/eventing/pkg/apis/sources"
+	"knative.dev/eventing/pkg/auth"
 
 	"knative.dev/pkg/kmeta"
 
@@ -54,7 +54,7 @@ func MakeOIDCRole(source *v1.ApiServerSource) (*rbacv1.Role, error) {
 				"description": fmt.Sprintf("Role for OIDC Authentication for ApiServerSource %q", source.GetName()),
 			},
 			Labels: map[string]string{
-				sources.OIDCLabelKey: "",
+				auth.OIDCLabelKey: "enabled",
 			},
 			OwnerReferences: []metav1.OwnerReference{
 				*kmeta.NewControllerRef(source),
@@ -92,7 +92,7 @@ func MakeOIDCRoleBinding(source *v1.ApiServerSource) (*rbacv1.RoleBinding, error
 				"description": fmt.Sprintf("Role Binding for OIDC Authentication for ApiServerSource %q", source.GetName()),
 			},
 			Labels: map[string]string{
-				sources.OIDCLabelKey: "",
+				auth.OIDCLabelKey: "enabled",
 			},
 			OwnerReferences: []metav1.OwnerReference{
 				*kmeta.NewControllerRef(source),

diff --git a/pkg/reconciler/broker/trigger/controller.go b/pkg/reconciler/broker/trigger/controller.go
@@ -19,12 +19,13 @@ package mttrigger
 import (
 	"context"
 
+	"knative.dev/eventing/pkg/auth"
+
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/tools/cache"
 	"knative.dev/pkg/client/injection/ducks/duck/v1/source"
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap"
-	serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
 	"knative.dev/pkg/configmap"
 	"knative.dev/pkg/controller"
 	"knative.dev/pkg/injection/clients/dynamicclient"
@@ -45,6 +46,8 @@ import (
 	eventinglisters "knative.dev/eventing/pkg/client/listers/eventing/v1"
 	"knative.dev/eventing/pkg/duck"
 	kubeclient "knative.dev/pkg/client/injection/kube/client"
+
+	serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered"
 )
 
 // NewController initializes the controller and is called by the generated code
@@ -59,7 +62,7 @@ func NewController(
 	subscriptionInformer := subscriptioninformer.Get(ctx)
 	configmapInformer := configmapinformer.Get(ctx)
 	secretInformer := secretinformer.Get(ctx)
-	serviceaccountInformer := serviceaccountinformer.Get(ctx)
+	oidcServiceaccountInformer := serviceaccountinformer.Get(ctx, auth.OIDCLabelSelector)
 
 	featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"))
 	featureStore.WatchConfigs(cmw)
@@ -74,7 +77,7 @@ func NewController(
 		triggerLister:        triggerLister,
 		configmapLister:      configmapInformer.Lister(),
 		secretLister:         secretInformer.Lister(),
-		serviceAccountLister: serviceaccountInformer.Lister(),
+		serviceAccountLister: oidcServiceaccountInformer.Lister(),
 	}
 	impl := triggerreconciler.NewImpl(ctx, r, func(impl *controller.Impl) controller.Options {
 		return controller.Options{
@@ -112,7 +115,7 @@ func NewController(
 	})
 
 	// Reconciler Trigger when the OIDC service account changes
-	serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
+	oidcServiceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
 		FilterFunc: controller.FilterController(&eventing.Trigger{}),
 		Handler:    controller.HandleAll(impl.EnqueueControllerOf),
 	})

diff --git a/pkg/reconciler/broker/trigger/controller_test.go b/pkg/reconciler/broker/trigger/controller_test.go
@@ -17,9 +17,13 @@ limitations under the License.
 package mttrigger
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
+	"knative.dev/eventing/pkg/auth"
+	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
+
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -44,11 +48,12 @@ import (
 	_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/broker/fake"
 	_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger/fake"
 	_ "knative.dev/eventing/pkg/client/injection/informers/messaging/v1/subscription/fake"
-	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake"
+	_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/fake"
+	_ "knative.dev/pkg/client/injection/kube/informers/factory/filtered/fake"
 )
 
 func TestNew(t *testing.T) {
-	ctx, _ := SetupFakeContext(t)
+	ctx, _ := SetupFakeContext(t, SetUpInformerSelector)
 
 	c := NewController(ctx, configmap.NewStaticWatcher(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "config-features"}}))
 
@@ -57,8 +62,13 @@ func TestNew(t *testing.T) {
 	}
 }
 
+func SetUpInformerSelector(ctx context.Context) context.Context {
+	ctx = filteredFactory.WithSelectors(ctx, auth.OIDCLabelSelector)
+	return ctx
+}
+
 func TestFilterTriggers(t *testing.T) {
-	ctx, _ := SetupFakeContext(t)
+	ctx, _ := SetupFakeContext(t, SetUpInformerSelector)
 
 	tt := []struct {
 		name    string