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

support auto generation of Sequence identity service account [OIDC] #7361

Merged
merged 7 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion pkg/apis/flows/v1/sequence_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import (
duckv1 "knative.dev/pkg/apis/duck/v1"
)

var sCondSet = apis.NewLivingConditionSet(SequenceConditionReady, SequenceConditionChannelsReady, SequenceConditionSubscriptionsReady, SequenceConditionAddressable)
var sCondSet = apis.NewLivingConditionSet(SequenceConditionReady, SequenceConditionChannelsReady, SequenceConditionSubscriptionsReady, SequenceConditionAddressable,
SequenceConditionOIDCIdentityCreated)

const (
// SequenceConditionReady has status True when all subconditions below have been set to True.
Expand All @@ -45,6 +46,10 @@ const (
// SequenceConditionAddressable has status true when this Sequence meets
// the Addressable contract and has a non-empty hostname.
SequenceConditionAddressable apis.ConditionType = "Addressable"

// SequenceConditionOIDCIdentityCreated has status True when the OIDCIdentity has been created.
// This condition is only relevant if the OIDC feature is enabled.
SequenceConditionOIDCIdentityCreated apis.ConditionType = "OIDCIdentityCreated"
)

// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
Expand Down Expand Up @@ -190,3 +195,23 @@ func (ss *SequenceStatus) setAddress(address *duckv1.Addressable) {
sCondSet.Manage(ss).MarkTrue(SequenceConditionAddressable)
}
}

// MarkOIDCIdentityCreatedSucceeded marks the OIDCIdentityCreated condition as true.
func (ss *SequenceStatus) MarkOIDCIdentityCreatedSucceeded() {
sCondSet.Manage(ss).MarkTrue(SequenceConditionOIDCIdentityCreated)
}

// MarkOIDCIdentityCreatedSucceededWithReason marks the OIDCIdentityCreated condition as true with the given reason.
func (ss *SequenceStatus) MarkOIDCIdentityCreatedSucceededWithReason(reason, messageFormat string, messageA ...interface{}) {
sCondSet.Manage(ss).MarkTrueWithReason(SequenceConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

// MarkOIDCIdentityCreatedFailed marks the OIDCIdentityCreated condition as false with the given reason.
func (ss *SequenceStatus) MarkOIDCIdentityCreatedFailed(reason, messageFormat string, messageA ...interface{}) {
sCondSet.Manage(ss).MarkFalse(SequenceConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}

// MarkOIDCIdentityCreatedUnknown marks the OIDCIdentityCreated condition as unknown with the given reason.
func (ss *SequenceStatus) MarkOIDCIdentityCreatedUnknown(reason, messageFormat string, messageA ...interface{}) {
sCondSet.Manage(ss).MarkUnknown(SequenceConditionOIDCIdentityCreated, reason, messageFormat, messageA...)
}
91 changes: 59 additions & 32 deletions pkg/apis/flows/v1/sequence_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -165,6 +168,9 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionFalse,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -192,6 +198,9 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -310,52 +319,70 @@ func TestSequencePropagateChannelStatuses(t *testing.T) {

func TestSequenceReady(t *testing.T) {
tests := []struct {
name string
subs []*messagingv1.Subscription
channels []*eventingduckv1.Channelable
want bool
name string
subs []*messagingv1.Subscription
channels []*eventingduckv1.Channelable
oidcSACreated bool
want bool
}{{
name: "empty",
subs: []*messagingv1.Subscription{},
channels: []*eventingduckv1.Channelable{},
want: false,
name: "empty",
subs: []*messagingv1.Subscription{},
channels: []*eventingduckv1.Channelable{},
oidcSACreated: false,
want: false,
}, {
name: "one channelable not ready, one subscription ready",
channels: []*eventingduckv1.Channelable{getChannelable(false)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
want: false,
name: "one channelable not ready, one subscription ready",
channels: []*eventingduckv1.Channelable{getChannelable(false)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
oidcSACreated: false,
want: false,
}, {
name: "one channelable ready, one subscription not ready",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", false)},
want: false,
name: "one channelable ready, one subscription not ready",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", false)},
oidcSACreated: false,
want: false,
}, {
name: "one channelable ready, one subscription ready",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
want: true,
name: "one channelable ready, one subscription ready, oidc SA created",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
oidcSACreated: true,
want: true,
}, {
name: "one channelable ready, one not, two subscriptions ready",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(false)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", true)},
want: false,
name: "one channelable ready, one not, two subscriptions ready",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(false)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", true)},
oidcSACreated: false,
want: false,
}, {
name: "two channelables ready, one subscription ready, one not",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", false)},
want: false,
name: "two channelables ready, one subscription ready, one not",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", false)},
oidcSACreated: false,
want: false,
}, {
name: "two channelables ready, two subscriptions ready",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", true)},
want: true,
name: "two channelables ready, two subscriptions ready, oidc SA not created",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", true)},
oidcSACreated: false,
want: false,
}, {
name: "two channelables ready, two subscriptions ready, oidc SA created",
channels: []*eventingduckv1.Channelable{getChannelable(true), getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true), getSubscription("sub1", true)},
oidcSACreated: true,
want: true,
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ps := SequenceStatus{}
ps.PropagateChannelStatuses(test.channels)
ps.PropagateSubscriptionStatuses(test.subs)
if test.oidcSACreated {
ps.MarkOIDCIdentityCreatedSucceeded()
}

got := ps.IsReady()
want := test.want
if want != got {
Expand Down
31 changes: 26 additions & 5 deletions pkg/reconciler/sequence/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@ import (
"context"

"k8s.io/client-go/tools/cache"
"knative.dev/eventing/pkg/apis/feature"
v1 "knative.dev/eventing/pkg/apis/flows/v1"
"knative.dev/eventing/pkg/duck"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
"knative.dev/pkg/logging"

eventingclient "knative.dev/eventing/pkg/client/injection/client"
"knative.dev/eventing/pkg/client/injection/ducks/duck/v1/channelable"
"knative.dev/eventing/pkg/client/injection/informers/flows/v1/sequence"
"knative.dev/eventing/pkg/client/injection/informers/messaging/v1/subscription"
sequencereconciler "knative.dev/eventing/pkg/client/injection/reconciler/flows/v1/sequence"
kubeclient "knative.dev/pkg/client/injection/kube/client"
serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
"knative.dev/pkg/injection/clients/dynamicclient"
)

Expand All @@ -42,14 +46,25 @@ func NewController(

sequenceInformer := sequence.Get(ctx)
subscriptionInformer := subscription.Get(ctx)
serviceaccountInformer := serviceaccountinformer.Get(ctx)

featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"))
featureStore.WatchConfigs(cmw)
creydr marked this conversation as resolved.
Show resolved Hide resolved

r := &Reconciler{
sequenceLister: sequenceInformer.Lister(),
subscriptionLister: subscriptionInformer.Lister(),
dynamicClientSet: dynamicclient.Get(ctx),
eventingClientSet: eventingclient.Get(ctx),
sequenceLister: sequenceInformer.Lister(),
subscriptionLister: subscriptionInformer.Lister(),
dynamicClientSet: dynamicclient.Get(ctx),
eventingClientSet: eventingclient.Get(ctx),
serviceAccountLister: serviceaccountInformer.Lister(),
kubeclient: kubeclient.Get(ctx),
}
impl := sequencereconciler.NewImpl(ctx, r)

impl := sequencereconciler.NewImpl(ctx, r, func(impl *controller.Impl) controller.Options {
return controller.Options{
ConfigStore: featureStore,
}
})

r.channelableTracker = duck.NewListableTrackerFromTracker(ctx, channelable.Get, impl.Tracker)
sequenceInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))
Expand All @@ -61,5 +76,11 @@ func NewController(
Handler: controller.HandleAll(impl.EnqueueControllerOf),
})

// Reconcile Sequence when the OIDC service account changes
serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: controller.FilterController(&v1.Sequence{}),
Handler: controller.HandleAll(impl.EnqueueControllerOf),
})

return impl
}
11 changes: 10 additions & 1 deletion pkg/reconciler/sequence/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,28 @@ package sequence
import (
"testing"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/configmap"
. "knative.dev/pkg/reconciler/testing"

// Fake injection informers
"knative.dev/eventing/pkg/apis/feature"
_ "knative.dev/eventing/pkg/client/injection/ducks/duck/v1/channelable/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/flows/v1/sequence/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/messaging/v1/subscription/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake"
)

func TestNew(t *testing.T) {
ctx, _ := SetupFakeContext(t)

c := NewController(ctx, configmap.NewStaticWatcher())
c := NewController(ctx, configmap.NewStaticWatcher(
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: feature.FlagsConfigName,
},
}))

if c == nil {
t.Fatal("Expected NewController to return a non-nil value")
Expand Down
26 changes: 25 additions & 1 deletion pkg/reconciler/sequence/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,27 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"knative.dev/pkg/kmeta"

duckapis "knative.dev/pkg/apis/duck"
"knative.dev/pkg/logging"
pkgreconciler "knative.dev/pkg/reconciler"

eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1"
"knative.dev/eventing/pkg/apis/feature"
v1 "knative.dev/eventing/pkg/apis/flows/v1"
messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1"
"knative.dev/eventing/pkg/auth"
clientset "knative.dev/eventing/pkg/client/clientset/versioned"
sequencereconciler "knative.dev/eventing/pkg/client/injection/reconciler/flows/v1/sequence"
listers "knative.dev/eventing/pkg/client/listers/flows/v1"
messaginglisters "knative.dev/eventing/pkg/client/listers/messaging/v1"
"knative.dev/eventing/pkg/duck"

corev1listers "k8s.io/client-go/listers/core/v1"
"knative.dev/eventing/pkg/reconciler/sequence/resources"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

type Reconciler struct {
Expand All @@ -57,7 +62,9 @@ type Reconciler struct {
eventingClientSet clientset.Interface

// dynamicClientSet allows us to configure pluggable Build objects
dynamicClientSet dynamic.Interface
dynamicClientSet dynamic.Interface
serviceAccountLister corev1listers.ServiceAccountLister
kubeclient kubernetes.Interface
}

// Check that our Reconciler implements sequencereconciler.Interface
Expand Down Expand Up @@ -122,6 +129,23 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, s *v1.Sequence) pkgrecon
return err
}

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

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

return r.removeUnwantedSubscriptions(ctx, s, subs)
}

Expand Down
Loading
Loading