Remove assumption on default namespace for EventType reference (knati…

…ve#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 <[email protected]> Co-authored-by: Calum Murray <[email protected]>
openshift-knative · Mar 5, 2024 · fa0a998 · fa0a998
1 parent 15e55a4
commit fa0a998
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 26 deletions.
diff --git a/pkg/apis/eventing/v1beta1/eventtype_defaults.go b/pkg/apis/eventing/v1beta1/eventtype_defaults.go
@@ -16,14 +16,22 @@ 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)
 }
 
 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
@@ -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
@@ -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
@@ -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
@@ -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))),