From ccdcd380eda5dd9eaebe722088543d8fb5b9b80d Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Mon, 26 Feb 2024 17:09:22 +0100 Subject: [PATCH 1/4] Remove assumption on default namespace for EventType reference Signed-off-by: Pierangelo Di Pilato --- pkg/apis/eventing/v1beta1/eventtype_defaults.go | 10 +++++++++- pkg/apis/eventing/v1beta2/eventtype_defaults.go | 11 +++++------ pkg/apis/eventing/v1beta3/eventtype_defaults.go | 11 +++++------ pkg/reconciler/source/duck/duck.go | 5 +---- 4 files changed, 20 insertions(+), 17 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) } From 851de68ff0511e1a66aad97e46e070323660a2f9 Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Mon, 26 Feb 2024 19:54:34 +0100 Subject: [PATCH 2/4] Add and fix tests Signed-off-by: Pierangelo Di Pilato --- pkg/reconciler/source/duck/duck_test.go | 61 +++++++++++++++++++++---- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/pkg/reconciler/source/duck/duck_test.go b/pkg/reconciler/source/duck/duck_test.go index 1bd6dc44458..c79ab665c04 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))), From dee82b3f9b675481194d74e777855e4615770dac Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Mon, 26 Feb 2024 20:37:08 +0100 Subject: [PATCH 3/4] Update pkg/reconciler/source/duck/duck_test.go Co-authored-by: Calum Murray --- pkg/reconciler/source/duck/duck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/source/duck/duck_test.go b/pkg/reconciler/source/duck/duck_test.go index c79ab665c04..0227db21174 100644 --- a/pkg/reconciler/source/duck/duck_test.go +++ b/pkg/reconciler/source/duck/duck_test.go @@ -294,7 +294,7 @@ func TestAllCases(t *testing.T) { return et }(), }, - Key: testNS + "/" + sourceName, + Key: testNS + "/" + sourceName, WantCreates: []runtime.Object{}, }} From e8c9f8ea282f562238c0b5895bcb2ab64d916870 Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Mon, 26 Feb 2024 20:37:13 +0100 Subject: [PATCH 4/4] Update pkg/reconciler/source/duck/duck_test.go Co-authored-by: Calum Murray --- pkg/reconciler/source/duck/duck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/source/duck/duck_test.go b/pkg/reconciler/source/duck/duck_test.go index 0227db21174..c68482ac480 100644 --- a/pkg/reconciler/source/duck/duck_test.go +++ b/pkg/reconciler/source/duck/duck_test.go @@ -270,7 +270,7 @@ func TestAllCases(t *testing.T) { }(), makeEventType("my-type-1", "http://my-source-1"), }, - Key: testNS + "/" + sourceName, + Key: testNS + "/" + sourceName, WantCreates: []runtime.Object{}, }, { Name: "no op with namespace",