From 22951b5406298cb81ce5f0df14d373388c1ea8fc Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 25 Mar 2021 11:38:07 +0100 Subject: [PATCH 1/8] Added DeliverySpec.Timeout Signed-off-by: Francesco Guardiani --- pkg/apis/duck/v1/delivery_types.go | 14 +++++++++++++ pkg/apis/duck/v1/delivery_types_test.go | 22 ++++++++++++++------ pkg/apis/duck/v1beta1/delivery_conversion.go | 2 ++ pkg/apis/duck/v1beta1/delivery_types.go | 14 +++++++++++++ pkg/apis/duck/v1beta1/delivery_types_test.go | 22 ++++++++++++++------ 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/pkg/apis/duck/v1/delivery_types.go b/pkg/apis/duck/v1/delivery_types.go index 0577bd584fe..8198e04929a 100644 --- a/pkg/apis/duck/v1/delivery_types.go +++ b/pkg/apis/duck/v1/delivery_types.go @@ -37,6 +37,13 @@ type DeliverySpec struct { // +optional Retry *int32 `json:"retry,omitempty"` + // Timeout is the timeout of each single request. + // More information on Duration format: + // - https://www.iso.org/iso-8601-date-and-time-format.html + // - https://en.wikipedia.org/wiki/ISO_8601 + // + Timeout *string `json:"timeout,omitempty"` + // BackoffPolicy is the retry backoff policy (linear, exponential). // +optional BackoffPolicy *BackoffPolicyType `json:"backoffPolicy,omitempty"` @@ -65,6 +72,13 @@ func (ds *DeliverySpec) Validate(ctx context.Context) *apis.FieldError { errs = errs.Also(apis.ErrInvalidValue(*ds.Retry, "retry")) } + if ds.Timeout != nil { + _, te := period.Parse(*ds.Timeout) + if te != nil { + errs = errs.Also(apis.ErrInvalidValue(*ds.Timeout, "timeout")) + } + } + if ds.BackoffPolicy != nil { switch *ds.BackoffPolicy { case BackoffPolicyExponential, BackoffPolicyLinear: diff --git a/pkg/apis/duck/v1/delivery_types_test.go b/pkg/apis/duck/v1/delivery_types_test.go index 5238c1033fd..4afc24e7092 100644 --- a/pkg/apis/duck/v1/delivery_types_test.go +++ b/pkg/apis/duck/v1/delivery_types_test.go @@ -28,8 +28,8 @@ import ( func TestDeliverySpecValidation(t *testing.T) { invalidString := "invalid time" bop := BackoffPolicyExponential - validBackoffDelay := "PT2S" - invalidBackoffDelay := "1985-04-12T23:20:50.52Z" + validDuration := "PT2S" + invalidDuration := "1985-04-12T23:20:50.52Z" tests := []struct { name string spec *DeliverySpec @@ -50,25 +50,35 @@ func TestDeliverySpecValidation(t *testing.T) { want: func() *apis.FieldError { return apis.ErrGeneric("expected at least one, got none", "ref", "uri").ViaField("deadLetterSink") }(), + }, { + name: "valid timeout", + spec: &DeliverySpec{Timeout: &validDuration}, + want: nil, + }, { + name: "invalid timeout", + spec: &DeliverySpec{Timeout: &invalidDuration}, + want: func() *apis.FieldError { + return apis.ErrInvalidValue(invalidDuration, "timeout") + }(), }, { name: "valid backoffPolicy", spec: &DeliverySpec{BackoffPolicy: &bop}, want: nil, }, { name: "valid backoffDelay", - spec: &DeliverySpec{BackoffDelay: &validBackoffDelay}, + spec: &DeliverySpec{BackoffDelay: &validDuration}, want: nil, }, { name: "invalid backoffDelay", - spec: &DeliverySpec{BackoffDelay: &invalidBackoffDelay}, + spec: &DeliverySpec{BackoffDelay: &invalidDuration}, want: func() *apis.FieldError { - return apis.ErrGeneric("invalid value: "+invalidBackoffDelay, "backoffDelay") + return apis.ErrInvalidValue(invalidDuration, "backoffDelay") }(), }, { name: "negative retry", spec: &DeliverySpec{Retry: pointer.Int32Ptr(-1)}, want: func() *apis.FieldError { - return apis.ErrGeneric("invalid value: -1", "retry") + return apis.ErrInvalidValue("-1", "retry") }(), }, { name: "valid retry 0", diff --git a/pkg/apis/duck/v1beta1/delivery_conversion.go b/pkg/apis/duck/v1beta1/delivery_conversion.go index d8a3bc880a2..d48ccde0c18 100644 --- a/pkg/apis/duck/v1beta1/delivery_conversion.go +++ b/pkg/apis/duck/v1beta1/delivery_conversion.go @@ -31,6 +31,7 @@ func (source *DeliverySpec) ConvertTo(ctx context.Context, to apis.Convertible) case *eventingduckv1.DeliverySpec: sink.Retry = source.Retry sink.BackoffDelay = source.BackoffDelay + sink.Timeout = source.Timeout if source.BackoffPolicy != nil { if *source.BackoffPolicy == BackoffPolicyLinear { linear := eventingduckv1.BackoffPolicyLinear @@ -55,6 +56,7 @@ func (sink *DeliverySpec) ConvertFrom(ctx context.Context, from apis.Convertible case *eventingduckv1.DeliverySpec: sink.Retry = source.Retry sink.BackoffDelay = source.BackoffDelay + sink.Timeout = source.Timeout if source.BackoffPolicy != nil { if *source.BackoffPolicy == eventingduckv1.BackoffPolicyLinear { linear := BackoffPolicyLinear diff --git a/pkg/apis/duck/v1beta1/delivery_types.go b/pkg/apis/duck/v1beta1/delivery_types.go index ac2001704ef..33ebd3ae2dc 100644 --- a/pkg/apis/duck/v1beta1/delivery_types.go +++ b/pkg/apis/duck/v1beta1/delivery_types.go @@ -38,6 +38,13 @@ type DeliverySpec struct { // +optional Retry *int32 `json:"retry,omitempty"` + // Timeout is the timeout of each single request. + // More information on Duration format: + // - https://www.iso.org/iso-8601-date-and-time-format.html + // - https://en.wikipedia.org/wiki/ISO_8601 + // + Timeout *string `json:"timeout,omitempty"` + // BackoffPolicy is the retry backoff policy (linear, exponential). // +optional BackoffPolicy *BackoffPolicyType `json:"backoffPolicy,omitempty"` @@ -66,6 +73,13 @@ func (ds *DeliverySpec) Validate(ctx context.Context) *apis.FieldError { errs = errs.Also(apis.ErrInvalidValue(*ds.Retry, "retry")) } + if ds.Timeout != nil { + _, te := period.Parse(*ds.Timeout) + if te != nil { + errs = errs.Also(apis.ErrInvalidValue(*ds.Timeout, "timeout")) + } + } + if ds.BackoffPolicy != nil { switch *ds.BackoffPolicy { case BackoffPolicyExponential, BackoffPolicyLinear: diff --git a/pkg/apis/duck/v1beta1/delivery_types_test.go b/pkg/apis/duck/v1beta1/delivery_types_test.go index ff5de5a494a..f1936554949 100644 --- a/pkg/apis/duck/v1beta1/delivery_types_test.go +++ b/pkg/apis/duck/v1beta1/delivery_types_test.go @@ -28,8 +28,8 @@ import ( func TestDeliverySpecValidation(t *testing.T) { invalidString := "invalid time" bop := BackoffPolicyExponential - validBackoffDelay := "PT2S" - invalidBackoffDelay := "1985-04-12T23:20:50.52Z" + validDuration := "PT2S" + invalidDuration := "1985-04-12T23:20:50.52Z" tests := []struct { name string spec *DeliverySpec @@ -50,25 +50,35 @@ func TestDeliverySpecValidation(t *testing.T) { want: func() *apis.FieldError { return apis.ErrGeneric("expected at least one, got none", "ref", "uri").ViaField("deadLetterSink") }(), + }, { + name: "valid timeout", + spec: &DeliverySpec{Timeout: &validDuration}, + want: nil, + }, { + name: "invalid timeout", + spec: &DeliverySpec{Timeout: &invalidDuration}, + want: func() *apis.FieldError { + return apis.ErrInvalidValue(invalidDuration, "timeout") + }(), }, { name: "valid backoffPolicy", spec: &DeliverySpec{BackoffPolicy: &bop}, want: nil, }, { name: "valid backoffDelay", - spec: &DeliverySpec{BackoffDelay: &validBackoffDelay}, + spec: &DeliverySpec{BackoffDelay: &validDuration}, want: nil, }, { name: "invalid backoffDelay", - spec: &DeliverySpec{BackoffDelay: &invalidBackoffDelay}, + spec: &DeliverySpec{BackoffDelay: &invalidDuration}, want: func() *apis.FieldError { - return apis.ErrGeneric("invalid value: "+invalidBackoffDelay, "backoffDelay") + return apis.ErrInvalidValue(invalidDuration, "backoffDelay") }(), }, { name: "negative retry", spec: &DeliverySpec{Retry: pointer.Int32Ptr(-1)}, want: func() *apis.FieldError { - return apis.ErrGeneric("invalid value: -1", "retry") + return apis.ErrInvalidValue("-1", "retry") }(), }, { name: "valid retry 0", From 803713e2d765773f7f4f927b1d2a7f6271740743 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 25 Mar 2021 11:51:09 +0100 Subject: [PATCH 2/8] codegen Signed-off-by: Francesco Guardiani --- pkg/apis/duck/v1/zz_generated.deepcopy.go | 5 +++++ pkg/apis/duck/v1beta1/zz_generated.deepcopy.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/pkg/apis/duck/v1/zz_generated.deepcopy.go b/pkg/apis/duck/v1/zz_generated.deepcopy.go index 902fb4b1f23..0501d0e6045 100644 --- a/pkg/apis/duck/v1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1/zz_generated.deepcopy.go @@ -146,6 +146,11 @@ func (in *DeliverySpec) DeepCopyInto(out *DeliverySpec) { *out = new(int32) **out = **in } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(string) + **out = **in + } if in.BackoffPolicy != nil { in, out := &in.BackoffPolicy, &out.BackoffPolicy *out = new(BackoffPolicyType) diff --git a/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go b/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go index c68ae747165..175fbad9895 100644 --- a/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go @@ -146,6 +146,11 @@ func (in *DeliverySpec) DeepCopyInto(out *DeliverySpec) { *out = new(int32) **out = **in } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(string) + **out = **in + } if in.BackoffPolicy != nil { in, out := &in.BackoffPolicy, &out.BackoffPolicy *out = new(BackoffPolicyType) From 5825f063754478c94884a245a81946e145eeb1ce Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 25 Mar 2021 11:51:24 +0100 Subject: [PATCH 3/8] Yamls Signed-off-by: Francesco Guardiani --- .../in-memory-channel/resources/in-memory-channel.yaml | 6 ++++++ config/core/resources/broker.yaml | 3 +++ config/core/resources/channel.yaml | 3 +++ config/core/resources/parallel.yaml | 3 +++ config/core/resources/sequence.yaml | 3 +++ config/core/resources/subscription.yaml | 3 +++ 6 files changed, 21 insertions(+) diff --git a/config/channels/in-memory-channel/resources/in-memory-channel.yaml b/config/channels/in-memory-channel/resources/in-memory-channel.yaml index 8c6854eebe9..caf68c6f112 100644 --- a/config/channels/in-memory-channel/resources/in-memory-channel.yaml +++ b/config/channels/in-memory-channel/resources/in-memory-channel.yaml @@ -47,6 +47,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -90,6 +93,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object diff --git a/config/core/resources/broker.yaml b/config/core/resources/broker.yaml index a4014de5700..8a2c5f1cb1e 100644 --- a/config/core/resources/broker.yaml +++ b/config/core/resources/broker.yaml @@ -63,6 +63,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object diff --git a/config/core/resources/channel.yaml b/config/core/resources/channel.yaml index 33f5b8b1ff7..23baa314c6a 100644 --- a/config/core/resources/channel.yaml +++ b/config/core/resources/channel.yaml @@ -75,6 +75,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object diff --git a/config/core/resources/parallel.yaml b/config/core/resources/parallel.yaml index 93f64c864e8..d216f3924bb 100644 --- a/config/core/resources/parallel.yaml +++ b/config/core/resources/parallel.yaml @@ -62,6 +62,9 @@ spec: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. diff --git a/config/core/resources/sequence.yaml b/config/core/resources/sequence.yaml index b8953d6bfcd..c1e55db4a23 100644 --- a/config/core/resources/sequence.yaml +++ b/config/core/resources/sequence.yaml @@ -88,6 +88,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object diff --git a/config/core/resources/subscription.yaml b/config/core/resources/subscription.yaml index ce90004a2b1..c026b673831 100644 --- a/config/core/resources/subscription.yaml +++ b/config/core/resources/subscription.yaml @@ -57,6 +57,9 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string + timeout: + description: Timeout is the timeout of each single request. + type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object From 7df4e6250901a1528b58b9f19e487f5cd3e30194 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 8 Jun 2021 14:33:19 +0200 Subject: [PATCH 4/8] Added the feature flag Fixed validation Fixed CRD Signed-off-by: Francesco Guardiani --- .../resources/in-memory-channel.yaml | 7 +------ config/core/configmaps/features.yaml | 3 +++ config/core/resources/broker.yaml | 4 +--- config/core/resources/channel.yaml | 4 +--- config/core/resources/parallel.yaml | 4 +--- config/core/resources/sequence.yaml | 4 +--- config/core/resources/subscription.yaml | 4 +--- pkg/apis/duck/v1/delivery_types.go | 13 ++++++++++--- pkg/apis/feature/flag_names.go | 1 + 9 files changed, 20 insertions(+), 24 deletions(-) diff --git a/config/channels/in-memory-channel/resources/in-memory-channel.yaml b/config/channels/in-memory-channel/resources/in-memory-channel.yaml index caf68c6f112..452c1a90d75 100644 --- a/config/channels/in-memory-channel/resources/in-memory-channel.yaml +++ b/config/channels/in-memory-channel/resources/in-memory-channel.yaml @@ -47,9 +47,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -77,6 +74,7 @@ spec: description: Retry is the minimum number of retries the sender should attempt when sending an event before moving it to the dead letter sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout subscribers: description: This is the list of subscriptions for this subscribable. type: array @@ -93,9 +91,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index 6856d9b3351..b7b78d1e0f9 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -25,3 +25,6 @@ data: # ALPHA feature: The kreference-group allows you to use the Group field in KReferences. # For more details: https://github.com/knative/eventing/issues/5086 kreference-group: "disabled" + + # ALPHA feature: The delivery-timeout allows you to use the Timeout field in DeliverySpec. + delivery-timeout: "disabled" diff --git a/config/core/resources/broker.yaml b/config/core/resources/broker.yaml index 8a2c5f1cb1e..ccbd8095e56 100644 --- a/config/core/resources/broker.yaml +++ b/config/core/resources/broker.yaml @@ -63,9 +63,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -93,6 +90,7 @@ spec: description: Retry is the minimum number of retries the sender should attempt when sending an event before moving it to the dead letter sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout status: description: Status represents the current state of the Broker. This data may be out of date. type: object diff --git a/config/core/resources/channel.yaml b/config/core/resources/channel.yaml index 23baa314c6a..25b2e18dd78 100644 --- a/config/core/resources/channel.yaml +++ b/config/core/resources/channel.yaml @@ -75,9 +75,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -105,6 +102,7 @@ spec: description: Retry is the minimum number of retries the sender should attempt when sending an event before moving it to the dead letter sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout subscribers: description: This is the list of subscriptions for this subscribable. type: array diff --git a/config/core/resources/parallel.yaml b/config/core/resources/parallel.yaml index d216f3924bb..74cc15aee5f 100644 --- a/config/core/resources/parallel.yaml +++ b/config/core/resources/parallel.yaml @@ -62,9 +62,6 @@ spec: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. @@ -107,6 +104,7 @@ spec: sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout filter: description: Filter is the expression guarding the branch type: object diff --git a/config/core/resources/sequence.yaml b/config/core/resources/sequence.yaml index c1e55db4a23..bf3fe3ba7a6 100644 --- a/config/core/resources/sequence.yaml +++ b/config/core/resources/sequence.yaml @@ -88,9 +88,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -118,6 +115,7 @@ spec: description: Retry is the minimum number of retries the sender should attempt when sending an event before moving it to the dead letter sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout ref: description: Ref points to an Addressable. type: object diff --git a/config/core/resources/subscription.yaml b/config/core/resources/subscription.yaml index c026b673831..53ac8931962 100644 --- a/config/core/resources/subscription.yaml +++ b/config/core/resources/subscription.yaml @@ -57,9 +57,6 @@ spec: backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). type: string - timeout: - description: Timeout is the timeout of each single request. - type: string deadLetterSink: description: DeadLetterSink is the sink receiving event that could not be sent to a destination. type: object @@ -87,6 +84,7 @@ spec: description: Retry is the minimum number of retries the sender should attempt when sending an event before moving it to the dead letter sink. type: integer format: int32 + x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout reply: description: Reply specifies (optionally) how to handle events returned from the Subscriber target. type: object diff --git a/pkg/apis/duck/v1/delivery_types.go b/pkg/apis/duck/v1/delivery_types.go index 8198e04929a..6857a73978c 100644 --- a/pkg/apis/duck/v1/delivery_types.go +++ b/pkg/apis/duck/v1/delivery_types.go @@ -20,6 +20,7 @@ import ( "context" "github.com/rickb777/date/period" + "knative.dev/eventing/pkg/apis/feature" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" ) @@ -42,6 +43,8 @@ type DeliverySpec struct { // - https://www.iso.org/iso-8601-date-and-time-format.html // - https://en.wikipedia.org/wiki/ISO_8601 // + // Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5148 + // +optional Timeout *string `json:"timeout,omitempty"` // BackoffPolicy is the retry backoff policy (linear, exponential). @@ -73,9 +76,13 @@ func (ds *DeliverySpec) Validate(ctx context.Context) *apis.FieldError { } if ds.Timeout != nil { - _, te := period.Parse(*ds.Timeout) - if te != nil { - errs = errs.Also(apis.ErrInvalidValue(*ds.Timeout, "timeout")) + if feature.FromContext(ctx).IsEnabled(feature.DeliveryTimeout) { + _, te := period.Parse(*ds.Timeout) + if te != nil { + errs = errs.Also(apis.ErrInvalidValue(*ds.Timeout, "timeout")) + } + } else { + errs = errs.Also(apis.ErrDisallowedFields("timeout")) } } diff --git a/pkg/apis/feature/flag_names.go b/pkg/apis/feature/flag_names.go index fdc49d7e144..05031685863 100644 --- a/pkg/apis/feature/flag_names.go +++ b/pkg/apis/feature/flag_names.go @@ -18,4 +18,5 @@ package feature const ( KReferenceGroup = "kreference-group" + DeliveryTimeout = "delivery-timeout" ) From 291b51c2da58cac52a0a32219ec58d2a10a024ad Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 8 Jun 2021 15:05:33 +0200 Subject: [PATCH 5/8] Fix validation UT Signed-off-by: Francesco Guardiani --- pkg/apis/duck/v1/delivery_types_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/apis/duck/v1/delivery_types_test.go b/pkg/apis/duck/v1/delivery_types_test.go index 4afc24e7092..cb4860ee88a 100644 --- a/pkg/apis/duck/v1/delivery_types_test.go +++ b/pkg/apis/duck/v1/delivery_types_test.go @@ -21,11 +21,16 @@ import ( "github.com/google/go-cmp/cmp" "k8s.io/utils/pointer" + "knative.dev/eventing/pkg/apis/feature" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" ) func TestDeliverySpecValidation(t *testing.T) { + deliveryTimeoutEnabledCtx := feature.ToContext(context.TODO(), feature.Flags{ + feature.DeliveryTimeout: feature.Enabled, + }) + invalidString := "invalid time" bop := BackoffPolicyExponential validDuration := "PT2S" @@ -33,6 +38,7 @@ func TestDeliverySpecValidation(t *testing.T) { tests := []struct { name string spec *DeliverySpec + ctx context.Context want *apis.FieldError }{{ name: "nil is valid", @@ -53,13 +59,19 @@ func TestDeliverySpecValidation(t *testing.T) { }, { name: "valid timeout", spec: &DeliverySpec{Timeout: &validDuration}, + ctx: deliveryTimeoutEnabledCtx, want: nil, }, { name: "invalid timeout", spec: &DeliverySpec{Timeout: &invalidDuration}, + ctx: deliveryTimeoutEnabledCtx, want: func() *apis.FieldError { return apis.ErrInvalidValue(invalidDuration, "timeout") }(), + }, { + name: "disabled timeout", + spec: &DeliverySpec{Timeout: &validDuration}, + want: apis.ErrDisallowedFields("timeout"), }, { name: "valid backoffPolicy", spec: &DeliverySpec{BackoffPolicy: &bop}, @@ -90,7 +102,11 @@ func TestDeliverySpecValidation(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := test.spec.Validate(context.TODO()) + ctx := test.ctx + if ctx == nil { + ctx = context.TODO() + } + got := test.spec.Validate(ctx) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { t.Error("DeliverySpec.Validate (-want, +got) =", diff) } From 2461bc71b4e0caf1e4e23a5f9215b11f5ab5ecd8 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 14 Jun 2021 15:30:53 +0200 Subject: [PATCH 6/8] Added comment Signed-off-by: Francesco Guardiani --- config/core/configmaps/features.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index b7b78d1e0f9..8a86755bfd0 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -27,4 +27,5 @@ data: kreference-group: "disabled" # ALPHA feature: The delivery-timeout allows you to use the Timeout field in DeliverySpec. + # For more details: https://github.com/knative/eventing/issues/5148 delivery-timeout: "disabled" From 94a9dfae74957df7747ee9865c2b14dc0379d50f Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 15 Jun 2021 09:38:35 +0200 Subject: [PATCH 7/8] Specify the value has to be greater than 0 Signed-off-by: Francesco Guardiani --- pkg/apis/duck/v1/delivery_types.go | 6 +++--- pkg/apis/duck/v1/delivery_types_test.go | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/apis/duck/v1/delivery_types.go b/pkg/apis/duck/v1/delivery_types.go index 6857a73978c..b82bbc4f9b7 100644 --- a/pkg/apis/duck/v1/delivery_types.go +++ b/pkg/apis/duck/v1/delivery_types.go @@ -38,7 +38,7 @@ type DeliverySpec struct { // +optional Retry *int32 `json:"retry,omitempty"` - // Timeout is the timeout of each single request. + // Timeout is the timeout of each single request. The value must be greater than 0. // More information on Duration format: // - https://www.iso.org/iso-8601-date-and-time-format.html // - https://en.wikipedia.org/wiki/ISO_8601 @@ -77,8 +77,8 @@ func (ds *DeliverySpec) Validate(ctx context.Context) *apis.FieldError { if ds.Timeout != nil { if feature.FromContext(ctx).IsEnabled(feature.DeliveryTimeout) { - _, te := period.Parse(*ds.Timeout) - if te != nil { + t, te := period.Parse(*ds.Timeout) + if te != nil || t.IsZero() { errs = errs.Also(apis.ErrInvalidValue(*ds.Timeout, "timeout")) } } else { diff --git a/pkg/apis/duck/v1/delivery_types_test.go b/pkg/apis/duck/v1/delivery_types_test.go index cb4860ee88a..ca82ed2b15b 100644 --- a/pkg/apis/duck/v1/delivery_types_test.go +++ b/pkg/apis/duck/v1/delivery_types_test.go @@ -68,6 +68,13 @@ func TestDeliverySpecValidation(t *testing.T) { want: func() *apis.FieldError { return apis.ErrInvalidValue(invalidDuration, "timeout") }(), + }, { + name: "zero timeout", + spec: &DeliverySpec{Timeout: pointer.StringPtr("PT0S")}, + ctx: deliveryTimeoutEnabledCtx, + want: func() *apis.FieldError { + return apis.ErrInvalidValue("PT0S", "timeout") + }(), }, { name: "disabled timeout", spec: &DeliverySpec{Timeout: &validDuration}, From fb56da6270d945e7bd91bb7581ccf51602873e4f Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Fri, 18 Jun 2021 09:25:42 +0200 Subject: [PATCH 8/8] Update docs Signed-off-by: Francesco Guardiani --- docs/eventing-api.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/eventing-api.md b/docs/eventing-api.md index b92a8afc7ee..3bbb760fad5 100644 --- a/docs/eventing-api.md +++ b/docs/eventing-api.md @@ -325,6 +325,22 @@ sending an event before moving it to the dead letter sink.

+timeout
+ +string + + + +(Optional) +

Timeout is the timeout of each single request. The value must be greater than 0. +More information on Duration format: +- https://www.iso.org/iso-8601-date-and-time-format.html +- https://en.wikipedia.org/wiki/ISO_8601

+

Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5148

+ + + + backoffPolicy
@@ -977,6 +993,20 @@ sending an event before moving it to the dead letter sink.

+timeout
+ +string + + + +

Timeout is the timeout of each single request. +More information on Duration format: +- https://www.iso.org/iso-8601-date-and-time-format.html +- https://en.wikipedia.org/wiki/ISO_8601

+ + + + backoffPolicy