From c8f5519c61acb918ad6341526a7e54eff31de393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Tue, 26 Sep 2023 14:56:27 +0200 Subject: [PATCH] Update trigger reconciler to create OIDC service account --- pkg/apis/eventing/v1/trigger_lifecycle.go | 20 +++++++++++- pkg/auth/serviceaccount.go | 35 ++++++++++++++++++++- pkg/auth/serviceaccount_test.go | 2 +- pkg/reconciler/broker/trigger/controller.go | 10 ++++++ pkg/reconciler/broker/trigger/trigger.go | 19 +++++++++++ 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/pkg/apis/eventing/v1/trigger_lifecycle.go b/pkg/apis/eventing/v1/trigger_lifecycle.go index c2ebce95cfa..a5d00a78665 100644 --- a/pkg/apis/eventing/v1/trigger_lifecycle.go +++ b/pkg/apis/eventing/v1/trigger_lifecycle.go @@ -23,7 +23,7 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" ) -var triggerCondSet = apis.NewLivingConditionSet(TriggerConditionBroker, TriggerConditionSubscribed, TriggerConditionDependency, TriggerConditionSubscriberResolved, TriggerConditionDeadLetterSinkResolved) +var triggerCondSet = apis.NewLivingConditionSet(TriggerConditionBroker, TriggerConditionSubscribed, TriggerConditionDependency, TriggerConditionSubscriberResolved, TriggerConditionDeadLetterSinkResolved, TriggerConditionOIDCServiceAccountResolved) const ( // TriggerConditionReady has status True when all subconditions below have been set to True. @@ -39,6 +39,8 @@ const ( TriggerConditionDeadLetterSinkResolved apis.ConditionType = "DeadLetterSinkResolved" + TriggerConditionOIDCServiceAccountResolved apis.ConditionType = "OIDCServiceAccountResolved" + // TriggerAnyFilter Constant to represent that we should allow anything. TriggerAnyFilter = "" ) @@ -199,3 +201,19 @@ func (ts *TriggerStatus) PropagateDependencyStatus(ks *duckv1.Source) { ts.MarkDependencyUnknown("DependencyUnknown", "The status of Dependency is invalid: %v", kc.Status) } } + +func (ts *TriggerStatus) MarkOIDCServiceAccountResolvedSucceeded() { + triggerCondSet.Manage(ts).MarkTrue(TriggerConditionOIDCServiceAccountResolved) +} + +func (ts *TriggerStatus) MarkOIDCServiceAccountResolvedSucceededWithReason(reason, messageFormat string, messageA ...interface{}) { + triggerCondSet.Manage(ts).MarkTrueWithReason(TriggerConditionOIDCServiceAccountResolved, reason, messageFormat, messageA...) +} + +func (ts *TriggerStatus) MarkOIDCServiceAccountResolvedFailed(reason, messageFormat string, messageA ...interface{}) { + triggerCondSet.Manage(ts).MarkFalse(TriggerConditionOIDCServiceAccountResolved, reason, messageFormat, messageA...) +} + +func (ts *TriggerStatus) MarkOIDCServiceAccountResolvedUnknown(reason, messageFormat string, messageA ...interface{}) { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionOIDCServiceAccountResolved, reason, messageFormat, messageA...) +} diff --git a/pkg/auth/serviceaccount.go b/pkg/auth/serviceaccount.go index c31f30ea649..cf0f3e5a62b 100644 --- a/pkg/auth/serviceaccount.go +++ b/pkg/auth/serviceaccount.go @@ -17,12 +17,17 @@ limitations under the License. package auth import ( + "context" "fmt" "strings" + "go.uber.org/zap" v1 "k8s.io/api/core/v1" + apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "knative.dev/pkg/logging" "knative.dev/pkg/ptr" ) @@ -47,7 +52,7 @@ func GetOIDCServiceAccountForResource(gvk schema.GroupVersionKind, objectMeta me Kind: gvk.GroupKind().Kind, Name: objectMeta.GetName(), UID: objectMeta.GetUID(), - Controller: ptr.Bool(false), + Controller: ptr.Bool(true), BlockOwnerDeletion: ptr.Bool(false), }, }, @@ -57,3 +62,31 @@ func GetOIDCServiceAccountForResource(gvk schema.GroupVersionKind, objectMeta me }, } } + +func EnsureOIDCServiceAccountExistsForResource(ctx context.Context, kubeclient kubernetes.Interface, gvk schema.GroupVersionKind, objectMeta metav1.ObjectMeta) error { + saName := GetOIDCServiceAccountNameForResource(gvk, objectMeta) + sa, err := kubeclient.CoreV1().ServiceAccounts(objectMeta.Namespace).Get(ctx, saName, metav1.GetOptions{}) + + // If the resource doesn't exist, we'll create it. + if apierrs.IsNotFound(err) { + logging.FromContext(ctx).Infow("Creating OIDC service account", zap.Error(err)) + + expected := GetOIDCServiceAccountForResource(gvk, objectMeta) + + _, err = kubeclient.CoreV1().ServiceAccounts(objectMeta.Namespace).Create(ctx, expected, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("could not create OIDC service account %s/%s for %s: %w", objectMeta.Name, objectMeta.Namespace, gvk.Kind, err) + } + return nil + } else if err != nil { + logging.FromContext(ctx).Errorw("Failed to get OIDC service account", zap.Error(err)) + + return fmt.Errorf("could not get OIDC service account %s/%s for %s: %w", objectMeta.Name, objectMeta.Namespace, gvk.Kind, err) + } else if !metav1.IsControlledBy(&sa.ObjectMeta, &objectMeta) { + logging.FromContext(ctx).Errorw("Service account not owned by parent resource", zap.Any("service account", sa), zap.Any("parent resource ("+gvk.Kind+")", objectMeta)) + + return fmt.Errorf("%s %q does not own service account %q", gvk.Kind, objectMeta.Name, sa.Name) + } + + return nil +} diff --git a/pkg/auth/serviceaccount_test.go b/pkg/auth/serviceaccount_test.go index 22e52ee4144..c938b203ec7 100644 --- a/pkg/auth/serviceaccount_test.go +++ b/pkg/auth/serviceaccount_test.go @@ -84,7 +84,7 @@ func TestGetOIDCServiceAccountForResource(t *testing.T) { Kind: "Broker", Name: "my-broker", UID: "my-uuid", - Controller: ptr.Bool(false), + Controller: ptr.Bool(true), BlockOwnerDeletion: ptr.Bool(false), }, }, diff --git a/pkg/reconciler/broker/trigger/controller.go b/pkg/reconciler/broker/trigger/controller.go index a21dd9f4e5d..2da1f57bde0 100644 --- a/pkg/reconciler/broker/trigger/controller.go +++ b/pkg/reconciler/broker/trigger/controller.go @@ -24,6 +24,7 @@ import ( "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" @@ -43,6 +44,7 @@ import ( triggerreconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/trigger" eventinglisters "knative.dev/eventing/pkg/client/listers/eventing/v1" "knative.dev/eventing/pkg/duck" + kubeclient "knative.dev/pkg/client/injection/kube/client" ) // NewController initializes the controller and is called by the generated code @@ -57,6 +59,7 @@ func NewController( subscriptionInformer := subscriptioninformer.Get(ctx) configmapInformer := configmapinformer.Get(ctx) secretInformer := secretinformer.Get(ctx) + serviceaccountInformer := serviceaccountinformer.Get(ctx) featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store")) featureStore.WatchConfigs(cmw) @@ -65,6 +68,7 @@ func NewController( r := &Reconciler{ eventingClientSet: eventingclient.Get(ctx), dynamicClientSet: dynamicclient.Get(ctx), + kubeclient: kubeclient.Get(ctx), subscriptionLister: subscriptionInformer.Lister(), brokerLister: brokerInformer.Lister(), triggerLister: triggerLister, @@ -106,6 +110,12 @@ func NewController( Handler: controller.HandleAll(impl.EnqueueControllerOf), }) + // Reconciler Trigger when the OIDC service account changes + serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterController(&eventing.Trigger{}), + Handler: controller.HandleAll(impl.EnqueueControllerOf), + }) + return impl } diff --git a/pkg/reconciler/broker/trigger/trigger.go b/pkg/reconciler/broker/trigger/trigger.go index dd8ae714509..2d75636cc83 100644 --- a/pkg/reconciler/broker/trigger/trigger.go +++ b/pkg/reconciler/broker/trigger/trigger.go @@ -27,6 +27,7 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/utils/pointer" "knative.dev/pkg/apis" @@ -44,6 +45,7 @@ import ( eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1" + "knative.dev/eventing/pkg/auth" clientset "knative.dev/eventing/pkg/client/clientset/versioned" eventinglisters "knative.dev/eventing/pkg/client/listers/eventing/v1" messaginglisters "knative.dev/eventing/pkg/client/listers/messaging/v1" @@ -65,6 +67,7 @@ const ( type Reconciler struct { eventingClientSet clientset.Interface dynamicClientSet dynamic.Interface + kubeclient kubernetes.Interface // listers index properties about resources subscriptionLister messaginglisters.SubscriptionLister @@ -137,6 +140,22 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, t *eventingv1.Trigger) p return err } + featureFlags := feature.FromContext(ctx) + if featureFlags.IsOIDCAuthentication() { + saName := auth.GetOIDCServiceAccountNameForResource(eventingv1.SchemeGroupVersion.WithKind("Trigger"), t.ObjectMeta) + t.Status.Auth = &duckv1.AuthStatus{ + ServiceAccountName: &saName, + } + + if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, r.kubeclient, eventingv1.SchemeGroupVersion.WithKind("Trigger"), t.ObjectMeta); err != nil { + t.Status.MarkOIDCServiceAccountResolvedFailed("Unable to resolve service account for OIDC authentication", "%v", err) + return err + } + t.Status.MarkOIDCServiceAccountResolvedSucceeded() + } else { + t.Status.MarkOIDCServiceAccountResolvedSucceededWithReason("OIDC authentication feature disabled", "") + } + sub, err := r.subscribeToBrokerChannel(ctx, b, t, brokerTrigger) if err != nil { logging.FromContext(ctx).Errorw("Unable to Subscribe", zap.Error(err))