diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index cceaf3d16a4..e0159504a96 100644 --- a/config/core/configmaps/features.yaml +++ b/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" diff --git a/pkg/apis/feature/flag_names.go b/pkg/apis/feature/flag_names.go index 1595c8c049d..1559d64f9d4 100644 --- a/pkg/apis/feature/flag_names.go +++ b/pkg/apis/feature/flag_names.go @@ -20,4 +20,5 @@ const ( KReferenceGroup = "kreference-group" DeliveryTimeout = "delivery-timeout" KReferenceMapping = "kreference-mapping" + StrictSubscriber = "strict-subscriber" ) diff --git a/pkg/apis/messaging/v1/subscription_validation.go b/pkg/apis/messaging/v1/subscription_validation.go index 93bcdf8c0ea..793a86bad92 100644 --- a/pkg/apis/messaging/v1/subscription_validation.go +++ b/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 { diff --git a/pkg/apis/messaging/v1/subscription_validation_test.go b/pkg/apis/messaging/v1/subscription_validation_test.go index e2871f71a93..c75ceb15494 100644 --- a/pkg/apis/messaging/v1/subscription_validation_test.go +++ b/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) + } + }) + } +} diff --git a/test/experimental/config/features.yaml b/test/experimental/config/features.yaml index d8b990c8cd7..76b436b94c6 100644 --- a/test/experimental/config/features.yaml +++ b/test/experimental/config/features.yaml @@ -24,3 +24,4 @@ metadata: data: kreference-group: "enabled" delivery-timeout: "enabled" + strict-subscriber: "disabled"