Skip to content

Commit

Permalink
support auto generation of Sequence identity service account [OIDC] (#…
Browse files Browse the repository at this point in the history 
…7361)

* add oidc auth to sequence

Signed-off-by: rahulii <r.sawra@gmail.com>

* imlement oidc for sequence

* run go lint

* Add unit tests

* reconcile sequence on configmap changes

* run go fmt

---------

Signed-off-by: rahulii <r.sawra@gmail.com>
Co-authored-by: Christoph Stäbler <cstabler@redhat.com>
  • Loading branch information
@rahulii @creydr
rahulii and creydr authored Nov 16, 2023
1 parent 8ebe869 commit e5f2814
Show file tree
Hide file tree
Showing 7 changed files with 357 additions and 54 deletions.
27 changes: 26 additions & 1 deletion pkg/apis/flows/v1/sequence_lifecycle.go
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,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 @@ -166,6 +169,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 @@ -193,6 +199,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 @@ -311,52 +320,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
40 changes: 35 additions & 5 deletions pkg/reconciler/sequence/controller.go
View file
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,34 @@ func NewController(

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

var globalResync func(obj interface{})
featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(name string, value interface{}) {
if globalResync != nil {
globalResync(nil)
}
})
featureStore.WatchConfigs(cmw)

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, func(impl *controller.Impl) controller.Options {
return controller.Options{
ConfigStore: featureStore,
}
})

globalResync = func(_ interface{}) {
impl.GlobalResync(sequenceInformer.Informer())
}
impl := sequencereconciler.NewImpl(ctx, r)

r.channelableTracker = duck.NewListableTrackerFromTracker(ctx, channelable.Get, impl.Tracker)
sequenceInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))
Expand All @@ -61,5 +85,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
View file
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
View file
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

0 comments on commit e5f2814

Please sign in to comment.