diff --git a/pkg/apis/eventing/v1/broker_validation.go b/pkg/apis/eventing/v1/broker_validation.go index e7e0e8a3c46..404545216ad 100644 --- a/pkg/apis/eventing/v1/broker_validation.go +++ b/pkg/apis/eventing/v1/broker_validation.go @@ -29,7 +29,14 @@ const ( func (b *Broker) Validate(ctx context.Context) *apis.FieldError { withNS := apis.AllowDifferentNamespace(apis.WithinParent(ctx, b.ObjectMeta)) - return b.Spec.Validate(withNS).ViaField("spec") + + // Make sure a BrokerClassAnnotation exists + var errs *apis.FieldError + if bc, ok := b.GetAnnotations()[BrokerClassAnnotationKey]; !ok || bc == "" { + errs = errs.Also(apis.ErrMissingField(BrokerClassAnnotationKey)) + } + + return errs.Also(b.Spec.Validate(withNS).ViaField("spec")) } func (bs *BrokerSpec) Validate(ctx context.Context) *apis.FieldError { diff --git a/pkg/apis/eventing/v1/broker_validation_test.go b/pkg/apis/eventing/v1/broker_validation_test.go index 774d55fd27a..3f26a469b4f 100644 --- a/pkg/apis/eventing/v1/broker_validation_test.go +++ b/pkg/apis/eventing/v1/broker_validation_test.go @@ -46,11 +46,11 @@ func TestBrokerImmutableFields(t *testing.T) { "nil original": { wantErr: nil, }, - "no ChannelTemplateSpec mutation": { + "no BrokerClassAnnotation mutation": { og: current, wantErr: nil, }, - "ChannelTemplateSpec mutated": { + "BrokerClassAnnotation mutated": { og: original, wantErr: &apis.FieldError{ Message: "Immutable fields changed (-old +new)", @@ -80,11 +80,30 @@ func TestValidate(t *testing.T) { b Broker want *apis.FieldError }{{ - name: "valid empty", + name: "missing annotation", b: Broker{}, + want: apis.ErrMissingField("eventing.knative.dev/broker.class"), + }, { + name: "empty annotation", + b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": ""}, + }, + }, + want: apis.ErrMissingField("eventing.knative.dev/broker.class"), + }, { + name: "valid empty", + b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, + }, }, { name: "valid config", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -97,6 +116,9 @@ func TestValidate(t *testing.T) { }, { name: "valid config, no namespace", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Name: "name", @@ -108,6 +130,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing name", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -120,6 +145,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing apiVersion", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -132,6 +160,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing kind", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -144,6 +175,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid delivery, invalid delay string", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Delivery: &eventingduckv1.DeliverySpec{ BackoffDelay: &invalidString, @@ -151,12 +185,12 @@ func TestValidate(t *testing.T) { }, }, want: apis.ErrInvalidValue(invalidString, "spec.delivery.backoffDelay"), - }, {}} + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { got := test.b.Validate(context.Background()) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { - t.Errorf("BrokerSpec.Validate (-want, +got) = %v", diff) + t.Errorf("Broker.Validate (-want, +got) = %v", diff) } }) } diff --git a/pkg/apis/eventing/v1beta1/broker_validation.go b/pkg/apis/eventing/v1beta1/broker_validation.go index a3cac84d95f..7f408c48829 100644 --- a/pkg/apis/eventing/v1beta1/broker_validation.go +++ b/pkg/apis/eventing/v1beta1/broker_validation.go @@ -29,7 +29,14 @@ const ( func (b *Broker) Validate(ctx context.Context) *apis.FieldError { withNS := apis.AllowDifferentNamespace(apis.WithinParent(ctx, b.ObjectMeta)) - return b.Spec.Validate(withNS).ViaField("spec") + + // Make sure a BrokerClassAnnotation exists + var errs *apis.FieldError + if bc, ok := b.GetAnnotations()[BrokerClassAnnotationKey]; !ok || bc == "" { + errs = errs.Also(apis.ErrMissingField(BrokerClassAnnotationKey)) + } + + return errs.Also(b.Spec.Validate(withNS).ViaField("spec")) } func (bs *BrokerSpec) Validate(ctx context.Context) *apis.FieldError { diff --git a/pkg/apis/eventing/v1beta1/broker_validation_test.go b/pkg/apis/eventing/v1beta1/broker_validation_test.go index 4a73e3c89e9..4e0a473d578 100644 --- a/pkg/apis/eventing/v1beta1/broker_validation_test.go +++ b/pkg/apis/eventing/v1beta1/broker_validation_test.go @@ -46,11 +46,11 @@ func TestBrokerImmutableFields(t *testing.T) { "nil original": { wantErr: nil, }, - "no ChannelTemplateSpec mutation": { + "no BrokerClassAnnotation mutation": { og: current, wantErr: nil, }, - "ChannelTemplateSpec mutated": { + "BrokerClassAnnotation mutated": { og: original, wantErr: &apis.FieldError{ Message: "Immutable fields changed (-old +new)", @@ -80,11 +80,30 @@ func TestValidate(t *testing.T) { b Broker want *apis.FieldError }{{ - name: "valid empty", + name: "missing annotation", b: Broker{}, + want: apis.ErrMissingField("eventing.knative.dev/broker.class"), + }, { + name: "empty annotation", + b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": ""}, + }, + }, + want: apis.ErrMissingField("eventing.knative.dev/broker.class"), + }, { + name: "valid empty", + b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, + }, }, { name: "valid config", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -97,6 +116,9 @@ func TestValidate(t *testing.T) { }, { name: "valid config, no namespace", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Name: "name", @@ -108,6 +130,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing name", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -120,6 +145,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing apiVersion", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -132,6 +160,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid config, missing kind", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Config: &duckv1.KReference{ Namespace: "namespace", @@ -144,6 +175,9 @@ func TestValidate(t *testing.T) { }, { name: "invalid delivery, invalid delay string", b: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, + }, Spec: BrokerSpec{ Delivery: &eventingduckv1beta1.DeliverySpec{ BackoffDelay: &invalidString, @@ -151,12 +185,12 @@ func TestValidate(t *testing.T) { }, }, want: apis.ErrInvalidValue(invalidString, "spec.delivery.backoffDelay"), - }, {}} + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { got := test.b.Validate(context.Background()) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { - t.Errorf("BrokerSpec.Validate (-want, +got) = %v", diff) + t.Errorf("Broker.Validate (-want, +got) = %v", diff) } }) } diff --git a/pkg/reconciler/mtnamespace/resources/broker.go b/pkg/reconciler/mtnamespace/resources/broker.go index 2c0c5fedd88..3b6273ac798 100644 --- a/pkg/reconciler/mtnamespace/resources/broker.go +++ b/pkg/reconciler/mtnamespace/resources/broker.go @@ -31,6 +31,7 @@ const ( func MakeBroker(ns *corev1.Namespace) *v1beta1.Broker { return &v1beta1.Broker{ ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"}, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(ns.GetObjectMeta(), schema.GroupVersionKind{ Group: corev1.SchemeGroupVersion.Group,