Skip to content

Commit

Permalink
Make SubscriptionSpec.Delivery mutable (knative#6139)
Browse files Browse the repository at this point in the history
Just by creating a subscription in v0.24 and upgrading the cluster to
0.26, the webhook starts throwing errors like this:
```
admission webhook "validation.webhook.eventing.knative.dev" denied the request: validation failed: Immutable fields changed (-old +new): spec
{v1.SubscriptionSpec}.Delivery.DeadLetterSink.Ref.Namespace:
    -: ""
    +: "kc-broker-v1-dlq"
```

In addition, Subscription.Spec.Delivery wasn't meant to be immutable,
in fact, TriggerSpec.Spec.Delivery is mutable.

Signed-off-by: Pierangelo Di Pilato <[email protected]>
  • Loading branch information
pierDipi authored Feb 9, 2022
1 parent 20718e2 commit a7f83b1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/apis/messaging/v1/subscription_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (s *Subscription) CheckImmutableFields(ctx context.Context, original *Subsc
}

// Only Subscriber and Reply are mutable.
ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, "Subscriber", "Reply")
ignoreArguments := cmpopts.IgnoreFields(SubscriptionSpec{}, "Subscriber", "Reply", "Delivery")
if diff, err := kmp.ShortDiff(original.Spec, s.Spec, ignoreArguments); err != nil {
return &apis.FieldError{
Message: "Failed to diff Subscription",
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/messaging/v1/subscription_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"k8s.io/utils/pointer"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"

Expand Down Expand Up @@ -528,6 +529,23 @@ func TestSubscriptionImmutable(t *testing.T) {
},
},
want: nil,
}, {
name: "valid, change delivery spec",
c: &Subscription{
Spec: SubscriptionSpec{
Channel: getValidChannelRef(),
Subscriber: getValidDestination(),
Delivery: &eventingduckv1.DeliverySpec{Retry: pointer.Int32(2)},
},
},
og: &Subscription{
Spec: SubscriptionSpec{
Channel: getValidChannelRef(),
Subscriber: getValidDestination(),
Delivery: &eventingduckv1.DeliverySpec{Retry: pointer.Int32(3)},
},
},
want: nil,
}, {
name: "Channel changed",
c: &Subscription{
Expand Down

0 comments on commit a7f83b1

Please sign in to comment.