diff --git a/pkg/apis/eventing/v1beta1/broker_conversion.go b/pkg/apis/eventing/v1beta1/broker_conversion.go index 8e6e22f8366..9537e433f2a 100644 --- a/pkg/apis/eventing/v1beta1/broker_conversion.go +++ b/pkg/apis/eventing/v1beta1/broker_conversion.go @@ -20,7 +20,9 @@ import ( "context" "fmt" - v1 "knative.dev/eventing/pkg/apis/eventing/v1" + duckv1 "knative.dev/eventing/pkg/apis/duck/v1" + duckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" + "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/pkg/apis" ) @@ -28,8 +30,10 @@ import ( func (source *Broker) ConvertTo(ctx context.Context, to apis.Convertible) error { switch sink := to.(type) { case *v1.Broker: + sink.ObjectMeta = source.ObjectMeta sink.Spec.Config = source.Spec.Config if source.Spec.Delivery != nil { + sink.Spec.Delivery = &duckv1.DeliverySpec{} if err := source.Spec.Delivery.ConvertTo(ctx, sink.Spec.Delivery); err != nil { return err } @@ -46,9 +50,11 @@ func (source *Broker) ConvertTo(ctx context.Context, to apis.Convertible) error func (sink *Broker) ConvertFrom(ctx context.Context, from apis.Convertible) error { switch source := from.(type) { case *v1.Broker: + sink.ObjectMeta = source.ObjectMeta sink.Spec.Config = source.Spec.Config if source.Spec.Delivery != nil { - if err := source.Spec.Delivery.ConvertFrom(ctx, sink.Spec.Delivery); err != nil { + sink.Spec.Delivery = &duckv1beta1.DeliverySpec{} + if err := sink.Spec.Delivery.ConvertFrom(ctx, source.Spec.Delivery); err != nil { return err } } diff --git a/pkg/apis/eventing/v1beta1/broker_conversion_test.go b/pkg/apis/eventing/v1beta1/broker_conversion_test.go index e6b76b06c47..8966dc418d2 100644 --- a/pkg/apis/eventing/v1beta1/broker_conversion_test.go +++ b/pkg/apis/eventing/v1beta1/broker_conversion_test.go @@ -20,21 +20,16 @@ import ( "context" "testing" - v1 "knative.dev/eventing/pkg/apis/eventing/v1" + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + eventingduckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1" + "knative.dev/eventing/pkg/apis/eventing/v1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" ) -func TestBrokerConversion(t *testing.T) { - objv1betav1, objv1 := &Broker{}, &v1.Broker{} - - if err := objv1betav1.ConvertTo(context.Background(), objv1); err != nil { - t.Errorf("ConvertTo() = %#v, unexpected error", objv1betav1) - } - - if err := objv1betav1.ConvertFrom(context.Background(), objv1); err != nil { - t.Errorf("ConvertFrom() = %#v, unexpected error", objv1) - } -} - func TestBrokerConversionBadType(t *testing.T) { good, bad := &Broker{}, &Trigger{} @@ -58,3 +53,173 @@ func TestBrokerConversionBadVersion(t *testing.T) { t.Errorf("ConvertFrom() = %#v, wanted error", good) } } + +// Test v1beta1 -> v1 -> v1beta1 +func TestBrokerConversionRoundTripV1beta1(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&v1.Broker{}} + + linear := eventingduckv1beta1.BackoffPolicyLinear + + tests := []struct { + name string + in *Broker + }{{ + name: "min configuration", + in: &Broker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "broker-name", + Namespace: "broker-ns", + Generation: 17, + }, + Spec: BrokerSpec{}, + }, + }, { + name: "full configuration", + in: &Broker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "broker-name", + Namespace: "broker-ns", + Generation: 17, + }, + Spec: BrokerSpec{ + Config: &duckv1.KReference{ + Kind: "config-kind", + Namespace: "config-ns", + Name: "config-name", + APIVersion: "config-version", + }, + Delivery: &eventingduckv1beta1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "dl-sink-kind", + Namespace: "dl-sink-ns", + Name: "dl-sink-name", + APIVersion: "dl-sink-version", + }, + URI: apis.HTTP("dl-sink.example.com"), + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: &linear, + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + Status: BrokerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + Address: duckv1.Addressable{ + URL: apis.HTTP("address"), + }, + }, + }, + }} + + 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 := &Broker{} + 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{&Broker{}} + + linear := eventingduckv1.BackoffPolicyLinear + + tests := []struct { + name string + in *v1.Broker + }{{ + name: "min configuration", + in: &v1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "par-name", + Namespace: "par-ns", + Generation: 17, + }, + Spec: v1.BrokerSpec{}, + }, + }, { + name: "full configuration", + in: &v1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "broker-name", + Namespace: "broker-ns", + Generation: 17, + }, + Spec: v1.BrokerSpec{ + Config: &duckv1.KReference{ + Kind: "config-kind", + Namespace: "config-ns", + Name: "config-name", + APIVersion: "config-version", + }, + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "dl-sink-kind", + Namespace: "dl-sink-ns", + Name: "dl-sink-name", + APIVersion: "dl-sink-version", + }, + URI: apis.HTTP("dl-sink.example.com"), + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: &linear, + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + Status: v1.BrokerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + Address: duckv1.Addressable{ + URL: apis.HTTP("address"), + }, + }, + }, + }} + + 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("ConvertDown() = %v", err) + } + got := &v1.Broker{} + if err := ver.ConvertTo(context.Background(), got); err != nil { + t.Errorf("ConvertUp() = %v", err) + } + + if diff := cmp.Diff(test.in, got); diff != "" { + t.Errorf("roundtrip (-want, +got) = %v", diff) + } + }) + } + } +} diff --git a/pkg/apis/eventing/v1beta1/trigger_conversion.go b/pkg/apis/eventing/v1beta1/trigger_conversion.go index 46abecaf6eb..4cd9ea2868c 100644 --- a/pkg/apis/eventing/v1beta1/trigger_conversion.go +++ b/pkg/apis/eventing/v1beta1/trigger_conversion.go @@ -20,7 +20,7 @@ import ( "context" "fmt" - v1 "knative.dev/eventing/pkg/apis/eventing/v1" + "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/pkg/apis" ) @@ -28,10 +28,13 @@ import ( func (source *Trigger) ConvertTo(_ context.Context, to apis.Convertible) error { switch sink := to.(type) { case *v1.Trigger: + sink.ObjectMeta = source.ObjectMeta sink.Spec.Broker = source.Spec.Broker sink.Spec.Subscriber = source.Spec.Subscriber if source.Spec.Filter != nil { - sink.Spec.Filter = &v1.TriggerFilter{} + sink.Spec.Filter = &v1.TriggerFilter{ + Attributes: make(v1.TriggerFilterAttributes, 0), + } for k, v := range source.Spec.Filter.Attributes { sink.Spec.Filter.Attributes[k] = v } @@ -48,12 +51,16 @@ func (source *Trigger) ConvertTo(_ context.Context, to apis.Convertible) error { func (sink *Trigger) ConvertFrom(_ context.Context, from apis.Convertible) error { switch source := from.(type) { case *v1.Trigger: + sink.ObjectMeta = source.ObjectMeta sink.Spec.Broker = source.Spec.Broker sink.Spec.Subscriber = source.Spec.Subscriber if source.Spec.Filter != nil { - sink.Spec.Filter = &TriggerFilter{} + attributes := TriggerFilterAttributes{} for k, v := range source.Spec.Filter.Attributes { - sink.Spec.Filter.Attributes[k] = v + attributes[k] = v + } + sink.Spec.Filter = &TriggerFilter{ + Attributes: attributes, } } sink.Status.Status = source.Status.Status diff --git a/pkg/apis/eventing/v1beta1/trigger_conversion_test.go b/pkg/apis/eventing/v1beta1/trigger_conversion_test.go index 19de939df67..5b07804cbfe 100644 --- a/pkg/apis/eventing/v1beta1/trigger_conversion_test.go +++ b/pkg/apis/eventing/v1beta1/trigger_conversion_test.go @@ -20,23 +20,16 @@ import ( "context" "testing" - v1 "knative.dev/eventing/pkg/apis/eventing/v1" -) - -func TestTriggerConversion(t *testing.T) { - objv1betav1, objv1 := &Trigger{}, &v1.Trigger{} + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - if err := objv1betav1.ConvertTo(context.Background(), objv1); err != nil { - t.Errorf("ConvertTo() = %#v, unexpected error", objv1betav1) - } - - if err := objv1betav1.ConvertFrom(context.Background(), objv1); err != nil { - t.Errorf("ConvertFrom() = %#v, unexpected error", objv1) - } -} + "knative.dev/eventing/pkg/apis/eventing/v1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) func TestTriggerConversionBadType(t *testing.T) { - good, bad := &Trigger{}, &Broker{} + good, bad := &Trigger{}, &Trigger{} if err := good.ConvertTo(context.Background(), bad); err == nil { t.Errorf("ConvertTo() = %#v, wanted error", bad) @@ -58,3 +51,261 @@ func TestTriggerConversionBadVersion(t *testing.T) { t.Errorf("ConvertFrom() = %#v, wanted error", good) } } + +// Test v1beta1 -> v1 -> v1beta1 +func TestTriggerConversionRoundTripV1beta1(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&v1.Trigger{}} + + tests := []struct { + name string + in *Trigger + }{{name: "simple configuration", + in: &Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: TriggerSpec{ + Broker: "default", + }, + Status: TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "filter rules", + in: &Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: TriggerSpec{ + Broker: "default", + Filter: &TriggerFilter{ + Attributes: TriggerFilterAttributes{"source": "mysource", "type": "mytype"}, + }, + }, + Status: TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "filter rules, many", + in: &Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: TriggerSpec{ + Broker: "default", + Filter: &TriggerFilter{ + Attributes: TriggerFilterAttributes{"source": "mysource", "type": "mytype", "customkey": "customvalue"}, + }, + }, + Status: TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "full configuration", + in: &Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: TriggerSpec{ + Broker: "default", + Filter: &TriggerFilter{ + Attributes: TriggerFilterAttributes{"source": "mysource", "type": "mytype"}, + }, + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "subscriberKind", + Namespace: "subscriberNamespace", + Name: "subscriberName", + APIVersion: "subscriberAPIVersion", + }, + URI: apis.HTTP("subscriberURI"), + }, + }, + Status: TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + SubscriberURI: apis.HTTP("subscriberURI"), + }, + }, + }} + + 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 := &Trigger{} + 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 TestTriggerConversionRoundTripV1(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&Trigger{}} + + tests := []struct { + name string + in *v1.Trigger + }{{name: "simple configuration", + in: &v1.Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: v1.TriggerSpec{ + Broker: "default", + }, + Status: v1.TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "filter rules", + in: &v1.Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: v1.TriggerSpec{ + Broker: "default", + Filter: &v1.TriggerFilter{ + Attributes: v1.TriggerFilterAttributes{"source": "mysource", "type": "mytype"}, + }, + }, + Status: v1.TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "filter rules, many", + in: &v1.Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: v1.TriggerSpec{ + Broker: "default", + Filter: &v1.TriggerFilter{ + Attributes: v1.TriggerFilterAttributes{"source": "mysource", "type": "mytype", "customkey": "customvalue"}, + }, + }, + Status: v1.TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + }, + }, + }, {name: "full configuration", + in: &v1.Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: v1.TriggerSpec{ + Broker: "default", + Filter: &v1.TriggerFilter{ + Attributes: v1.TriggerFilterAttributes{"source": "mysource", "type": "mytype"}, + }, + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "subscriberKind", + Namespace: "subscriberNamespace", + Name: "subscriberName", + APIVersion: "subscriberAPIVersion", + }, + URI: apis.HTTP("subscriberURI"), + }, + }, + Status: v1.TriggerStatus{ + Status: duckv1.Status{ + ObservedGeneration: 1, + Conditions: duckv1.Conditions{{ + Type: "Ready", + Status: "True", + }}, + }, + SubscriberURI: apis.HTTP("subscriberURI"), + }, + }, + }} + + 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("ConvertDown() = %v", err) + } + got := &v1.Trigger{} + if err := ver.ConvertTo(context.Background(), got); err != nil { + t.Errorf("ConvertUp() = %v", err) + } + + if diff := cmp.Diff(test.in, got); diff != "" { + t.Errorf("roundtrip (-want, +got) = %v", diff) + } + }) + } + } +}