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 reconciliation for Sequence #8106

Merged
merged 32 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
fb45ef6
feat: initial commit
Leo6Leo Jul 19, 2024
3298698
Merge branch 'main' into Create-EventPolicies-for-Sequence
Leo6Leo Jul 22, 2024
043141a
feat: add the test for eventpolicy in sequence reconciler
Leo6Leo Jul 23, 2024
2c99a12
fix: fix the typo and remove the unused helper function
Leo6Leo Jul 23, 2024
1696769
fix: trying to fix the git diff issue
Leo6Leo Jul 24, 2024
0adb907
fix: trying to fix the git diff issue
Leo6Leo Jul 24, 2024
e82a1db
fix: fix the nit minor comments
Leo6Leo Jul 24, 2024
5e73f53
fix: update the reconcilation mechanism
Leo6Leo Jul 24, 2024
ca8b3ae
Merge branch 'main' into Create-EventPolicies-for-Sequence
Leo6Leo Jul 25, 2024
78a8b0a
fix: fix the goimports and remove unused helper functions and input p…
Leo6Leo Jul 25, 2024
fe72c3c
fix: add more unit tests to test out remove steps from the sequence
Leo6Leo Jul 26, 2024
f088cf8
Update pkg/reconciler/sequence/sequence.go
Leo6Leo Jul 26, 2024
22520ed
Update pkg/reconciler/sequence/sequence.go
Leo6Leo Jul 26, 2024
f048559
Apply suggestions from code review
Leo6Leo Jul 26, 2024
092f87e
fix: fix the nit review comments from pierdipi and rahul
Leo6Leo Jul 26, 2024
8126f8c
fix: using auth.GetEventPoliciesForResource when trying to list all S…
Leo6Leo Jul 26, 2024
2f23635
Merge remote-tracking branch 'upstream/main' into Create-EventPolicie…
Leo6Leo Jul 29, 2024
268c595
feat: add the sorting to avoid flaky test when there are multiple eve…
Leo6Leo Jul 31, 2024
38017b8
feat: remove the nil condition for channel name when creating the seq…
Leo6Leo Jul 31, 2024
fc8be57
feat: add more unit tests
Leo6Leo Jul 31, 2024
b7bfc92
fix: lint & goimports
Leo6Leo Jul 31, 2024
56b071b
Merge remote-tracking branch 'upstream/main' into Create-EventPolicie…
Leo6Leo Jul 31, 2024
dce2679
Merge remote-tracking branch 'upstream/main' into Create-EventPolicie…
Leo6Leo Aug 1, 2024
18373ca
Merge branch 'main' into Create-EventPolicies-for-Sequence
Leo6Leo Aug 7, 2024
5da98e0
fix: fix the review comments
Leo6Leo Aug 7, 2024
36a1cb3
fix: fix Christoph's review comments
Leo6Leo Aug 7, 2024
6042cbb
feat: adding a test for sequence with existing intermediate eventpoli…
Leo6Leo Aug 7, 2024
9d1e73b
fix: the deepDerivative failed to compare the eventpolicy's From.Spec…
Leo6Leo Aug 7, 2024
e6cddbe
fix: change back to use DeepDerivative
Leo6Leo Aug 8, 2024
cf4d257
fix: fix the test case to make the eventpolicy has a valid spec
Leo6Leo Aug 8, 2024
ad4df83
fix: fix the flaky issue by soring the policies
Leo6Leo Aug 8, 2024
6c8fa1a
fix: change input channel's ownerref to sequence's eventpolicy
Leo6Leo Aug 9, 2024
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
111 changes: 111 additions & 0 deletions pkg/reconciler/sequence/resources/eventpolicy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2024 The Knative Authors

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

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resources

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1"
eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
flowsv1 "knative.dev/eventing/pkg/apis/flows/v1"
messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1"
"knative.dev/pkg/kmeta"
)

const (
SequenceChannelEventPolicyLabelPrefix = "flows.knative.dev/"
sequenceKind = "Sequence"
)

func MakeEventPolicyForSequenceChannel(s *flowsv1.Sequence, channel *eventingduckv1.Channelable, subscription *messagingv1.Subscription) *eventingv1alpha1.EventPolicy {
return &eventingv1alpha1.EventPolicy{
ObjectMeta: metav1.ObjectMeta{
Namespace: channel.Namespace,
Name: SequenceEventPolicyName(s.Name, channel.Name),
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: flowsv1.SchemeGroupVersion.String(),
Kind: sequenceKind,
Name: s.Name,
creydr marked this conversation as resolved.
Show resolved Hide resolved
},
},
Labels: LabelsForSequenceChannelsEventPolicy(s.Name),
},
Spec: eventingv1alpha1.EventPolicySpec{
To: []eventingv1alpha1.EventPolicySpecTo{
{
Ref: &eventingv1alpha1.EventPolicyToReference{
APIVersion: channel.APIVersion,
Kind: channel.Kind,
Name: channel.Name,
},
},
},
From: []eventingv1alpha1.EventPolicySpecFrom{
{
Ref: &eventingv1alpha1.EventPolicyFromReference{
APIVersion: subscription.APIVersion,
Kind: subscription.Kind,
Name: subscription.Name,
Namespace: subscription.Namespace,
},
},
},
},
}
}

func LabelsForSequenceChannelsEventPolicy(sequenceName string) map[string]string {
return map[string]string{
SequenceChannelEventPolicyLabelPrefix + "sequence-name": sequenceName,
}
}

func SequenceEventPolicyName(sequenceName, channelName string) string {
// if channel name is empty, it means the event policy is for the output channel
Leo6Leo marked this conversation as resolved.
Show resolved Hide resolved
return kmeta.ChildName(sequenceName, channelName+"-ep")
Leo6Leo marked this conversation as resolved.
Show resolved Hide resolved

}

// MakeEventPolicyForSequenceInputChannel creates an EventPolicy for the input channel of a Sequence
func MakeEventPolicyForSequenceInputChannel(s *flowsv1.Sequence, inputChannel *eventingduckv1.Channelable, sequencePolicy *eventingv1alpha1.EventPolicy) *eventingv1alpha1.EventPolicy {
return &eventingv1alpha1.EventPolicy{
ObjectMeta: metav1.ObjectMeta{
Namespace: inputChannel.Namespace,
Name: SequenceEventPolicyName(s.Name, sequencePolicy.Name),
creydr marked this conversation as resolved.
Show resolved Hide resolved
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: flowsv1.SchemeGroupVersion.String(),
Kind: sequenceKind,
Name: s.Name,
creydr marked this conversation as resolved.
Show resolved Hide resolved
},
},
Labels: LabelsForSequenceChannelsEventPolicy(s.Name),
},
Spec: eventingv1alpha1.EventPolicySpec{
To: []eventingv1alpha1.EventPolicySpecTo{
{
Ref: &eventingv1alpha1.EventPolicyToReference{
APIVersion: inputChannel.APIVersion,
Kind: inputChannel.Kind,
Name: inputChannel.Name,
},
},
},
From: sequencePolicy.Spec.From,
},
}
}
151 changes: 151 additions & 0 deletions pkg/reconciler/sequence/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package sequence
import (
"context"
"fmt"
"sort"

"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -37,6 +39,7 @@ import (
"knative.dev/pkg/kmp"

eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1"
eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
"knative.dev/eventing/pkg/apis/feature"
v1 "knative.dev/eventing/pkg/apis/flows/v1"
messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1"
Expand Down Expand Up @@ -130,6 +133,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, s *v1.Sequence) pkgrecon
return err
}

if err := r.reconcileEventPolicies(ctx, s, channels, subs, featureFlags); err != nil {
return fmt.Errorf("failed to reconcile EventPolicies: %w", err)
}

err := auth.UpdateStatusWithEventPolicies(featureFlags, &s.Status.AppliedEventPoliciesStatus, &s.Status, r.eventPolicyLister, v1.SchemeGroupVersion.WithKind("Sequence"), s.ObjectMeta)
if err != nil {
return fmt.Errorf("could not update Sequence status with EventPolicies: %v", err)
Expand Down Expand Up @@ -333,3 +340,147 @@ func (r *Reconciler) removeUnwantedSubscriptions(ctx context.Context, seq *v1.Se

return nil
}

func (r *Reconciler) reconcileEventPolicies(ctx context.Context, s *v1.Sequence, channels []*eventingduckv1.Channelable, subs []*messagingv1.Subscription, featureFlags feature.Flags) error {
if !featureFlags.IsOIDCAuthentication() {
return r.cleanupAllEventPolicies(ctx, s)
}

existingPolicies, err := r.listEventPoliciesForSequence(s)
if err != nil {
return fmt.Errorf("failed to list existing EventPolicies: %w", err)
}

// Prepare maps for efficient lookups, updates, and deletions of policies
existingPolicyMap := make(map[string]*eventingv1alpha1.EventPolicy)
for _, policy := range existingPolicies {
existingPolicyMap[policy.Name] = policy
}

// Prepare lists for different actions so that policies can be categorized
var policiesToUpdate, policiesToCreate []*eventingv1alpha1.EventPolicy
policiesToDelete := make([]*eventingv1alpha1.EventPolicy, 0, len(existingPolicies))

// Handle intermediate channel policies (skip the first channel as it's the input channel!)
for i := 1; i < len(channels); i++ {
expectedPolicy := resources.MakeEventPolicyForSequenceChannel(s, channels[i], subs[i-1])
existingPolicy, exists := existingPolicyMap[expectedPolicy.Name]

if exists {
if !equality.Semantic.DeepDerivative(expectedPolicy.Spec, existingPolicy.Spec) {
Leo6Leo marked this conversation as resolved.
Show resolved Hide resolved
policiesToUpdate = append(policiesToUpdate, expectedPolicy)
creydr marked this conversation as resolved.
Show resolved Hide resolved
}
delete(existingPolicyMap, expectedPolicy.Name)
} else {
policiesToCreate = append(policiesToCreate, expectedPolicy)
}
}

// Handle input channel policies
inputPolicies, err := r.prepareInputChannelEventPolicy(s, channels[0])
if err != nil {
return fmt.Errorf("failed to prepare input channel EventPolicies: %w", err)
}
for _, inputPolicy := range inputPolicies {
existingInputPolicy, exists := existingPolicyMap[inputPolicy.Name]
if exists {
if !equality.Semantic.DeepDerivative(inputPolicy.Spec, existingInputPolicy.Spec) {
Leo6Leo marked this conversation as resolved.
Show resolved Hide resolved
policiesToUpdate = append(policiesToUpdate, inputPolicy)
creydr marked this conversation as resolved.
Show resolved Hide resolved
}
delete(existingPolicyMap, inputPolicy.Name)
} else {
policiesToCreate = append(policiesToCreate, inputPolicy)
}
}

// Any remaining policies in the map should be deleted
for _, policy := range existingPolicyMap {
policiesToDelete = append(policiesToDelete, policy)
}

// Perform the actual CRUD operations
if err := r.createEventPolicies(ctx, policiesToCreate); err != nil {
return fmt.Errorf("failed to create EventPolicies: %w", err)
}
if err := r.updateEventPolicies(ctx, policiesToUpdate); err != nil {
return fmt.Errorf("failed to update EventPolicies: %w", err)
}
if err := r.deleteEventPolicies(ctx, policiesToDelete); err != nil {
return fmt.Errorf("failed to delete EventPolicies: %w", err)
}

return nil
}

// listEventPoliciesForSequence lists all EventPolicies (e.g. the policies for the input channel and the intermediate channels) created during reconcileKind that are associated with the given Sequence.
func (r *Reconciler) listEventPoliciesForSequence(s *v1.Sequence) ([]*eventingv1alpha1.EventPolicy, error) {
labelSelector := labels.SelectorFromSet(map[string]string{
resources.SequenceChannelEventPolicyLabelPrefix + "sequence-name": s.Name,
Leo6Leo marked this conversation as resolved.
Show resolved Hide resolved
})
return r.eventPolicyLister.EventPolicies(s.Namespace).List(labelSelector)
}

func (r *Reconciler) prepareInputChannelEventPolicy(s *v1.Sequence, inputChannel *eventingduckv1.Channelable) ([]*eventingv1alpha1.EventPolicy, error) {
matchingPolicies, err := auth.GetEventPoliciesForResource(
r.eventPolicyLister,
v1.SchemeGroupVersion.WithKind("Sequence"),
s.ObjectMeta,
)
if err != nil {
return nil, fmt.Errorf("failed to get matching EventPolicies for Sequence: %w", err)
}

if len(matchingPolicies) == 0 {
return nil, nil
}

sort.Slice(matchingPolicies, func(i, j int) bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

Do sorting here as the order of the matching policies is indeterminant. it cause the unit test (when sequence has multiple eventpolicies) become flaky.

Copy link
Member Author

@Leo6Leo Leo6Leo Jul 31, 2024

Choose a reason for hiding this comment

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

=== RUN   TestAllCases/sequence_with_multiple_eventpolicies
   sequence_test.go:3420: Unexpected status update (-want, +got):
          &v1.Sequence{
          	TypeMeta:   {},
          	ObjectMeta: {Name: "test-sequence", Namespace: "test-namespace"},
          	Spec:       {Steps: {{Destination: {URI: &{Scheme: "http", Host: "example.com", Path: "0"}}}, {Destination: {URI: &{Scheme: "http", Host: "example.com", Path: "1"}}}}, ChannelTemplate: &{TypeMeta: {Kind: "InMemoryChannel", APIVersion: "messaging.knative.dev/v1"}, Spec: &{Raw: "{}"}}},
          	Status: v1.SequenceStatus{
          		Status: v1.Status{
          			ObservedGeneration: 0,
          			Conditions: v1.Conditions{
          				{Type: "Addressable", Status: "Unknown", Reason: "emptyAddress", Message: "addressable is nil", ...},
          				{Type: "ChannelsReady", Status: "Unknown", Reason: "ChannelsNotReady", Message: "Channels are not ready yet, or there are none", ...},
          				{
          					... // 1 ignored and 3 identical fields
          					Reason: "EventPoliciesNotReady",
          					Message: strings.Join({
          						"event policies test-sequence-",
        - 						"ep, test-sequence-additional-1-ep, test-sequence-additional-2",
        + 						"additional-1-ep, test-sequence-additional-2-ep, test-sequence",
          						"-ep are not ready",
          					}, ""),
          				},
          				{
          					... // 1 ignored and 3 identical fields
          					Reason: "EventPoliciesNotReady",
          					Message: strings.Join({
          						"event policies test-sequence-",
        - 						"ep, test-sequence-additional-1-ep, test-sequence-additional-2",
        + 						"additional-1-ep, test-sequence-additional-2-ep, test-sequence",
          						"-ep are not ready",
          					}, ""),
          				},
          				{Type: "SubscriptionsReady", Status: "Unknown", Reason: "SubscriptionsNotReady", Message: "Subscriptions are not ready yet, or there are none", ...},
          			},
          			Annotations: nil,
          		},
          		SubscriptionStatuses: {{Subscription: {Kind: "Subscription", Namespace: "test-namespace", Name: "test-sequence-kn-sequence-0", APIVersion: "messaging.knative.dev/v1", ...}, ReadyCondition: {Type: "Ready", Status: "Unknown", Reason: "NoReady", Message: "Subscription does not have Ready condition"}}, {Subscription: {Kind: "Subscription", Namespace: "test-namespace", Name: "test-sequence-kn-sequence-1", APIVersion: "messaging.knative.dev/v1", ...}, ReadyCondition: {Type: "Ready", Status: "Unknown", Reason: "NoReady", Message: "Subscription does not have Ready condition"}}},
          		ChannelStatuses:      {{Channel: {Kind: "InMemoryChannel", Namespace: "test-namespace", Name: "test-sequence-kn-sequence-0", APIVersion: "messaging.knative.dev/v1", ...}, ReadyCondition: {Type: "Ready", Status: "Unknown", Reason: "NoReady", Message: "Channel does not have Ready condition"}}, {Channel: {Kind: "InMemoryChannel", Namespace: "test-namespace", Name: "test-sequence-kn-sequence-1", APIVersion: "messaging.knative.dev/v1", ...}, ReadyCondition: {Type: "Ready", Status: "Unknown", Reason: "NoReady", Message: "Channel does not have Ready condition"}}},
          		... // 3 identical fields
          	},
          }
        
        Full: &{{ } {test-sequence  test-namespace    0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] [] []} {[{{<nil> http://example.com/0 <nil> <nil>} <nil>} {{<nil> http://example.com/1 <nil> <nil>} <nil>}] &TypeMeta{Kind:InMemoryChannel,APIVersion:messaging.knative.dev/v1,} <nil>} {{0 [{Addressable Unknown  {2024-07-31 01:38:04.798862164 -0400 EDT m=+3.880229171} emptyAddress addressable is nil} {ChannelsReady Unknown  {2024-07-31 01:38:04.798862164 -0400 EDT m=+3.880229171} ChannelsNotReady Channels are not ready yet, or there are none} {EventPoliciesReady False  {2024-07-31 01:38:04.798862164 -0400 EDT m=+3.880229171} EventPoliciesNotReady event policies test-sequence-additional-1-ep, test-sequence-additional-2-ep, test-sequence-ep are not ready} {Ready False  {2024-07-31 01:38:04.798862164 -0400 EDT m=+3.880229171} EventPoliciesNotReady event policies test-sequence-additional-1-ep, test-sequence-additional-2-ep, test-sequence-ep are not ready} {SubscriptionsReady Unknown  {2024-07-31 01:38:04.798862164 -0400 EDT m=+3.880229171} SubscriptionsNotReady Subscriptions are not ready yet, or there are none}] map[]} [{{Subscription test-namespace test-sequence-kn-sequence-0  messaging.knative.dev/v1  } {Ready Unknown  {2024-07-31 01:38:04.798688281 -0400 EDT m=+3.880055291} NoReady Subscription does not have Ready condition}} {{Subscription test-namespace test-sequence-kn-sequence-1  messaging.knative.dev/v1  } {Ready Unknown  {2024-07-31 01:38:04.798688999 -0400 EDT m=+3.880056004} NoReady Subscription does not have Ready condition}}] [{{InMemoryChannel test-namespace test-sequence-kn-sequence-0  messaging.knative.dev/v1  } {Ready Unknown  {2024-07-31 01:38:04.798241015 -0400 EDT m=+3.879608024} NoReady Channel does not have Ready condition}} {{InMemoryChannel test-namespace test-sequence-kn-sequence-1  messaging.knative.dev/v1  } {Ready Unknown  {2024-07-31 01:38:04.798241621 -0400 EDT m=+3.879608619} NoReady Channel does not have Ready condition}}] {<nil>  <nil> <nil>} <nil> {[]}}}
    --- FAIL: TestAllCases/sequence_with_multiple_eventpolicies (0.14s)

return matchingPolicies[i].Name < matchingPolicies[j].Name
})

inputChannelPolicies := make([]*eventingv1alpha1.EventPolicy, 0, len(matchingPolicies))
for _, policy := range matchingPolicies {
inputChannelPolicy := resources.MakeEventPolicyForSequenceInputChannel(s, inputChannel, policy)
inputChannelPolicies = append(inputChannelPolicies, inputChannelPolicy)
}

return inputChannelPolicies, nil
}

func (r *Reconciler) createEventPolicies(ctx context.Context, policies []*eventingv1alpha1.EventPolicy) error {
for _, policy := range policies {
_, err := r.eventingClientSet.EventingV1alpha1().EventPolicies(policy.Namespace).Create(ctx, policy, metav1.CreateOptions{})
if err != nil {
return err
}
}
return nil
}

func (r *Reconciler) updateEventPolicies(ctx context.Context, policies []*eventingv1alpha1.EventPolicy) error {
for _, policy := range policies {
_, err := r.eventingClientSet.EventingV1alpha1().EventPolicies(policy.Namespace).Update(ctx, policy, metav1.UpdateOptions{})
if err != nil {
return err
}
}
return nil
}

func (r *Reconciler) deleteEventPolicies(ctx context.Context, policies []*eventingv1alpha1.EventPolicy) error {
for _, policy := range policies {
err := r.eventingClientSet.EventingV1alpha1().EventPolicies(policy.Namespace).Delete(ctx, policy.Name, metav1.DeleteOptions{})
if err != nil && !apierrs.IsNotFound(err) {
return err
}
}
return nil
}

func (r *Reconciler) cleanupAllEventPolicies(ctx context.Context, s *v1.Sequence) error {
policies, err := r.listEventPoliciesForSequence(s)
if err != nil {
return fmt.Errorf("failed to list EventPolicies for cleanup: %w", err)
}
return r.deleteEventPolicies(ctx, policies)
}
Loading
Loading