From 3fedccddd2c8e68800ec1be377a13d93a80ac022 Mon Sep 17 00:00:00 2001 From: Ville Aikas Date: Wed, 5 Feb 2020 14:59:50 -0800 Subject: [PATCH] add tests for trigger conversion --- .../eventing/v1alpha1/trigger_conversion.go | 12 +- .../v1alpha1/trigger_conversion_test.go | 190 ++++++++++++++++++ 2 files changed, 198 insertions(+), 4 deletions(-) create mode 100644 pkg/apis/eventing/v1alpha1/trigger_conversion_test.go diff --git a/pkg/apis/eventing/v1alpha1/trigger_conversion.go b/pkg/apis/eventing/v1alpha1/trigger_conversion.go index cf5d5160de3..c257c8751b0 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_conversion.go +++ b/pkg/apis/eventing/v1alpha1/trigger_conversion.go @@ -33,7 +33,9 @@ func (source *Trigger) ConvertUp(ctx context.Context, obj apis.Convertible) erro sink.Spec.Broker = source.Spec.Broker sink.Spec.Subscriber = source.Spec.Subscriber if source.Spec.Filter != nil { - sink.Spec.Filter = &v1beta1.TriggerFilter{} + sink.Spec.Filter = &v1beta1.TriggerFilter{ + Attributes: make(v1beta1.TriggerFilterAttributes, 0), + } if source.Spec.Filter.Attributes != nil { for k, v := range *source.Spec.Filter.Attributes { sink.Spec.Filter.Attributes[k] = v @@ -62,10 +64,12 @@ func (sink *Trigger) ConvertDown(ctx context.Context, obj apis.Convertible) erro sink.Spec.Broker = source.Spec.Broker sink.Spec.Subscriber = source.Spec.Subscriber if source.Spec.Filter != nil { - sink.Spec.Filter = &TriggerFilter{} - tfa := TriggerFilterAttributes{} + attributes := TriggerFilterAttributes{} for k, v := range source.Spec.Filter.Attributes { - tfa[k] = v + attributes[k] = v + } + sink.Spec.Filter = &TriggerFilter{ + Attributes: &attributes, } } diff --git a/pkg/apis/eventing/v1alpha1/trigger_conversion_test.go b/pkg/apis/eventing/v1alpha1/trigger_conversion_test.go new file mode 100644 index 00000000000..2bbe5a766fe --- /dev/null +++ b/pkg/apis/eventing/v1alpha1/trigger_conversion_test.go @@ -0,0 +1,190 @@ +/* +Copyright 2020 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 v1alpha1 + +import ( + "context" + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/eventing/pkg/apis/eventing/v1beta1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +// TODO: Replace dummy some other Eventing object once they +// implement apis.Convertible +type dummy struct{} + +func (*dummy) ConvertUp(ctx context.Context, obj apis.Convertible) error { + return errors.New("Won't go") +} + +func (*dummy) ConvertDown(ctx context.Context, obj apis.Convertible) error { + return errors.New("Won't go") +} + +func TestTriggerConversionBadType(t *testing.T) { + good, bad := &Trigger{}, &dummy{} + + if err := good.ConvertUp(context.Background(), bad); err == nil { + t.Errorf("ConvertUp() = %#v, wanted error", bad) + } + + if err := good.ConvertDown(context.Background(), bad); err == nil { + t.Errorf("ConvertDown() = %#v, wanted error", good) + } +} + +func TestTriggerConversion(t *testing.T) { + // Just one for now, just adding the for loop for ease of future changes. + versions := []apis.Convertible{&v1beta1.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, deprecated", + in: &Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "trigger-name", + Namespace: "trigger-ns", + Generation: 17, + }, + Spec: TriggerSpec{ + Broker: "default", + Filter: &TriggerFilter{ + DeprecatedSourceAndType: &TriggerFilterSourceAndType{ + Source: "mysource", + Type: "mytype", + }, + }, + }, + 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", + }}, + }, + }, + }, + }} + for _, test := range tests { + for _, version := range versions { + t.Run(test.name, func(t *testing.T) { + ver := version + if err := test.in.ConvertUp(context.Background(), ver); err != nil { + t.Errorf("ConvertUp() = %v", err) + } + got := &Trigger{} + if err := got.ConvertDown(context.Background(), ver); err != nil { + t.Errorf("ConvertDown() = %v", err) + } + // Since on the way down, we lose the DeprecatedSourceAndType, + // convert the in to equivalent out. + fixed := fixDeprecated(test.in) + if diff := cmp.Diff(fixed, got); diff != "" { + t.Errorf("roundtrip (-want, +got) = %v", diff) + } + }) + } + } +} + +// Since DeprecatedSourceAndType is lossy but semanctically equivalent +// if source,type are present and equivalent in the attributes map, +// fix that so diff works. +func fixDeprecated(in *Trigger) *Trigger { + if in.Spec.Filter != nil && in.Spec.Filter.DeprecatedSourceAndType != nil { + // attributes must be nil, can't have both Deprecated / Attributes + attributes := TriggerFilterAttributes{} + attributes["source"] = in.Spec.Filter.DeprecatedSourceAndType.Source + attributes["type"] = in.Spec.Filter.DeprecatedSourceAndType.Type + in.Spec.Filter.DeprecatedSourceAndType = nil + in.Spec.Filter.Attributes = &attributes + } + return in +}