From 71d68a421178a7ea38c56e3aba8e79732e50b8ae Mon Sep 17 00:00:00 2001 From: rahulii Date: Mon, 2 Oct 2023 16:17:34 +0530 Subject: [PATCH 01/11] support auto generation of SinkBindings identity service account Signed-off-by: rahulii --- pkg/apis/sources/v1/sinkbinding_lifecycle.go | 17 +++++++++ .../sources/v1/sinkbinding_lifecycle_test.go | 34 ++++++++++++++++++ pkg/apis/sources/v1/sinkbinding_types.go | 5 +++ pkg/reconciler/sinkbinding/controller.go | 36 +++++++++++++++++++ 4 files changed, 92 insertions(+) diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle.go b/pkg/apis/sources/v1/sinkbinding_lifecycle.go index 81371999ede..5a8d1003554 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle.go @@ -35,6 +35,7 @@ import ( var sbCondSet = apis.NewLivingConditionSet( SinkBindingConditionSinkProvided, + SinkBindingConditionOIDCIdentityCreated, ) // GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface. @@ -95,6 +96,22 @@ func (sbs *SinkBindingStatus) MarkSink(addr *duckv1.Addressable) { } } +func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedSucceeded() { + sbCondSet.Manage(sbs).MarkTrue(SinkBindingConditionOIDCIdentityCreated) +} + +func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedSucceededWithReason(reason, messageFormat string, messageA ...interface{}) { + sbCondSet.Manage(sbs).MarkTrueWithReason(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...) +} + +func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedFailed(reason, messageFormat string, messageA ...interface{}) { + sbCondSet.Manage(sbs).MarkFalse(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...) +} + +func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedUnknown(reason, messageFormat string, messageA ...interface{}) { + sbCondSet.Manage(sbs).MarkUnknown(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...) +} + // Do implements psbinding.Bindable func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) { // First undo so that we can just unconditionally append below. diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go index 99288bb595e..dc3b08bde11 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go @@ -173,9 +173,43 @@ func TestSinkBindingStatusIsReady(t *testing.T) { s.MarkBindingAvailable() return s }(), + want: false, + }, { + name: "mark OIDC identity created", + s: func() *SinkBindingStatus { + s := &SinkBindingStatus{} + s.InitializeConditions() + s.MarkSink(sink) + s.MarkBindingAvailable() + s.MarkOIDCIdentityCreatedSucceeded() + return s + }(), + want: true, + }, { + name: "mark OIDC identity created with reason", + s: func() *SinkBindingStatus { + s := &SinkBindingStatus{} + s.InitializeConditions() + s.MarkSink(sink) + s.MarkBindingAvailable() + s.MarkOIDCIdentityCreatedSucceededWithReason("TheReason", "feature is disbaled") + return s + }(), want: true, + }, { + name: "mark OIDC identity created failed", + s: func() *SinkBindingStatus { + s := &SinkBindingStatus{} + s.InitializeConditions() + s.MarkSink(sink) + s.MarkBindingAvailable() + s.MarkOIDCIdentityCreatedFailed("TheReason", "this is a message") + return s + }(), + want: false, }} + for _, test := range tests { t.Run(test.name, func(t *testing.T) { got := test.s.IsReady() diff --git a/pkg/apis/sources/v1/sinkbinding_types.go b/pkg/apis/sources/v1/sinkbinding_types.go index d06a4c6eb6e..676b345f057 100644 --- a/pkg/apis/sources/v1/sinkbinding_types.go +++ b/pkg/apis/sources/v1/sinkbinding_types.go @@ -77,6 +77,10 @@ const ( // SinkBindingConditionSinkProvided is configured to indicate whether the // sink has been properly extracted from the resolver. SinkBindingConditionSinkProvided apis.ConditionType = "SinkProvided" + + // SinkBindingConditionOIDCIdentityCreated is configured to indicate whether + // the OIDC identity has been created for the sink. + SinkBindingConditionOIDCIdentityCreated apis.ConditionType = "OIDCIdentityCreated" ) // SinkBindingStatus communicates the observed state of the SinkBinding (from the controller). @@ -88,6 +92,7 @@ type SinkBindingStatus struct { // state. // * SinkURI - the current active sink URI that has been configured for the // Source. + // * AuthStatus - the current status of the authentication configuration duckv1.SourceStatus `json:",inline"` } diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 9aa753d58ea..c8802b53ed1 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -19,7 +19,9 @@ package sinkbinding import ( "context" "errors" + "fmt" + "knative.dev/eventing/pkg/auth" sbinformer "knative.dev/eventing/pkg/client/injection/informers/sources/v1/sinkbinding" "knative.dev/pkg/client/injection/ducks/duck/v1/podspecable" "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" @@ -30,12 +32,17 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" + corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" + "knative.dev/eventing/pkg/apis/feature" v1 "knative.dev/eventing/pkg/apis/sources/v1" "knative.dev/pkg/apis/duck" + duckv1 "knative.dev/pkg/apis/duck/v1" kubeclient "knative.dev/pkg/client/injection/kube/client" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" @@ -43,6 +50,7 @@ import ( "knative.dev/pkg/logging" "knative.dev/pkg/tracker" "knative.dev/pkg/webhook/psbinding" + serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount" ) const ( @@ -52,6 +60,8 @@ const ( type SinkBindingSubResourcesReconciler struct { res *resolver.URIResolver tracker tracker.Interface + serviceAccountLister corev1listers.ServiceAccountLister + kubeclient kubernetes.Interface } // NewController returns a new SinkBinding reconciler. @@ -65,6 +75,7 @@ func NewController( dc := dynamicclient.Get(ctx) psInformerFactory := podspecable.Get(ctx) namespaceInformer := namespace.Get(ctx) + serviceaccountInformer := serviceaccountinformer.Get(ctx) c := &psbinding.BaseReconciler{ LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { @@ -101,6 +112,7 @@ func NewController( c.SubResourcesReconciler = &SinkBindingSubResourcesReconciler{ res: sbResolver, tracker: impl.Tracker, + kubeclient: kubeclient.Get(ctx), } c.WithContext = func(ctx context.Context, b psbinding.Bindable) (context.Context, error) { @@ -114,6 +126,12 @@ func NewController( }, } + // Reconciler Trigger when the OIDC service account changes + serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterController(&v1.SinkBinding{}), + Handler: controller.HandleAll(impl.EnqueueControllerOf), + }) + return impl } @@ -161,6 +179,24 @@ func (s *SinkBindingSubResourcesReconciler) Reconcile(ctx context.Context, b psb Name: sb.Spec.Sink.Ref.Name, }, b) } + + featureFlags := feature.FromContext(ctx) + if featureFlags.IsOIDCAuthentication() { + saName := auth.GetOIDCServiceAccountNameForResource(eventingv1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta) + sb.Status.Auth = &duckv1.AuthStatus{ + ServiceAccountName: &saName, + } + + if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, s.serviceAccountLister, s.kubeclient, eventingv1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta); err != nil { + sb.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err) + return err + } + sb.Status.MarkOIDCIdentityCreatedSucceeded() + } else { + sb.Status.Auth = nil + sb.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "") + } + addr, err := s.res.AddressableFromDestinationV1(ctx, sb.Spec.Sink, sb) if err != nil { logging.FromContext(ctx).Errorf("Failed to get Addressable from Destination: %w", err) From 6409b6127c986ca57ab751b8cac8676cb36f5021 Mon Sep 17 00:00:00 2001 From: rahulii Date: Mon, 2 Oct 2023 16:31:37 +0530 Subject: [PATCH 02/11] pass serviceAccountLister to reconciler Signed-off-by: rahulii --- pkg/reconciler/sinkbinding/controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index c8802b53ed1..9349a0b5505 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -113,6 +113,7 @@ func NewController( res: sbResolver, tracker: impl.Tracker, kubeclient: kubeclient.Get(ctx), + serviceAccountLister: serviceaccountInformer.Lister(), } c.WithContext = func(ctx context.Context, b psbinding.Bindable) (context.Context, error) { From 88f52b448987cd760a7f8b1bd9e0692d17c8341e Mon Sep 17 00:00:00 2001 From: rahulii Date: Wed, 4 Oct 2023 17:31:57 +0530 Subject: [PATCH 03/11] give service account permission and minor fixes Signed-off-by: rahulii --- config/core/roles/webhook-clusterrole.yaml | 12 ++++++++++++ pkg/apis/sources/v1/sinkbinding_lifecycle.go | 6 ++++++ pkg/reconciler/sinkbinding/controller.go | 16 ++++++++++++---- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/config/core/roles/webhook-clusterrole.yaml b/config/core/roles/webhook-clusterrole.yaml index 023c383af29..944c2ae5f20 100644 --- a/config/core/roles/webhook-clusterrole.yaml +++ b/config/core/roles/webhook-clusterrole.yaml @@ -109,6 +109,18 @@ rules: - "create" - "patch" + - apiGroups: + - "" + resources: + - "serviceaccounts" + verbs: + - "get" + - "create" + - "update" + - "list" + - "watch" + - "patch" + # Necessary for conversion webhook. These are copied from the serving # TODO: Do we really need all these permissions? - apiGroups: ["apiextensions.k8s.io"] diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle.go b/pkg/apis/sources/v1/sinkbinding_lifecycle.go index 5a8d1003554..410aa8cb58a 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "knative.dev/eventing/pkg/apis/feature" "knative.dev/pkg/apis" "knative.dev/pkg/apis/duck" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -72,6 +73,7 @@ func (sbs *SinkBindingStatus) SetObservedGeneration(gen int64) { // with all of its conditions configured to Unknown. func (sbs *SinkBindingStatus) InitializeConditions() { sbCondSet.Manage(sbs).InitializeConditions() + sbs.MarkOIDCIdentityCreatedNotSupported() } // MarkBindingUnavailable marks the SinkBinding's Ready condition to False with @@ -112,6 +114,10 @@ func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedUnknown(reason, messageForm sbCondSet.Manage(sbs).MarkUnknown(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...) } +func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedNotSupported() { + // in case the OIDC feature is not supported, we mark the condition as true, to not mark the SinkBinding unready. + sbCondSet.Manage(sbs).MarkTrueWithReason(SinkBindingConditionOIDCIdentityCreated, fmt.Sprintf("%s feature not yet supported for SinkBinding", feature.OIDCAuthentication), "") +} // Do implements psbinding.Bindable func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) { // First undo so that we can just unconditionally append below. diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 9349a0b5505..1038442eddf 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -38,7 +38,6 @@ import ( corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" - eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" v1 "knative.dev/eventing/pkg/apis/sources/v1" "knative.dev/pkg/apis/duck" @@ -62,6 +61,7 @@ type SinkBindingSubResourcesReconciler struct { tracker tracker.Interface serviceAccountLister corev1listers.ServiceAccountLister kubeclient kubernetes.Interface + featureStore *feature.Store } // NewController returns a new SinkBinding reconciler. @@ -76,6 +76,10 @@ func NewController( psInformerFactory := podspecable.Get(ctx) namespaceInformer := namespace.Get(ctx) serviceaccountInformer := serviceaccountinformer.Get(ctx) + + featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store")) + featureStore.WatchConfigs(cmw) + c := &psbinding.BaseReconciler{ LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { @@ -114,6 +118,7 @@ func NewController( tracker: impl.Tracker, kubeclient: kubeclient.Get(ctx), serviceAccountLister: serviceaccountInformer.Lister(), + featureStore: featureStore, } c.WithContext = func(ctx context.Context, b psbinding.Bindable) (context.Context, error) { @@ -127,7 +132,7 @@ func NewController( }, } - // Reconciler Trigger when the OIDC service account changes + // Reconciler SinkBinding when the OIDC service account changes serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: controller.FilterController(&v1.SinkBinding{}), Handler: controller.HandleAll(impl.EnqueueControllerOf), @@ -181,14 +186,17 @@ func (s *SinkBindingSubResourcesReconciler) Reconcile(ctx context.Context, b psb }, b) } + // Add feature flags to context + ctx = s.featureStore.ToContext(ctx) + featureFlags := feature.FromContext(ctx) if featureFlags.IsOIDCAuthentication() { - saName := auth.GetOIDCServiceAccountNameForResource(eventingv1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta) + saName := auth.GetOIDCServiceAccountNameForResource(v1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta) sb.Status.Auth = &duckv1.AuthStatus{ ServiceAccountName: &saName, } - if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, s.serviceAccountLister, s.kubeclient, eventingv1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta); err != nil { + if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, s.serviceAccountLister, s.kubeclient, v1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta); err != nil { sb.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err) return err } From 7e48b41ba302131a8ab3103b850d6781975258b6 Mon Sep 17 00:00:00 2001 From: rahulii Date: Wed, 4 Oct 2023 19:14:35 +0530 Subject: [PATCH 04/11] fix review comments Signed-off-by: rahulii --- pkg/reconciler/sinkbinding/controller.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 1038442eddf..1af412553c3 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -27,6 +27,7 @@ import ( "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" "knative.dev/pkg/reconciler" "knative.dev/pkg/resolver" + "knative.dev/pkg/system" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -43,13 +44,14 @@ import ( "knative.dev/pkg/apis/duck" duckv1 "knative.dev/pkg/apis/duck/v1" kubeclient "knative.dev/pkg/client/injection/kube/client" + 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" "knative.dev/pkg/logging" "knative.dev/pkg/tracker" "knative.dev/pkg/webhook/psbinding" - serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount" ) const ( @@ -76,6 +78,7 @@ func NewController( psInformerFactory := podspecable.Get(ctx) namespaceInformer := namespace.Get(ctx) serviceaccountInformer := serviceaccountinformer.Get(ctx) + configmapInformer := configmapinformer.Get(ctx) featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store")) featureStore.WatchConfigs(cmw) @@ -132,12 +135,20 @@ func NewController( }, } - // Reconciler SinkBinding when the OIDC service account changes + // Reconcile SinkBinding when the OIDC service account changes serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: controller.FilterController(&v1.SinkBinding{}), Handler: controller.HandleAll(impl.EnqueueControllerOf), }) + // reconcile sinkindings on changes on the features configmap + configmapInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), feature.FlagsConfigName), + Handler: controller.HandleAll(func(i interface{}) { + impl.GlobalResync(sbInformer.Informer()) + }), + }) + return impl } @@ -186,10 +197,7 @@ func (s *SinkBindingSubResourcesReconciler) Reconcile(ctx context.Context, b psb }, b) } - // Add feature flags to context - ctx = s.featureStore.ToContext(ctx) - - featureFlags := feature.FromContext(ctx) + featureFlags := s.featureStore.Load() if featureFlags.IsOIDCAuthentication() { saName := auth.GetOIDCServiceAccountNameForResource(v1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta) sb.Status.Auth = &duckv1.AuthStatus{ From 910f5813dcfe47409404e6b6c029e17eeae6a61c Mon Sep 17 00:00:00 2001 From: rahulii Date: Thu, 5 Oct 2023 15:53:25 +0530 Subject: [PATCH 05/11] fix linting and review comments Signed-off-by: rahulii --- config/core/roles/webhook-clusterrole.yaml | 8 +------- pkg/apis/sources/v1/sinkbinding_lifecycle.go | 1 + .../sources/v1/sinkbinding_lifecycle_test.go | 3 +-- pkg/reconciler/sinkbinding/controller.go | 16 ++++++++-------- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/config/core/roles/webhook-clusterrole.yaml b/config/core/roles/webhook-clusterrole.yaml index 944c2ae5f20..481d46ab7d9 100644 --- a/config/core/roles/webhook-clusterrole.yaml +++ b/config/core/roles/webhook-clusterrole.yaml @@ -113,13 +113,7 @@ rules: - "" resources: - "serviceaccounts" - verbs: - - "get" - - "create" - - "update" - - "list" - - "watch" - - "patch" + verbs: *everything # Necessary for conversion webhook. These are copied from the serving # TODO: Do we really need all these permissions? diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle.go b/pkg/apis/sources/v1/sinkbinding_lifecycle.go index 410aa8cb58a..455ffbc96fa 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle.go @@ -118,6 +118,7 @@ func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedNotSupported() { // in case the OIDC feature is not supported, we mark the condition as true, to not mark the SinkBinding unready. sbCondSet.Manage(sbs).MarkTrueWithReason(SinkBindingConditionOIDCIdentityCreated, fmt.Sprintf("%s feature not yet supported for SinkBinding", feature.OIDCAuthentication), "") } + // Do implements psbinding.Bindable func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) { // First undo so that we can just unconditionally append below. diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go index dc3b08bde11..bcd1c873b5b 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go @@ -192,7 +192,7 @@ func TestSinkBindingStatusIsReady(t *testing.T) { s.InitializeConditions() s.MarkSink(sink) s.MarkBindingAvailable() - s.MarkOIDCIdentityCreatedSucceededWithReason("TheReason", "feature is disbaled") + s.MarkOIDCIdentityCreatedSucceededWithReason("TheReason", "feature is disabled") return s }(), want: true, @@ -209,7 +209,6 @@ func TestSinkBindingStatusIsReady(t *testing.T) { want: false, }} - for _, test := range tests { t.Run(test.name, func(t *testing.T) { got := test.s.IsReady() diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 1af412553c3..2a37f97bba5 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -59,11 +59,11 @@ const ( ) type SinkBindingSubResourcesReconciler struct { - res *resolver.URIResolver - tracker tracker.Interface + res *resolver.URIResolver + tracker tracker.Interface serviceAccountLister corev1listers.ServiceAccountLister - kubeclient kubernetes.Interface - featureStore *feature.Store + kubeclient kubernetes.Interface + featureStore *feature.Store } // NewController returns a new SinkBinding reconciler. @@ -117,11 +117,11 @@ func NewController( sbResolver := resolver.NewURIResolverFromTracker(ctx, impl.Tracker) c.SubResourcesReconciler = &SinkBindingSubResourcesReconciler{ - res: sbResolver, - tracker: impl.Tracker, + res: sbResolver, + tracker: impl.Tracker, kubeclient: kubeclient.Get(ctx), serviceAccountLister: serviceaccountInformer.Lister(), - featureStore: featureStore, + featureStore: featureStore, } c.WithContext = func(ctx context.Context, b psbinding.Bindable) (context.Context, error) { @@ -196,7 +196,7 @@ func (s *SinkBindingSubResourcesReconciler) Reconcile(ctx context.Context, b psb Name: sb.Spec.Sink.Ref.Name, }, b) } - + featureFlags := s.featureStore.Load() if featureFlags.IsOIDCAuthentication() { saName := auth.GetOIDCServiceAccountNameForResource(v1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta) From 426c9756f7f367d7f55bd9dcc6c63303dad8d954 Mon Sep 17 00:00:00 2001 From: rahulii Date: Thu, 5 Oct 2023 15:56:36 +0530 Subject: [PATCH 06/11] fix review comments Signed-off-by: rahulii --- config/core/roles/webhook-clusterrole.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/core/roles/webhook-clusterrole.yaml b/config/core/roles/webhook-clusterrole.yaml index 481d46ab7d9..1f248ba3864 100644 --- a/config/core/roles/webhook-clusterrole.yaml +++ b/config/core/roles/webhook-clusterrole.yaml @@ -109,6 +109,7 @@ rules: - "create" - "patch" +# For the SinkBinding reconciler adding the OIDC identity service accounts - apiGroups: - "" resources: From d0c24d059759b0c8d96f115dcab94d24bc666f1f Mon Sep 17 00:00:00 2001 From: rahulii Date: Thu, 5 Oct 2023 16:40:11 +0530 Subject: [PATCH 07/11] fix test Signed-off-by: rahulii --- pkg/apis/sources/v1/sinkbinding_lifecycle_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go index bcd1c873b5b..d38e426f655 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go @@ -173,7 +173,7 @@ func TestSinkBindingStatusIsReady(t *testing.T) { s.MarkBindingAvailable() return s }(), - want: false, + want: true, }, { name: "mark OIDC identity created", s: func() *SinkBindingStatus { From 650ae20e51d41b137334accd0909496bce0ca1b9 Mon Sep 17 00:00:00 2001 From: rahulii Date: Tue, 10 Oct 2023 01:35:08 +0530 Subject: [PATCH 08/11] review comments Signed-off-by: rahulii --- pkg/apis/sources/v1/sinkbinding_lifecycle.go | 7 ------- pkg/apis/sources/v1/sinkbinding_lifecycle_test.go | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle.go b/pkg/apis/sources/v1/sinkbinding_lifecycle.go index 455ffbc96fa..5a8d1003554 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle.go @@ -26,7 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "knative.dev/eventing/pkg/apis/feature" "knative.dev/pkg/apis" "knative.dev/pkg/apis/duck" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -73,7 +72,6 @@ func (sbs *SinkBindingStatus) SetObservedGeneration(gen int64) { // with all of its conditions configured to Unknown. func (sbs *SinkBindingStatus) InitializeConditions() { sbCondSet.Manage(sbs).InitializeConditions() - sbs.MarkOIDCIdentityCreatedNotSupported() } // MarkBindingUnavailable marks the SinkBinding's Ready condition to False with @@ -114,11 +112,6 @@ func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedUnknown(reason, messageForm sbCondSet.Manage(sbs).MarkUnknown(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...) } -func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedNotSupported() { - // in case the OIDC feature is not supported, we mark the condition as true, to not mark the SinkBinding unready. - sbCondSet.Manage(sbs).MarkTrueWithReason(SinkBindingConditionOIDCIdentityCreated, fmt.Sprintf("%s feature not yet supported for SinkBinding", feature.OIDCAuthentication), "") -} - // Do implements psbinding.Bindable func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) { // First undo so that we can just unconditionally append below. diff --git a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go index d38e426f655..6795e4e84ec 100644 --- a/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go +++ b/pkg/apis/sources/v1/sinkbinding_lifecycle_test.go @@ -171,6 +171,7 @@ func TestSinkBindingStatusIsReady(t *testing.T) { s.InitializeConditions() s.MarkSink(sink) s.MarkBindingAvailable() + s.MarkOIDCIdentityCreatedSucceeded() return s }(), want: true, From 5bf370cedaa49446a35a2543c223d0165b7ab272 Mon Sep 17 00:00:00 2001 From: rahulii Date: Tue, 10 Oct 2023 12:40:08 +0530 Subject: [PATCH 09/11] add handler to feature store Signed-off-by: rahulii --- config/core/roles/webhook-clusterrole.yaml | 1 - pkg/reconciler/sinkbinding/controller.go | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/config/core/roles/webhook-clusterrole.yaml b/config/core/roles/webhook-clusterrole.yaml index 1f248ba3864..91b6e330712 100644 --- a/config/core/roles/webhook-clusterrole.yaml +++ b/config/core/roles/webhook-clusterrole.yaml @@ -115,7 +115,6 @@ rules: resources: - "serviceaccounts" verbs: *everything - # Necessary for conversion webhook. These are copied from the serving # TODO: Do we really need all these permissions? - apiGroups: ["apiextensions.k8s.io"] diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 2a37f97bba5..21ea1430b48 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -80,9 +80,6 @@ func NewController( serviceaccountInformer := serviceaccountinformer.Get(ctx) configmapInformer := configmapinformer.Get(ctx) - featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store")) - featureStore.WatchConfigs(cmw) - c := &psbinding.BaseReconciler{ LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { @@ -112,6 +109,12 @@ func NewController( Logger: logger, }) + featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(name string, value interface{}) { + impl.GlobalResync(sbInformer.Informer()) + }) + + featureStore.WatchConfigs(cmw) + sbInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) namespaceInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) From ac09c62282c336e20ef481577782ba93996a98f9 Mon Sep 17 00:00:00 2001 From: rahulii Date: Tue, 10 Oct 2023 12:48:06 +0530 Subject: [PATCH 10/11] fix lint Signed-off-by: rahulii --- pkg/reconciler/sinkbinding/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/sinkbinding/controller.go b/pkg/reconciler/sinkbinding/controller.go index 21ea1430b48..d60b25169ff 100644 --- a/pkg/reconciler/sinkbinding/controller.go +++ b/pkg/reconciler/sinkbinding/controller.go @@ -112,7 +112,7 @@ func NewController( featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(name string, value interface{}) { impl.GlobalResync(sbInformer.Informer()) }) - + featureStore.WatchConfigs(cmw) sbInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) From fdda99e79c3ee578ae2e0c155e5c04cfe0219246 Mon Sep 17 00:00:00 2001 From: rahulii Date: Tue, 10 Oct 2023 14:16:50 +0530 Subject: [PATCH 11/11] remove comment Signed-off-by: rahulii --- pkg/apis/sources/v1/sinkbinding_types.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/apis/sources/v1/sinkbinding_types.go b/pkg/apis/sources/v1/sinkbinding_types.go index 676b345f057..512d687d31a 100644 --- a/pkg/apis/sources/v1/sinkbinding_types.go +++ b/pkg/apis/sources/v1/sinkbinding_types.go @@ -92,7 +92,6 @@ type SinkBindingStatus struct { // state. // * SinkURI - the current active sink URI that has been configured for the // Source. - // * AuthStatus - the current status of the authentication configuration duckv1.SourceStatus `json:",inline"` }