Skip to content

Commit

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

This reverts commit e5f2814.
  • Loading branch information
creydr committed May 7, 2024
1 parent 0d4c672 commit 865edbc
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 360 deletions.
27 changes: 1 addition & 26 deletions pkg/apis/flows/v1/sequence_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ import (
duckv1 "knative.dev/pkg/apis/duck/v1"
)

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

const (
// SequenceConditionReady has status True when all subconditions below have been set to True.
Expand All @@ -46,10 +45,6 @@ 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 @@ -199,23 +194,3 @@ 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: 32 additions & 59 deletions pkg/apis/flows/v1/sequence_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -173,9 +170,6 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionFalse,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -203,9 +197,6 @@ func TestSequenceInitializeConditions(t *testing.T) {
}, {
Type: SequenceConditionChannelsReady,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionOIDCIdentityCreated,
Status: corev1.ConditionUnknown,
}, {
Type: SequenceConditionReady,
Status: corev1.ConditionUnknown,
Expand Down Expand Up @@ -324,70 +315,52 @@ func TestSequencePropagateChannelStatuses(t *testing.T) {

func TestSequenceReady(t *testing.T) {
tests := []struct {
name string
subs []*messagingv1.Subscription
channels []*eventingduckv1.Channelable
oidcSACreated bool
want bool
name string
subs []*messagingv1.Subscription
channels []*eventingduckv1.Channelable
want bool
}{{
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)},
oidcSACreated: false,
want: false,
name: "empty",
subs: []*messagingv1.Subscription{},
channels: []*eventingduckv1.Channelable{},
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 not ready, one subscription ready",
channels: []*eventingduckv1.Channelable{getChannelable(false)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
want: false,
}, {
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 subscription not ready",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", false)},
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: "one channelable ready, one subscription ready",
channels: []*eventingduckv1.Channelable{getChannelable(true)},
subs: []*messagingv1.Subscription{getSubscription("sub0", true)},
want: true,
}, {
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: "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: "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, 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, 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,
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,
}}

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
42 changes: 5 additions & 37 deletions pkg/reconciler/sequence/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,17 @@ package sequence
import (
"context"

"knative.dev/eventing/pkg/auth"

"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/filtered"
"knative.dev/pkg/injection/clients/dynamicclient"
)

Expand All @@ -48,34 +42,14 @@ func NewController(

sequenceInformer := sequence.Get(ctx)
subscriptionInformer := subscription.Get(ctx)
oidcServiceaccountInformer := serviceaccountinformer.Get(ctx, auth.OIDCLabelSelector)

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),
serviceAccountLister: oidcServiceaccountInformer.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())
sequenceLister: sequenceInformer.Lister(),
subscriptionLister: subscriptionInformer.Lister(),
dynamicClientSet: dynamicclient.Get(ctx),
eventingClientSet: eventingclient.Get(ctx),
}
impl := sequencereconciler.NewImpl(ctx, r)

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

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

return impl
}
23 changes: 2 additions & 21 deletions pkg/reconciler/sequence/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,23 @@ limitations under the License.
package sequence

import (
"context"
"testing"

"knative.dev/eventing/pkg/auth"
filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"

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/filtered/fake"
_ "knative.dev/pkg/client/injection/kube/informers/factory/filtered/fake"
)

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

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

if c == nil {
t.Fatal("Expected NewController to return a non-nil value")
}
}

func SetUpInformerSelector(ctx context.Context) context.Context {
ctx = filteredFactory.WithSelectors(ctx, auth.OIDCLabelSelector)
return ctx
}
16 changes: 1 addition & 15 deletions pkg/reconciler/sequence/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,22 @@ 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 @@ -62,9 +57,7 @@ type Reconciler struct {
eventingClientSet clientset.Interface

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

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

featureFlags := feature.FromContext(ctx)
if err := auth.SetupOIDCServiceAccount(ctx, featureFlags, r.serviceAccountLister, r.kubeclient, v1.SchemeGroupVersion.WithKind("Sequence"), s.ObjectMeta, &s.Status, func(as *duckv1.AuthStatus) {
s.Status.Auth = as
}); err != nil {
return err
}

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

Expand Down

0 comments on commit 865edbc

Please sign in to comment.