diff --git a/config/core/resources/trigger.yaml b/config/core/resources/trigger.yaml index b0a74b5d01a..0b460e730e1 100644 --- a/config/core/resources/trigger.yaml +++ b/config/core/resources/trigger.yaml @@ -58,6 +58,7 @@ spec: delivery: description: Delivery contains the delivery spec for this specific trigger. type: object + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout properties: backoffDelay: description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' diff --git a/go.mod b/go.mod index 72442874e6f..33c928ac3a4 100644 --- a/go.mod +++ b/go.mod @@ -43,7 +43,7 @@ require ( k8s.io/utils v0.0.0-20201110183641-67b214c5f920 knative.dev/hack v0.0.0-20210622141627-e28525d8d260 knative.dev/hack/schema v0.0.0-20210622141627-e28525d8d260 - knative.dev/pkg v0.0.0-20210803160015-21eb4c167cc5 + knative.dev/pkg v0.0.0-20210902173607-844a6bc45596 knative.dev/reconciler-test v0.0.0-20210803183715-b61cc77c06f6 sigs.k8s.io/yaml v1.2.0 ) diff --git a/go.sum b/go.sum index c5f1ed52191..8e9f080bb80 100644 --- a/go.sum +++ b/go.sum @@ -1088,8 +1088,9 @@ knative.dev/hack v0.0.0-20210622141627-e28525d8d260 h1:f2eMtOubAOc/Q7JlvFPDKXiPl knative.dev/hack v0.0.0-20210622141627-e28525d8d260/go.mod h1:PHt8x8yX5Z9pPquBEfIj0X66f8iWkWfR0S/sarACJrI= knative.dev/hack/schema v0.0.0-20210622141627-e28525d8d260 h1:YkMkZ7qdafyRHNIuKttYzEmM1ilKTGyEtPWeVLcLcDE= knative.dev/hack/schema v0.0.0-20210622141627-e28525d8d260/go.mod h1:ffjwmdcrH5vN3mPhO8RrF2KfNnbHeCE2C60A+2cv3U0= -knative.dev/pkg v0.0.0-20210803160015-21eb4c167cc5 h1:jpOTmAXg1oLS8u5HPBrFP1XsOSFCQIvlTRxP8TDGg2E= knative.dev/pkg v0.0.0-20210803160015-21eb4c167cc5/go.mod h1:RPk5txNA3apR9X40D4MpUOP9/VqOG8CrtABWfOwGVS4= +knative.dev/pkg v0.0.0-20210902173607-844a6bc45596 h1:LCSg0O51V8I7sfnhw+j9WLBol8f2lCV5HkPyxJT9zzA= +knative.dev/pkg v0.0.0-20210902173607-844a6bc45596/go.mod h1:RPk5txNA3apR9X40D4MpUOP9/VqOG8CrtABWfOwGVS4= knative.dev/reconciler-test v0.0.0-20210803183715-b61cc77c06f6 h1:jSz98FX9JfAMJX3qVeRF7RU7A3XLJJxBNz8GYU5z0bM= knative.dev/reconciler-test v0.0.0-20210803183715-b61cc77c06f6/go.mod h1:+Kovy+G5zXZNcuO/uB+zfo37vFKZzsLIlWezt/nKMz0= pgregory.net/rapid v0.3.3 h1:jCjBsY4ln4Atz78QoBWxUEvAHaFyNDQg9+WU62aCn1U= diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index 4fd7c636f75..cc31958e733 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/duck/v1/delivery_defaults.go b/pkg/apis/duck/v1/delivery_defaults.go new file mode 100644 index 00000000000..b06c792ef89 --- /dev/null +++ b/pkg/apis/duck/v1/delivery_defaults.go @@ -0,0 +1,28 @@ +/* +Copyright 2021 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import "context" + +func (ds *DeliverySpec) SetDefaults(ctx context.Context) { + if ds == nil { + return + } + if ds.DeadLetterSink != nil { + ds.DeadLetterSink.SetDefaults(ctx) + } +} diff --git a/pkg/apis/duck/v1/delivery_defaults_test.go b/pkg/apis/duck/v1/delivery_defaults_test.go new file mode 100644 index 00000000000..7f77f8fc3cc --- /dev/null +++ b/pkg/apis/duck/v1/delivery_defaults_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2021 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +func TestDeliverySpecSetDefaults(t *testing.T) { + t.Parallel() + + tt := []struct { + name string + given *DeliverySpec + want *DeliverySpec + ctx context.Context + }{ + { + name: "nil", + ctx: context.Background(), + }, + { + name: "deadLetterSink nil", + ctx: context.Background(), + given: &DeliverySpec{}, + want: &DeliverySpec{}, + }, + { + name: "deadLetterSink.ref nil", + ctx: context.Background(), + given: &DeliverySpec{DeadLetterSink: &duckv1.Destination{}}, + want: &DeliverySpec{DeadLetterSink: &duckv1.Destination{}}, + }, + { + name: "deadLetterSink.ref.namespace empty string", + ctx: apis.WithinParent(context.Background(), metav1.ObjectMeta{Name: "b", Namespace: "custom"}), + given: &DeliverySpec{DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Namespace: "", + Name: "svc", + APIVersion: "v1", + }, + }}, + want: &DeliverySpec{DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Namespace: "custom", + Name: "svc", + APIVersion: "v1", + }, + }}, + }, + } + + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + tc.given.SetDefaults(tc.ctx) + if diff := cmp.Diff(tc.want, tc.given); diff != "" { + t.Error("(-want, +got)", diff) + } + }) + } +} diff --git a/pkg/apis/duck/v1/zz_generated.deepcopy.go b/pkg/apis/duck/v1/zz_generated.deepcopy.go index 0501d0e6045..c96550411a8 100644 --- a/pkg/apis/duck/v1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go b/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go index 175fbad9895..7a0e6e05ea7 100644 --- a/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/eventing/v1/broker_defaults.go b/pkg/apis/eventing/v1/broker_defaults.go index 468964bb64e..e101713e79a 100644 --- a/pkg/apis/eventing/v1/broker_defaults.go +++ b/pkg/apis/eventing/v1/broker_defaults.go @@ -21,9 +21,10 @@ import ( eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + "knative.dev/pkg/apis" + "knative.dev/eventing/pkg/apis/config" "knative.dev/eventing/pkg/apis/eventing" - "knative.dev/pkg/apis" ) func (b *Broker) SetDefaults(ctx context.Context) { @@ -53,4 +54,5 @@ func (bs *BrokerSpec) SetDefaults(ctx context.Context) { if bs.Config != nil { bs.Config.SetDefaults(ctx) } + bs.Delivery.SetDefaults(ctx) } diff --git a/pkg/apis/eventing/v1/broker_defaults_test.go b/pkg/apis/eventing/v1/broker_defaults_test.go index 7ecbd9a2ca3..4f341a68a44 100644 --- a/pkg/apis/eventing/v1/broker_defaults_test.go +++ b/pkg/apis/eventing/v1/broker_defaults_test.go @@ -21,14 +21,16 @@ import ( "testing" "k8s.io/utils/pointer" + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + duckv1 "knative.dev/pkg/apis/duck/v1" + "knative.dev/eventing/pkg/apis/config" "knative.dev/eventing/pkg/apis/eventing" - duckv1 "knative.dev/pkg/apis/duck/v1" ) var ( @@ -404,6 +406,61 @@ func TestBrokerSetDefaults(t *testing.T) { }, }, }, + "missing deadLetterSink.ref.namespace, defaulted": { + initial: Broker{ + ObjectMeta: metav1.ObjectMeta{Name: "broker", Namespace: "custom"}, + Spec: BrokerSpec{ + Config: &duckv1.KReference{ + Kind: "ConfigMap", + Name: "natss-channel", + Namespace: "custom1", + APIVersion: "v1", + }, + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Name: "handle-error", + APIVersion: "serving.knative.dev/v1", + }, + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.StringPtr("linear")), + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + }, + expected: Broker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "broker", + Namespace: "custom", + Annotations: map[string]string{ + eventing.BrokerClassKey: "MTChannelBasedBroker", + }, + }, + Spec: BrokerSpec{ + Config: &duckv1.KReference{ + Kind: "ConfigMap", + Namespace: "custom1", + Name: "natss-channel", + APIVersion: "v1", + }, + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Namespace: "custom", + Name: "handle-error", + APIVersion: "serving.knative.dev/v1", + }, + }, + Retry: pointer.Int32Ptr(5), + BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.StringPtr("linear")), + BackoffDelay: pointer.StringPtr("5s"), + }, + }, + }, + }, } for n, tc := range testCases { t.Run(n, func(t *testing.T) { diff --git a/pkg/apis/eventing/v1/trigger_defaults.go b/pkg/apis/eventing/v1/trigger_defaults.go index 4c90811f355..1dde6bab9d3 100644 --- a/pkg/apis/eventing/v1/trigger_defaults.go +++ b/pkg/apis/eventing/v1/trigger_defaults.go @@ -39,6 +39,7 @@ func (ts *TriggerSpec) SetDefaults(ctx context.Context) { } // Default the Subscriber namespace ts.Subscriber.SetDefaults(ctx) + ts.Delivery.SetDefaults(ctx) } func setLabels(t *Trigger) { diff --git a/pkg/apis/eventing/v1/trigger_defaults_test.go b/pkg/apis/eventing/v1/trigger_defaults_test.go index 0d01bc10ea1..7ee2f51466a 100644 --- a/pkg/apis/eventing/v1/trigger_defaults_test.go +++ b/pkg/apis/eventing/v1/trigger_defaults_test.go @@ -25,6 +25,8 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" "github.com/google/go-cmp/cmp" + + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" ) var ( @@ -80,6 +82,50 @@ func TestTriggerDefaults(t *testing.T) { }, }}, }, + "deadLetterSink, ns defaulted": { + initial: Trigger{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "custom", + }, + Spec: TriggerSpec{ + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + }, + }, + }, + Broker: otherBroker, + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + Namespace: "custom1", + }, + }}}, + expected: Trigger{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "custom", + Labels: map[string]string{brokerLabel: otherBroker}, + }, + Spec: TriggerSpec{ + Broker: otherBroker, + Filter: emptyTriggerFilter, + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + Namespace: "custom", + }, + }, + }, + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + Namespace: "custom1", + }, + }, + }}, + }, "nil broker and nil filter": { initial: Trigger{}, expected: defaultTrigger, diff --git a/pkg/apis/eventing/v1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1/zz_generated.deepcopy.go index 7be79a4ee5c..3fe406e0625 100644 --- a/pkg/apis/eventing/v1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go index f50a3b144f0..e24968fcba3 100644 --- a/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/flows/v1/zz_generated.deepcopy.go b/pkg/apis/flows/v1/zz_generated.deepcopy.go index 2e550d0612f..ecc46585927 100644 --- a/pkg/apis/flows/v1/zz_generated.deepcopy.go +++ b/pkg/apis/flows/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/messaging/config/zz_generated.deepcopy.go b/pkg/apis/messaging/config/zz_generated.deepcopy.go index 50ae7673f56..4f9f9c0992f 100644 --- a/pkg/apis/messaging/config/zz_generated.deepcopy.go +++ b/pkg/apis/messaging/config/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/messaging/v1/channel_defaults.go b/pkg/apis/messaging/v1/channel_defaults.go index 82746cdd4fe..682e4d171f6 100644 --- a/pkg/apis/messaging/v1/channel_defaults.go +++ b/pkg/apis/messaging/v1/channel_defaults.go @@ -19,9 +19,10 @@ package v1 import ( "context" + "knative.dev/pkg/apis" + "knative.dev/eventing/pkg/apis/messaging" "knative.dev/eventing/pkg/apis/messaging/config" - "knative.dev/pkg/apis" ) func (c *Channel) SetDefaults(ctx context.Context) { @@ -48,6 +49,7 @@ func (cs *ChannelSpec) SetDefaults(ctx context.Context) { c.Spec, } } + cs.Delivery.SetDefaults(ctx) } // ChannelDefaulter sets the default Channel CRD and Arguments on Channels that do not diff --git a/pkg/apis/messaging/v1/channel_validation.go b/pkg/apis/messaging/v1/channel_validation.go index 8220d6515aa..5f0a10b4c07 100644 --- a/pkg/apis/messaging/v1/channel_validation.go +++ b/pkg/apis/messaging/v1/channel_validation.go @@ -49,6 +49,13 @@ func (cs *ChannelSpec) Validate(ctx context.Context) *apis.FieldError { if len(cs.SubscribableSpec.Subscribers) > 0 { errs = errs.Also(apis.ErrDisallowedFields("subscribers").ViaField("subscribable")) } + + if cs.Delivery != nil { + if fe := cs.Delivery.Validate(ctx); fe != nil { + errs = errs.Also(fe.ViaField("delivery")) + } + } + return errs } diff --git a/pkg/apis/messaging/v1/channel_validation_test.go b/pkg/apis/messaging/v1/channel_validation_test.go index 6b78514fefe..35dbe93af44 100644 --- a/pkg/apis/messaging/v1/channel_validation_test.go +++ b/pkg/apis/messaging/v1/channel_validation_test.go @@ -23,9 +23,9 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "knative.dev/pkg/apis" eventingduck "knative.dev/eventing/pkg/apis/duck/v1" - "knative.dev/pkg/apis" ) func TestChannelValidation(t *testing.T) { @@ -95,6 +95,51 @@ func TestChannelValidation(t *testing.T) { errs = errs.Also(apis.ErrDisallowedFields("spec.subscribable.subscribers")) return errs }(), + }, { + name: "invalid Delivery", + cr: &Channel{ + Spec: ChannelSpec{ + ChannelTemplate: &ChannelTemplateSpec{ + TypeMeta: v1.TypeMeta{ + Kind: "Channel", + APIVersion: SchemeGroupVersion.String(), + }, + }, + ChannelableSpec: eventingduck.ChannelableSpec{ + Delivery: getDelivery(backoffDelayInvalid), + }, + }, + }, + want: apis.ErrInvalidValue(backoffDelayInvalid, "spec.delivery.backoffDelay"), + }, { + name: "valid Delivery", + cr: &Channel{ + Spec: ChannelSpec{ + ChannelTemplate: &ChannelTemplateSpec{ + TypeMeta: v1.TypeMeta{ + Kind: "Channel", + APIVersion: SchemeGroupVersion.String(), + }, + }, + ChannelableSpec: eventingduck.ChannelableSpec{ + Delivery: getDelivery(backoffDelayValid), + }, + }, + }, + want: nil, + }, { + name: "valid minimal spec", + cr: &Channel{ + Spec: ChannelSpec{ + ChannelTemplate: &ChannelTemplateSpec{ + TypeMeta: v1.TypeMeta{ + Kind: "Channel", + APIVersion: SchemeGroupVersion.String(), + }, + }, + }, + }, + want: nil, }} doValidateTest(t, tests) diff --git a/pkg/apis/messaging/v1/in_memory_channel_defaults.go b/pkg/apis/messaging/v1/in_memory_channel_defaults.go index a081d6bb678..a3f3bedff54 100644 --- a/pkg/apis/messaging/v1/in_memory_channel_defaults.go +++ b/pkg/apis/messaging/v1/in_memory_channel_defaults.go @@ -19,6 +19,8 @@ package v1 import ( "context" + "knative.dev/pkg/apis" + "knative.dev/eventing/pkg/apis/messaging" ) @@ -35,9 +37,10 @@ func (imc *InMemoryChannel) SetDefaults(ctx context.Context) { imc.Annotations[messaging.SubscribableDuckVersionAnnotation] = "v1" } + ctx = apis.WithinParent(ctx, imc.ObjectMeta) imc.Spec.SetDefaults(ctx) } func (imcs *InMemoryChannelSpec) SetDefaults(ctx context.Context) { - // TODO: Nothing to default here... + imcs.Delivery.SetDefaults(ctx) } diff --git a/pkg/apis/messaging/v1/in_memory_channel_defaults_test.go b/pkg/apis/messaging/v1/in_memory_channel_defaults_test.go index 326c9b091df..e0c948469ad 100644 --- a/pkg/apis/messaging/v1/in_memory_channel_defaults_test.go +++ b/pkg/apis/messaging/v1/in_memory_channel_defaults_test.go @@ -21,6 +21,9 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + duckv1 "knative.dev/pkg/apis/duck/v1" + + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" "knative.dev/eventing/pkg/apis/messaging/config" "github.com/google/go-cmp/cmp" @@ -44,6 +47,41 @@ func TestInMemoryChannelSetDefaults(t *testing.T) { initial: InMemoryChannel{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"somethingelse": "yup"}}}, expected: InMemoryChannel{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"messaging.knative.dev/subscribable": "v1", "somethingelse": "yup"}}}, }, + "deadLetterSink.ref.namespace gets defaulted": { + initial: InMemoryChannel{ + ObjectMeta: metav1.ObjectMeta{ + Name: "imc", + Namespace: "custom", + Annotations: map[string]string{"messaging.knative.dev/subscribable": "v1"}, + }, + Spec: InMemoryChannelSpec{eventingduckv1.ChannelableSpec{ + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + }, + }, + }, + }}, + }, + expected: InMemoryChannel{ + ObjectMeta: metav1.ObjectMeta{ + Name: "imc", + Namespace: "custom", + Annotations: map[string]string{"messaging.knative.dev/subscribable": "v1"}, + }, + Spec: InMemoryChannelSpec{eventingduckv1.ChannelableSpec{ + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Name: "foo", + Namespace: "custom", + }, + }, + }, + }}, + }, + }, } for n, tc := range testCases { t.Run(n, func(t *testing.T) { diff --git a/pkg/apis/messaging/v1/subscription_defaults.go b/pkg/apis/messaging/v1/subscription_defaults.go index 2beb73d00d1..0e894fc6379 100644 --- a/pkg/apis/messaging/v1/subscription_defaults.go +++ b/pkg/apis/messaging/v1/subscription_defaults.go @@ -18,10 +18,21 @@ package v1 import ( "context" + + "knative.dev/pkg/apis" ) func (s *Subscription) SetDefaults(ctx context.Context) { + if s == nil { + return + } + ctx = apis.WithinParent(ctx, s.ObjectMeta) s.Spec.SetDefaults(ctx) } -func (ss *SubscriptionSpec) SetDefaults(ctx context.Context) {} +func (ss *SubscriptionSpec) SetDefaults(ctx context.Context) { + if ss == nil { + return + } + ss.Delivery.SetDefaults(ctx) +} diff --git a/pkg/apis/messaging/v1/subscription_defaults_test.go b/pkg/apis/messaging/v1/subscription_defaults_test.go index 6e7d47d74ee..3dbe541fdfa 100644 --- a/pkg/apis/messaging/v1/subscription_defaults_test.go +++ b/pkg/apis/messaging/v1/subscription_defaults_test.go @@ -19,10 +19,100 @@ package v1 import ( "context" "testing" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + duckv1 "knative.dev/pkg/apis/duck/v1" + + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" ) -// No-op test because method does nothing. func TestSubscriptionDefaults(t *testing.T) { s := Subscription{} s.SetDefaults(context.TODO()) + + tt := []struct { + name string + given *Subscription + want *Subscription + }{ + { + name: "subscription empty", + }, + { + name: "subscription.spec nil", + given: &Subscription{}, + want: &Subscription{}, + }, + { + name: "subscription.spec empty", + given: &Subscription{ + Spec: SubscriptionSpec{}, + }, + want: &Subscription{ + Spec: SubscriptionSpec{}, + }, + }, + { + name: "subscription.spec.delivery empty", + given: &Subscription{ + Spec: SubscriptionSpec{ + Delivery: &eventingduckv1.DeliverySpec{}, + }, + }, + want: &Subscription{ + Spec: SubscriptionSpec{ + Delivery: &eventingduckv1.DeliverySpec{}, + }, + }, + }, + { + name: "subscription.spec.delivery.deadLetterSink.ref.namespace empty", + given: &Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "custom", + Name: "s", + }, + Spec: SubscriptionSpec{ + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Name: "svc", + APIVersion: "v1", + }, + }, + }, + }, + }, + want: &Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "custom", + Name: "s", + }, + Spec: SubscriptionSpec{ + Delivery: &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Service", + Namespace: "custom", + Name: "svc", + APIVersion: "v1", + }, + }, + }, + }, + }, + }, + } + + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + tc.given.SetDefaults(context.Background()) + if diff := cmp.Diff(tc.want, tc.given); diff != "" { + t.Error("(-want, +got)", diff) + } + }) + } } diff --git a/pkg/apis/messaging/v1/subscription_validation.go b/pkg/apis/messaging/v1/subscription_validation.go index 93bcdf8c0ea..03365452963 100644 --- a/pkg/apis/messaging/v1/subscription_validation.go +++ b/pkg/apis/messaging/v1/subscription_validation.go @@ -68,6 +68,12 @@ func (ss *SubscriptionSpec) Validate(ctx context.Context) *apis.FieldError { } } + if ss.Delivery != nil { + if fe := ss.Delivery.Validate(ctx); fe != nil { + errs = errs.Also(fe.ViaField("delivery")) + } + } + return errs } diff --git a/pkg/apis/messaging/v1/subscription_validation_test.go b/pkg/apis/messaging/v1/subscription_validation_test.go index e2871f71a93..27c974b770e 100644 --- a/pkg/apis/messaging/v1/subscription_validation_test.go +++ b/pkg/apis/messaging/v1/subscription_validation_test.go @@ -20,20 +20,26 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "knative.dev/eventing/pkg/apis/feature" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" + + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + "knative.dev/eventing/pkg/apis/feature" ) const ( - channelKind = "MyChannel" - channelAPIVersion = "messaging.knative.dev/v1" - routeKind = "Route" - routeAPIVersion = "serving.knative.dev/v1" - channelName = "subscribedChannel" - replyChannelName = "toChannel" - subscriberName = "subscriber" - namespace = "namespace" + channelKind = "MyChannel" + channelAPIVersion = "messaging.knative.dev/v1" + routeKind = "Route" + routeAPIVersion = "serving.knative.dev/v1" + channelName = "subscribedChannel" + replyChannelName = "toChannel" + subscriberName = "subscriber" + namespace = "namespace" + retryCount = int32(3) + backoffPolicy = eventingduckv1.BackoffPolicyExponential + backoffDelayValid = "PT0.5S" + backoffDelayInvalid = "invalid-delay" ) func getValidChannelRef() duckv1.KReference { @@ -66,6 +72,16 @@ func getValidDestination() *duckv1.Destination { } } +func getDelivery(delay string) *eventingduckv1.DeliverySpec { + retry := retryCount + policy := backoffPolicy + return &eventingduckv1.DeliverySpec{ + Retry: &retry, + BackoffPolicy: &policy, + BackoffDelay: &delay, + } +} + func TestSubscriptionValidation(t *testing.T) { name := "empty channel" c := &Subscription{ @@ -94,7 +110,7 @@ func TestSubscriptionSpecValidation(t *testing.T) { c *SubscriptionSpec want *apis.FieldError }{{ - name: "valid", + name: "valid minimal spec", c: &SubscriptionSpec{ Channel: getValidChannelRef(), Subscriber: getValidDestination(), @@ -108,6 +124,14 @@ func TestSubscriptionSpecValidation(t *testing.T) { Reply: getValidReply(), }, want: nil, + }, { + name: "valid with delivery", + c: &SubscriptionSpec{ + Channel: getValidChannelRef(), + Subscriber: getValidDestination(), + Delivery: getDelivery(backoffDelayValid), + }, + want: nil, }, { name: "empty Channel", c: &SubscriptionSpec{ @@ -153,13 +177,6 @@ func TestSubscriptionSpecValidation(t *testing.T) { fe.Details = "the Subscription must reference at least one of (reply or a subscriber)" return fe }(), - }, { - name: "missing Reply", - c: &SubscriptionSpec{ - Channel: getValidChannelRef(), - Subscriber: getValidDestination(), - }, - want: nil, }, { name: "empty Reply", c: &SubscriptionSpec{ @@ -218,7 +235,7 @@ func TestSubscriptionSpecValidation(t *testing.T) { }, want: apis.ErrMissingField("subscriber.ref.name"), }, { - name: "missing name in Subscriber.Ref", + name: "empty name in Subscriber.Ref", c: &SubscriptionSpec{ Channel: getValidChannelRef(), Subscriber: getValidDestination(), @@ -232,6 +249,14 @@ func TestSubscriptionSpecValidation(t *testing.T) { }, }, want: apis.ErrMissingField("reply.ref.name"), + }, { + name: "invalid Delivery", + c: &SubscriptionSpec{ + Channel: getValidChannelRef(), + Subscriber: getValidDestination(), + Delivery: getDelivery(backoffDelayInvalid), + }, + want: apis.ErrInvalidValue(backoffDelayInvalid, "delivery.backoffDelay"), }} for _, test := range tests { diff --git a/pkg/apis/messaging/v1/zz_generated.deepcopy.go b/pkg/apis/messaging/v1/zz_generated.deepcopy.go index 752f11d6e10..bc0dc043c99 100644 --- a/pkg/apis/messaging/v1/zz_generated.deepcopy.go +++ b/pkg/apis/messaging/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/sources/v1/zz_generated.deepcopy.go b/pkg/apis/sources/v1/zz_generated.deepcopy.go index 62c83377820..3d8878ca514 100644 --- a/pkg/apis/sources/v1/zz_generated.deepcopy.go +++ b/pkg/apis/sources/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go b/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go index 5326d83b373..86b4e093aa2 100644 --- a/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go +++ b/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/reconciler/broker/trigger/trigger_test.go b/pkg/reconciler/broker/trigger/trigger_test.go index 5d3cbbc15bd..a3e6b14bd4c 100644 --- a/pkg/reconciler/broker/trigger/trigger_test.go +++ b/pkg/reconciler/broker/trigger/trigger_test.go @@ -28,17 +28,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" clientgotesting "k8s.io/client-go/testing" - eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" - v1 "knative.dev/eventing/pkg/apis/duck/v1" - "knative.dev/eventing/pkg/apis/eventing" - eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" - messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1" - "knative.dev/eventing/pkg/apis/sources/v1beta2" - fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" - "knative.dev/eventing/pkg/client/injection/ducks/duck/v1/channelable" - "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/trigger" - "knative.dev/eventing/pkg/duck" - "knative.dev/eventing/pkg/reconciler/broker/resources" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" v1addr "knative.dev/pkg/client/injection/ducks/duck/v1/addressable" @@ -53,6 +42,18 @@ import ( "knative.dev/pkg/ptr" "knative.dev/pkg/resolver" + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + v1 "knative.dev/eventing/pkg/apis/duck/v1" + "knative.dev/eventing/pkg/apis/eventing" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" + messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1" + "knative.dev/eventing/pkg/apis/sources/v1beta2" + fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" + "knative.dev/eventing/pkg/client/injection/ducks/duck/v1/channelable" + "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/trigger" + "knative.dev/eventing/pkg/duck" + "knative.dev/eventing/pkg/reconciler/broker/resources" + _ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger/fake" . "knative.dev/eventing/pkg/reconciler/testing/v1" rtv1beta2 "knative.dev/eventing/pkg/reconciler/testing/v1beta2" @@ -306,6 +307,72 @@ func TestReconcile(t *testing.T) { WithTriggerStatusDeadLetterSinkURI("http://example.com"), WithTriggerDeadLetterSinkResolvedSucceeded()), }}, + }, { + Name: "Trigger with Broker DLQ", + Key: testKey, + Objects: []runtime.Object{ + NewBroker(brokerName, testNS, + WithBrokerClass(eventing.MTChannelBrokerClassValue), + WithBrokerConfig(config()), + WithInitBrokerConditions, + WithBrokerReady, + WithBrokerResourceVersion(""), + WithBrokerAddressURI(brokerAddress), + WithChannelAddressAnnotation(triggerChannelURL), + WithChannelAPIVersionAnnotation(triggerChannelAPIVersion), + WithChannelKindAnnotation(triggerChannelKind), + WithChannelNameAnnotation(triggerChannelName), + WithDeadLeaderSink(&duckv1.KReference{ + Kind: "Broker", + Name: brokerName, + APIVersion: "eventing.knative.dev/v1", + }, ""), + ), + createChannel(testNS, true), + imcConfigMap(), + NewEndpoints(filterServiceName, systemNS, + WithEndpointsLabels(FilterLabels()), + WithEndpointsAddresses(corev1.EndpointAddress{IP: "127.0.0.1"})), + NewEndpoints(ingressServiceName, systemNS, + WithEndpointsLabels(IngressLabels()), + WithEndpointsAddresses(corev1.EndpointAddress{IP: "127.0.0.1"})), + makeSubscriberAddressableAsUnstructured(testNS), + NewTrigger(triggerName, testNS, brokerName, + WithTriggerUID(triggerUID), + WithTriggerSubscriberRefAndURIReference(subscriberGVK, subscriberName, testNS, subscriberURIReference), + WithInitTriggerConditions, + ), + }, + WantErr: false, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewTrigger(triggerName, testNS, brokerName, + WithTriggerUID(triggerUID), + WithTriggerSubscriberRefAndURIReference(subscriberGVK, subscriberName, testNS, subscriberURIReference), + // The first reconciliation will initialize the status conditions. + WithInitTriggerConditions, + WithTriggerBrokerReady(), + WithTriggerSubscriptionNotConfigured(), + WithTriggerStatusSubscriberURI(subscriberResolvedTargetURI), + WithTriggerSubscriberResolvedSucceeded(), + WithTriggerDependencyReady(), + WithTriggerStatusDeadLetterSinkURI("http://broker-ingress.knative-testing.svc.cluster.local/test-namespace/test-broker"), + WithTriggerDeadLetterSinkResolvedSucceeded(), + ), + }}, + WantCreates: []runtime.Object{ + makeFilterSubscription(testNS, func(subscription *messagingv1.Subscription) { + subscription.Spec.Delivery = &eventingduckv1.DeliverySpec{ + DeadLetterSink: &duckv1.Destination{ + Ref: &duckv1.KReference{ + Kind: "Broker", + Name: brokerName, + Namespace: testNS, + APIVersion: "eventing.knative.dev/v1", + }, + }, + } + }), + }, }, { Name: "Subscription Create fails", Key: testKey, @@ -1054,8 +1121,12 @@ func makeServiceURI() *apis.URL { Path: fmt.Sprintf("/triggers/%s/%s/%s", testNS, triggerName, triggerUID), } } -func makeFilterSubscription(subscriberNamespace string) *messagingv1.Subscription { - return resources.NewSubscription(makeTrigger(subscriberNamespace), createTriggerChannelRef(), makeBrokerRef(), makeServiceURI(), makeEmptyDelivery()) +func makeFilterSubscription(subscriberNamespace string, opts ...func(subscription *messagingv1.Subscription)) *messagingv1.Subscription { + s := resources.NewSubscription(makeTrigger(subscriberNamespace), createTriggerChannelRef(), makeBrokerRef(), makeServiceURI(), makeEmptyDelivery()) + for _, opt := range opts { + opt(s) + } + return s } func makeTrigger(subscriberNamespace string) *eventingv1.Trigger { diff --git a/test/experimental/delivery_timeout_test.go b/test/experimental/delivery_timeout_test.go index 8a2d1c57187..74e66cd95e5 100644 --- a/test/experimental/delivery_timeout_test.go +++ b/test/experimental/delivery_timeout_test.go @@ -21,9 +21,14 @@ package experimental import ( "testing" + "knative.dev/eventing/pkg/apis/eventing" "knative.dev/eventing/test/experimental/features/delivery_timeout" + "knative.dev/eventing/test/rekt/features/broker" + b "knative.dev/eventing/test/rekt/resources/broker" + rt "knative.dev/eventing/test/rekt/resources/trigger" "knative.dev/pkg/system" "knative.dev/reconciler-test/pkg/environment" + "knative.dev/reconciler-test/pkg/feature" "knative.dev/reconciler-test/pkg/k8s" "knative.dev/reconciler-test/pkg/knative" ) @@ -41,3 +46,21 @@ func TestDeliveryTimeout(t *testing.T) { env.Test(ctx, t, delivery_timeout.ChannelToSink()) } + +func TestBrokerTriggerWithDeliveryTimeout(t *testing.T) { + class := eventing.MTChannelBrokerClassValue + + ctx, env := global.Environment( + knative.WithKnativeNamespace(system.Namespace()), + knative.WithLoggingConfig, + knative.WithTracingConfig, + k8s.WithEventListener, + environment.Managed(t), + ) + + brokerName := feature.MakeRandomK8sName("broker") + triggerName := feature.MakeRandomK8sName("trigger") + + env.Test(ctx, t, broker.GoesReady(brokerName, b.WithBrokerClass(class), b.WithTimeout("PT1S"))) + env.Test(ctx, t, broker.TriggerGoesReady(triggerName, brokerName, rt.WithTimeout("PT10S"))) +} diff --git a/test/rekt/features/broker/readyness.go b/test/rekt/features/broker/readyness.go index 9e5cb3a780e..e97c38adf33 100644 --- a/test/rekt/features/broker/readyness.go +++ b/test/rekt/features/broker/readyness.go @@ -29,15 +29,14 @@ import ( // TriggerGoesReady returns a feature that tests after the creation of a // Trigger, it becomes ready. This feature assumes the Broker already exists. -func TriggerGoesReady(name, brokerName string) *feature.Feature { - cfg := []manifest.CfgFn(nil) - +func TriggerGoesReady(name, brokerName string, cfg ...manifest.CfgFn) *feature.Feature { f := new(feature.Feature) // The test needs a subscriber. sub := feature.MakeRandomK8sName("sub") f.Setup("install a service", svc.Install(sub, "app", "rekt")) - cfg = append(cfg, trigger.WithSubscriber(svc.AsKReference(sub), "")) + // Append user-provided cfg to the end, in case they are providing their own subscriber. + cfg = append([]manifest.CfgFn{trigger.WithSubscriber(svc.AsKReference(sub), "")}, cfg...) // Install the trigger f.Setup(fmt.Sprintf("install trigger %q", name), trigger.Install(name, brokerName, cfg...)) diff --git a/test/rekt/resources/broker/broker.go b/test/rekt/resources/broker/broker.go index da631b61e90..6d84ae24578 100644 --- a/test/rekt/resources/broker/broker.go +++ b/test/rekt/resources/broker/broker.go @@ -93,6 +93,9 @@ var WithDeadLetterSink = delivery.WithDeadLetterSink // WithRetry adds the retry related config to a Broker spec. var WithRetry = delivery.WithRetry +// WithTimeout adds the timeout related config to the config. +var WithTimeout = delivery.WithTimeout + // Install will create a Broker resource, augmented with the config fn options. func Install(name string, opts ...manifest.CfgFn) feature.StepFn { cfg := map[string]interface{}{ diff --git a/test/rekt/resources/broker/broker.yaml b/test/rekt/resources/broker/broker.yaml index 91ec6d821f5..694cd44d376 100644 --- a/test/rekt/resources/broker/broker.yaml +++ b/test/rekt/resources/broker/broker.yaml @@ -31,6 +31,9 @@ spec: {{ end }} {{ if .delivery }} delivery: + {{ if .delivery.timeout }} + timeout: {{ .delivery.timeout }} + {{ end }} {{ if .delivery.deadLetterSink }} deadLetterSink: {{ if .delivery.deadLetterSink.ref }} diff --git a/test/rekt/resources/delivery/delivery.go b/test/rekt/resources/delivery/delivery.go index 7f38cecd7e1..05342a4132e 100644 --- a/test/rekt/resources/delivery/delivery.go +++ b/test/rekt/resources/delivery/delivery.go @@ -66,3 +66,15 @@ func WithRetry(count int32, backoffPolicy *eventingv1.BackoffPolicyType, backoff } } } + +// WithTimeout adds the timeout related config to the config. +func WithTimeout(timeout string) manifest.CfgFn { + return func(cfg map[string]interface{}) { + if _, set := cfg["delivery"]; !set { + cfg["delivery"] = map[string]interface{}{} + } + delivery := cfg["delivery"].(map[string]interface{}) + + delivery["timeout"] = timeout + } +} diff --git a/test/rekt/resources/trigger/trigger.go b/test/rekt/resources/trigger/trigger.go index 9b883dec5b7..d5bc9c2257a 100644 --- a/test/rekt/resources/trigger/trigger.go +++ b/test/rekt/resources/trigger/trigger.go @@ -94,6 +94,9 @@ var WithDeadLetterSink = delivery.WithDeadLetterSink // WithRetry adds the retry related config to a Trigger spec. var WithRetry = delivery.WithRetry +// WithTimeout adds the timeout related config to the config. +var WithTimeout = delivery.WithTimeout + // Install will create a Trigger resource, augmented with the config fn options. func Install(name, brokerName string, opts ...manifest.CfgFn) feature.StepFn { cfg := map[string]interface{}{ diff --git a/test/rekt/resources/trigger/trigger.yaml b/test/rekt/resources/trigger/trigger.yaml index e0fd13d1985..7974149405e 100644 --- a/test/rekt/resources/trigger/trigger.yaml +++ b/test/rekt/resources/trigger/trigger.yaml @@ -49,6 +49,9 @@ spec: {{ end }} {{ if .delivery }} delivery: + {{ if .delivery.timeout }} + timeout: {{ .delivery.timeout }} + {{ end }} {{ if .delivery.deadLetterSink }} deadLetterSink: {{ if .delivery.deadLetterSink.ref }} diff --git a/vendor/knative.dev/pkg/webhook/configmaps/configmaps.go b/vendor/knative.dev/pkg/webhook/configmaps/configmaps.go index 4d7ea5bfd35..5a4c4d88887 100644 --- a/vendor/knative.dev/pkg/webhook/configmaps/configmaps.go +++ b/vendor/knative.dev/pkg/webhook/configmaps/configmaps.go @@ -17,7 +17,6 @@ limitations under the License. package configmaps import ( - "bytes" "context" "encoding/json" "errors" @@ -192,8 +191,7 @@ func (ac *reconciler) validate(ctx context.Context, req *admissionv1.AdmissionRe var newObj corev1.ConfigMap if len(newBytes) != 0 { - newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes)) - if err := newDecoder.Decode(&newObj); err != nil { + if err := json.Unmarshal(newBytes, &newObj); err != nil { return fmt.Errorf("cannot decode incoming new object: %w", err) } } diff --git a/vendor/knative.dev/pkg/webhook/json/decode.go b/vendor/knative.dev/pkg/webhook/json/decode.go new file mode 100644 index 00000000000..9e206a10ba3 --- /dev/null +++ b/vendor/knative.dev/pkg/webhook/json/decode.go @@ -0,0 +1,131 @@ +/* +Copyright 2021 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package json + +import ( + "bytes" + "encoding/json" + "io" +) + +var ( + emptyMeta = []byte(`:{}`) + metaPrefix = []byte(`{"metadata"`) + metaSuffix = []byte(`}`) +) + +var ( + // Unmarshal is an alias for json.Unmarshal + Unmarshal = json.Unmarshal + + //Marshal is an alias for json.Marshal + Marshal = json.Marshal +) + +// Decode will parse the json byte array to the target object. When +// unknown fields are _not_ allowed we still accept unknown +// fields in the Object's metadata +// +// See https://github.com/knative/serving/issues/11448 for details +func Decode(bites []byte, target interface{}, disallowUnknownFields bool) error { + if !disallowUnknownFields { + return json.Unmarshal(bites, target) + } + + // If we don't allow unknown fields we skip validating fields in the metadata + // block since that is opaque to us and validated by the API server + start, end, err := findMetadataOffsets(bites) + if err != nil { + return err + } else if start == -1 || end == -1 { + // If for some reason the json does not have metadata continue with normal parsing + dec := json.NewDecoder(bytes.NewReader(bites)) + dec.DisallowUnknownFields() + return dec.Decode(target) + } + + before := bites[:start] + metadata := bites[start:end] + after := bites[end:] + + // Parse everything but skip metadata + dec := json.NewDecoder(io.MultiReader( + bytes.NewReader(before), + bytes.NewReader(emptyMeta), + bytes.NewReader(after), + )) + + dec.DisallowUnknownFields() + if err := dec.Decode(target); err != nil { + return err + } + + // Now we parse just the metadata + dec = json.NewDecoder(io.MultiReader( + bytes.NewReader(metaPrefix), + bytes.NewReader(metadata), + bytes.NewReader(metaSuffix), + )) + + if err := dec.Decode(target); err != nil { + return err + } + + return nil +} + +func findMetadataOffsets(bites []byte) (start, end int64, err error) { + start, end = -1, -1 + level := 0 + + var ( + dec = json.NewDecoder(bytes.NewReader(bites)) + t json.Token + ) + + for { + t, err = dec.Token() + if err == io.EOF { //nolint + break + } + if err != nil { + return + } + + switch v := t.(type) { + case json.Delim: + if v == '{' { + level++ + } else if v == '}' { + level-- + } + case string: + if v == "metadata" && level == 1 { + start = dec.InputOffset() + x := struct{}{} + if err = dec.Decode(&x); err != nil { + return -1, -1, err + } + end = dec.InputOffset() + + // we exit early to stop processing the rest of the object + return + } + } + } + return -1, -1, nil +} diff --git a/vendor/knative.dev/pkg/webhook/psbinding/psbinding.go b/vendor/knative.dev/pkg/webhook/psbinding/psbinding.go index ef16a9d83c2..9fa34d2c0c8 100644 --- a/vendor/knative.dev/pkg/webhook/psbinding/psbinding.go +++ b/vendor/knative.dev/pkg/webhook/psbinding/psbinding.go @@ -17,7 +17,6 @@ limitations under the License. package psbinding import ( - "bytes" "context" "encoding/json" "fmt" @@ -178,8 +177,7 @@ func (ac *Reconciler) Admit(ctx context.Context, request *admissionv1.AdmissionR } orig := &duckv1.WithPod{} - decoder := json.NewDecoder(bytes.NewBuffer(request.Object.Raw)) - if err := decoder.Decode(&orig); err != nil { + if err := json.Unmarshal(request.Object.Raw, orig); err != nil { return webhook.MakeErrorStatus("unable to decode object: %v", err) } diff --git a/vendor/knative.dev/pkg/webhook/resourcesemantics/defaulting/defaulting.go b/vendor/knative.dev/pkg/webhook/resourcesemantics/defaulting/defaulting.go index a5bcc4137d4..504dcaa0661 100644 --- a/vendor/knative.dev/pkg/webhook/resourcesemantics/defaulting/defaulting.go +++ b/vendor/knative.dev/pkg/webhook/resourcesemantics/defaulting/defaulting.go @@ -17,9 +17,7 @@ limitations under the License. package defaulting import ( - "bytes" "context" - "encoding/json" "errors" "fmt" "sort" @@ -47,6 +45,7 @@ import ( "knative.dev/pkg/system" "knative.dev/pkg/webhook" certresources "knative.dev/pkg/webhook/certificates/resources" + "knative.dev/pkg/webhook/json" "knative.dev/pkg/webhook/resourcesemantics" ) @@ -241,21 +240,15 @@ func (ac *reconciler) mutate(ctx context.Context, req *admissionv1.AdmissionRequ if len(newBytes) != 0 { newObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD) - newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes)) - if ac.disallowUnknownFields { - newDecoder.DisallowUnknownFields() - } - if err := newDecoder.Decode(&newObj); err != nil { + err := json.Decode(newBytes, newObj, ac.disallowUnknownFields) + if err != nil { return nil, fmt.Errorf("cannot decode incoming new object: %w", err) } } if len(oldBytes) != 0 { oldObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD) - oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes)) - if ac.disallowUnknownFields { - oldDecoder.DisallowUnknownFields() - } - if err := oldDecoder.Decode(&oldObj); err != nil { + err := json.Decode(oldBytes, oldObj, ac.disallowUnknownFields) + if err != nil { return nil, fmt.Errorf("cannot decode incoming old object: %w", err) } } diff --git a/vendor/knative.dev/pkg/webhook/resourcesemantics/validation/validation_admit.go b/vendor/knative.dev/pkg/webhook/resourcesemantics/validation/validation_admit.go index f2d7ad9bf5b..0544d88a6b1 100644 --- a/vendor/knative.dev/pkg/webhook/resourcesemantics/validation/validation_admit.go +++ b/vendor/knative.dev/pkg/webhook/resourcesemantics/validation/validation_admit.go @@ -17,9 +17,7 @@ limitations under the License. package validation import ( - "bytes" "context" - "encoding/json" "errors" "fmt" @@ -31,6 +29,7 @@ import ( kubeclient "knative.dev/pkg/client/injection/kube/client" "knative.dev/pkg/logging" "knative.dev/pkg/webhook" + "knative.dev/pkg/webhook/json" "knative.dev/pkg/webhook/resourcesemantics" ) @@ -110,11 +109,8 @@ func (ac *reconciler) decodeRequestAndPrepareContext( var newObj resourcesemantics.GenericCRD if len(newBytes) != 0 { newObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD) - newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes)) - if ac.disallowUnknownFields { - newDecoder.DisallowUnknownFields() - } - if err := newDecoder.Decode(&newObj); err != nil { + err := json.Decode(newBytes, newObj, ac.disallowUnknownFields) + if err != nil { return ctx, nil, fmt.Errorf("cannot decode incoming new object: %w", err) } } @@ -122,11 +118,8 @@ func (ac *reconciler) decodeRequestAndPrepareContext( var oldObj resourcesemantics.GenericCRD if len(oldBytes) != 0 { oldObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD) - oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes)) - if ac.disallowUnknownFields { - oldDecoder.DisallowUnknownFields() - } - if err := oldDecoder.Decode(&oldObj); err != nil { + err := json.Decode(oldBytes, oldObj, ac.disallowUnknownFields) + if err != nil { return ctx, nil, fmt.Errorf("cannot decode incoming old object: %w", err) } } @@ -201,8 +194,7 @@ func (ac *reconciler) callback(ctx context.Context, req *admissionv1.AdmissionRe if c, ok := ac.callbacks[gvk]; ok { if _, supported := c.supportedVerbs[req.Operation]; supported { unstruct := &unstructured.Unstructured{} - newDecoder := json.NewDecoder(bytes.NewBuffer(toDecode)) - if err := newDecoder.Decode(&unstruct); err != nil { + if err := json.Unmarshal(toDecode, unstruct); err != nil { return fmt.Errorf("cannot decode incoming new object: %w", err) } diff --git a/vendor/modules.txt b/vendor/modules.txt index f3c19ac5d2d..b6bc8d8bfaa 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -939,7 +939,7 @@ knative.dev/hack/schema/commands knative.dev/hack/schema/docs knative.dev/hack/schema/registry knative.dev/hack/schema/schema -# knative.dev/pkg v0.0.0-20210803160015-21eb4c167cc5 +# knative.dev/pkg v0.0.0-20210902173607-844a6bc45596 ## explicit knative.dev/pkg/apiextensions/storageversion knative.dev/pkg/apiextensions/storageversion/cmd/migrate @@ -1061,6 +1061,7 @@ knative.dev/pkg/webhook knative.dev/pkg/webhook/certificates knative.dev/pkg/webhook/certificates/resources knative.dev/pkg/webhook/configmaps +knative.dev/pkg/webhook/json knative.dev/pkg/webhook/psbinding knative.dev/pkg/webhook/resourcesemantics knative.dev/pkg/webhook/resourcesemantics/conversion