From fa0a998deac4466bfb5aa61e4f1503c56f352d58 Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Tue, 5 Mar 2024 16:16:41 +0100 Subject: [PATCH] Remove assumption on default namespace for EventType reference (#7724) (#548) * Remove assumption on default namespace for EventType reference * Add and fix tests * Update pkg/reconciler/source/duck/duck_test.go * Update pkg/reconciler/source/duck/duck_test.go --------- Signed-off-by: Pierangelo Di Pilato Co-authored-by: Calum Murray --- .../eventing/v1beta1/eventtype_defaults.go | 10 ++- .../eventing/v1beta2/eventtype_defaults.go | 11 ++-- .../eventing/v1beta3/eventtype_defaults.go | 11 ++-- pkg/reconciler/source/duck/duck.go | 5 +- pkg/reconciler/source/duck/duck_test.go | 61 ++++++++++++++++--- 5 files changed, 72 insertions(+), 26 deletions(-) diff --git a/pkg/apis/eventing/v1beta1/eventtype_defaults.go b/pkg/apis/eventing/v1beta1/eventtype_defaults.go index 115df98d460..07a7a3d400c 100644 --- a/pkg/apis/eventing/v1beta1/eventtype_defaults.go +++ b/pkg/apis/eventing/v1beta1/eventtype_defaults.go @@ -16,9 +16,14 @@ limitations under the License. package v1beta1 -import "context" +import ( + "context" + + "knative.dev/pkg/apis" +) func (et *EventType) SetDefaults(ctx context.Context) { + ctx = apis.WithinParent(ctx, et.ObjectMeta) et.Spec.SetDefaults(ctx) } @@ -26,4 +31,7 @@ func (ets *EventTypeSpec) SetDefaults(ctx context.Context) { if ets.Reference == nil && ets.Broker == "" { ets.Broker = "default" } + if ets.Reference != nil { + ets.Reference.SetDefaults(ctx) + } } diff --git a/pkg/apis/eventing/v1beta2/eventtype_defaults.go b/pkg/apis/eventing/v1beta2/eventtype_defaults.go index 02b6f0f2a3c..217cc51588a 100644 --- a/pkg/apis/eventing/v1beta2/eventtype_defaults.go +++ b/pkg/apis/eventing/v1beta2/eventtype_defaults.go @@ -18,18 +18,17 @@ package v1beta2 import ( "context" + + "knative.dev/pkg/apis" ) func (et *EventType) SetDefaults(ctx context.Context) { + ctx = apis.WithinParent(ctx, et.ObjectMeta) et.Spec.SetDefaults(ctx) - setReferenceNs(et) } func (ets *EventTypeSpec) SetDefaults(ctx context.Context) { -} - -func setReferenceNs(et *EventType) { - if et.Spec.Reference != nil && et.Spec.Reference.Namespace == "" { - et.Spec.Reference.Namespace = et.GetNamespace() + if ets.Reference != nil { + ets.Reference.SetDefaults(ctx) } } diff --git a/pkg/apis/eventing/v1beta3/eventtype_defaults.go b/pkg/apis/eventing/v1beta3/eventtype_defaults.go index f9c73a94c26..4c894901f78 100644 --- a/pkg/apis/eventing/v1beta3/eventtype_defaults.go +++ b/pkg/apis/eventing/v1beta3/eventtype_defaults.go @@ -18,18 +18,17 @@ package v1beta3 import ( "context" + + "knative.dev/pkg/apis" ) func (et *EventType) SetDefaults(ctx context.Context) { + ctx = apis.WithinParent(ctx, et.ObjectMeta) et.Spec.SetDefaults(ctx) - setReferenceNs(et) } func (ets *EventTypeSpec) SetDefaults(ctx context.Context) { -} - -func setReferenceNs(et *EventType) { - if et.Spec.Reference != nil && et.Spec.Reference.Namespace == "" { - et.Spec.Reference.Namespace = et.GetNamespace() + if ets.Reference != nil { + ets.Reference.SetDefaults(ctx) } } diff --git a/pkg/reconciler/source/duck/duck.go b/pkg/reconciler/source/duck/duck.go index 7ca55ec48c0..f41b3774bd6 100644 --- a/pkg/reconciler/source/duck/duck.go +++ b/pkg/reconciler/source/duck/duck.go @@ -215,9 +215,6 @@ func (r *Reconciler) makeEventTypes(ctx context.Context, src *duckv1.Source) []v CeSchema: schemaURL, Description: description, }) - if eventType.Spec.Reference.Namespace == "" { - eventType.Spec.Reference.Namespace = defaultNamespace - } eventTypes = append(eventTypes, *eventType) } return eventTypes @@ -235,7 +232,7 @@ func (r *Reconciler) computeDiff(current []v1beta2.EventType, expected []v1beta2 if c, ok := currentMap[keyFromEventType(&e)]; !ok { toCreate = append(toCreate, e) } else { - if !equality.Semantic.DeepEqual(e.Spec, c.Spec) { + if !equality.Semantic.DeepDerivative(e.Spec, c.Spec) { toDelete = append(toDelete, c) toCreate = append(toCreate, e) } diff --git a/pkg/reconciler/source/duck/duck_test.go b/pkg/reconciler/source/duck/duck_test.go index 1bd6dc44458..c68482ac480 100644 --- a/pkg/reconciler/source/duck/duck_test.go +++ b/pkg/reconciler/source/duck/duck_test.go @@ -31,10 +31,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" clientgotesting "k8s.io/client-go/testing" - v1 "knative.dev/eventing/pkg/apis/duck/v1" - "knative.dev/eventing/pkg/apis/eventing" - fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" - "knative.dev/eventing/pkg/reconciler/source/duck/resources" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/client/injection/ducks/duck/v1/source" @@ -43,8 +39,14 @@ import ( logtesting "knative.dev/pkg/logging/testing" "knative.dev/pkg/ptr" - . "knative.dev/eventing/pkg/reconciler/testing/v1beta2" + v1 "knative.dev/eventing/pkg/apis/duck/v1" + "knative.dev/eventing/pkg/apis/eventing" + fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" + "knative.dev/eventing/pkg/reconciler/source/duck/resources" + . "knative.dev/pkg/reconciler/testing" + + . "knative.dev/eventing/pkg/reconciler/testing/v1beta2" ) const ( @@ -250,6 +252,50 @@ func TestAllCases(t *testing.T) { WantCreates: []runtime.Object{ makeEventType("my-type-1", "http://my-source-1"), }, + }, { + Name: "no op", + Objects: []runtime.Object{ + makeSource([]duckv1.CloudEventAttributes{{ + Type: "my-type-1", + Source: "http://my-source-1", + }}), + func() runtime.Object { + s := makeSourceCRD([]eventTypeEntry{{ + Type: "my-type-1", + Schema: "/some-schema-from-crd", + Description: "This came from the annotation in a crd for the source.", + }}) + s.Annotations[eventing.EventTypesAnnotationKey] = "something that is not valid json" + return s + }(), + makeEventType("my-type-1", "http://my-source-1"), + }, + Key: testNS + "/" + sourceName, + WantCreates: []runtime.Object{}, + }, { + Name: "no op with namespace", + Objects: []runtime.Object{ + makeSource([]duckv1.CloudEventAttributes{{ + Type: "my-type-1", + Source: "http://my-source-1", + }}), + func() runtime.Object { + s := makeSourceCRD([]eventTypeEntry{{ + Type: "my-type-1", + Schema: "/some-schema-from-crd", + Description: "This came from the annotation in a crd for the source.", + }}) + s.Annotations[eventing.EventTypesAnnotationKey] = "something that is not valid json" + return s + }(), + func() runtime.Object { + et := makeEventType("my-type-1", "http://my-source-1") + et.Spec.Reference.Namespace = testNS + return et + }(), + }, + Key: testNS + "/" + sourceName, + WantCreates: []runtime.Object{}, }} logger := logtesting.TestLogger(t) @@ -340,14 +386,11 @@ func makeSourceCRD(eventTypes []eventTypeEntry) *apix1.CustomResourceDefinition } func makeEventType(ceType, ceSource string) *v1beta2.EventType { - return makeEventTypeWithReference(ceType, ceSource, brokerDest.Ref) + return makeEventTypeWithReference(ceType, ceSource, brokerDest.Ref.DeepCopy()) } func makeEventTypeWithReference(ceType, ceSource string, ref *duckv1.KReference) *v1beta2.EventType { ceSourceURL, _ := apis.ParseURL(ceSource) - if ref.Namespace == "" { - ref.Namespace = "default" - } return &v1beta2.EventType{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%x", md5.Sum([]byte(ceType+ceSource+sourceUID))),