Updating validation tests to follow the spec for spec.subscriber

config/core/configmaps/features.yaml

@@ -35,3 +35,7 @@ data:
   # ALPHA feature: The kreference-mapping allows you to map kreference onto templated URI
   # For more details: https://github.com/knative/eventing/issues/5593
   kreference-mapping: "disabled"
+
+  # ALPHA feature: The subscriber-strict flag force subscriptions to define a subscriber
+  # For more details: https://github.com/knative/eventing/issues/5756
+  strict-subscriber: "disabled"

pkg/apis/feature/flag_names.go

@@ -20,4 +20,5 @@ const (
 	KReferenceGroup   = "kreference-group"
 	DeliveryTimeout   = "delivery-timeout"
 	KReferenceMapping = "kreference-mapping"
+	StrictSubscriber  = "strict-subscriber"
 )

pkg/apis/messaging/v1/subscription_validation.go

@@ -19,6 +19,8 @@ package v1
 import (
 	"context"
 
+	"knative.dev/eventing/pkg/apis/feature"
+
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"knative.dev/pkg/apis"
@@ -50,10 +52,20 @@ func (ss *SubscriptionSpec) Validate(ctx context.Context) *apis.FieldError {
 
 	missingSubscriber := isDestinationNilOrEmpty(ss.Subscriber)
 	missingReply := isDestinationNilOrEmpty(ss.Reply)
-	if missingSubscriber && missingReply {
-		fe := apis.ErrMissingField("reply", "subscriber")
-		fe.Details = "the Subscription must reference at least one of (reply or a subscriber)"
+
+	// Check if StrictSubscriber flag is enabled, if so we follow the spec and check for a valid reference to a subscriber
+	if missingSubscriber && feature.FromContext(ctx).IsEnabled(feature.StrictSubscriber) {
+		fe := apis.ErrMissingField("subscriber")
+		fe.Details = "the Subscription must reference a subscriber"
 		errs = errs.Also(fe)
+
+	} else { // if the flag is not set, we use pre 0.26 behavior
+		if missingSubscriber && missingReply {
+			fe := apis.ErrMissingField("reply", "subscriber")
+			fe.Details = "the Subscription must reference at least one of (reply or a subscriber)"
+			errs = errs.Also(fe)
+		}
+
 	}
 
 	if !missingSubscriber {

pkg/apis/messaging/v1/subscription_validation_test.go

@@ -605,3 +605,80 @@ func TestValidChannel(t *testing.T) {
 		})
 	}
 }
+
+func TestSubscriptionSpecValidationWithStrictSubscriber(t *testing.T) {
+	tests := []struct {
+		name string
+		c    *SubscriptionSpec
+		want *apis.FieldError
+	}{{
+		name: "missing Subscriber and Reply",
+		c: &SubscriptionSpec{
+			Channel: getValidChannelRef(),
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("subscriber")
+			fe.Details = "the Subscription must reference a subscriber"
+			return fe
+		}(),
+	}, {
+		name: "empty Subscriber and Reply",
+		c: &SubscriptionSpec{
+			Channel:    getValidChannelRef(),
+			Subscriber: &duckv1.Destination{},
+			Reply:      &duckv1.Destination{},
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("subscriber")
+			fe.Details = "the Subscription must reference a subscriber"
+			return fe
+		}(),
+	}, {
+		name: "missing Subscriber",
+		c: &SubscriptionSpec{
+			Channel: getValidChannelRef(),
+			Reply:   getValidReply(),
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("subscriber")
+			fe.Details = "the Subscription must reference a subscriber"
+			return fe
+		}(),
+	}, {
+		name: "empty Subscriber",
+		c: &SubscriptionSpec{
+			Channel:    getValidChannelRef(),
+			Subscriber: &duckv1.Destination{},
+			Reply:      getValidReply(),
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("subscriber")
+			fe.Details = "the Subscription must reference a subscriber"
+			return fe
+		}(),
+	}, {
+		name: "missing name in channel, and missing subscriber, reply",
+		c: &SubscriptionSpec{
+			Channel: duckv1.KReference{
+				Kind:       channelKind,
+				APIVersion: channelAPIVersion,
+			},
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("subscriber")
+			fe.Details = "the Subscription must reference a subscriber"
+			return apis.ErrMissingField("channel.name").Also(fe)
+		}(),
+	}}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := feature.ToContext(context.TODO(), feature.Flags{
+				feature.StrictSubscriber: feature.Enabled,
+			})
+			got := test.c.Validate(ctx)
+			if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
+				t.Errorf("%s: strictSubscriber (-want, +got) = %v", test.name, diff)
+			}
+		})
+	}
+}

test/experimental/config/features.yaml

@@ -24,3 +24,4 @@ metadata:
 data:
   kreference-group: "enabled"
   delivery-timeout: "enabled"
+  strict-subscriber: "disabled"