From c396fdf788837ffe4721d0cb44c372dce77e6c79 Mon Sep 17 00:00:00 2001 From: Ali Ok Date: Fri, 26 Jun 2020 10:49:33 +0300 Subject: [PATCH 1/3] Subscription v1<>v1beta conversion and simple tests --- .../v1beta1/subscription_conversion.go | 59 ++++++++- .../v1beta1/subscription_conversion_test.go | 123 ++++++++++++++++++ 2 files changed, 176 insertions(+), 6 deletions(-) diff --git a/pkg/apis/messaging/v1beta1/subscription_conversion.go b/pkg/apis/messaging/v1beta1/subscription_conversion.go index 568bf41b56b..172b9490996 100644 --- a/pkg/apis/messaging/v1beta1/subscription_conversion.go +++ b/pkg/apis/messaging/v1beta1/subscription_conversion.go @@ -19,16 +19,63 @@ package v1beta1 import ( "context" "fmt" + duckv1 "knative.dev/eventing/pkg/apis/duck/v1" + duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" + "knative.dev/eventing/pkg/apis/messaging/v1" "knative.dev/pkg/apis" ) -// ConvertTo implements apis.Convertible -func (source *Subscription) ConvertTo(ctx context.Context, sink apis.Convertible) error { - return fmt.Errorf("v1beta1 is the highest known version, got: %T", sink) +// ConvertTo implements apis.Convertible. +// Converts source (from v1beta1.Subscription) into v1.Subscription +func (source *Subscription) ConvertTo(ctx context.Context, obj apis.Convertible) error { + switch sink := obj.(type) { + case *v1.Subscription: + sink.ObjectMeta = source.ObjectMeta + sink.Spec.Channel = source.Spec.Channel + if source.Spec.Delivery != nil { + sink.Spec.Delivery = &duckv1.DeliverySpec{} + if err := source.Spec.Delivery.ConvertTo(ctx, sink.Spec.Delivery); err != nil { + return err + } + } + sink.Spec.Subscriber = source.Spec.Subscriber + sink.Spec.Reply = source.Spec.Reply + + sink.Status.Status = source.Status.Status + sink.Status.PhysicalSubscription.SubscriberURI = source.Status.PhysicalSubscription.SubscriberURI + sink.Status.PhysicalSubscription.ReplyURI = source.Status.PhysicalSubscription.ReplyURI + sink.Status.PhysicalSubscription.DeadLetterSinkURI = source.Status.PhysicalSubscription.DeadLetterSinkURI + return nil + default: + return fmt.Errorf("Unknown conversion, got: %T", sink) + + } } -// ConvertFrom implements apis.Convertible -func (sink *Subscription) ConvertFrom(ctx context.Context, source apis.Convertible) error { - return fmt.Errorf("v1beta1 is the highest known version, got: %T", source) +// ConvertFrom implements apis.Convertible. +// Converts obj from v1.Subscription into v1beta1.Subscription +func (sink *Subscription) ConvertFrom(ctx context.Context, obj apis.Convertible) error { + switch source := obj.(type) { + case *v1.Subscription: + sink.ObjectMeta = source.ObjectMeta + sink.Spec.Channel = source.Spec.Channel + if source.Spec.Delivery != nil { + sink.Spec.Delivery = &duckv1beta1.DeliverySpec{} + if err := sink.Spec.Delivery.ConvertFrom(ctx, source.Spec.Delivery); err != nil { + return err + } + } + sink.Spec.Subscriber = source.Spec.Subscriber + sink.Spec.Reply = source.Spec.Reply + + sink.Status.Status = source.Status.Status + sink.Status.PhysicalSubscription.SubscriberURI = source.Status.PhysicalSubscription.SubscriberURI + sink.Status.PhysicalSubscription.ReplyURI = source.Status.PhysicalSubscription.ReplyURI + sink.Status.PhysicalSubscription.DeadLetterSinkURI = source.Status.PhysicalSubscription.DeadLetterSinkURI + + return nil + default: + return fmt.Errorf("Unknown conversion, got: %T", source) + } } diff --git a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go index 5205e9a2223..4d80ee4b67d 100644 --- a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go +++ b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go @@ -19,9 +19,30 @@ package v1beta1 import ( "context" "testing" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" + duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" + "knative.dev/eventing/pkg/apis/messaging/v1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" ) func TestSubscriptionConversionBadType(t *testing.T) { + good, bad := &Subscription{}, &Channel{} + + if err := good.ConvertTo(context.Background(), bad); err == nil { + t.Errorf("ConvertTo() = %#v, wanted error", bad) + } + + if err := good.ConvertFrom(context.Background(), bad); err == nil { + t.Errorf("ConvertFrom() = %#v, wanted error", good) + } +} + +func TestBrokerConversionBadVersion(t *testing.T) { good, bad := &Subscription{}, &Subscription{} if err := good.ConvertTo(context.Background(), bad); err == nil { @@ -32,3 +53,105 @@ func TestSubscriptionConversionBadType(t *testing.T) { t.Errorf("ConvertFrom() = %#v, wanted error", good) } } + +func TestSubscriptionConversion(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&v1.Subscription{}} + + linear := duckv1beta1.BackoffPolicyLinear + + tests := []struct { + name string + in *Subscription + }{{ + name: "min configuration", + in: &Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Name: "subscription-name", + Namespace: "subscription-ns", + Generation: 17, + }, + Spec: SubscriptionSpec{}, + }, + }, { + name: "full configuration", + in: &Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Name: "subscription-name", + Namespace: "subscription-ns", + Generation: 17, + }, + Spec: SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Kind: "channelKind", + Namespace: "channelNamespace", + Name: "channelName", + APIVersion: "channelAPIVersion", + }, + Subscriber: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "subscriber-dest-kind", + Namespace: "subscriber-dest-ns", + Name: "subscriber-dest-name", + APIVersion: "subscriber-dest-version", + }, + URI: apis.HTTP("address"), + }, + Reply: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "reply-dest-kind", + Namespace: "reply-dest-ns", + Name: "reply-dest-name", + APIVersion: "reply-dest-version", + }, + URI: apis.HTTP("address"), + }, + Delivery: &duckv1beta1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "dlKind", + Namespace: "dlNamespace", + Name: "dlName", + APIVersion: "dlAPIVersion", + }, + URI: apis.HTTP("dls"), + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: &linear, + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + Status: SubscriptionStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + PhysicalSubscription: SubscriptionStatusPhysicalSubscription{ + SubscriberURI: apis.HTTP("subscriber.example.com"), + ReplyURI: apis.HTTP("reply.example.com"), + DeadLetterSinkURI: apis.HTTP("dlc.example.com"), + }, + }, + }, + }} + for _, test := range tests { + for _, version := range versions { + t.Run(test.name, func(t *testing.T) { + ver := version + if err := test.in.ConvertTo(context.Background(), ver); err != nil { + t.Errorf("ConvertTo() = %v", err) + } + got := &Subscription{} + if err := got.ConvertFrom(context.Background(), ver); err != nil { + t.Errorf("ConvertFrom() = %v", err) + } + if diff := cmp.Diff(test.in, got); diff != "" { + t.Errorf("roundtrip (-want, +got) = %v", diff) + } + }) + } + } +} From b462de785cbfea14457f31a010d50c3b5d6dfbb9 Mon Sep 17 00:00:00 2001 From: Ali Ok Date: Fri, 26 Jun 2020 11:06:22 +0300 Subject: [PATCH 2/3] Formatting stuff --- pkg/apis/messaging/v1beta1/subscription_conversion.go | 3 ++- pkg/apis/messaging/v1beta1/subscription_conversion_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/apis/messaging/v1beta1/subscription_conversion.go b/pkg/apis/messaging/v1beta1/subscription_conversion.go index 172b9490996..2196e795244 100644 --- a/pkg/apis/messaging/v1beta1/subscription_conversion.go +++ b/pkg/apis/messaging/v1beta1/subscription_conversion.go @@ -19,10 +19,11 @@ package v1beta1 import ( "context" "fmt" + duckv1 "knative.dev/eventing/pkg/apis/duck/v1" duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" - "knative.dev/eventing/pkg/apis/messaging/v1" + v1 "knative.dev/eventing/pkg/apis/messaging/v1" "knative.dev/pkg/apis" ) diff --git a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go index 4d80ee4b67d..87dec2d9883 100644 --- a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go +++ b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go @@ -25,7 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" - "knative.dev/eventing/pkg/apis/messaging/v1" + v1 "knative.dev/eventing/pkg/apis/messaging/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" ) From 37fa48bb9f02b03dce48a5f032a7393de475d8e2 Mon Sep 17 00:00:00 2001 From: Ali Ok Date: Fri, 26 Jun 2020 11:04:19 +0300 Subject: [PATCH 3/3] Subscription v1<>v1beta conversion roundtrip tests --- .../v1beta1/subscription_conversion_test.go | 111 +++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go index 87dec2d9883..90b12f97420 100644 --- a/pkg/apis/messaging/v1beta1/subscription_conversion_test.go +++ b/pkg/apis/messaging/v1beta1/subscription_conversion_test.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" v1 "knative.dev/eventing/pkg/apis/messaging/v1" "knative.dev/pkg/apis" @@ -54,7 +55,8 @@ func TestBrokerConversionBadVersion(t *testing.T) { } } -func TestSubscriptionConversion(t *testing.T) { +// Test v1beta1 -> v1 -> v1beta1 +func TestSubscriptionConversionRoundTripV1beta1(t *testing.T) { // Just one for now, just adding the for loop for ease of future changes. versions := []apis.Convertible{&v1.Subscription{}} @@ -137,6 +139,7 @@ func TestSubscriptionConversion(t *testing.T) { }, }, }} + for _, test := range tests { for _, version := range versions { t.Run(test.name, func(t *testing.T) { @@ -148,6 +151,112 @@ func TestSubscriptionConversion(t *testing.T) { if err := got.ConvertFrom(context.Background(), ver); err != nil { t.Errorf("ConvertFrom() = %v", err) } + + if diff := cmp.Diff(test.in, got); diff != "" { + t.Errorf("roundtrip (-want, +got) = %v", diff) + } + }) + } + } +} + +// Test v1 -> v1beta1 -> v1 +func TestBrokerConversionRoundTripV1(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&Subscription{}} + + linear := eventingduckv1.BackoffPolicyLinear + + tests := []struct { + name string + in *v1.Subscription + }{{ + name: "min configuration", + in: &v1.Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Name: "subscription-name", + Namespace: "subscription-ns", + Generation: 17, + }, + Spec: v1.SubscriptionSpec{}, + }, + }, { + name: "full configuration", + in: &v1.Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Name: "subscription-name", + Namespace: "subscription-ns", + Generation: 17, + }, + Spec: v1.SubscriptionSpec{ + Channel: corev1.ObjectReference{ + Kind: "channelKind", + Namespace: "channelNamespace", + Name: "channelName", + APIVersion: "channelAPIVersion", + }, + Subscriber: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "subscriber-dest-kind", + Namespace: "subscriber-dest-ns", + Name: "subscriber-dest-name", + APIVersion: "subscriber-dest-version", + }, + URI: apis.HTTP("address"), + }, + Reply: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "reply-dest-kind", + Namespace: "reply-dest-ns", + Name: "reply-dest-name", + APIVersion: "reply-dest-version", + }, + URI: apis.HTTP("address"), + }, + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "dlKind", + Namespace: "dlNamespace", + Name: "dlName", + APIVersion: "dlAPIVersion", + }, + URI: apis.HTTP("dls"), + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: &linear, + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + Status: v1.SubscriptionStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + PhysicalSubscription: v1.SubscriptionStatusPhysicalSubscription{ + SubscriberURI: apis.HTTP("subscriber.example.com"), + ReplyURI: apis.HTTP("reply.example.com"), + DeadLetterSinkURI: apis.HTTP("dlc.example.com"), + }, + }, + }, + }} + + for _, test := range tests { + for _, version := range versions { + t.Run(test.name, func(t *testing.T) { + ver := version + if err := ver.ConvertFrom(context.Background(), test.in); err != nil { + t.Errorf("ConvertFrom() = %v", err) + } + got := &v1.Subscription{} + if err := ver.ConvertTo(context.Background(), got); err != nil { + t.Errorf("ConvertTo() = %v", err) + } + if diff := cmp.Diff(test.in, got); diff != "" { t.Errorf("roundtrip (-want, +got) = %v", diff) }