check KReference namespace matches parent. Allow escape hatch. (#1052)

apis/contexts.go

@@ -180,3 +180,21 @@ func DisallowDeprecated(ctx context.Context) context.Context {
 func IsDeprecatedAllowed(ctx context.Context) bool {
 	return ctx.Value(disallowDeprecated{}) == nil
 }
+
+// This is attached to contexts as they are passed down through a resource
+// being validated to direct them to allow namespaces (or missing namespace)
+// outside the parent (as indicated by WithinParent.
+type allowDifferentNamespace struct{}
+
+// AllowDifferentNamespace notes on the context that further validation
+// should allow different namespaces from the encapsulating object. Mainly
+// used by KReference, since it by default requires namespaces to match.
+func AllowDifferentNamespace(ctx context.Context) context.Context {
+	return context.WithValue(ctx, allowDifferentNamespace{}, struct{}{})
+}
+
+// IsDifferentNamespaceAllowed checks the context to see whether different
+// namespace is allowed from the encapsulating object.
+func IsDifferentNamespaceAllowed(ctx context.Context) bool {
+	return ctx.Value(allowDifferentNamespace{}) != nil
+}

apis/contexts_test.go

@@ -118,6 +118,16 @@ func TestContexts(t *testing.T) {
 		ctx:   ctx,
 		check: IsDeprecatedAllowed,
 		want:  true,
+	}, {
+		name:  "allow different namespace",
+		ctx:   AllowDifferentNamespace(ctx),
+		check: IsDifferentNamespaceAllowed,
+		want:  true,
+	}, {
+		name:  "don't allow different namespace",
+		ctx:   ctx,
+		check: IsDifferentNamespaceAllowed,
+		want:  false,
 	}}
 
 	for _, tc := range tests {

apis/duck/v1/destination.go

@@ -33,6 +33,8 @@ type Destination struct {
 	URI *apis.URL `json:"uri,omitempty"`
 }
 
+// Validate the Destination has all the necessary fields and check the
+// Namespace matches that of the parent object (using apis.ParentMeta).
 func (dest *Destination) Validate(ctx context.Context) *apis.FieldError {
 	if dest == nil {
 		return nil

apis/duck/v1/knative_reference.go

@@ -18,6 +18,7 @@ package v1
 
 import (
 	"context"
+	"fmt"
 
 	"knative.dev/pkg/apis"
 )
@@ -59,6 +60,23 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
 	if kr.Kind == "" {
 		errs = errs.Also(apis.ErrMissingField("kind"))
 	}
+	// Only if namespace is empty validate it. This is to deal with legacy
+	// objects in the storage that may now have the namespace filled in.
+	// Because things get defaulted in other cases, moving forward the
+	// kr.Namespace will not be empty.
+	if kr.Namespace != "" {
+		parentNS := apis.ParentMeta(ctx).Namespace
+		if !apis.IsDifferentNamespaceAllowed(ctx) {
+			if parentNS != "" && kr.Namespace != parentNS {
+				errs = errs.Also(&apis.FieldError{
+					Message: "mismatched namespaces",
+					Paths:   []string{"namespace"},
+					Details: fmt.Sprintf("parent namespace: %q does not match ref: %q", parentNS, kr.Namespace),
+				})
+			}
+
+		}
+	}
 	return errs
 }
 

apis/duck/v1/knative_reference_test.go

@@ -37,21 +37,22 @@ func TestValidate(t *testing.T) {
 
 	tests := map[string]struct {
 		ref  *KReference
+		ctx  context.Context
 		want *apis.FieldError
 	}{
 		"nil valid": {
-			ref: nil,
-			want: func() *apis.FieldError {
-				fe := apis.ErrMissingField("name", "kind", "apiVersion")
-				return fe
-			}(),
+			ref:  nil,
+			ctx:  ctx,
+			want: apis.ErrMissingField("name", "kind", "apiVersion"),
 		},
 		"valid ref": {
 			ref:  &validRef,
+			ctx:  ctx,
 			want: nil,
 		},
 		"invalid ref, empty": {
 			ref:  &KReference{},
+			ctx:  ctx,
 			want: apis.ErrMissingField("name", "kind", "apiVersion"),
 		},
 		"invalid ref, missing kind": {
@@ -60,10 +61,8 @@ func TestValidate(t *testing.T) {
 				Name:       name,
 				APIVersion: apiVersion,
 			},
-			want: func() *apis.FieldError {
-				fe := apis.ErrMissingField("kind")
-				return fe
-			}(),
+			ctx:  ctx,
+			want: apis.ErrMissingField("kind"),
 		},
 		"invalid ref, missing api version": {
 			ref: &KReference{
@@ -71,15 +70,37 @@ func TestValidate(t *testing.T) {
 				Name:      name,
 				Kind:      kind,
 			},
-			want: func() *apis.FieldError {
-				return apis.ErrMissingField("apiVersion")
-			}(),
+			ctx:  ctx,
+			want: apis.ErrMissingField("apiVersion"),
+		},
+		"invalid ref, mismatched namespaces": {
+			ref: &KReference{
+				Namespace:  namespace,
+				Name:       name,
+				Kind:       kind,
+				APIVersion: apiVersion,
+			},
+			ctx: apis.WithinParent(ctx, metav1.ObjectMeta{Namespace: "diffns"}),
+			want: &apis.FieldError{
+				Message: "mismatched namespaces",
+				Paths:   []string{"namespace"},
+				Details: `parent namespace: "diffns" does not match ref: "b-namespace"`,
+			},
+		},
+		"valid ref, mismatched namespaces, but overridden": {
+			ref: &KReference{
+				Namespace:  namespace,
+				Name:       name,
+				Kind:       kind,
+				APIVersion: apiVersion,
+			},
+			ctx: apis.AllowDifferentNamespace(apis.WithinParent(ctx, metav1.ObjectMeta{Namespace: "diffns"})),
 		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			gotErr := tc.ref.Validate(ctx)
+			gotErr := tc.ref.Validate(tc.ctx)
 
 			if tc.want != nil {
 				if diff := cmp.Diff(tc.want.Error(), gotErr.Error()); diff != "" {