From 11bfab01eb5d728fb11fb5a2d345d1ffe053edda Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 17 Mar 2021 08:40:52 +0100 Subject: [PATCH 1/4] KafkaChannel hack Signed-off-by: Francesco Guardiani --- pkg/reconciler/subscription/subscription.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/reconciler/subscription/subscription.go b/pkg/reconciler/subscription/subscription.go index 3c2a8c018f3..20bc53e505b 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -321,6 +321,12 @@ func (r *Reconciler) trackAndFetchChannel(ctx context.Context, sub *v1.Subscript func (r *Reconciler) getChannel(ctx context.Context, sub *v1.Subscription) (*eventingduckv1.Channelable, pkgreconciler.Event) { logging.FromContext(ctx).Infow("Getting channel", zap.Any("channel", sub.Spec.Channel)) + // HACK if a channel ref is a kafka channel ref, we need to hack it around to use only v1beta1 + // TODO REMOVE AFTER 0.22 release + if sub.Spec.Channel.Kind == "KafkaChannel" && sub.Spec.Channel.APIVersion == "messaging.knative.dev/v1alpha1" { + sub.Spec.Channel.APIVersion = "messaging.knative.dev/v1beta1" + } + // 1. Track the channel pointed by subscription. // a. If channel is a Channel.messaging.knative.dev obj, err := r.trackAndFetchChannel(ctx, sub, sub.Spec.Channel) From c5082c54a231acfda1e050f923e28af11feb3cd2 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 18 Mar 2021 10:56:45 +0100 Subject: [PATCH 2/4] This hack will do the trick? Signed-off-by: Francesco Guardiani --- pkg/apis/messaging/v1/subscription_defaults.go | 6 +++++- pkg/apis/messaging/v1beta1/subscription_defaults.go | 6 +++++- pkg/reconciler/subscription/subscription.go | 6 ------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/apis/messaging/v1/subscription_defaults.go b/pkg/apis/messaging/v1/subscription_defaults.go index c2ddc46ac0d..6cdf2bab9b5 100644 --- a/pkg/apis/messaging/v1/subscription_defaults.go +++ b/pkg/apis/messaging/v1/subscription_defaults.go @@ -25,5 +25,9 @@ func (s *Subscription) SetDefaults(ctx context.Context) { } func (ss *SubscriptionSpec) SetDefaults(ctx context.Context) { - // TODO anything? + // HACK if a channel ref is a kafka channel ref, we need to hack it around to use only v1beta1 + // TODO(slinkydeveloper) REMOVE AFTER 0.22 release + if ss.Channel.Kind == "KafkaChannel" && ss.Channel.APIVersion == "messaging.knative.dev/v1alpha1" { + ss.Channel.APIVersion = "messaging.knative.dev/v1beta1" + } } diff --git a/pkg/apis/messaging/v1beta1/subscription_defaults.go b/pkg/apis/messaging/v1beta1/subscription_defaults.go index 16fd42ea588..6694bb17a45 100644 --- a/pkg/apis/messaging/v1beta1/subscription_defaults.go +++ b/pkg/apis/messaging/v1beta1/subscription_defaults.go @@ -25,5 +25,9 @@ func (s *Subscription) SetDefaults(ctx context.Context) { } func (ss *SubscriptionSpec) SetDefaults(ctx context.Context) { - // TODO anything? + // HACK if a channel ref is a kafka channel ref, we need to hack it around to use only v1beta1 + // TODO(slinkydeveloper) REMOVE AFTER 0.22 release + if ss.Channel.Kind == "KafkaChannel" && ss.Channel.APIVersion == "messaging.knative.dev/v1alpha1" { + ss.Channel.APIVersion = "messaging.knative.dev/v1beta1" + } } diff --git a/pkg/reconciler/subscription/subscription.go b/pkg/reconciler/subscription/subscription.go index 20bc53e505b..3c2a8c018f3 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -321,12 +321,6 @@ func (r *Reconciler) trackAndFetchChannel(ctx context.Context, sub *v1.Subscript func (r *Reconciler) getChannel(ctx context.Context, sub *v1.Subscription) (*eventingduckv1.Channelable, pkgreconciler.Event) { logging.FromContext(ctx).Infow("Getting channel", zap.Any("channel", sub.Spec.Channel)) - // HACK if a channel ref is a kafka channel ref, we need to hack it around to use only v1beta1 - // TODO REMOVE AFTER 0.22 release - if sub.Spec.Channel.Kind == "KafkaChannel" && sub.Spec.Channel.APIVersion == "messaging.knative.dev/v1alpha1" { - sub.Spec.Channel.APIVersion = "messaging.knative.dev/v1beta1" - } - // 1. Track the channel pointed by subscription. // a. If channel is a Channel.messaging.knative.dev obj, err := r.trackAndFetchChannel(ctx, sub, sub.Spec.Channel) From 8679b1bf88ffa52c32d5e5b4aead0e4b59c337ca Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 18 Mar 2021 11:41:04 +0100 Subject: [PATCH 3/4] This is a proper hack Signed-off-by: Francesco Guardiani --- .../messaging/v1/subscription_validation.go | 10 +++++++- .../v1/subscription_validation_test.go | 23 +++++++++++++++++++ .../v1beta1/subscription_validation.go | 10 +++++++- .../v1beta1/subscription_validation_test.go | 23 +++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/pkg/apis/messaging/v1/subscription_validation.go b/pkg/apis/messaging/v1/subscription_validation.go index f2496bc7afa..e3f3a517290 100644 --- a/pkg/apis/messaging/v1/subscription_validation.go +++ b/pkg/apis/messaging/v1/subscription_validation.go @@ -80,8 +80,16 @@ func (s *Subscription) CheckImmutableFields(ctx context.Context, original *Subsc return nil } + // TODO(slinkydeveloper) + // HACK around the immutability check to make sure the update script can upgrade the api version + // REMOVE AFTER 0.22 release + ignoredFields := []string{"Subscriber", "Reply"} + if original.Spec.Channel.Kind == "KafkaChannel" && original.Spec.Channel.APIVersion == "messaging.knative.dev/v1alpha1" && s.Spec.Channel.APIVersion == "messaging.knative.dev/v1beta1" { + ignoredFields = append(ignoredFields, "Channel.APIVersion") + } + // Only Subscriber and Reply are mutable. - ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, "Subscriber", "Reply") + ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, ignoredFields...) if diff, err := kmp.ShortDiff(original.Spec, s.Spec, ignoreArguments); err != nil { return &apis.FieldError{ Message: "Failed to diff Subscription", diff --git a/pkg/apis/messaging/v1/subscription_validation_test.go b/pkg/apis/messaging/v1/subscription_validation_test.go index e31086eba62..b956b8215ae 100644 --- a/pkg/apis/messaging/v1/subscription_validation_test.go +++ b/pkg/apis/messaging/v1/subscription_validation_test.go @@ -329,6 +329,29 @@ func TestSubscriptionImmutable(t *testing.T) { }, }, want: nil, + }, { + name: "valid, kafkachannel hack", + c: &Subscription{ + Spec: SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Name: channelName, + Kind: "KafkaChannel", + APIVersion: "messaging.knative.dev/v1beta1", + }, + Subscriber: getValidDestination(), + }, + }, + og: &Subscription{ + Spec: SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Name: channelName, + Kind: "KafkaChannel", + APIVersion: "messaging.knative.dev/v1alpha1", + }, + Subscriber: getValidDestination(), + }, + }, + want: nil, }, { name: "Channel changed", c: &Subscription{ diff --git a/pkg/apis/messaging/v1beta1/subscription_validation.go b/pkg/apis/messaging/v1beta1/subscription_validation.go index 5cf8c2e8515..6d316efbb70 100644 --- a/pkg/apis/messaging/v1beta1/subscription_validation.go +++ b/pkg/apis/messaging/v1beta1/subscription_validation.go @@ -75,8 +75,16 @@ func (s *Subscription) CheckImmutableFields(ctx context.Context, original *Subsc return nil } + // TODO(slinkydeveloper) + // HACK around the immutability check to make sure the update script can upgrade the api version + // REMOVE AFTER 0.22 release + ignoredFields := []string{"Subscriber", "Reply"} + if original.Spec.Channel.Kind == "KafkaChannel" && original.Spec.Channel.APIVersion == "messaging.knative.dev/v1alpha1" && s.Spec.Channel.APIVersion == "messaging.knative.dev/v1beta1" { + ignoredFields = append(ignoredFields, "Channel.APIVersion") + } + // Only Subscriber and Reply are mutable. - ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, "Subscriber", "Reply") + ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, ignoredFields...) if diff, err := kmp.ShortDiff(original.Spec, s.Spec, ignoreArguments); err != nil { return &apis.FieldError{ Message: "Failed to diff Subscription", diff --git a/pkg/apis/messaging/v1beta1/subscription_validation_test.go b/pkg/apis/messaging/v1beta1/subscription_validation_test.go index c9a1a9efcdc..54956c713c1 100644 --- a/pkg/apis/messaging/v1beta1/subscription_validation_test.go +++ b/pkg/apis/messaging/v1beta1/subscription_validation_test.go @@ -282,6 +282,29 @@ func TestSubscriptionImmutable(t *testing.T) { }, }, want: nil, + }, { + name: "valid, kafkachannel hack", + c: &Subscription{ + Spec: SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Name: channelName, + Kind: "KafkaChannel", + APIVersion: "messaging.knative.dev/v1beta1", + }, + Subscriber: getValidDestination(), + }, + }, + og: &Subscription{ + Spec: SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Name: channelName, + Kind: "KafkaChannel", + APIVersion: "messaging.knative.dev/v1alpha1", + }, + Subscriber: getValidDestination(), + }, + }, + want: nil, }, { name: "valid, new Reply", c: &Subscription{ From a8a50cc1fb3dab6f6b1413bef8b5602d413daa41 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 18 Mar 2021 12:11:13 +0100 Subject: [PATCH 4/4] fmt Signed-off-by: Francesco Guardiani --- pkg/apis/messaging/v1beta1/subscription_validation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/messaging/v1beta1/subscription_validation_test.go b/pkg/apis/messaging/v1beta1/subscription_validation_test.go index 54956c713c1..71e20fa4495 100644 --- a/pkg/apis/messaging/v1beta1/subscription_validation_test.go +++ b/pkg/apis/messaging/v1beta1/subscription_validation_test.go @@ -286,7 +286,7 @@ func TestSubscriptionImmutable(t *testing.T) { name: "valid, kafkachannel hack", c: &Subscription{ Spec: SubscriptionSpec{ - Channel: corev1.ObjectReference{ + Channel: corev1.ObjectReference{ Name: channelName, Kind: "KafkaChannel", APIVersion: "messaging.knative.dev/v1beta1", @@ -296,7 +296,7 @@ func TestSubscriptionImmutable(t *testing.T) { }, og: &Subscription{ Spec: SubscriptionSpec{ - Channel: corev1.ObjectReference{ + Channel: corev1.ObjectReference{ Name: channelName, Kind: "KafkaChannel", APIVersion: "messaging.knative.dev/v1alpha1",