Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the AuthStatus Logic in Eventing OIDC Feature Track #7417

Merged
merged 12 commits into from
Nov 28, 2023
4 changes: 4 additions & 0 deletions pkg/apis/eventing/v1/trigger_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,7 @@ func (ts *TriggerStatus) MarkOIDCIdentityCreatedNotSupported() {
// in case the OIDC feature is not supported, we mark the condition as true, to not mark the Trigger unready.
triggerCondSet.Manage(ts).MarkTrueWithReason(TriggerConditionOIDCIdentityCreated, fmt.Sprintf("%s feature not yet supported for this Broker class", feature.OIDCAuthentication), "")
}

func (ts *TriggerStatus) MarkStatus(authStatus *duckv1.AuthStatus) {
ts.Auth = authStatus
}
5 changes: 5 additions & 0 deletions pkg/apis/messaging/v1/subscription_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1

import (
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

// SubCondSet is a condition set with Ready as the happy condition and
Expand Down Expand Up @@ -131,3 +132,7 @@ func (ss *SubscriptionStatus) MarkOIDCIdentityCreatedFailed(reason, messageForma
func (ss *SubscriptionStatus) MarkOIDCIdentityCreatedUnknown(reason, messageFormat string, messageA ...interface{}) {
SubCondSet.Manage(ss).MarkUnknown(SubscriptionConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

func (ss *SubscriptionStatus) MarkStatus(authStatus *duckv1.AuthStatus) {
ss.Auth = authStatus
}
4 changes: 4 additions & 0 deletions pkg/apis/sources/v1/apiserver_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,7 @@ func (s *ApiServerSourceStatus) MarkOIDCIdentityCreatedFailed(reason, messageFor
func (s *ApiServerSourceStatus) MarkOIDCIdentityCreatedUnknown(reason, messageFormat string, messageA ...interface{}) {
apiserverCondSet.Manage(s).MarkUnknown(ApiServerConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

func (s *ApiServerSourceStatus) MarkStatus(authStatus *duckv1.AuthStatus) {
s.Auth = authStatus
}
4 changes: 4 additions & 0 deletions pkg/apis/sources/v1/ping_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,7 @@ func (s *PingSourceStatus) MarkOIDCIdentityCreatedFailed(reason, messageFormat s
func (s *PingSourceStatus) MarkOIDCIdentityCreatedUnknown(reason, messageFormat string, messageA ...interface{}) {
PingSourceCondSet.Manage(s).MarkUnknown(PingSourceConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

func (s *PingSourceStatus) MarkStatus(authStatus *duckv1.AuthStatus) {
s.Auth = authStatus
}
4 changes: 4 additions & 0 deletions pkg/apis/sources/v1/sinkbinding_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func (sbs *SinkBindingStatus) MarkOIDCIdentityCreatedUnknown(reason, messageForm
sbCondSet.Manage(sbs).MarkUnknown(SinkBindingConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

func (sbs *SinkBindingStatus) MarkStatus(authStatus *duckv1.AuthStatus) {
sbs.Auth = authStatus
}

// Do implements psbinding.Bindable
func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) {
// First undo so that we can just unconditionally append below.
Expand Down
30 changes: 30 additions & 0 deletions pkg/auth/serviceaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/kubernetes"
corev1listers "k8s.io/client-go/listers/core/v1"
"knative.dev/eventing/pkg/apis/feature"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/logging"
"knative.dev/pkg/ptr"
pkgreconciler "knative.dev/pkg/reconciler"
)

// GetOIDCServiceAccountNameForResource returns the service account name to use
Expand Down Expand Up @@ -94,3 +97,30 @@ func EnsureOIDCServiceAccountExistsForResource(ctx context.Context, serviceAccou

return nil
}

type OIDCStatusMarker interface {
xiangpingjiang marked this conversation as resolved.
Show resolved Hide resolved
MarkOIDCIdentityCreatedSucceeded()
MarkOIDCIdentityCreatedSucceededWithReason(reason, messageFormat string, messageA ...interface{})
MarkOIDCIdentityCreatedFailed(reason, messageFormat string, messageA ...interface{})
MarkStatus(authStatus *duckv1.AuthStatus)
xiangpingjiang marked this conversation as resolved.
Show resolved Hide resolved
}

func OIDCAuthStatusUtility(ctx context.Context, serviceAccountLister corev1listers.ServiceAccountLister, kubeclient kubernetes.Interface, gvk schema.GroupVersionKind, objectMeta metav1.ObjectMeta, marker OIDCStatusMarker) pkgreconciler.Event {
xiangpingjiang marked this conversation as resolved.
Show resolved Hide resolved
featureFlags := feature.FromContext(ctx)
if featureFlags.IsOIDCAuthentication() {
xiangpingjiang marked this conversation as resolved.
Show resolved Hide resolved
saName := GetOIDCServiceAccountNameForResource(gvk, objectMeta)

marker.MarkStatus(&duckv1.AuthStatus{
ServiceAccountName: &saName,
})
if err := EnsureOIDCServiceAccountExistsForResource(ctx, serviceAccountLister, kubeclient, gvk, objectMeta); err != nil {
marker.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err)
return err
}
marker.MarkOIDCIdentityCreatedSucceeded()
} else {
marker.MarkStatus(nil)
marker.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
}
return nil
}
18 changes: 2 additions & 16 deletions pkg/reconciler/apiserversource/apiserversource.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
pkgreconciler "knative.dev/pkg/reconciler"
"knative.dev/pkg/resolver"

"knative.dev/eventing/pkg/apis/feature"
apisources "knative.dev/eventing/pkg/apis/sources"
v1 "knative.dev/eventing/pkg/apis/sources/v1"
"knative.dev/eventing/pkg/auth"
Expand Down Expand Up @@ -99,21 +98,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *v1.ApiServerSour
}

// OIDC authentication
featureFlags := feature.FromContext(ctx)
if featureFlags.IsOIDCAuthentication() {
saName := auth.GetOIDCServiceAccountNameForResource(v1.SchemeGroupVersion.WithKind("ApiServerSource"), source.ObjectMeta)
source.Status.Auth = &duckv1.AuthStatus{
ServiceAccountName: &saName,
}

if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, r.serviceAccountLister, r.kubeClientSet, v1.SchemeGroupVersion.WithKind("ApiServerSource"), source.ObjectMeta); err != nil {
source.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err)
return err
}
source.Status.MarkOIDCIdentityCreatedSucceeded()
} else {
source.Status.Auth = nil
source.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
if err := auth.OIDCAuthStatusUtility(ctx, r.serviceAccountLister, r.kubeClientSet, v1.SchemeGroupVersion.WithKind("ApiServerSource"), source.ObjectMeta, &source.Status); err != nil {
return err
}

sinkAddr, err := r.sinkResolver.AddressableFromDestinationV1(ctx, *dest, source)
Expand Down
17 changes: 2 additions & 15 deletions pkg/reconciler/broker/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,8 @@ 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.serviceAccountLister, r.kubeclient, eventingv1.SchemeGroupVersion.WithKind("Trigger"), t.ObjectMeta); err != nil {
t.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err)
return err
}
t.Status.MarkOIDCIdentityCreatedSucceeded()
} else {
t.Status.Auth = nil
t.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
if err = auth.OIDCAuthStatusUtility(ctx, r.serviceAccountLister, r.kubeclient, eventingv1.SchemeGroupVersion.WithKind("Trigger"), t.ObjectMeta, &t.Status); err != nil {
return err
}

sub, err := r.subscribeToBrokerChannel(ctx, b, t, brokerTrigger)
Expand Down
20 changes: 3 additions & 17 deletions pkg/reconciler/pingsource/pingsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"fmt"

v1 "k8s.io/client-go/listers/core/v1"
"knative.dev/eventing/pkg/apis/feature"
"knative.dev/eventing/pkg/auth"

"go.uber.org/zap"

Expand All @@ -46,6 +44,7 @@ import (
"knative.dev/eventing/pkg/adapter/mtping"
"knative.dev/eventing/pkg/adapter/v2"
sourcesv1 "knative.dev/eventing/pkg/apis/sources/v1"
"knative.dev/eventing/pkg/auth"
pingsourcereconciler "knative.dev/eventing/pkg/client/injection/reconciler/sources/v1/pingsource"
"knative.dev/eventing/pkg/reconciler/pingsource/resources"
reconcilersource "knative.dev/eventing/pkg/reconciler/source"
Expand Down Expand Up @@ -106,21 +105,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *sourcesv1.PingSo
}

// OIDC authentication
featureFlags := feature.FromContext(ctx)
if featureFlags.IsOIDCAuthentication() {
saName := auth.GetOIDCServiceAccountNameForResource(sourcesv1.SchemeGroupVersion.WithKind("PingSource"), source.ObjectMeta)
source.Status.Auth = &duckv1.AuthStatus{
ServiceAccountName: &saName,
}

if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, r.serviceAccountLister, r.kubeClientSet, sourcesv1.SchemeGroupVersion.WithKind("PingSource"), source.ObjectMeta); err != nil {
source.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err)
return err
}
source.Status.MarkOIDCIdentityCreatedSucceeded()
} else {
source.Status.Auth = nil
source.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
if err := auth.OIDCAuthStatusUtility(ctx, r.serviceAccountLister, r.kubeClientSet, sourcesv1.SchemeGroupVersion.WithKind("PingSource"), source.ObjectMeta, &source.Status); err != nil {
return err
}

sinkAddr, err := r.sinkResolver.AddressableFromDestinationV1(ctx, *dest, source)
Expand Down
20 changes: 2 additions & 18 deletions pkg/reconciler/sinkbinding/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package sinkbinding
import (
"context"
"errors"
"fmt"

"knative.dev/eventing/pkg/auth"
sbinformer "knative.dev/eventing/pkg/client/injection/informers/sources/v1/sinkbinding"
Expand All @@ -41,7 +40,6 @@ import (
"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"
serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
"knative.dev/pkg/configmap"
Expand Down Expand Up @@ -188,22 +186,8 @@ 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)
sb.Status.Auth = &duckv1.AuthStatus{
ServiceAccountName: &saName,
}

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
}
sb.Status.MarkOIDCIdentityCreatedSucceeded()
} else {
sb.Status.Auth = nil
sb.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
if err := auth.OIDCAuthStatusUtility(ctx, s.serviceAccountLister, s.kubeclient, v1.SchemeGroupVersion.WithKind("SinkBinding"), sb.ObjectMeta, &sb.Status); err != nil {
return err
}

addr, err := s.res.AddressableFromDestinationV1(ctx, sb.Spec.Sink, sb)
Expand Down
17 changes: 2 additions & 15 deletions pkg/reconciler/subscription/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,8 @@ var _ subscriptionreconciler.Finalizer = (*Reconciler)(nil)
// ReconcileKind implements Interface.ReconcileKind.
func (r *Reconciler) ReconcileKind(ctx context.Context, subscription *v1.Subscription) pkgreconciler.Event {
// OIDC authentication
featureFlags := feature.FromContext(ctx)
if featureFlags.IsOIDCAuthentication() {
saName := auth.GetOIDCServiceAccountNameForResource(v1.SchemeGroupVersion.WithKind("Subscription"), subscription.ObjectMeta)
subscription.Status.Auth = &duckv1.AuthStatus{
ServiceAccountName: &saName,
}

if err := auth.EnsureOIDCServiceAccountExistsForResource(ctx, r.serviceAccountLister, r.kubeclient, v1.SchemeGroupVersion.WithKind("Subscription"), subscription.ObjectMeta); err != nil {
subscription.Status.MarkOIDCIdentityCreatedFailed("Unable to resolve service account for OIDC authentication", "%v", err)
return err
}
subscription.Status.MarkOIDCIdentityCreatedSucceeded()
} else {
subscription.Status.Auth = nil
subscription.Status.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
if err := auth.OIDCAuthStatusUtility(ctx, r.serviceAccountLister, r.kubeclient, v1.SchemeGroupVersion.WithKind("Subscription"), subscription.ObjectMeta, &subscription.Status); err != nil {
return err
}

// Find the channel for this subscription.
Expand Down