diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 96a7cba0887..8ed3b25a14e 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -39,6 +39,10 @@ func (t *Trigger) Validate(ctx context.Context) *apis.FieldError { errs = t.validateAnnotation(errs, DependencyAnnotation, t.validateDependencyAnnotation) errs = t.validateAnnotation(errs, DeprecatedInjectionAnnotation, t.validateInjectionAnnotation) errs = t.validateAnnotation(errs, InjectionAnnotation, t.validateInjectionAnnotation) + if apis.IsInUpdate(ctx) { + original := apis.GetBaseline(ctx).(*Trigger) + errs = errs.Also(t.CheckImmutableFields(ctx, original)) + } return errs } diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index 524e810d239..29bb123b47b 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -233,7 +233,31 @@ func TestTriggerValidation(t *testing.T) { Paths: []string{injectionAnnotationPath}, Message: "The provided injection annotation value can only be \"enabled\" or \"disabled\", not \"wut\"", }, - }, { + }, + { + name: "invalid trigger spec, invalid dependency annotation(missing kind, name, apiVersion) and invalid injection", + t: &Trigger{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test-ns", + Annotations: map[string]string{ + DependencyAnnotation: "{}", + InjectionAnnotation: invalidInjectionAnnotation, + }}, + Spec: TriggerSpec{Broker: "default", Subscriber: validSubscriber}}, + want: func() *apis.FieldError { + var errs *apis.FieldError + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf(`The provided injection annotation value can only be "enabled", not "disabled"`), + Paths: []string{"metadata.annotations[knative-eventing-injection]"}, + }) + + errs = errs.Also(apis.ErrMissingField("metadata.annotations[knative.dev/dependency].apiVersion")) + errs = errs.Also(apis.ErrMissingField("metadata.annotations[knative.dev/dependency].kind")) + errs = errs.Also(apis.ErrMissingField("metadata.annotations[knative.dev/dependency].name")) + return errs + }(), + }, + { name: "valid injection annotation value, non-default broker specified", t: &Trigger{ ObjectMeta: v1.ObjectMeta{ @@ -400,12 +424,14 @@ func TestTriggerImmutableFields(t *testing.T) { name: "good (no change)", current: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", + Broker: "broker", + Subscriber: validSubscriber, }, }, original: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", + Broker: "broker", + Subscriber: validSubscriber, }, }, want: nil, @@ -413,7 +439,8 @@ func TestTriggerImmutableFields(t *testing.T) { name: "new nil is ok", current: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", + Broker: "broker", + Subscriber: validSubscriber, }, }, original: nil, @@ -422,13 +449,15 @@ func TestTriggerImmutableFields(t *testing.T) { name: "good (filter change)", current: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", + Broker: "broker", + Subscriber: validSubscriber, }, }, original: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", - Filter: validAttributesFilter, + Broker: "broker", + Filter: validAttributesFilter, + Subscriber: validSubscriber, }, }, want: nil, @@ -436,12 +465,14 @@ func TestTriggerImmutableFields(t *testing.T) { name: "bad (broker change)", current: &Trigger{ Spec: TriggerSpec{ - Broker: "broker", + Broker: "broker", + Subscriber: validSubscriber, }, }, original: &Trigger{ Spec: TriggerSpec{ - Broker: "original_broker", + Broker: "original_broker", + Subscriber: validSubscriber, }, }, want: &apis.FieldError{ @@ -456,7 +487,9 @@ func TestTriggerImmutableFields(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := test.current.CheckImmutableFields(context.TODO(), test.original) + ctx := context.Background() + ctx = apis.WithinUpdate(ctx, test.original) + got := test.current.Validate(ctx) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { t.Errorf("CheckImmutableFields (-want, +got) = %v", diff) }