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/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,19 @@ 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{
+			trigger := client_v1alpha1.NewTriggerBuilder(name)
+			trigger.Namespace(namespace)
+			trigger.Broker(triggerUpdateFlags.Broker)
+			for k, v := range filters {
+				trigger.AddFilter(k, v)
+			}
+			trigger.Subscriber(&duckv1.Destination{
 				Ref: objectRef.Ref,
 				URI: objectRef.URI,
-			}
+			})
 
-			err = eventingClient.CreateTrigger(trigger)
+			err = eventingClient.CreateTrigger(trigger.Build())
 			if err != nil {
 				return fmt.Errorf(
 					"cannot create trigger '%s' in namespace '%s' "+
@@ -99,27 +98,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 TestTriggerCreateNoFilter(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

@@ -80,8 +80,10 @@ func createTrigger(namespace string, name string, filters map[string]string, bro
 		Namespace(namespace).
 		Broker(broker)
 
-	for k, v := range filters {
-		triggerBuilder.AddFilter(k, v)
+	if filters != nil {
+		for k, v := range filters {
+			triggerBuilder.AddFilter(k, v)
+		}
 	}
 
 	triggerBuilder.Subscriber(&duckv1.Destination{

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")
 }

pkg/kn/commands/trigger/update_flags_test.go

@@ -24,7 +24,7 @@ import (
 func TestGetFilters(t *testing.T) {
 	t.Run("get multiple filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type=abc.edf.ghi", "attr=value"},
+			Filters: []string{"type=abc.edf.ghi", "attr=value"},
 		}
 		created, err := createFlag.GetFilters()
 		wanted := map[string]string{
@@ -37,34 +37,34 @@ func TestGetFilters(t *testing.T) {
 
 	t.Run("get filters with errors", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type"},
+			Filters: []string{"type"},
 		}
 		_, err := createFlag.GetFilters()
 		assert.ErrorContains(t, err, "Invalid --filter")
 
 		createFlag = TriggerUpdateFlags{
-			Filters: filterArray{"type="},
+			Filters: []string{"type="},
 		}
 		filters, err := createFlag.GetFilters()
 		wanted := map[string]string{"type": ""}
 		assert.DeepEqual(t, wanted, filters)
 
 		createFlag = TriggerUpdateFlags{
-			Filters: filterArray{"=value"},
+			Filters: []string{"=value"},
 		}
 		_, err = createFlag.GetFilters()
 		assert.ErrorContains(t, err, "Invalid --filter")
 
 		createFlag = TriggerUpdateFlags{
-			Filters: filterArray{"="},
+			Filters: []string{"="},
 		}
 		_, err = createFlag.GetFilters()
 		assert.ErrorContains(t, err, "Invalid --filter")
 	})
 
 	t.Run("get duplicate filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type=foo", "type=bar"},
+			Filters: []string{"type=foo", "type=bar"},
 		}
 		_, err := createFlag.GetFilters()
 		assert.ErrorContains(t, err, "duplicate")
@@ -74,7 +74,7 @@ func TestGetFilters(t *testing.T) {
 func TestGetUpdateFilters(t *testing.T) {
 	t.Run("get updated filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type=abc.edf.ghi", "attr=value"},
+			Filters: []string{"type=abc.edf.ghi", "attr=value"},
 		}
 		updated, removed, err := createFlag.GetUpdateFilters()
 		wanted := map[string]string{
@@ -88,7 +88,7 @@ func TestGetUpdateFilters(t *testing.T) {
 
 	t.Run("get deleted filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type-", "attr-"},
+			Filters: []string{"type-", "attr-"},
 		}
 		updated, removed, err := createFlag.GetUpdateFilters()
 		wanted := []string{"type", "attr"}
@@ -101,7 +101,7 @@ func TestGetUpdateFilters(t *testing.T) {
 
 	t.Run("get updated & deleted filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type=foo", "attr-", "source=bar", "env-"},
+			Filters: []string{"type=foo", "attr-", "source=bar", "env-"},
 		}
 		updated, removed, err := createFlag.GetUpdateFilters()
 		wantedRemoved := []string{"attr", "env"}
@@ -118,7 +118,7 @@ func TestGetUpdateFilters(t *testing.T) {
 
 	t.Run("update duplicate filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
-			Filters: filterArray{"type=foo", "type=bar"},
+			Filters: []string{"type=foo", "type=bar"},
 		}
 		_, _, err := createFlag.GetUpdateFilters()
 		assert.ErrorContains(t, err, "duplicate")