From 145a441a01f9f94e67b1f7090284ac45d123cc19 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 8 Sep 2021 22:46:49 +0200 Subject: [PATCH 1/8] Implement CE Subscriptions filters Signed-off-by: Ahmed Abdalla --- config/core/resources/trigger.yaml | 40 ++ docs/eventing-api.md | 196 +++++++++- pkg/apis/eventing/v1/trigger_types.go | 88 ++++- pkg/apis/eventing/v1/trigger_validation.go | 116 ++++-- .../eventing/v1/trigger_validation_test.go | 346 +++++++++++++++--- pkg/apis/eventing/v1/zz_generated.deepcopy.go | 78 ++++ 6 files changed, 772 insertions(+), 92 deletions(-) diff --git a/config/core/resources/trigger.yaml b/config/core/resources/trigger.yaml index 14e27549cd2..49e62b5c54e 100644 --- a/config/core/resources/trigger.yaml +++ b/config/core/resources/trigger.yaml @@ -103,6 +103,46 @@ spec: description: 'Attributes filters events by exact match on event context attributes. Each key in the map is compared with the equivalent key in the event context. An event passes the filter if all values are equal to the specified values. Nested context attributes are not supported as keys. Only string values are supported. ' type: object x-kubernetes-preserve-unknown-fields: true + filters: + description: 'Filters is the filter to apply against all events from the Broker. Only events that pass this filter will be sent to the Subscriber. If not specified, will default to allowing all events. ' + type: object + properties: + exact: + description: 'Exact evaluates to true if the value of the matching CloudEvents attribute is matches exactly the String value specified (case sensitive). Exact must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. The attribute name and value specified in the filter express cannot be be empty strings.' + type: object + x-kubernetes-preserve-unknown-fields: true + prefix: + description: 'Prefix evaluates to true if the value of the matching CloudEvents attribute starts with the String value specified (case sensitive). Prefix must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. The attribute name and value specified in the filter express cannot be be empty strings.' + type: object + x-kubernetes-preserve-unknown-fields: true + suffix: + description: 'Suffix evaluates to true if the value of the matching CloudEvents attribute ends with the String value specified (case sensitive). | Suffix must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. he attribute name and value specified in the filter express cannot be be empty strings.' + type: object + x-kubernetes-preserve-unknown-fields: true + not: + description: 'Not evaluates to true if the nested expression evaluates to false.' + type: object + # Because kube doesn't allow to use $ref, we can't recursively define this schema. + x-kubernetes-preserve-unknown-fields: true + all: + description: 'All evaluates to true if all the nested expressions evaluate to true. All must contain at least one filter expression.' + type: array + items: + # Because kube doesn't allow to use $ref, we can't recursively reference to the filter schema. + type: object + description: "Sub schema" + x-kubernetes-preserve-unknown-fields: true + any: + description: 'Any evaluates to true if at least one of the nested expressions evaluate to true. Any must contain at least one filter expression.' + type: array + items: + # Because kube doesn't allow to use $ref, we can't recursively reference to the filter schema. + type: object + description: "Sub schema" + x-kubernetes-preserve-unknown-fields: true + # This allows extension filter dialects + additionalProperties: true + x-kubernetes-preserve-unknown-fields: true subscriber: description: Subscriber is the addressable that receives events from the Broker that pass the Filter. It is required. type: object diff --git a/docs/eventing-api.md b/docs/eventing-api.md index 37cbe032b8e..50fdadd3615 100644 --- a/docs/eventing-api.md +++ b/docs/eventing-api.md @@ -1764,6 +1764,29 @@ filter will be sent to the Subscriber. If not specified, will default to allowin +filters
+ + +[]SubscriptionsAPIFilter + + + + +(Optional) +

Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions +API. It’s An array of filter expressions that evaluates to true or false. +If any filter expression in the array evaluates to false, the event MUST +NOT be sent to the Subscriber. If all the filter expressions in the array +evaluates to true, the event MUST be attempted to be delivered. Absence of +a filter or empty array implies a value of true. In the event of users +specifying both Filter and Filters, then the later will override the first. +This will allow our users to try out the effect of the new filters field +without compromising existing Filter objects and try it out on existing +Trigger objects.

+ + + + subscriber
@@ -1772,8 +1795,8 @@ knative.dev/pkg/apis/duck/v1.Destination -

Subscriber is the addressable that receives events from the Broker that pass the Filter. It -is required.

+

Subscriber is the addressable that receives events from the Broker that pass +the Filter. It is required.

@@ -1927,6 +1950,140 @@ resolved delivery options.

+

SubscriptionsAPIFilter +

+

+(Appears on:SubscriptionsAPIFilter, TriggerSpec) +

+

+

SubscriptionsAPIFilter allows defining a filter expression using CloudEvents +Subscriptions API. If multiple filters are specified, then the same semantics +of SubscriptionsAPIFilter.All is applied. If no filter dialect or empty +object is specified, then the filter always accept the events.

+

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+all
+ + +[]SubscriptionsAPIFilter + + +		 +(Optional) +

All evaluates to true if all the nested expressions evaluate to true. +It must contain at least one filter expression.

+
+any
+ + +[]SubscriptionsAPIFilter + + +		 +(Optional) +

Any evaluates to true if at least one of the nested expressions evaluates +to true. It must contain at least one filter expression.

+
+not
+ + +SubscriptionsAPIFilter + + +		 +(Optional) +

Not evaluates to true if the nested expression evaluates to false.

+
+exact
+ +map[string]string + +		 +(Optional) +

Exact evaluates to true if the value of the matching CloudEvents +attribute matches exactly the String value specified (case-sensitive). +Exact must contain exactly one property, where the key is the name of the +CloudEvents attribute to be matched, and its value is the String value to +use in the comparison. The attribute name and value specified in the filter +expression cannot be empty strings.

+
+prefix
+ +map[string]string + +		 +(Optional) +

Prefix evaluates to true if the value of the matching CloudEvents +attribute starts with the String value specified (case-sensitive). Prefix +must contain exactly one property, where the key is the name of the +CloudEvents attribute to be matched, and its value is the String value to +use in the comparison. The attribute name and value specified in the filter +expression cannot be empty strings.

+
+suffix
+ +map[string]string + +		 +(Optional) +

Suffix evaluates to true if the value of the matching CloudEvents +attribute ends with the String value specified (case-sensitive). Suffix +must contain exactly one property, where the key is the name of the +CloudEvents attribute to be matched, and its value is the String value to +use in the comparison. The attribute name and value specified in the filter +expression cannot be empty strings.

+
+Extensions
+ +map[string]*k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +

+(Members of Extensions are embedded into this type.) +

+(Optional) +

Extensions includes the list of additional filter dialects supported by +specific broker implementations. Check out the documentation of the +broker implementation you’re using to know about what additional filters +are supported.

+

TriggerFilter

@@ -1956,8 +2113,8 @@ TriggerFilterAttributes

Attributes filters events by exact match on event context attributes. Each key in the map is compared with the equivalent key in the event context. An event passes the filter if all values are equal to the -specified values.

-

Nested context attributes are not supported as keys. Only string values are supported.

+specified values. Nested context attributes are not supported as keys. Only +string values are supported.

@@ -1969,8 +2126,8 @@ specified values.

TriggerFilterAttributes is a map of context attribute names to values for -filtering by equality. Only exact matches will pass the filter. You can use the value “ -to indicate all strings match.

+filtering by equality. Only exact matches will pass the filter. You can use +the value “ to indicate all strings match.

TriggerSpec

@@ -2015,6 +2172,29 @@ filter will be sent to the Subscriber. If not specified, will default to allowin +filters
+ + +[]SubscriptionsAPIFilter + + + + +(Optional) +

Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions +API. It’s An array of filter expressions that evaluates to true or false. +If any filter expression in the array evaluates to false, the event MUST +NOT be sent to the Subscriber. If all the filter expressions in the array +evaluates to true, the event MUST be attempted to be delivered. Absence of +a filter or empty array implies a value of true. In the event of users +specifying both Filter and Filters, then the later will override the first. +This will allow our users to try out the effect of the new filters field +without compromising existing Filter objects and try it out on existing +Trigger objects.

+ + + + subscriber
@@ -2023,8 +2203,8 @@ knative.dev/pkg/apis/duck/v1.Destination -

Subscriber is the addressable that receives events from the Broker that pass the Filter. It -is required.

+

Subscriber is the addressable that receives events from the Broker that pass +the Filter. It is required.

diff --git a/pkg/apis/eventing/v1/trigger_types.go b/pkg/apis/eventing/v1/trigger_types.go index aa842be3f80..5c28bdb1a8c 100644 --- a/pkg/apis/eventing/v1/trigger_types.go +++ b/pkg/apis/eventing/v1/trigger_types.go @@ -83,8 +83,22 @@ type TriggerSpec struct { // +optional Filter *TriggerFilter `json:"filter,omitempty"` - // Subscriber is the addressable that receives events from the Broker that pass the Filter. It - // is required. + // Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions + // API. It's An array of filter expressions that evaluates to true or false. + // If any filter expression in the array evaluates to false, the event MUST + // NOT be sent to the Subscriber. If all the filter expressions in the array + // evaluates to true, the event MUST be attempted to be delivered. Absence of + // a filter or empty array implies a value of true. In the event of users + // specifying both Filter and Filters, then the later will override the first. + // This will allow our users to try out the effect of the new filters field + // without compromising existing Filter objects and try it out on existing + // Trigger objects. + // + // +optional + Filters []SubscriptionsAPIFilter `json:"filters,omitempty"` + + // Subscriber is the addressable that receives events from the Broker that pass + // the Filter. It is required. Subscriber duckv1.Destination `json:"subscriber"` // Delivery contains the delivery spec for this specific trigger. @@ -96,17 +110,77 @@ type TriggerFilter struct { // Attributes filters events by exact match on event context attributes. // Each key in the map is compared with the equivalent key in the event // context. An event passes the filter if all values are equal to the - // specified values. - // - // Nested context attributes are not supported as keys. Only string values are supported. + // specified values. Nested context attributes are not supported as keys. Only + // string values are supported. // // +optional Attributes TriggerFilterAttributes `json:"attributes,omitempty"` } +// SubscriptionsAPIFilter allows defining a filter expression using CloudEvents +// Subscriptions API. If multiple filters are specified, then the same semantics +// of SubscriptionsAPIFilter.All is applied. If no filter dialect or empty +// object is specified, then the filter always accept the events. +type SubscriptionsAPIFilter struct { + // All evaluates to true if all the nested expressions evaluate to true. + // It must contain at least one filter expression. + // + // +optional + All []SubscriptionsAPIFilter `json:"all,omitempty"` + + // Any evaluates to true if at least one of the nested expressions evaluates + // to true. It must contain at least one filter expression. + // + // +optional + Any []SubscriptionsAPIFilter `json:"any,omitempty"` + + // Not evaluates to true if the nested expression evaluates to false. + // + // +optional + Not *SubscriptionsAPIFilter `json:"not,omitempty"` + + // Exact evaluates to true if the value of the matching CloudEvents + // attribute matches exactly the String value specified (case-sensitive). + // Exact must contain exactly one property, where the key is the name of the + // CloudEvents attribute to be matched, and its value is the String value to + // use in the comparison. The attribute name and value specified in the filter + // expression cannot be empty strings. + // + // +optional + Exact map[string]string `json:"exact,omitempty"` + + // Prefix evaluates to true if the value of the matching CloudEvents + // attribute starts with the String value specified (case-sensitive). Prefix + // must contain exactly one property, where the key is the name of the + // CloudEvents attribute to be matched, and its value is the String value to + // use in the comparison. The attribute name and value specified in the filter + // expression cannot be empty strings. + // + // +optional + Prefix map[string]string `json:"prefix,omitempty"` + + // Suffix evaluates to true if the value of the matching CloudEvents + // attribute ends with the String value specified (case-sensitive). Suffix + // must contain exactly one property, where the key is the name of the + // CloudEvents attribute to be matched, and its value is the String value to + // use in the comparison. The attribute name and value specified in the filter + // expression cannot be empty strings. + // + // +optional + Suffix map[string]string `json:"suffix,omitempty"` + + // Extensions includes the list of additional filter dialects supported by + // specific broker implementations. Check out the documentation of the + // broker implementation you're using to know about what additional filters + // are supported. + // + // +optional + Extensions map[string]*runtime.RawExtension `json:",inline"` +} + // TriggerFilterAttributes is a map of context attribute names to values for -// filtering by equality. Only exact matches will pass the filter. You can use the value '' -// to indicate all strings match. +// filtering by equality. Only exact matches will pass the filter. You can use +// the value '' to indicate all strings match. type TriggerFilterAttributes map[string]string // TriggerStatus represents the current state of a Trigger. diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 4b367340a6b..28edad679b1 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -22,10 +22,9 @@ import ( "fmt" "regexp" + corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" "knative.dev/pkg/kmp" - - corev1 "k8s.io/api/core/v1" ) var ( @@ -35,7 +34,7 @@ var ( // Validate the Trigger. func (t *Trigger) Validate(ctx context.Context) *apis.FieldError { - errs := t.Spec.Validate(ctx).ViaField("spec") + errs := t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec") errs = t.validateAnnotation(errs, DependencyAnnotation, t.validateDependencyAnnotation) errs = t.validateAnnotation(errs, InjectionAnnotation, t.validateInjectionAnnotation) if apis.IsInUpdate(ctx) { @@ -46,36 +45,20 @@ func (t *Trigger) Validate(ctx context.Context) *apis.FieldError { } // Validate the TriggerSpec. -func (ts *TriggerSpec) Validate(ctx context.Context) *apis.FieldError { - var errs *apis.FieldError +func (ts *TriggerSpec) Validate(ctx context.Context) (errs *apis.FieldError) { if ts.Broker == "" { - fe := apis.ErrMissingField("broker") - errs = errs.Also(fe) - } - - if ts.Filter != nil { - for attr := range map[string]string(ts.Filter.Attributes) { - if !validAttributeName.MatchString(attr) { - fe := &apis.FieldError{ - Message: fmt.Sprintf("Invalid attribute name: %q", attr), - Paths: []string{"filter.attributes"}, - } - errs = errs.Also(fe) - } - } - } - - if fe := ts.Subscriber.Validate(ctx); fe != nil { - errs = errs.Also(fe.ViaField("subscriber")) - } - - if ts.Delivery != nil { - if de := ts.Delivery.Validate(ctx); de != nil { - errs = errs.Also(de.ViaField("delivery")) - } - } - - return errs + errs = errs.Also(apis.ErrMissingField("broker")) + } + + return errs.Also( + ValidateAttributeFilters(ts.Filter).ViaField("filter"), + ).Also( + ValidateSubscriptionAPIFiltersList(ts.Filters, "filters").ViaField("filters"), + ).Also( + ts.Subscriber.Validate(ctx).ViaField("subscriber"), + ).Also( + ts.Delivery.Validate(ctx).ViaField("delivery"), + ) } // CheckImmutableFields checks that any immutable fields were not changed. @@ -163,3 +146,72 @@ func (t *Trigger) validateInjectionAnnotation(injectionAnnotation string) *apis. } return nil } + +func ValidateAttributeFilters(filter *TriggerFilter) (errs *apis.FieldError) { + if filter == nil { + return nil + } + return errs.Also(ValidateAttributesNames(filter.Attributes).ViaField("attributes")) +} + +func ValidateAttributesNames(attrs map[string]string) (errs *apis.FieldError) { + if attrs == nil { + return nil + } + + for attr := range attrs { + if !validAttributeName.MatchString(attr) { + errs = errs.Also(apis.ErrInvalidKeyName(attr, apis.CurrentField, "Attribute name must start with a letter and can only contain lowercase alphanumeric").ViaKey(attr)) + } + } + return errs +} + +func ValidateSingleAttributeMap(expr map[string]string) (errs *apis.FieldError) { + if len(expr) == 0 { + return nil + } + + if len(expr) != 1 { + return apis.ErrGeneric("Multiple items found, can have only one key-value", apis.CurrentField) + } + for attr := range expr { + if !validAttributeName.MatchString(attr) { + errs = errs.Also(apis.ErrInvalidKeyName(attr, apis.CurrentField, "Attribute name must start with a letter and can only contain lowercase alphanumeric").ViaKey(attr)) + } + } + return errs +} + +func ValidateSubscriptionAPIFiltersList(filters []SubscriptionsAPIFilter, field string) (errs *apis.FieldError) { + if filters == nil { + return nil + } + if len(filters) == 0 { + return apis.ErrGeneric(fmt.Sprintf("%s must contain at least one nested filter", field), apis.CurrentField) + } + + for i, f := range filters { + f := f + errs = errs.Also(ValidateSubscriptionAPIFilter(&f)).ViaIndex(i) + } + return errs +} + +func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.FieldError) { + if filter == nil { + return nil + } + errs = errs.Also( + ValidateSingleAttributeMap(filter.Exact).ViaField("exact"), + ).Also( + ValidateSingleAttributeMap(filter.Prefix).ViaField("prefix"), + ).Also( + ValidateSingleAttributeMap(filter.Suffix).ViaField("suffix"), + ).Also( + ValidateSubscriptionAPIFiltersList(filter.All, "all").ViaField("all"), + ).Also( + ValidateSubscriptionAPIFiltersList(filter.Any, "any").ViaField("any"), + ).Also(ValidateSubscriptionAPIFilter(filter.Not).ViaField("not")) + return errs +} diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index 629164c479d..d0cf0cab419 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -23,17 +23,23 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + "k8s.io/apimachinery/pkg/runtime" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" + + eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" ) var ( - validEmptyFilter = &TriggerFilter{} - validAttributesFilter = &TriggerFilter{ - Attributes: TriggerFilterAttributes{ + validEmptyTriggerFilter = newTriggerFilter(nil) + validTriggerFilter = newTriggerFilter( + map[string]string{ "type": "other_type", "source": "other_source", + }) + validSubscriptionAPIFilter = &SubscriptionsAPIFilter{ + Exact: map[string]string{ + "type": "other_type", }, } validSubscriber = duckv1.Destination{ @@ -76,7 +82,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "default", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: nil, @@ -100,7 +106,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -118,7 +124,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -136,7 +142,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -153,7 +159,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -170,7 +176,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -187,7 +193,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -225,7 +231,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "default", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -266,7 +272,7 @@ func TestTriggerValidation(t *testing.T) { }}, Spec: TriggerSpec{ Broker: "test-broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -282,7 +288,7 @@ func TestTriggerValidation(t *testing.T) { }, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, Delivery: &eventingduckv1.DeliverySpec{ BackoffDelay: &invalidString, @@ -315,7 +321,7 @@ func TestTriggerUpdateValidation(t *testing.T) { }, Spec: TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, tNew: &Trigger{ @@ -324,7 +330,7 @@ func TestTriggerUpdateValidation(t *testing.T) { }, Spec: TriggerSpec{ Broker: "anotherBroker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }}, want: &apis.FieldError{ @@ -370,7 +376,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "missing broker", ts: &TriggerSpec{ Broker: "", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: validSubscriber, }, want: func() *apis.FieldError { @@ -380,10 +386,8 @@ func TestTriggerSpecValidation(t *testing.T) { }, { name: "missing attributes keys, match all", ts: &TriggerSpec{ - Broker: "test_broker", - Filter: &TriggerFilter{ - Attributes: TriggerFilterAttributes{}, - }, + Broker: "test_broker", + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }, want: &apis.FieldError{}, @@ -391,44 +395,40 @@ func TestTriggerSpecValidation(t *testing.T) { name: "invalid attribute name, start with number", ts: &TriggerSpec{ Broker: "test_broker", - Filter: &TriggerFilter{ - Attributes: TriggerFilterAttributes{ + Filter: newTriggerFilter( + map[string]string{ "0invalid": "my-value", - }, - }, + }), Subscriber: validSubscriber, }, - want: &apis.FieldError{ - Message: `Invalid attribute name: "0invalid"`, - Paths: []string{"filter.attributes"}, - }, + want: apis.ErrInvalidKeyName("0invalid", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("attributes", "0invalid").ViaField("filter"), }, { name: "invalid attribute name, capital letters", ts: &TriggerSpec{ Broker: "test_broker", - Filter: &TriggerFilter{ - Attributes: TriggerFilterAttributes{ + Filter: newTriggerFilter( + map[string]string{ "invALID": "my-value", - }, - }, + }), Subscriber: validSubscriber, }, - want: &apis.FieldError{ - Message: `Invalid attribute name: "invALID"`, - Paths: []string{"filter.attributes"}, - }, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("attributes", "invALID").ViaField("filter"), }, { name: "missing subscriber", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validAttributesFilter, + Filter: validTriggerFilter, }, want: apis.ErrGeneric("expected at least one, got none", "subscriber.ref", "subscriber.uri"), }, { name: "missing subscriber.ref.name", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: invalidSubscriber, }, want: apis.ErrMissingField("subscriber.ref.name"), @@ -436,7 +436,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "missing broker", ts: &TriggerSpec{ Broker: "", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: validSubscriber, }, want: apis.ErrMissingField("broker"), @@ -444,7 +444,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "valid empty filter", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, }, want: &apis.FieldError{}, @@ -452,7 +452,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "valid SourceAndType filter", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: validSubscriber, }, want: &apis.FieldError{}, @@ -460,7 +460,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "valid Attributes filter", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: validSubscriber, }, want: &apis.FieldError{}, @@ -468,7 +468,7 @@ func TestTriggerSpecValidation(t *testing.T) { name: "invalid delivery, invalid delay string", ts: &TriggerSpec{ Broker: "test_broker", - Filter: validEmptyFilter, + Filter: validEmptyTriggerFilter, Subscriber: validSubscriber, Delivery: &eventingduckv1.DeliverySpec{ BackoffDelay: &invalidString, @@ -487,6 +487,256 @@ func TestTriggerSpecValidation(t *testing.T) { } } +func TestFilterSpecValidation(t *testing.T) { + tests := []struct { + name string + filter *TriggerFilter + filters []SubscriptionsAPIFilter + want *apis.FieldError + }{{ + name: "missing filters, match all", + filters: nil, + want: &apis.FieldError{}, + }, { + name: "invalid exact filter attribute name, start with number", + filters: []SubscriptionsAPIFilter{ + { + Exact: map[string]string{ + "0invalid": "some-value", + }, + }}, + want: apis.ErrInvalidKeyName("0invalid", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("exact", "0invalid").ViaFieldIndex("filters", 0), + }, { + name: "invalid exact filter attribute name, capital letters", + filters: []SubscriptionsAPIFilter{ + { + Exact: map[string]string{ + "invALID": "some-value", + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("exact", "invALID").ViaFieldIndex("filters", 0), + }, { + name: "valid empty filter", + filter: validEmptyTriggerFilter, + want: &apis.FieldError{}, + }, { + name: "valid SourceAndType filter", + filter: validTriggerFilter, + want: &apis.FieldError{}, + }, { + name: "valid Attributes filter", + filter: validTriggerFilter, + want: &apis.FieldError{}, + }, { + name: "exact filter contains more than one attribute", + filters: []SubscriptionsAPIFilter{ + { + Exact: map[string]string{ + "myext": "abc", + "anotherext": "xyz", + }, + }}, + want: apis.ErrGeneric("Multiple items found, can have only one key-value", "exact").ViaFieldIndex("filters", 0), + }, { + name: "valid exact filter", + filters: []SubscriptionsAPIFilter{ + { + Exact: map[string]string{ + "valid": "abc", + }, + }}, + want: &apis.FieldError{}, + }, { + name: "suffix filter contains more than one attribute", + filters: []SubscriptionsAPIFilter{ + { + Suffix: map[string]string{ + "myext": "abc", + "anotherext": "xyz", + }, + }}, + want: apis.ErrGeneric("Multiple items found, can have only one key-value", "suffix").ViaFieldIndex("filters", 0), + }, { + name: "suffix filter contains invalid attribute name", + filters: []SubscriptionsAPIFilter{ + { + Suffix: map[string]string{ + "invALID": "abc", + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("suffix", "invALID").ViaFieldIndex("filters", 0), + }, { + name: "valid suffix filter", + filters: []SubscriptionsAPIFilter{ + { + Suffix: map[string]string{ + "valid": "abc", + }, + }}, + want: &apis.FieldError{}, + }, { + name: "prefix filter contains more than one attribute", + filters: []SubscriptionsAPIFilter{ + { + Prefix: map[string]string{ + "myext": "abc", + "anotherext": "xyz", + }, + }}, + want: apis.ErrGeneric("Multiple items found, can have only one key-value", "prefix").ViaFieldIndex("filters", 0), + }, { + name: "prefix filter contains invalid attribute name", + filters: []SubscriptionsAPIFilter{ + { + Prefix: map[string]string{ + "invALID": "abc", + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaFieldIndex("filters", 0), + }, { + name: "valid prefix filter", + filters: []SubscriptionsAPIFilter{ + { + Prefix: map[string]string{ + "valid": "abc", + }, + }}, + want: &apis.FieldError{}, + }, { + name: "not nested expression is valid", + filters: []SubscriptionsAPIFilter{ + { + Not: validSubscriptionAPIFilter, + }}, + want: &apis.FieldError{}, + }, { + name: "not nested expression is invalid", + filters: []SubscriptionsAPIFilter{ + { + Not: &SubscriptionsAPIFilter{ + Prefix: map[string]string{ + "invALID": "abc", + }, + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaField("not").ViaFieldIndex("filters", 0), + }, { + name: "all filter is empty", + filters: []SubscriptionsAPIFilter{ + { + All: []SubscriptionsAPIFilter{}, + }}, + want: apis.ErrGeneric("all must contain at least one nested filter", "all").ViaFieldIndex("filters", 0), + }, { + name: "all filter is nil", + filters: []SubscriptionsAPIFilter{ + { + All: nil, + }}, + want: &apis.FieldError{}, + }, { + name: "all filter is valid", + filters: []SubscriptionsAPIFilter{ + { + All: []SubscriptionsAPIFilter{ + *validSubscriptionAPIFilter, + { + Exact: map[string]string{"myattr": "myval"}, + }, + }, + }}, + want: &apis.FieldError{}, + }, { + name: "all filter sub expression is invalid", + filters: []SubscriptionsAPIFilter{ + { + All: []SubscriptionsAPIFilter{ + *validSubscriptionAPIFilter, + { + Exact: map[string]string{"myattr": "myval"}, + }, + { + Prefix: map[string]string{ + "invALID": "abc", + }, + }, + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaFieldIndex("all", 2).ViaFieldIndex("filters", 0), + }, { + name: "any filter is empty", + filters: []SubscriptionsAPIFilter{ + { + Any: []SubscriptionsAPIFilter{}, + }}, + want: apis.ErrGeneric("any must contain at least one nested filter", "any").ViaFieldIndex("filters", 0), + }, { + name: "any filter is valid", + filters: []SubscriptionsAPIFilter{ + { + Any: []SubscriptionsAPIFilter{ + *validSubscriptionAPIFilter, + { + Exact: map[string]string{"myattr": "myval"}, + }, + }, + }}, + want: &apis.FieldError{}, + }, { + name: "any filter sub expression is invalid", + filters: []SubscriptionsAPIFilter{ + { + Any: []SubscriptionsAPIFilter{ + *validSubscriptionAPIFilter, + { + Exact: map[string]string{"myattr": "myval"}, + }, + { + Prefix: map[string]string{"invALID": "abc"}, + }, + }, + }}, + want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, + "Attribute name must start with a letter and can only contain "+ + "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaFieldIndex("any", 2).ViaFieldIndex("filters", 0)}, { + name: "raw extension expression is valid", + filters: []SubscriptionsAPIFilter{ + { + Extensions: map[string]*runtime.RawExtension{ + "juel": {Raw: []byte("\"myExpressionUsingJUEL\"")}, + }, + }}, + want: &apis.FieldError{}, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ts := &TriggerSpec{ + Broker: "test_broker", + Filter: test.filter, + Filters: test.filters, + Subscriber: validSubscriber, + } + got := ts.Validate(context.TODO()) + if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { + t.Errorf("Validate TriggerSpec (-want, +got) =\n%s", diff) + } + }) + } +} + func TestTriggerImmutableFields(t *testing.T) { tests := []struct { name string @@ -529,7 +779,7 @@ func TestTriggerImmutableFields(t *testing.T) { original: &Trigger{ Spec: TriggerSpec{ Broker: "broker", - Filter: validAttributesFilter, + Filter: validTriggerFilter, Subscriber: validSubscriber, }, }, @@ -569,3 +819,9 @@ func TestTriggerImmutableFields(t *testing.T) { }) } } + +func newTriggerFilter(attrs map[string]string) *TriggerFilter { + return &TriggerFilter{ + Attributes: attrs, + } +} diff --git a/pkg/apis/eventing/v1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1/zz_generated.deepcopy.go index 80d8532638a..df852553761 100644 --- a/pkg/apis/eventing/v1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1/zz_generated.deepcopy.go @@ -134,6 +134,77 @@ func (in *BrokerStatus) DeepCopy() *BrokerStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SubscriptionsAPIFilter) DeepCopyInto(out *SubscriptionsAPIFilter) { + *out = *in + if in.All != nil { + in, out := &in.All, &out.All + *out = make([]SubscriptionsAPIFilter, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Any != nil { + in, out := &in.Any, &out.Any + *out = make([]SubscriptionsAPIFilter, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Not != nil { + in, out := &in.Not, &out.Not + *out = new(SubscriptionsAPIFilter) + (*in).DeepCopyInto(*out) + } + if in.Exact != nil { + in, out := &in.Exact, &out.Exact + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Prefix != nil { + in, out := &in.Prefix, &out.Prefix + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Suffix != nil { + in, out := &in.Suffix, &out.Suffix + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Extensions != nil { + in, out := &in.Extensions, &out.Extensions + *out = make(map[string]*runtime.RawExtension, len(*in)) + for key, val := range *in { + var outVal *runtime.RawExtension + if val == nil { + (*out)[key] = nil + } else { + in, out := &val, &outVal + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SubscriptionsAPIFilter. +func (in *SubscriptionsAPIFilter) DeepCopy() *SubscriptionsAPIFilter { + if in == nil { + return nil + } + out := new(SubscriptionsAPIFilter) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Trigger) DeepCopyInto(out *Trigger) { *out = *in @@ -248,6 +319,13 @@ func (in *TriggerSpec) DeepCopyInto(out *TriggerSpec) { *out = new(TriggerFilter) (*in).DeepCopyInto(*out) } + if in.Filters != nil { + in, out := &in.Filters, &out.Filters + *out = make([]SubscriptionsAPIFilter, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } in.Subscriber.DeepCopyInto(&out.Subscriber) if in.Delivery != nil { in, out := &in.Delivery, &out.Delivery From d4f67f2d2e9e572771848941920c508fa8f7fbf4 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 10 Nov 2021 19:46:38 +0100 Subject: [PATCH 2/8] add oneof validation to filters Signed-off-by: Ahmed Abdalla --- pkg/apis/eventing/v1/trigger_validation.go | 11 ++++ .../eventing/v1/trigger_validation_test.go | 12 ++++ pkg/utils/struct_validation.go | 55 +++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 pkg/utils/struct_validation.go diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 28edad679b1..b1767673033 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -22,6 +22,8 @@ import ( "fmt" "regexp" + "knative.dev/eventing/pkg/utils" + corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" "knative.dev/pkg/kmp" @@ -203,6 +205,8 @@ func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.F return nil } errs = errs.Also( + ValidateOneOf(filter), + ).Also( ValidateSingleAttributeMap(filter.Exact).ViaField("exact"), ).Also( ValidateSingleAttributeMap(filter.Prefix).ViaField("prefix"), @@ -215,3 +219,10 @@ func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.F ).Also(ValidateSubscriptionAPIFilter(filter.Not).ViaField("not")) return errs } + +func ValidateOneOf(filter *SubscriptionsAPIFilter) (err *apis.FieldError) { + if filter != nil && utils.HasMultipleSetFields(*filter) { + return apis.ErrGeneric("multiple dialects found, filters can have only one dialect set") + } + return nil +} diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index d0cf0cab419..77be5d3a4ab 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -527,6 +527,18 @@ func TestFilterSpecValidation(t *testing.T) { name: "valid SourceAndType filter", filter: validTriggerFilter, want: &apis.FieldError{}, + }, { + name: "invalid multiple dialects", + filters: []SubscriptionsAPIFilter{ + { + Exact: map[string]string{ + "myext": "abc", + }, + Suffix: map[string]string{ + "myext": "abc", + }, + }}, + want: apis.ErrGeneric("multiple dialects found, filters can have only one dialect set"), }, { name: "valid Attributes filter", filter: validTriggerFilter, diff --git a/pkg/utils/struct_validation.go b/pkg/utils/struct_validation.go new file mode 100644 index 00000000000..2f66f999cf5 --- /dev/null +++ b/pkg/utils/struct_validation.go @@ -0,0 +1,55 @@ +/* +Copyright 2020 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 utils + +import ( + "reflect" +) + +// HasMultipleSetFields checks if the struct has more than one field with a non-zero value +func HasMultipleSetFields(o interface{}) bool { + val := reflect.ValueOf(o) + fieldSet := false + for i := 0; i < val.NumField(); i++ { + f := val.Field(i) + if !IsEmptyField(f) { + if !fieldSet { + fieldSet = true + } else { + return true + } + } + } + return false +} + +// IsEmptyField checks if a struct field is empty by comparing it against its zero value +func IsEmptyField(v reflect.Value) bool { + switch v.Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return v.Int() == 0 + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return v.Uint() == 0 + case reflect.String: + return v.String() == "" + case reflect.Ptr, reflect.Slice, reflect.Map, reflect.Interface, reflect.Chan: + return v.IsNil() + case reflect.Bool: + return !v.Bool() + } + return false +} From 97f9ef1f6838beea3c5b602552c2305455f33d29 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 10 Nov 2021 20:31:20 +0100 Subject: [PATCH 3/8] Removing field schema Signed-off-by: Ahmed Abdalla --- config/core/resources/trigger.yaml | 41 +------------------ docs/eventing-api.md | 24 +++++------ pkg/apis/config/zz_generated.deepcopy.go | 1 - pkg/apis/duck/v1/zz_generated.deepcopy.go | 1 - .../duck/v1beta1/zz_generated.deepcopy.go | 1 - .../eventing}/struct_validation.go | 2 +- pkg/apis/eventing/v1/trigger_types.go | 12 +++--- pkg/apis/eventing/v1/trigger_validation.go | 19 +++------ .../eventing/v1/trigger_validation_test.go | 14 ------- pkg/apis/eventing/v1/zz_generated.deepcopy.go | 1 - .../eventing/v1beta1/zz_generated.deepcopy.go | 1 - pkg/apis/flows/v1/zz_generated.deepcopy.go | 1 - .../messaging/config/zz_generated.deepcopy.go | 1 - .../messaging/v1/zz_generated.deepcopy.go | 1 - pkg/apis/sources/v1/zz_generated.deepcopy.go | 1 - .../sources/v1beta2/zz_generated.deepcopy.go | 1 - 16 files changed, 26 insertions(+), 96 deletions(-) rename pkg/{utils => apis/eventing}/struct_validation.go (98%) diff --git a/config/core/resources/trigger.yaml b/config/core/resources/trigger.yaml index 49e62b5c54e..8c81d76716b 100644 --- a/config/core/resources/trigger.yaml +++ b/config/core/resources/trigger.yaml @@ -53,6 +53,7 @@ spec: spec: description: Spec defines the desired state of the Trigger. type: object + x-kubernetes-preserve-unknown-fields: true properties: broker: description: Broker is the broker that this trigger receives events from. @@ -103,46 +104,6 @@ spec: description: 'Attributes filters events by exact match on event context attributes. Each key in the map is compared with the equivalent key in the event context. An event passes the filter if all values are equal to the specified values. Nested context attributes are not supported as keys. Only string values are supported. ' type: object x-kubernetes-preserve-unknown-fields: true - filters: - description: 'Filters is the filter to apply against all events from the Broker. Only events that pass this filter will be sent to the Subscriber. If not specified, will default to allowing all events. ' - type: object - properties: - exact: - description: 'Exact evaluates to true if the value of the matching CloudEvents attribute is matches exactly the String value specified (case sensitive). Exact must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. The attribute name and value specified in the filter express cannot be be empty strings.' - type: object - x-kubernetes-preserve-unknown-fields: true - prefix: - description: 'Prefix evaluates to true if the value of the matching CloudEvents attribute starts with the String value specified (case sensitive). Prefix must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. The attribute name and value specified in the filter express cannot be be empty strings.' - type: object - x-kubernetes-preserve-unknown-fields: true - suffix: - description: 'Suffix evaluates to true if the value of the matching CloudEvents attribute ends with the String value specified (case sensitive). | Suffix must contain exactly one property, where the key is the name of the CloudEvents attribute to be matched, and its value is the String value to use in the comparison. he attribute name and value specified in the filter express cannot be be empty strings.' - type: object - x-kubernetes-preserve-unknown-fields: true - not: - description: 'Not evaluates to true if the nested expression evaluates to false.' - type: object - # Because kube doesn't allow to use $ref, we can't recursively define this schema. - x-kubernetes-preserve-unknown-fields: true - all: - description: 'All evaluates to true if all the nested expressions evaluate to true. All must contain at least one filter expression.' - type: array - items: - # Because kube doesn't allow to use $ref, we can't recursively reference to the filter schema. - type: object - description: "Sub schema" - x-kubernetes-preserve-unknown-fields: true - any: - description: 'Any evaluates to true if at least one of the nested expressions evaluate to true. Any must contain at least one filter expression.' - type: array - items: - # Because kube doesn't allow to use $ref, we can't recursively reference to the filter schema. - type: object - description: "Sub schema" - x-kubernetes-preserve-unknown-fields: true - # This allows extension filter dialects - additionalProperties: true - x-kubernetes-preserve-unknown-fields: true subscriber: description: Subscriber is the addressable that receives events from the Broker that pass the Filter. It is required. type: object diff --git a/docs/eventing-api.md b/docs/eventing-api.md index 50fdadd3615..d4e35db0acb 100644 --- a/docs/eventing-api.md +++ b/docs/eventing-api.md @@ -1773,15 +1773,15 @@ filter will be sent to the Subscriber. If not specified, will default to allowin (Optional) -

Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions -API. It’s An array of filter expressions that evaluates to true or false. +

Filters is an experimental field that conforms to the CNCF CloudEvents Subscriptions +API. It’s an array of filter expressions that evaluate to true or false. If any filter expression in the array evaluates to false, the event MUST NOT be sent to the Subscriber. If all the filter expressions in the array -evaluates to true, the event MUST be attempted to be delivered. Absence of +evaluate to true, the event MUST be attempted to be delivered. Absence of a filter or empty array implies a value of true. In the event of users -specifying both Filter and Filters, then the later will override the first. -This will allow our users to try out the effect of the new filters field -without compromising existing Filter objects and try it out on existing +specifying both Filter and Filters, then the latter will override the former. +This will allow users to try out the effect of the new Filters field +without compromising the existing attribute-based Filter and try it out on existing Trigger objects.

@@ -2181,15 +2181,15 @@ filter will be sent to the Subscriber. If not specified, will default to allowin (Optional) -

Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions -API. It’s An array of filter expressions that evaluates to true or false. +

Filters is an experimental field that conforms to the CNCF CloudEvents Subscriptions +API. It’s an array of filter expressions that evaluate to true or false. If any filter expression in the array evaluates to false, the event MUST NOT be sent to the Subscriber. If all the filter expressions in the array -evaluates to true, the event MUST be attempted to be delivered. Absence of +evaluate to true, the event MUST be attempted to be delivered. Absence of a filter or empty array implies a value of true. In the event of users -specifying both Filter and Filters, then the later will override the first. -This will allow our users to try out the effect of the new filters field -without compromising existing Filter objects and try it out on existing +specifying both Filter and Filters, then the latter will override the former. +This will allow users to try out the effect of the new Filters field +without compromising the existing attribute-based Filter and try it out on existing Trigger objects.

diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index cc31958e733..4fd7c636f75 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/duck/v1/zz_generated.deepcopy.go b/pkg/apis/duck/v1/zz_generated.deepcopy.go index c22e2bc3b9c..ffd6985aed6 100644 --- a/pkg/apis/duck/v1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 7a0e6e05ea7..175fbad9895 100644 --- a/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1beta1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/utils/struct_validation.go b/pkg/apis/eventing/struct_validation.go similarity index 98% rename from pkg/utils/struct_validation.go rename to pkg/apis/eventing/struct_validation.go index 2f66f999cf5..94c7e8ae5f5 100644 --- a/pkg/utils/struct_validation.go +++ b/pkg/apis/eventing/struct_validation.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package utils +package eventing import ( "reflect" diff --git a/pkg/apis/eventing/v1/trigger_types.go b/pkg/apis/eventing/v1/trigger_types.go index 5c28bdb1a8c..25dd0f0ac03 100644 --- a/pkg/apis/eventing/v1/trigger_types.go +++ b/pkg/apis/eventing/v1/trigger_types.go @@ -83,15 +83,15 @@ type TriggerSpec struct { // +optional Filter *TriggerFilter `json:"filter,omitempty"` - // Filters is an experimental field that conforms to CNCF CloudEvents Subscriptions - // API. It's An array of filter expressions that evaluates to true or false. + // Filters is an experimental field that conforms to the CNCF CloudEvents Subscriptions + // API. It's an array of filter expressions that evaluate to true or false. // If any filter expression in the array evaluates to false, the event MUST // NOT be sent to the Subscriber. If all the filter expressions in the array - // evaluates to true, the event MUST be attempted to be delivered. Absence of + // evaluate to true, the event MUST be attempted to be delivered. Absence of // a filter or empty array implies a value of true. In the event of users - // specifying both Filter and Filters, then the later will override the first. - // This will allow our users to try out the effect of the new filters field - // without compromising existing Filter objects and try it out on existing + // specifying both Filter and Filters, then the latter will override the former. + // This will allow users to try out the effect of the new Filters field + // without compromising the existing attribute-based Filter and try it out on existing // Trigger objects. // // +optional diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index b1767673033..4d5561f43c2 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -22,7 +22,7 @@ import ( "fmt" "regexp" - "knative.dev/eventing/pkg/utils" + "knative.dev/eventing/pkg/apis/eventing" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" @@ -55,7 +55,7 @@ func (ts *TriggerSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs.Also( ValidateAttributeFilters(ts.Filter).ViaField("filter"), ).Also( - ValidateSubscriptionAPIFiltersList(ts.Filters, "filters").ViaField("filters"), + ValidateSubscriptionAPIFiltersList(ts.Filters).ViaField("filters"), ).Also( ts.Subscriber.Validate(ctx).ViaField("subscriber"), ).Also( @@ -157,10 +157,6 @@ func ValidateAttributeFilters(filter *TriggerFilter) (errs *apis.FieldError) { } func ValidateAttributesNames(attrs map[string]string) (errs *apis.FieldError) { - if attrs == nil { - return nil - } - for attr := range attrs { if !validAttributeName.MatchString(attr) { errs = errs.Also(apis.ErrInvalidKeyName(attr, apis.CurrentField, "Attribute name must start with a letter and can only contain lowercase alphanumeric").ViaKey(attr)) @@ -185,13 +181,10 @@ func ValidateSingleAttributeMap(expr map[string]string) (errs *apis.FieldError) return errs } -func ValidateSubscriptionAPIFiltersList(filters []SubscriptionsAPIFilter, field string) (errs *apis.FieldError) { +func ValidateSubscriptionAPIFiltersList(filters []SubscriptionsAPIFilter) (errs *apis.FieldError) { if filters == nil { return nil } - if len(filters) == 0 { - return apis.ErrGeneric(fmt.Sprintf("%s must contain at least one nested filter", field), apis.CurrentField) - } for i, f := range filters { f := f @@ -213,15 +206,15 @@ func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.F ).Also( ValidateSingleAttributeMap(filter.Suffix).ViaField("suffix"), ).Also( - ValidateSubscriptionAPIFiltersList(filter.All, "all").ViaField("all"), + ValidateSubscriptionAPIFiltersList(filter.All).ViaField("all"), ).Also( - ValidateSubscriptionAPIFiltersList(filter.Any, "any").ViaField("any"), + ValidateSubscriptionAPIFiltersList(filter.Any).ViaField("any"), ).Also(ValidateSubscriptionAPIFilter(filter.Not).ViaField("not")) return errs } func ValidateOneOf(filter *SubscriptionsAPIFilter) (err *apis.FieldError) { - if filter != nil && utils.HasMultipleSetFields(*filter) { + if filter != nil && eventing.HasMultipleSetFields(*filter) { return apis.ErrGeneric("multiple dialects found, filters can have only one dialect set") } return nil diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index 77be5d3a4ab..70b5274944e 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -642,13 +642,6 @@ func TestFilterSpecValidation(t *testing.T) { want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, "Attribute name must start with a letter and can only contain "+ "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaField("not").ViaFieldIndex("filters", 0), - }, { - name: "all filter is empty", - filters: []SubscriptionsAPIFilter{ - { - All: []SubscriptionsAPIFilter{}, - }}, - want: apis.ErrGeneric("all must contain at least one nested filter", "all").ViaFieldIndex("filters", 0), }, { name: "all filter is nil", filters: []SubscriptionsAPIFilter{ @@ -687,13 +680,6 @@ func TestFilterSpecValidation(t *testing.T) { want: apis.ErrInvalidKeyName("invALID", apis.CurrentField, "Attribute name must start with a letter and can only contain "+ "lowercase alphanumeric").ViaFieldKey("prefix", "invALID").ViaFieldIndex("all", 2).ViaFieldIndex("filters", 0), - }, { - name: "any filter is empty", - filters: []SubscriptionsAPIFilter{ - { - Any: []SubscriptionsAPIFilter{}, - }}, - want: apis.ErrGeneric("any must contain at least one nested filter", "any").ViaFieldIndex("filters", 0), }, { name: "any filter is valid", filters: []SubscriptionsAPIFilter{ diff --git a/pkg/apis/eventing/v1/zz_generated.deepcopy.go b/pkg/apis/eventing/v1/zz_generated.deepcopy.go index df852553761..c2aee43a866 100644 --- a/pkg/apis/eventing/v1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 e24968fcba3..f50a3b144f0 100644 --- a/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 ecc46585927..2e550d0612f 100644 --- a/pkg/apis/flows/v1/zz_generated.deepcopy.go +++ b/pkg/apis/flows/v1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 4f9f9c0992f..50ae7673f56 100644 --- a/pkg/apis/messaging/config/zz_generated.deepcopy.go +++ b/pkg/apis/messaging/config/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/pkg/apis/messaging/v1/zz_generated.deepcopy.go b/pkg/apis/messaging/v1/zz_generated.deepcopy.go index 2876f8388a8..dd756192282 100644 --- a/pkg/apis/messaging/v1/zz_generated.deepcopy.go +++ b/pkg/apis/messaging/v1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 3d8878ca514..62c83377820 100644 --- a/pkg/apis/sources/v1/zz_generated.deepcopy.go +++ b/pkg/apis/sources/v1/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//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 86b4e093aa2..5326d83b373 100644 --- a/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go +++ b/pkg/apis/sources/v1beta2/zz_generated.deepcopy.go @@ -1,4 +1,3 @@ -//go:build !ignore_autogenerated // +build !ignore_autogenerated /* From 61fc963fa592c8c3a48a26cd2e9e47f1796eafe0 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 10 Nov 2021 23:13:33 +0100 Subject: [PATCH 4/8] Adding feature flag Signed-off-by: Ahmed Abdalla --- config/core/configmaps/features.yaml | 5 +++++ pkg/apis/eventing/v1/trigger_validation.go | 18 ++++++++++-------- .../eventing/v1/trigger_validation_test.go | 7 ++++++- pkg/apis/feature/flag_names.go | 1 + 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index e0159504a96..0b351176338 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -39,3 +39,8 @@ data: # ALPHA feature: The subscriber-strict flag force subscriptions to define a subscriber # For more details: https://github.com/knative/eventing/issues/5756 strict-subscriber: "disabled" + + # ALPHA feature: The new-trigger-filters flag allows you to use the new `filters` field + # in Trigger objects with its rich filtering capabilities. + # For more details: https://github.com/knative/eventing/issues/5204 + new-trigger-filters: "disabled" diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 4d5561f43c2..070c3e586a2 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -22,6 +22,8 @@ import ( "fmt" "regexp" + "knative.dev/eventing/pkg/apis/feature" + "knative.dev/eventing/pkg/apis/eventing" corev1 "k8s.io/api/core/v1" @@ -55,7 +57,7 @@ func (ts *TriggerSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs.Also( ValidateAttributeFilters(ts.Filter).ViaField("filter"), ).Also( - ValidateSubscriptionAPIFiltersList(ts.Filters).ViaField("filters"), + ValidateSubscriptionAPIFiltersList(ctx, ts.Filters).ViaField("filters"), ).Also( ts.Subscriber.Validate(ctx).ViaField("subscriber"), ).Also( @@ -181,19 +183,19 @@ func ValidateSingleAttributeMap(expr map[string]string) (errs *apis.FieldError) return errs } -func ValidateSubscriptionAPIFiltersList(filters []SubscriptionsAPIFilter) (errs *apis.FieldError) { - if filters == nil { +func ValidateSubscriptionAPIFiltersList(ctx context.Context, filters []SubscriptionsAPIFilter) (errs *apis.FieldError) { + if filters == nil || !feature.FromContext(ctx).IsEnabled(feature.NewTriggerFilters) { return nil } for i, f := range filters { f := f - errs = errs.Also(ValidateSubscriptionAPIFilter(&f)).ViaIndex(i) + errs = errs.Also(ValidateSubscriptionAPIFilter(ctx, &f)).ViaIndex(i) } return errs } -func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.FieldError) { +func ValidateSubscriptionAPIFilter(ctx context.Context, filter *SubscriptionsAPIFilter) (errs *apis.FieldError) { if filter == nil { return nil } @@ -206,10 +208,10 @@ func ValidateSubscriptionAPIFilter(filter *SubscriptionsAPIFilter) (errs *apis.F ).Also( ValidateSingleAttributeMap(filter.Suffix).ViaField("suffix"), ).Also( - ValidateSubscriptionAPIFiltersList(filter.All).ViaField("all"), + ValidateSubscriptionAPIFiltersList(ctx, filter.All).ViaField("all"), ).Also( - ValidateSubscriptionAPIFiltersList(filter.Any).ViaField("any"), - ).Also(ValidateSubscriptionAPIFilter(filter.Not).ViaField("not")) + ValidateSubscriptionAPIFiltersList(ctx, filter.Any).ViaField("any"), + ).Also(ValidateSubscriptionAPIFilter(ctx, filter.Not).ViaField("not")) return errs } diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index 70b5274944e..efda596aa89 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -21,6 +21,8 @@ import ( "fmt" "testing" + "knative.dev/eventing/pkg/apis/feature" + "github.com/google/go-cmp/cmp" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -488,6 +490,9 @@ func TestTriggerSpecValidation(t *testing.T) { } func TestFilterSpecValidation(t *testing.T) { + newTriggerFiltersEnabledCtx := feature.ToContext(context.TODO(), feature.Flags{ + feature.NewTriggerFilters: feature.Enabled, + }) tests := []struct { name string filter *TriggerFilter @@ -727,7 +732,7 @@ func TestFilterSpecValidation(t *testing.T) { Filters: test.filters, Subscriber: validSubscriber, } - got := ts.Validate(context.TODO()) + got := ts.Validate(newTriggerFiltersEnabledCtx) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { t.Errorf("Validate TriggerSpec (-want, +got) =\n%s", diff) } diff --git a/pkg/apis/feature/flag_names.go b/pkg/apis/feature/flag_names.go index 1559d64f9d4..a404c8fdc92 100644 --- a/pkg/apis/feature/flag_names.go +++ b/pkg/apis/feature/flag_names.go @@ -21,4 +21,5 @@ const ( DeliveryTimeout = "delivery-timeout" KReferenceMapping = "kreference-mapping" StrictSubscriber = "strict-subscriber" + NewTriggerFilters = "new-trigger-filters" ) From f3e8ff9dde403cbbf3b89d7e8b77f302a244485f Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Wed, 10 Nov 2021 23:19:52 +0100 Subject: [PATCH 5/8] Organize imports Signed-off-by: Ahmed Abdalla --- pkg/apis/eventing/v1/trigger_validation.go | 7 +++---- pkg/apis/eventing/v1/trigger_validation_test.go | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 070c3e586a2..87deb22af13 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -22,13 +22,12 @@ import ( "fmt" "regexp" - "knative.dev/eventing/pkg/apis/feature" - - "knative.dev/eventing/pkg/apis/eventing" - corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" "knative.dev/pkg/kmp" + + "knative.dev/eventing/pkg/apis/eventing" + "knative.dev/eventing/pkg/apis/feature" ) var ( diff --git a/pkg/apis/eventing/v1/trigger_validation_test.go b/pkg/apis/eventing/v1/trigger_validation_test.go index efda596aa89..7fd70f124af 100644 --- a/pkg/apis/eventing/v1/trigger_validation_test.go +++ b/pkg/apis/eventing/v1/trigger_validation_test.go @@ -21,8 +21,6 @@ import ( "fmt" "testing" - "knative.dev/eventing/pkg/apis/feature" - "github.com/google/go-cmp/cmp" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -30,6 +28,7 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1" + "knative.dev/eventing/pkg/apis/feature" ) var ( From f2b8c8e1a302796212fffb6e7b12d9b2dc0d9e0a Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Thu, 11 Nov 2021 00:23:00 +0100 Subject: [PATCH 6/8] update codegen Signed-off-by: Ahmed Abdalla --- pkg/apis/config/zz_generated.deepcopy.go | 1 + pkg/apis/duck/v1/zz_generated.deepcopy.go | 1 + pkg/apis/duck/v1beta1/zz_generated.deepcopy.go | 1 + pkg/apis/eventing/v1/zz_generated.deepcopy.go | 1 + pkg/apis/eventing/v1beta1/zz_generated.deepcopy.go | 1 + pkg/apis/flows/v1/zz_generated.deepcopy.go | 1 + pkg/apis/messaging/config/zz_generated.deepcopy.go | 1 + pkg/apis/messaging/v1/zz_generated.deepcopy.go | 1 + pkg/apis/sources/v1/zz_generated.deepcopy.go | 1 + pkg/apis/sources/v1beta2/zz_generated.deepcopy.go | 1 + 10 files changed, 10 insertions(+) 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/zz_generated.deepcopy.go b/pkg/apis/duck/v1/zz_generated.deepcopy.go index ffd6985aed6..c22e2bc3b9c 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/zz_generated.deepcopy.go b/pkg/apis/eventing/v1/zz_generated.deepcopy.go index c2aee43a866..df852553761 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/zz_generated.deepcopy.go b/pkg/apis/messaging/v1/zz_generated.deepcopy.go index dd756192282..2876f8388a8 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 /* From 81ef75ea71099518e7c784e27969779c900c60d5 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Thu, 11 Nov 2021 14:59:25 +0100 Subject: [PATCH 7/8] change filters oneOf validation Signed-off-by: Ahmed Abdalla --- pkg/apis/eventing/struct_validation.go | 55 ---------------------- pkg/apis/eventing/v1/trigger_validation.go | 49 ++++++++++++++++++- 2 files changed, 47 insertions(+), 57 deletions(-) delete mode 100644 pkg/apis/eventing/struct_validation.go diff --git a/pkg/apis/eventing/struct_validation.go b/pkg/apis/eventing/struct_validation.go deleted file mode 100644 index 94c7e8ae5f5..00000000000 --- a/pkg/apis/eventing/struct_validation.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2020 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 eventing - -import ( - "reflect" -) - -// HasMultipleSetFields checks if the struct has more than one field with a non-zero value -func HasMultipleSetFields(o interface{}) bool { - val := reflect.ValueOf(o) - fieldSet := false - for i := 0; i < val.NumField(); i++ { - f := val.Field(i) - if !IsEmptyField(f) { - if !fieldSet { - fieldSet = true - } else { - return true - } - } - } - return false -} - -// IsEmptyField checks if a struct field is empty by comparing it against its zero value -func IsEmptyField(v reflect.Value) bool { - switch v.Kind() { - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return v.Int() == 0 - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return v.Uint() == 0 - case reflect.String: - return v.String() == "" - case reflect.Ptr, reflect.Slice, reflect.Map, reflect.Interface, reflect.Chan: - return v.IsNil() - case reflect.Bool: - return !v.Bool() - } - return false -} diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 87deb22af13..46d82660eaf 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -26,7 +26,6 @@ import ( "knative.dev/pkg/apis" "knative.dev/pkg/kmp" - "knative.dev/eventing/pkg/apis/eventing" "knative.dev/eventing/pkg/apis/feature" ) @@ -215,8 +214,54 @@ func ValidateSubscriptionAPIFilter(ctx context.Context, filter *SubscriptionsAPI } func ValidateOneOf(filter *SubscriptionsAPIFilter) (err *apis.FieldError) { - if filter != nil && eventing.HasMultipleSetFields(*filter) { + if filter != nil && hasMultipleDialects(filter) { return apis.ErrGeneric("multiple dialects found, filters can have only one dialect set") } return nil } + +func hasMultipleDialects(filter *SubscriptionsAPIFilter) bool { + dialectFound := false + if len(filter.Exact) > 0 { + dialectFound = true + } + if len(filter.Prefix) > 0 { + if dialectFound { + return true + } else { + dialectFound = true + } + } + if len(filter.Suffix) > 0 { + if dialectFound { + return true + } else { + dialectFound = true + } + } + if len(filter.All) > 0 { + if dialectFound { + return true + } else { + dialectFound = true + } + } + if len(filter.Any) > 0 { + if dialectFound { + return true + } else { + dialectFound = true + } + } + if filter.Not != nil { + if dialectFound { + return true + } else { + dialectFound = true + } + } + if len(filter.Extensions) > 0 && dialectFound { + return true + } + return false +} From 5363c2bceb64031c65b36fb48bedf05e2e2dd5b1 Mon Sep 17 00:00:00 2001 From: Ahmed Abdalla Date: Thu, 11 Nov 2021 15:01:07 +0100 Subject: [PATCH 8/8] fix formatting Signed-off-by: Ahmed Abdalla --- pkg/apis/eventing/v1/trigger_validation.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/apis/eventing/v1/trigger_validation.go b/pkg/apis/eventing/v1/trigger_validation.go index 46d82660eaf..772a33f17a1 100644 --- a/pkg/apis/eventing/v1/trigger_validation.go +++ b/pkg/apis/eventing/v1/trigger_validation.go @@ -209,7 +209,9 @@ func ValidateSubscriptionAPIFilter(ctx context.Context, filter *SubscriptionsAPI ValidateSubscriptionAPIFiltersList(ctx, filter.All).ViaField("all"), ).Also( ValidateSubscriptionAPIFiltersList(ctx, filter.Any).ViaField("any"), - ).Also(ValidateSubscriptionAPIFilter(ctx, filter.Not).ViaField("not")) + ).Also( + ValidateSubscriptionAPIFilter(ctx, filter.Not).ViaField("not"), + ) return errs }