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

Add EventPolicy Reconciler #8024

Merged
merged 6 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
44 changes: 32 additions & 12 deletions pkg/apis/eventing/v1alpha1/eventpolicy_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import (
"knative.dev/pkg/apis"
)

var eventPolicyCondSet = apis.NewLivingConditionSet(EventPolicyConditionAuthnEnabled, EventPolicyConditionSubjectsResolved)
var eventPolicyCondSet = apis.NewLivingConditionSet(EventPolicyConditionAuthenticationEnabled, EventPolicyConditionSubjectsResolved)

const (
EventPolicyConditionReady = apis.ConditionReady
EventPolicyConditionAuthnEnabled apis.ConditionType = "AuthenticationEnabled"
EventPolicyConditionSubjectsResolved apis.ConditionType = "SubjectsResolved"
EventPolicyConditionReady = apis.ConditionReady
EventPolicyConditionAuthenticationEnabled apis.ConditionType = "AuthenticationEnabled"
EventPolicyConditionSubjectsResolved apis.ConditionType = "SubjectsResolved"
)

// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
Expand All @@ -34,21 +34,41 @@ func (*EventPolicy) GetConditionSet() apis.ConditionSet {
}

// GetCondition returns the condition currently associated with the given type, or nil.
func (et *EventPolicyStatus) GetCondition(t apis.ConditionType) *apis.Condition {
return eventPolicyCondSet.Manage(et).GetCondition(t)
func (ep *EventPolicyStatus) GetCondition(t apis.ConditionType) *apis.Condition {
return eventPolicyCondSet.Manage(ep).GetCondition(t)
creydr marked this conversation as resolved.
Show resolved Hide resolved
}

// IsReady returns true if the resource is ready overall.
func (et *EventPolicyStatus) IsReady() bool {
return et.GetTopLevelCondition().IsTrue()
func (ep *EventPolicyStatus) IsReady() bool {
return ep.GetTopLevelCondition().IsTrue()
}

// GetTopLevelCondition returns the top level Condition.
func (et *EventPolicyStatus) GetTopLevelCondition() *apis.Condition {
return eventPolicyCondSet.Manage(et).GetTopLevelCondition()
func (ep *EventPolicyStatus) GetTopLevelCondition() *apis.Condition {
return eventPolicyCondSet.Manage(ep).GetTopLevelCondition()
}

// InitializeConditions sets relevant unset conditions to Unknown state.
func (et *EventPolicyStatus) InitializeConditions() {
eventPolicyCondSet.Manage(et).InitializeConditions()
func (ep *EventPolicyStatus) InitializeConditions() {
eventPolicyCondSet.Manage(ep).InitializeConditions()
}

// MarkOIDCAuthenticationEnabled sets EventPolicyConditionAuthenticationEnabled condition to true.
func (ep *EventPolicyStatus) MarkOIDCAuthenticationEnabled() {
eventPolicyCondSet.Manage(ep).MarkTrue(EventPolicyConditionAuthenticationEnabled)
}

// MarkOIDCAuthenticationNotEnabled sets EventPolicyConditionAuthenticationEnabled condition to false.
func (ep *EventPolicyStatus) MarkOIDCAuthenticationNotEnabled(reason, messageFormat string, messageA ...interface{}) {
eventPolicyCondSet.Manage(ep).MarkFalse(EventPolicyConditionAuthenticationEnabled, reason, messageFormat, messageA...)
}

// MarkSubjectsResolved sets EventPolicyConditionAuthenticationEnabled condition to true.
creydr marked this conversation as resolved.
Show resolved Hide resolved
func (ep *EventPolicyStatus) MarkSubjectsResolved() {
creydr marked this conversation as resolved.
Show resolved Hide resolved
eventPolicyCondSet.Manage(ep).MarkTrue(EventPolicyConditionSubjectsResolved)
}

// MarkSubjectsNotResolved sets EventPolicyConditionAuthenticationEnabled condition to false.
creydr marked this conversation as resolved.
Show resolved Hide resolved
func (ep *EventPolicyStatus) MarkSubjectsNotResolved(reason, messageFormat string, messageA ...interface{}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (ep *EventPolicyStatus) MarkSubjectsNotResolved(reason, messageFormat string, messageA ...interface{}) {
func (ep *EventPolicyStatus) MarkSubjectsResolvedFailed(reason, messageFormat string, messageA ...interface{}) {

nit (to align with the other marker names)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I updated it in the newer commit.

eventPolicyCondSet.Manage(ep).MarkFalse(EventPolicyConditionSubjectsResolved, reason, messageFormat, messageA...)
}
2 changes: 1 addition & 1 deletion pkg/apis/eventing/v1alpha1/eventpolicy_lifecycle_test.go
creydr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestEventPolicyInitializeConditions(t *testing.T) {
Status: duckv1.Status{
Conditions: []apis.Condition{
{
Type: EventPolicyConditionAuthnEnabled,
Type: EventPolicyConditionAuthenticationEnabled,
Status: corev1.ConditionUnknown,
},
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/eventpolicy/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewController(
r := &Reconciler{}
impl := eventpolicyreconciler.NewImpl(ctx, r)

r.fromRefResolver = resolver.NewAuthenticatableResolverFromTracker(ctx, impl.Tracker)
r.authResolver = resolver.NewAuthenticatableResolverFromTracker(ctx, impl.Tracker)

// Set up event handlers
eventPolicyInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/eventpolicy/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
Expand Down
18 changes: 9 additions & 9 deletions pkg/reconciler/eventpolicy/eventpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,28 @@ import (
)

type Reconciler struct {
fromRefResolver *resolver.AuthenticatableResolver
authResolver *resolver.AuthenticatableResolver
}

// ReconcileKind implements Interface.ReconcileKind.
// 1. Verify the Reference exists.
func (r *Reconciler) ReconcileKind(ctx context.Context, ep *v1alpha1.EventPolicy) pkgreconciler.Event {
featureFlags := feature.FromContext(ctx)
if featureFlags.IsOIDCAuthentication() {
ep.GetConditionSet().Manage(ep.GetStatus()).MarkTrue(v1alpha1.EventPolicyConditionAuthnEnabled)
ep.Status.MarkOIDCAuthenticationEnabled()
} else {
ep.GetConditionSet().Manage(ep.GetStatus()).MarkFalse(v1alpha1.EventPolicyConditionAuthnEnabled, "AuthOIDCFeatureNotEnabled", "")
ep.Status.MarkOIDCAuthenticationNotEnabled("AuthOIDCFeatureNotEnabled", "")
creydr marked this conversation as resolved.
Show resolved Hide resolved
return nil
creydr marked this conversation as resolved.
Show resolved Hide resolved
}
// We reconcile the status of the EventPolicy
// by looking at all from[].refs have subjects
// by looking at all .spec.from[].refs have subjects
// and accordingly set the eventpolicy status
serverAccts, err := auth.ResolveSubjects(r.fromRefResolver, ep)
subjects, err := auth.ResolveSubjects(r.authResolver, ep)
if err != nil {
ep.GetConditionSet().Manage(ep.GetStatus()).MarkFalse(v1alpha1.EventPolicyConditionSubjectsResolved, "FromSubjectsNotResolved", "")
return fmt.Errorf("failed to resolve from[].ref: %w", err)
ep.Status.MarkSubjectsNotResolved("SubjectsNotResolved", err.Error())
return fmt.Errorf("failed to resolve .spec.from[].ref: %w", err)
}
ep.GetConditionSet().Manage(ep.GetStatus()).MarkTrue(v1alpha1.EventPolicyConditionSubjectsResolved)
ep.Status.From = serverAccts
ep.Status.MarkSubjectsResolved()
ep.Status.From = subjects
return nil
}
38 changes: 34 additions & 4 deletions pkg/reconciler/eventpolicy/eventpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ var (
apiServerSourceWithServiceAccount = NewApiServerSource(apiServerSourceName, testNS, WithApiServerSourceOIDCServiceAccountName((serviceAccountname)))
pingSourceGVK = v1.GroupVersionKind(sourcesv1.SchemeGroupVersion.WithKind("PingSource"))
apiServerSourceGVK = v1.GroupVersionKind(sourcesv1.SchemeGroupVersion.WithKind("APIServerSource"))
SubjectsNotResolvedErrorMessage = fmt.Sprintf("could not resolve subjects from reference: could not resolve auth status: failed to get authenticatable %s/%s: failed to get object %s/%s: pingsources.sources.knative.dev \"%s\" not found", testNS, pingSourceName, testNS, pingSourceName, pingSourceName)
SubjectsNotResolvedEventMessage = fmt.Sprintf("Warning InternalError failed to resolve .spec.from[].ref: could not resolve subjects from reference: could not resolve auth status: failed to get authenticatable %s/%s: failed to get object %s/%s: pingsources.sources.knative.dev \"%s\" not found", testNS, pingSourceName, testNS, pingSourceName, pingSourceName)
)

func TestReconcile(t *testing.T) {
Expand Down Expand Up @@ -105,12 +107,12 @@ func TestReconcile(t *testing.T) {
Object: NewEventPolicy(eventPolicyName, testNS,
creydr marked this conversation as resolved.
Show resolved Hide resolved
WithEventPolicyFrom(pingSourceGVK, pingSourceName, testNS),
WithTrueAuthenticationEnabledCondition,
WithUnreadyEventPolicyCondition("FromSubjectsNotResolved", ""),
WithFalseSubjectsResolvedCondition,
WithUnreadyEventPolicyCondition("SubjectsNotResolved", SubjectsNotResolvedErrorMessage),
WithFalseSubjectsResolvedCondition("SubjectsNotResolved", SubjectsNotResolvedErrorMessage),
),
},
},
WantEvents: []string{fmt.Sprintf("Warning InternalError failed to resolve from[].ref: could not resolve subjects from reference: could not resolve auth status: failed to get authenticatable %s/%s: failed to get object test-namespace/test-pingsource: pingsources.sources.knative.dev \"%s\" not found", testNS, pingSourceName, pingSourceName)},
WantEvents: []string{SubjectsNotResolvedEventMessage},
WantErr: true,
},
{
Expand Down Expand Up @@ -193,12 +195,40 @@ func TestReconcile(t *testing.T) {
},
WantErr: false,
},

// test cases for authentication-oidc feature disabled afterwards
{
Name: "Ready status EventPolicy updated to NotReady",
Ctx: feature.ToContext(context.TODO(), feature.Flags{
feature.OIDCAuthentication: feature.Disabled,
}),
Key: testNS + "/" + eventPolicyName,
Objects: []runtime.Object{
pingSourceWithServiceAccount,
NewEventPolicy(eventPolicyName, testNS,
WithReadyEventPolicyCondition,
WithTrueAuthenticationEnabledCondition,
WithTrueSubjectsResolvedCondition,
WithEventPolicyFrom(pingSourceGVK, pingSourceName, testNS)),
},
WantStatusUpdates: []clientgotesting.UpdateActionImpl{
{
Object: NewEventPolicy(eventPolicyName, testNS,
creydr marked this conversation as resolved.
Show resolved Hide resolved
WithEventPolicyFrom(pingSourceGVK, pingSourceName, testNS),
WithFalseAuthenticationEnabledCondition,
WithUnreadyEventPolicyCondition("AuthOIDCFeatureNotEnabled", ""),
WithTrueSubjectsResolvedCondition,
),
},
},
WantErr: false,
},
}
logger := logtesting.TestLogger(t)
table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
ctx = duckv1authstatus.WithDuck(ctx)
r := &Reconciler{
fromRefResolver: resolver.NewAuthenticatableResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0))}
authResolver: resolver.NewAuthenticatableResolverFromTracker(ctx, tracker.New(func(types.NamespacedName) {}, 0))}
return eventpolicy.NewReconciler(ctx, logger,
fakeeventingclient.Get(ctx), listers.GetEventPolicyLister(),
controller.GetEventRecorder(ctx), r)
Expand Down
21 changes: 12 additions & 9 deletions pkg/reconciler/testing/v1/eventpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ func WithInitEventPolicyConditions(ep *v1alpha1.EventPolicy) {
func WithTrueAuthenticationEnabledCondition(ep *v1alpha1.EventPolicy) {
creydr marked this conversation as resolved.
Show resolved Hide resolved
ep.Status.Conditions = append(ep.Status.Conditions,
apis.Condition{
Type: v1alpha1.EventPolicyConditionAuthnEnabled,
Type: v1alpha1.EventPolicyConditionAuthenticationEnabled,
Status: corev1.ConditionTrue,
})
}

func WithFalseAuthenticationEnabledCondition(ep *v1alpha1.EventPolicy) {
creydr marked this conversation as resolved.
Show resolved Hide resolved
ep.Status.Conditions = append(ep.Status.Conditions,
apis.Condition{
Type: v1alpha1.EventPolicyConditionAuthnEnabled,
Type: v1alpha1.EventPolicyConditionAuthenticationEnabled,
Status: corev1.ConditionFalse,
Reason: "AuthOIDCFeatureNotEnabled",
})
Expand All @@ -73,13 +73,16 @@ func WithTrueSubjectsResolvedCondition(ep *v1alpha1.EventPolicy) {
})
}

func WithFalseSubjectsResolvedCondition(ep *v1alpha1.EventPolicy) {
ep.Status.Conditions = append(ep.Status.Conditions,
apis.Condition{
Type: v1alpha1.EventPolicyConditionSubjectsResolved,
Status: corev1.ConditionFalse,
Reason: "FromSubjectsNotResolved",
})
func WithFalseSubjectsResolvedCondition(reason, message string) EventPolicyOption {
creydr marked this conversation as resolved.
Show resolved Hide resolved
return func(ep *v1alpha1.EventPolicy) {
ep.Status.Conditions = append(ep.Status.Conditions,
apis.Condition{
Type: v1alpha1.EventPolicyConditionSubjectsResolved,
Status: corev1.ConditionFalse,
Reason: reason,
Message: message,
})
}
}

func WithUnknownSubjectsResolvedCondition(ep *v1alpha1.EventPolicy) {
creydr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
8 changes: 4 additions & 4 deletions pkg/reconciler/testing/v1/listers.go
creydr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ func (l *Listers) GetAllObjects() []runtime.Object {
return all
}

func (l *Listers) GetEventPolicyLister() eventingv1alpha1listers.EventPolicyLister {
return eventingv1alpha1listers.NewEventPolicyLister(l.indexerFor(&eventingv1alpha1.EventPolicy{}))
}

func (l *Listers) GetEventTypeLister() eventingv1beta2listers.EventTypeLister {
return eventingv1beta2listers.NewEventTypeLister(l.indexerFor(&eventingv1beta2.EventType{}))
}

func (l *Listers) GetEventPolicyLister() eventingv1alpha1listers.EventPolicyLister {
return eventingv1alpha1listers.NewEventPolicyLister(l.indexerFor(&eventingv1alpha1.EventPolicy{}))
}

func (l *Listers) GetPingSourceLister() sourcelisters.PingSourceLister {
return sourcelisters.NewPingSourceLister(l.indexerFor(&sourcesv1.PingSource{}))
}
Expand Down