add trigger update test

docs/cmd/kn_trigger.md

@@ -31,4 +31,5 @@ kn trigger [flags]
 * [kn trigger delete](kn_trigger_delete.md)	 - Delete a trigger.
 * [kn trigger describe](kn_trigger_describe.md)	 - Describe a trigger.
 * [kn trigger list](kn_trigger_list.md)	 - List available triggers.
+* [kn trigger update](kn_trigger_update.md)	 - Update a trigger
 

docs/cmd/kn_trigger_update.md

@@ -0,0 +1,49 @@
+## kn trigger update
+
+Update a trigger
+
+### Synopsis
+
+Update a trigger
+
+```
+kn trigger update NAME --filter KEY=VALUE --sink SINK [flags]
+```
+
+### Examples
+
+```
+
+  # Update the filter which key is 'type' to value 'type=knative.dev.bar' in a trigger 'mytrigger'
+  kn trigger update mytrigger --filter type=knative.dev.bar
+
+  # Delete the filter which key is 'type' from a trigger 'mytrigger' 
+  kn trigger update mytrigger --filter type-
+
+  # Update the sink of a trigger 'mytrigger' to 'svc:new-service'
+  kn trigger update mytrigger --sink svc:new-service
+  
+```
+
+### Options
+
+```
+      --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
+  -h, --help               help for update
+  -n, --namespace string   Specify the namespace to operate in.
+  -s, --sink string        Addressable sink for events
+```
+
+### Options inherited from parent commands
+
+```
+      --config string       kn config file (default is $HOME/.kn/config.yaml)
+      --kubeconfig string   kubectl config file (default is $HOME/.kube/config)
+      --log-http            log http traffic
+```
+
+### SEE ALSO
+
+* [kn trigger](kn_trigger.md)	 - Trigger command group
+

pkg/eventing/v1alpha1/client.go

@@ -23,6 +23,7 @@ import (
 	"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	"knative.dev/eventing/pkg/client/clientset/versioned/scheme"
 	client_v1alpha1 "knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1alpha1"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
 
 const (
@@ -42,6 +43,8 @@ type KnEventingClient interface {
 	GetTrigger(name string) (*v1alpha1.Trigger, error)
 	// ListTrigger returns list of trigger CRDs
 	ListTriggers() (*v1alpha1.TriggerList, error)
+	// UpdateTrigger is used to update an instance of trigger
+	UpdateTrigger(trigger *v1alpha1.Trigger) error
 }
 
 // KnEventingClient is a combination of Sources client interface and namespace
@@ -110,6 +113,15 @@ func (c *knEventingClient) ListTriggers() (*v1alpha1.TriggerList, error) {
 	return triggerListNew, nil
 }
 
+//CreateTrigger is used to create an instance of trigger
+func (c *knEventingClient) UpdateTrigger(trigger *v1alpha1.Trigger) error {
+	trigger, err := c.client.Triggers(c.namespace).Update(trigger)
+	if err != nil {
+		return kn_errors.GetError(err)
+	}
+	return nil
+}
+
 // Return the client's namespace
 func (c *knEventingClient) Namespace() string {
 	return c.namespace
@@ -139,22 +151,50 @@ func NewTriggerBuilderFromExisting(tr *v1alpha1.Trigger) *TriggerBuilder {
 	return &TriggerBuilder{trigger: tr.DeepCopy()}
 }
 
-// Broker with which this source should operate
+// Broker to set the broker of trigger object
 func (b *TriggerBuilder) Broker(broker string) *TriggerBuilder {
 	if broker != "" {
 		b.trigger.Spec.Broker = broker
 	}
 	return b
 }
 
-// Filter with which this source should operate
-func (b *TriggerBuilder) Filter(filters map[string]string) *TriggerBuilder {
+// Filters to set the filters of trigger object
+func (b *TriggerBuilder) Filters(filters map[string]string) *TriggerBuilder {
 	if filters != nil {
-		if b.trigger.Spec.Filter==nil || 
 		triggerFilterAttributes := v1alpha1.TriggerFilterAttributes(filters)
 		b.trigger.Spec.Filter = &v1alpha1.TriggerFilter{
 			Attributes: &triggerFilterAttributes,
 		}
 	}
 	return b
 }
+
+// UpdateFilters to update the filters of trigger object
+func (b *TriggerBuilder) UpdateFilters(toUpdate map[string]string, toRemove []string) *TriggerBuilder {
+	if b.trigger.Spec.Filter == nil {
+		b.Filters(toUpdate)
+		return b
+	}
+
+	existing := map[string]string(*b.trigger.Spec.Filter.Attributes)
+	for key, value := range toUpdate {
+		existing[key] = value
+	}
+	for _, key := range toRemove {
+		delete(existing, key)
+	}
+	b.Filters(existing)
+	return b
+}
+
+// Sink to set the subscriber of trigger object
+func (b *TriggerBuilder) Sink(sink *duckv1.Destination) *TriggerBuilder {
+	b.trigger.Spec.Subscriber = sink
+	return b
+}
+
+// Build to return an instance of trigger object
+func (b *TriggerBuilder) Build() *v1alpha1.Trigger {
+	return b.trigger
+}

pkg/eventing/v1alpha1/client_mock.go

@@ -102,6 +102,17 @@ func (c *MockKnEventingClient) ListTriggers() (*v1alpha1.TriggerList, error) {
 	return call.Result[0].(*v1alpha1.TriggerList), mock.ErrorOrNil(call.Result[1])
 }
 
+// UpdateTrigger records a call for ListTriggers with the expected result and error (nil if none)
+func (sr *EventingRecorder) UpdateTrigger(trigger interface{}, err error) {
+	sr.r.Add("UpdateTrigger", []interface{}{trigger}, []interface{}{err})
+}
+
+// UpdateTrigger performs a previously recorded action
+func (c *MockKnEventingClient) UpdateTrigger(trigger *v1alpha1.Trigger) error {
+	call := c.recorder.r.VerifyCall("UpdateTrigger")
+	return mock.ErrorOrNil(call.Result[0])
+}
+
 // Validate validates whether every recorded action has been called
 func (sr *EventingRecorder) Validate() {
 	sr.r.CheckThatAllRecordedMethodsHaveBeenCalled()

pkg/eventing/v1alpha1/client_mock_test.go

@@ -31,12 +31,14 @@ func TestMockKnClient(t *testing.T) {
 	recorder.CreateTrigger(&v1alpha1.Trigger{}, nil)
 	recorder.DeleteTrigger("hello", nil)
 	recorder.ListTriggers(nil, nil)
+	recorder.UpdateTrigger(&v1alpha1.Trigger{}, nil)
 
 	// Call all service
 	client.GetTrigger("hello")
 	client.CreateTrigger(&v1alpha1.Trigger{})
 	client.DeleteTrigger("hello")
 	client.ListTriggers()
+	client.UpdateTrigger(&v1alpha1.Trigger{})
 
 	// Validate
 	recorder.Validate()

pkg/eventing/v1alpha1/client_test.go

@@ -128,22 +128,54 @@ func TestListTrigger(t *testing.T) {
 	})
 }
 
-func newTrigger(name string) *v1alpha1.Trigger {
-	obj := &v1alpha1.Trigger{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: testNamespace,
-		},
-		Spec: v1alpha1.TriggerSpec{
-			Broker: "default",
-			Filter: &v1alpha1.TriggerFilter{
-				Attributes: &v1alpha1.TriggerFilterAttributes{
-					"type": "foo",
-				},
+func TestTriggerBuilder(t *testing.T) {
+	a := NewTriggerBuilder("testtrigger")
+	a.Filters(map[string]string{"type": "foo", "source": "bar"})
+
+	t.Run("update filter", func(t *testing.T) {
+		b := NewTriggerBuilderFromExisting(a.Build())
+		assert.DeepEqual(t, b.Build(), a.Build())
+		b.UpdateFilters(map[string]string{"type": "new"}, []string{"source"})
+		expected := &v1alpha1.TriggerFilter{
+			Attributes: &v1alpha1.TriggerFilterAttributes{
+				"type": "new",
+			},
+		}
+		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
+	})
+
+	t.Run("update filter with only deletions", func(t *testing.T) {
+		b := NewTriggerBuilderFromExisting(a.Build())
+		assert.DeepEqual(t, b.Build(), a.Build())
+		b.UpdateFilters(nil, []string{"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.UpdateFilters(map[string]string{"type": "new"}, nil)
+		expected := &v1alpha1.TriggerFilter{
+			Attributes: &v1alpha1.TriggerFilterAttributes{
+				"type":   "new",
+				"source": "bar",
 			},
-		},
-	}
-	obj.Name = name
-	obj.Namespace = testNamespace
-	return obj
+		}
+		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
+	})
+
+}
+
+func newTrigger(name string) *v1alpha1.Trigger {
+	b := NewTriggerBuilder(name)
+	b.Filters(map[string]string{"type": "foo"})
+	b.Broker("default")
+	b.trigger.Name = name
+	b.trigger.Namespace = testNamespace
+	return b.Build()
 }

pkg/kn/commands/trigger/trigger.go

@@ -30,5 +30,6 @@ func NewTriggerCommand(p *commands.KnParams) *cobra.Command {
 	triggerCmd.AddCommand(NewTriggerDeleteCommand(p))
 	triggerCmd.AddCommand(NewTriggerDescribeCommand(p))
 	triggerCmd.AddCommand(NewTriggerListCommand(p))
+	triggerCmd.AddCommand(NewTriggerUpdateCommand(p))
 	return triggerCmd
 }

pkg/kn/commands/trigger/update.go

@@ -23,6 +23,7 @@ import (
 	client_v1alpha1 "knative.dev/client/pkg/eventing/v1alpha1"
 	"knative.dev/client/pkg/kn/commands"
 	"knative.dev/client/pkg/kn/commands/flags"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
 
 // NewTriggerUpdateCommand prepares the command for a CronJobSource update
@@ -31,15 +32,15 @@ func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command {
 	var sinkFlags flags.SinkFlags
 
 	cmd := &cobra.Command{
-		Use:   "update NAME --broker BROKER --filter KEY=VALUE --sink SINK",
+		Use:   "update NAME --filter KEY=VALUE --sink SINK",
 		Short: "Update a trigger",
 		Example: `
-  # Update the broker of a trigger 'mytrigger' to 'new-broker'
-  kn trigger update mytrigger --broker new-broker
-
-  # Update the filter of a trigger 'mytrigger' to 'type=knative.dev.bar'
+  # Update the filter which key is 'type' to value 'type=knative.dev.bar' in a trigger 'mytrigger'
   kn trigger update mytrigger --filter type=knative.dev.bar
 
+  # Delete the filter which key is 'type' from a trigger 'mytrigger' 
+  kn trigger update mytrigger --filter type-
+
   # Update the sink of a trigger 'mytrigger' to 'svc:new-service'
   kn trigger update mytrigger --sink svc:new-service
   `,
@@ -73,17 +74,28 @@ func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command {
 			b := client_v1alpha1.NewTriggerBuilderFromExisting(trigger)
 
 			if cmd.Flags().Changed("broker") {
-				b.Broker(triggerUpdateFlags.Broker)
+				return fmt.Errorf(
+					"cannot update trigger '%s' "+
+						"because broker is immutable", name)
 			}
 			if cmd.Flags().Changed("filter") {
-				b.Filter(triggerUpdateFlags.GetFilters())
+				updated, removed, err := triggerUpdateFlags.GetUpdateFilters()
+				if err != nil {
+					return fmt.Errorf(
+						"cannot update trigger '%s' "+
+							"because %s", name, err)
+				}
+				b.UpdateFilters(updated, removed)
 			}
 			if cmd.Flags().Changed("sink") {
 				destination, err := sinkFlags.ResolveSink(servingClient)
 				if err != nil {
 					return err
 				}
-				b.Sink(destination)
+				b.Sink(&duckv1.Destination{
+					Ref: destination.Ref,
+					URI: destination.URI,
+				})
 			}
 			err = eventingClient.UpdateTrigger(b.Build())
 			if err == nil {

pkg/kn/commands/trigger/update_flags.go

@@ -60,6 +60,24 @@ func (f *TriggerUpdateFlags) GetFilters() (map[string]string, error) {
 	return filters, nil
 }
 
+// GetFilter to return a map type of filters
+func (f *TriggerUpdateFlags) GetUpdateFilters() (map[string]string, []string, error) {
+	filters := map[string]string{}
+	removes := []string{}
+	for _, item := range f.Filters {
+		if strings.HasSuffix(item, "-") {
+			removes = append(removes, item[:len(item)-1])
+		} else {
+			parts := strings.Split(item, "=")
+			if len(parts) < 2 || parts[0] == "" || parts[1] == "" {
+				return nil, nil, fmt.Errorf("invalid filter %s", f.Filters)
+			}
+			filters[parts[0]] = parts[1]
+		}
+	}
+	return filters, removes, nil
+}
+
 //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.")

pkg/kn/commands/trigger/update_flags_test.go

@@ -20,7 +20,7 @@ import (
 	"gotest.tools/assert"
 )
 
-func TestGetFilter(t *testing.T) {
+func TestGetFilters(t *testing.T) {
 	t.Run("get multiple filters", func(t *testing.T) {
 		createFlag := TriggerUpdateFlags{
 			Filters: filterArray{"type=abc.edf.ghi", "attr=value"},
@@ -60,3 +60,45 @@ func TestGetFilter(t *testing.T) {
 		assert.ErrorContains(t, err, "invalid filter")
 	})
 }
+
+func TestGetUpdateFilters(t *testing.T) {
+	t.Run("get updated filters", func(t *testing.T) {
+		createFlag := TriggerUpdateFlags{
+			Filters: filterArray{"type=abc.edf.ghi", "attr=value"},
+		}
+		updated, removed, err := createFlag.GetUpdateFilters()
+		wanted := map[string]string{
+			"type": "abc.edf.ghi",
+			"attr": "value",
+		}
+		assert.NilError(t, err, "UpdateFilter should be created")
+		assert.DeepEqual(t, wanted, updated)
+		assert.Assert(t, len(removed) == 0)
+	})
+
+	t.Run("get deleted filters", func(t *testing.T) {
+		createFlag := TriggerUpdateFlags{
+			Filters: filterArray{"type-", "attr-"},
+		}
+		updated, removed, err := createFlag.GetUpdateFilters()
+		wanted := []string{"type", "attr"}
+		assert.NilError(t, err, "UpdateFilter should be created")
+		assert.DeepEqual(t, wanted, removed)
+		assert.Assert(t, len(updated) == 0)
+	})
+
+	t.Run("get updated & deleted filters", func(t *testing.T) {
+		createFlag := TriggerUpdateFlags{
+			Filters: filterArray{"type=foo", "attr-", "source=bar", "env-"},
+		}
+		updated, removed, err := createFlag.GetUpdateFilters()
+		wantedRemoved := []string{"attr", "env"}
+		wantedUpdated := map[string]string{
+			"type":   "foo",
+			"source": "bar",
+		}
+		assert.NilError(t, err, "UpdateFilter should be created")
+		assert.DeepEqual(t, wantedRemoved, removed)
+		assert.DeepEqual(t, wantedUpdated, updated)
+	})
+}