Enable trigger filter to be optional

docs/cmd/kn_trigger_create.md

@@ -22,7 +22,7 @@ kn trigger create NAME --broker BROKER --filter KEY=VALUE --sink SINK [flags]
 
 ```
       --broker string      Name of the Broker which the trigger associates with. (default "default")
-      --filter []string    Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
+      --filter strings     Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
   -h, --help               help for create
   -n, --namespace string   Specify the namespace to operate in.
   -s, --sink string        Addressable sink for events

docs/cmd/kn_trigger_update.md

@@ -29,7 +29,7 @@ kn trigger update NAME --filter KEY=VALUE --sink SINK [flags]
 
 ```
       --broker string      Name of the Broker which the trigger associates with. (default "default")
-      --filter []string    Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
+      --filter strings     Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
   -h, --help               help for update
   -n, --namespace string   Specify the namespace to operate in.
   -s, --sink string        Addressable sink for events

pkg/eventing/v1alpha1/client.go

@@ -170,7 +170,7 @@ func (b *TriggerBuilder) Broker(broker string) *TriggerBuilder {
 	return b
 }
 
-func (b *TriggerBuilder) AddFilter(key, value string) *TriggerBuilder {
+func (b *TriggerBuilder) Filters(filters map[string]string) *TriggerBuilder {
 	filter := b.trigger.Spec.Filter
 	if filter == nil {
 		filter = &v1alpha1.TriggerFilter{}
@@ -181,20 +181,9 @@ func (b *TriggerBuilder) AddFilter(key, value string) *TriggerBuilder {
 		attributes = &v1alpha1.TriggerFilterAttributes{}
 		filter.Attributes = attributes
 	}
-	(*attributes)[key] = value
-	return b
-}
-
-func (b *TriggerBuilder) RemoveFilter(key string) *TriggerBuilder {
-	filter := b.trigger.Spec.Filter
-	if filter == nil {
-		return b
-	}
-	attributes := filter.Attributes
-	if attributes == nil {
-		return b
+	for k, v := range filters {
+		(*attributes)[k] = v
 	}
-	delete(*attributes, key)
 	return b
 }
 

pkg/eventing/v1alpha1/client_test.go

@@ -128,12 +128,12 @@ func TestListTrigger(t *testing.T) {
 
 func TestTriggerBuilder(t *testing.T) {
 	a := NewTriggerBuilder("testtrigger")
-	a.AddFilter("type", "foo").AddFilter("source", "bar")
+	a.Filters(map[string]string{"type": "foo"})
 
-	t.Run("update filter", func(t *testing.T) {
+	t.Run("update filters", func(t *testing.T) {
 		b := NewTriggerBuilderFromExisting(a.Build())
 		assert.DeepEqual(t, b.Build(), a.Build())
-		b.AddFilter("type", "new").RemoveFilter("source")
+		b.Filters(map[string]string{"type": "new"})
 		expected := &v1alpha1.TriggerFilter{
 			Attributes: &v1alpha1.TriggerFilterAttributes{
 				"type": "new",
@@ -142,22 +142,10 @@ func TestTriggerBuilder(t *testing.T) {
 		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
 	})
 
-	t.Run("update filter with only deletions", func(t *testing.T) {
+	t.Run("update filters with both new entry and updated entry", func(t *testing.T) {
 		b := NewTriggerBuilderFromExisting(a.Build())
 		assert.DeepEqual(t, b.Build(), a.Build())
-		b.RemoveFilter("source")
-		expected := &v1alpha1.TriggerFilter{
-			Attributes: &v1alpha1.TriggerFilterAttributes{
-				"type": "foo",
-			},
-		}
-		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
-	})
-
-	t.Run("update filter with only updates", func(t *testing.T) {
-		b := NewTriggerBuilderFromExisting(a.Build())
-		assert.DeepEqual(t, b.Build(), a.Build())
-		b.AddFilter("type", "new")
+		b.Filters(map[string]string{"type": "new", "source": "bar"})
 		expected := &v1alpha1.TriggerFilter{
 			Attributes: &v1alpha1.TriggerFilterAttributes{
 				"type":   "new",
@@ -172,6 +160,6 @@ func newTrigger(name string) *v1alpha1.Trigger {
 	return NewTriggerBuilder(name).
 		Namespace(testNamespace).
 		Broker("default").
-		AddFilter("type", "foo").
+		Filters(map[string]string{"type": "foo"}).
 		Build()
 }

pkg/kn/commands/trigger/create.go

@@ -20,10 +20,9 @@ import (
 
 	"github.com/spf13/cobra"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	client_v1alpha1 "knative.dev/client/pkg/eventing/v1alpha1"
 	"knative.dev/client/pkg/kn/commands"
 	"knative.dev/client/pkg/kn/commands/flags"
-	"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
 
@@ -73,19 +72,18 @@ func NewTriggerCreateCommand(p *commands.KnParams) *cobra.Command {
 					"cannot create trigger '%s' "+
 						"because %s", name, err)
 			}
-			if filters == nil {
-				return fmt.Errorf(
-					"cannot create trigger '%s' "+
-						"because filters are required", name)
-			}
 
-			trigger := constructTrigger(name, namespace, triggerUpdateFlags.Broker, filters)
-			trigger.Spec.Subscriber = &duckv1.Destination{
-				Ref: objectRef.Ref,
-				URI: objectRef.URI,
-			}
-
-			err = eventingClient.CreateTrigger(trigger)
+			triggerBuilder := client_v1alpha1.
+				NewTriggerBuilder(name).
+				Namespace(namespace).
+				Broker(triggerUpdateFlags.Broker).
+				Filters(filters).
+				Subscriber(&duckv1.Destination{
+					Ref: objectRef.Ref,
+					URI: objectRef.URI,
+				})
+
+			err = eventingClient.CreateTrigger(triggerBuilder.Build())
 			if err != nil {
 				return fmt.Errorf(
 					"cannot create trigger '%s' in namespace '%s' "+
@@ -99,27 +97,6 @@ func NewTriggerCreateCommand(p *commands.KnParams) *cobra.Command {
 	triggerUpdateFlags.Add(cmd)
 	sinkFlags.Add(cmd)
 	cmd.MarkFlagRequired("sink")
-	cmd.MarkFlagRequired("filter")
 
 	return cmd
 }
-
-// constructTrigger is to create an instance of v1alpha1.Trigger
-func constructTrigger(name string, namespace string, broker string, filters map[string]string) *v1alpha1.Trigger {
-	trigger := v1alpha1.Trigger{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: namespace,
-		},
-		Spec: v1alpha1.TriggerSpec{
-			Broker: broker,
-		},
-	}
-
-	triggerFilterAttributes := v1alpha1.TriggerFilterAttributes(filters)
-	trigger.Spec.Filter = &v1alpha1.TriggerFilter{
-		Attributes: &triggerFilterAttributes,
-	}
-
-	return &trigger
-}

pkg/kn/commands/trigger/create_test.go

@@ -67,13 +67,6 @@ func TestNoSinkError(t *testing.T) {
 	assert.ErrorContains(t, err, "required flag(s)", "sink", "not set")
 }
 
-func TestNoFilterError(t *testing.T) {
-	eventingClient := eventing_client.NewMockKnEventingClient(t)
-	_, err := executeTriggerCommand(eventingClient, nil, "create", triggerName, "--broker", "mybroker",
-		"--sink", "svc:mysvc")
-	assert.ErrorContains(t, err, "required flag(s)", "filter", "not set")
-}
-
 func TestTriggerCreateMultipleFilter(t *testing.T) {
 	eventingClient := eventing_client.NewMockKnEventingClient(t)
 	dynamicClient := dynamic_fake.CreateFakeKnDynamicClient("default", &serving_v1alpha1.Service{
@@ -91,3 +84,20 @@ func TestTriggerCreateMultipleFilter(t *testing.T) {
 
 	eventingRecorder.Validate()
 }
+
+func TestTriggerCreateWithoutFilter(t *testing.T) {
+	eventingClient := eventing_client.NewMockKnEventingClient(t)
+	dynamicClient := dynamic_fake.CreateFakeKnDynamicClient("default", &serving_v1alpha1.Service{
+		TypeMeta:   metav1.TypeMeta{Kind: "Service", APIVersion: "serving.knative.dev/v1alpha1"},
+		ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "default"},
+	})
+
+	eventingRecorder := eventingClient.Recorder()
+	eventingRecorder.CreateTrigger(createTrigger("default", triggerName, nil, "mybroker", "mysvc"), nil)
+
+	out, err := executeTriggerCommand(eventingClient, dynamicClient, "create", triggerName, "--broker", "mybroker", "--sink", "svc:mysvc")
+	assert.NilError(t, err, "Trigger should be created")
+	util.ContainsAll(out, "Trigger", triggerName, "created", "namespace", "default")
+
+	eventingRecorder.Validate()
+}

pkg/kn/commands/trigger/trigger_test.go

@@ -78,20 +78,16 @@ func executeTriggerCommand(triggerClient eventc_v1alpha1.KnEventingClient, dynam
 func createTrigger(namespace string, name string, filters map[string]string, broker string, svcname string) *v1alpha1.Trigger {
 	triggerBuilder := eventc_v1alpha1.NewTriggerBuilder(name).
 		Namespace(namespace).
-		Broker(broker)
-
-	for k, v := range filters {
-		triggerBuilder.AddFilter(k, v)
-	}
-
-	triggerBuilder.Subscriber(&duckv1.Destination{
-		Ref: &corev1.ObjectReference{
-			Name:       svcname,
-			Kind:       "Service",
-			Namespace:  "default",
-			APIVersion: "serving.knative.dev/v1alpha1",
-		},
-	})
+		Broker(broker).
+		Filters(filters).
+		Subscriber(&duckv1.Destination{
+			Ref: &corev1.ObjectReference{
+				Name:       svcname,
+				Kind:       "Service",
+				Namespace:  "default",
+				APIVersion: "serving.knative.dev/v1alpha1",
+			},
+		})
 	return triggerBuilder.Build()
 }
 

pkg/kn/commands/trigger/update.go

@@ -23,6 +23,8 @@ import (
 	client_v1alpha1 "knative.dev/client/pkg/eventing/v1alpha1"
 	"knative.dev/client/pkg/kn/commands"
 	"knative.dev/client/pkg/kn/commands/flags"
+	"knative.dev/client/pkg/util"
+	"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
 
@@ -82,12 +84,8 @@ func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command {
 					return fmt.Errorf(
 						"cannot update trigger '%s' because %s", name, err)
 				}
-				for k, v := range updated {
-					b.AddFilter(k, v)
-				}
-				for _, k := range removed {
-					b.RemoveFilter(k)
-				}
+				existing := extractFilters(trigger)
+				b.Filters(existing.Merge(updated).Remove(removed))
 			}
 			if cmd.Flags().Changed("sink") {
 				destination, err := sinkFlags.ResolveSink(dynamicClient, namespace)
@@ -112,3 +110,13 @@ func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command {
 
 	return cmd
 }
+
+func extractFilters(trigger *v1alpha1.Trigger) util.StringMap {
+	attributes := make(util.StringMap)
+	if trigger.Spec.Filter != nil && trigger.Spec.Filter.Attributes != nil {
+		for k, v := range *trigger.Spec.Filter.Attributes {
+			attributes[k] = v
+		}
+	}
+	return attributes
+}

pkg/kn/commands/trigger/update_flags.go

@@ -21,32 +21,13 @@ import (
 	"knative.dev/client/pkg/util"
 )
 
-type filterArray []string
-
-func (filters *filterArray) String() string {
-	str := ""
-	for _, filter := range *filters {
-		str = str + filter + " "
-	}
-	return str
-}
-
-func (filters *filterArray) Set(value string) error {
-	*filters = append(*filters, value)
-	return nil
-}
-
-func (filters *filterArray) Type() string {
-	return "[]string"
-}
-
 // TriggerUpdateFlags are flags for create and update a trigger
 type TriggerUpdateFlags struct {
 	Broker  string
-	Filters filterArray
+	Filters []string
 }
 
-// GetFilter to return a map type of filters
+// GetFilters to return a map type of filters
 func (f *TriggerUpdateFlags) GetFilters() (map[string]string, error) {
 	filters, err := util.MapFromArray(f.Filters, "=")
 	if err != nil {
@@ -55,7 +36,7 @@ func (f *TriggerUpdateFlags) GetFilters() (map[string]string, error) {
 	return filters, nil
 }
 
-// GetFilter to return a map type of filters
+// GetUpdateFilters to return a map type of filters
 func (f *TriggerUpdateFlags) GetUpdateFilters() (map[string]string, []string, error) {
 	filters, err := util.MapFromArrayAllowingSingles(f.Filters, "=")
 	if err != nil {
@@ -68,5 +49,5 @@ func (f *TriggerUpdateFlags) GetUpdateFilters() (map[string]string, []string, er
 //Add is to set parameters
 func (f *TriggerUpdateFlags) Add(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&f.Broker, "broker", "default", "Name of the Broker which the trigger associates with.")
-	cmd.Flags().Var(&f.Filters, "filter", "Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo")
+	cmd.Flags().StringSliceVar(&f.Filters, "filter", nil, "Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo")
 }