From ce8cca625f527dbf11fd40c35834d1aeedabd655 Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Tue, 17 Dec 2019 17:37:41 +0800 Subject: [PATCH 1/2] add trigger update command --- pkg/eventing/v1alpha1/client.go | 40 ++++++++ pkg/kn/commands/trigger/create.go | 28 ++++-- pkg/kn/commands/trigger/create_test.go | 7 ++ pkg/kn/commands/trigger/update.go | 100 +++++++++++++++++++ pkg/kn/commands/trigger/update_flags.go | 10 +- pkg/kn/commands/trigger/update_flags_test.go | 29 +++++- pkg/kn/commands/trigger/update_test.go | 15 +++ 7 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 pkg/kn/commands/trigger/update.go create mode 100644 pkg/kn/commands/trigger/update_test.go diff --git a/pkg/eventing/v1alpha1/client.go b/pkg/eventing/v1alpha1/client.go index aabde5bcab..b1b7eba76b 100644 --- a/pkg/eventing/v1alpha1/client.go +++ b/pkg/eventing/v1alpha1/client.go @@ -16,6 +16,7 @@ package v1alpha1 import ( apis_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" kn_errors "knative.dev/client/pkg/errors" "knative.dev/client/pkg/util" @@ -118,3 +119,42 @@ func (c *knEventingClient) Namespace() string { func updateTriggerGvk(obj runtime.Object) error { return util.UpdateGroupVersionKindWithScheme(obj, v1alpha1.SchemeGroupVersion, scheme.Scheme) } + +// TriggerBuilder is for building the trigger +type TriggerBuilder struct { + trigger *v1alpha1.Trigger +} + +// NewTriggerBuilder for building trigger object +func NewTriggerBuilder(name string) *TriggerBuilder { + return &TriggerBuilder{trigger: &v1alpha1.Trigger{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: name, + }, + }} +} + +// NewTriggerBuilderFromExisting for building the object from existing Trigger object +func NewTriggerBuilderFromExisting(tr *v1alpha1.Trigger) *TriggerBuilder { + return &TriggerBuilder{trigger: tr.DeepCopy()} +} + +// Broker with which this source should operate +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 { + if filters != nil { + if b.trigger.Spec.Filter==nil || + triggerFilterAttributes := v1alpha1.TriggerFilterAttributes(filters) + b.trigger.Spec.Filter = &v1alpha1.TriggerFilter{ + Attributes: &triggerFilterAttributes, + } + } + return b +} diff --git a/pkg/kn/commands/trigger/create.go b/pkg/kn/commands/trigger/create.go index e39c3625d2..bd8d511656 100644 --- a/pkg/kn/commands/trigger/create.go +++ b/pkg/kn/commands/trigger/create.go @@ -67,7 +67,19 @@ func NewTriggerCreateCommand(p *commands.KnParams) *cobra.Command { "because %s", name, namespace, err) } - trigger := constructTrigger(name, namespace, triggerUpdateFlags) + filters, err := triggerUpdateFlags.GetFilters() + if err != nil { + return fmt.Errorf( + "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, @@ -87,28 +99,26 @@ 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, flags TriggerUpdateFlags) *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: flags.Broker, + Broker: broker, }, } - filters := flags.GetFilters() - if filters != nil { - triggerFilterAttributes := v1alpha1.TriggerFilterAttributes(filters) - trigger.Spec.Filter = &v1alpha1.TriggerFilter{ - Attributes: &triggerFilterAttributes, - } + triggerFilterAttributes := v1alpha1.TriggerFilterAttributes(filters) + trigger.Spec.Filter = &v1alpha1.TriggerFilter{ + Attributes: &triggerFilterAttributes, } return &trigger diff --git a/pkg/kn/commands/trigger/create_test.go b/pkg/kn/commands/trigger/create_test.go index 2c7ab2d6e0..d5b5720343 100644 --- a/pkg/kn/commands/trigger/create_test.go +++ b/pkg/kn/commands/trigger/create_test.go @@ -75,6 +75,13 @@ 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) servingClient := knserving_client.NewMockKnServiceClient(t) diff --git a/pkg/kn/commands/trigger/update.go b/pkg/kn/commands/trigger/update.go new file mode 100644 index 0000000000..246d03c1e0 --- /dev/null +++ b/pkg/kn/commands/trigger/update.go @@ -0,0 +1,100 @@ +// Copyright © 2019 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 trigger + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + + client_v1alpha1 "knative.dev/client/pkg/eventing/v1alpha1" + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/commands/flags" +) + +// NewTriggerUpdateCommand prepares the command for a CronJobSource update +func NewTriggerUpdateCommand(p *commands.KnParams) *cobra.Command { + var triggerUpdateFlags TriggerUpdateFlags + var sinkFlags flags.SinkFlags + + cmd := &cobra.Command{ + Use: "update NAME --broker BROKER --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' + kn trigger update mytrigger --filter type=knative.dev.bar + + # Update the sink of a trigger 'mytrigger' to 'svc:new-service' + kn trigger update mytrigger --sink svc:new-service + `, + + RunE: func(cmd *cobra.Command, args []string) (err error) { + if len(args) != 1 { + return errors.New("name of trigger required") + } + name := args[0] + + namespace, err := p.GetNamespace(cmd) + if err != nil { + return err + } + + eventingClient, err := p.NewEventingClient(namespace) + if err != nil { + return err + } + + servingClient, err := p.NewServingClient(namespace) + if err != nil { + return err + } + + trigger, err := eventingClient.GetTrigger(name) + if err != nil { + return err + } + + b := client_v1alpha1.NewTriggerBuilderFromExisting(trigger) + + if cmd.Flags().Changed("broker") { + b.Broker(triggerUpdateFlags.Broker) + } + if cmd.Flags().Changed("filter") { + b.Filter(triggerUpdateFlags.GetFilters()) + } + if cmd.Flags().Changed("sink") { + destination, err := sinkFlags.ResolveSink(servingClient) + if err != nil { + return err + } + b.Sink(destination) + } + err = eventingClient.UpdateTrigger(b.Build()) + if err == nil { + fmt.Fprintf(cmd.OutOrStdout(), "Trigger '%s' updated in namespace '%s'.\n", name, namespace) + } + return err + }, + } + commands.AddNamespaceFlags(cmd.Flags(), false) + triggerUpdateFlags.Add(cmd) + sinkFlags.Add(cmd) + + return cmd +} diff --git a/pkg/kn/commands/trigger/update_flags.go b/pkg/kn/commands/trigger/update_flags.go index be3b601223..1cec4b01f9 100644 --- a/pkg/kn/commands/trigger/update_flags.go +++ b/pkg/kn/commands/trigger/update_flags.go @@ -47,17 +47,17 @@ type TriggerUpdateFlags struct { } // GetFilter to return a map type of filters -func (f *TriggerUpdateFlags) GetFilters() map[string]string { +func (f *TriggerUpdateFlags) GetFilters() (map[string]string, error) { filters := map[string]string{} for _, item := range f.Filters { parts := strings.Split(item, "=") - if len(parts) == 2 { - filters[parts[0]] = parts[1] + if len(parts) < 2 || parts[0] == "" || parts[1] == "" { + return nil, fmt.Errorf("invalid filter %s", f.Filters) } else { - fmt.Printf("Ignore invalid filter %s", f) + filters[parts[0]] = parts[1] } } - return filters + return filters, nil } //Add is to set parameters diff --git a/pkg/kn/commands/trigger/update_flags_test.go b/pkg/kn/commands/trigger/update_flags_test.go index e82ff6fe27..18e6cf9e84 100644 --- a/pkg/kn/commands/trigger/update_flags_test.go +++ b/pkg/kn/commands/trigger/update_flags_test.go @@ -25,11 +25,38 @@ func TestGetFilter(t *testing.T) { createFlag := TriggerUpdateFlags{ Filters: filterArray{"type=abc.edf.ghi", "attr=value"}, } - created := createFlag.GetFilters() + created, err := createFlag.GetFilters() wanted := map[string]string{ "type": "abc.edf.ghi", "attr": "value", } + assert.NilError(t, err, "Filter should be created") assert.DeepEqual(t, wanted, created) }) + + t.Run("get filters with errors", func(t *testing.T) { + createFlag := TriggerUpdateFlags{ + Filters: filterArray{"type"}, + } + _, err := createFlag.GetFilters() + assert.ErrorContains(t, err, "invalid filter") + + createFlag = TriggerUpdateFlags{ + Filters: filterArray{"type="}, + } + _, err = createFlag.GetFilters() + assert.ErrorContains(t, err, "invalid filter") + + createFlag = TriggerUpdateFlags{ + Filters: filterArray{"=value"}, + } + _, err = createFlag.GetFilters() + assert.ErrorContains(t, err, "invalid filter") + + createFlag = TriggerUpdateFlags{ + Filters: filterArray{"="}, + } + _, err = createFlag.GetFilters() + assert.ErrorContains(t, err, "invalid filter") + }) } diff --git a/pkg/kn/commands/trigger/update_test.go b/pkg/kn/commands/trigger/update_test.go new file mode 100644 index 0000000000..0e98315afa --- /dev/null +++ b/pkg/kn/commands/trigger/update_test.go @@ -0,0 +1,15 @@ +// Copyright © 2019 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 trigger From 6bb9c0d1fbec63822dd079b78e472a3f05480216 Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Tue, 17 Dec 2019 19:42:50 +0800 Subject: [PATCH 2/2] add trigger update test --- docs/cmd/kn_trigger.md | 1 + docs/cmd/kn_trigger_update.md | 49 +++++++++++++++ pkg/eventing/v1alpha1/client.go | 48 ++++++++++++-- pkg/eventing/v1alpha1/client_mock.go | 11 ++++ pkg/eventing/v1alpha1/client_mock_test.go | 2 + pkg/eventing/v1alpha1/client_test.go | 66 +++++++++++++++----- pkg/kn/commands/trigger/trigger.go | 3 +- pkg/kn/commands/trigger/update.go | 26 +++++--- pkg/kn/commands/trigger/update_flags.go | 18 ++++++ pkg/kn/commands/trigger/update_flags_test.go | 44 ++++++++++++- pkg/kn/commands/trigger/update_test.go | 64 +++++++++++++++++++ 11 files changed, 301 insertions(+), 31 deletions(-) create mode 100644 docs/cmd/kn_trigger_update.md diff --git a/docs/cmd/kn_trigger.md b/docs/cmd/kn_trigger.md index 77f60d901d..b484a4de51 100644 --- a/docs/cmd/kn_trigger.md +++ b/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 diff --git a/docs/cmd/kn_trigger_update.md b/docs/cmd/kn_trigger_update.md new file mode 100644 index 0000000000..4980eef54d --- /dev/null +++ b/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 'knative.dev.bar' in a trigger 'mytrigger' + kn trigger update mytrigger --filter type=knative.dev.bar + + # Remove 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 + diff --git a/pkg/eventing/v1alpha1/client.go b/pkg/eventing/v1alpha1/client.go index b1b7eba76b..a4296d34bb 100644 --- a/pkg/eventing/v1alpha1/client.go +++ b/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,7 +151,7 @@ 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 @@ -147,10 +159,9 @@ func (b *TriggerBuilder) Broker(broker string) *TriggerBuilder { 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, @@ -158,3 +169,32 @@ func (b *TriggerBuilder) Filter(filters map[string]string) *TriggerBuilder { } 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 +} diff --git a/pkg/eventing/v1alpha1/client_mock.go b/pkg/eventing/v1alpha1/client_mock.go index 9fcb3b9ae7..cf3166e0f6 100644 --- a/pkg/eventing/v1alpha1/client_mock.go +++ b/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() diff --git a/pkg/eventing/v1alpha1/client_mock_test.go b/pkg/eventing/v1alpha1/client_mock_test.go index ffe746d638..129e4f7eec 100644 --- a/pkg/eventing/v1alpha1/client_mock_test.go +++ b/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() diff --git a/pkg/eventing/v1alpha1/client_test.go b/pkg/eventing/v1alpha1/client_test.go index 7f658037e5..efc38933f5 100644 --- a/pkg/eventing/v1alpha1/client_test.go +++ b/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() } diff --git a/pkg/kn/commands/trigger/trigger.go b/pkg/kn/commands/trigger/trigger.go index e5158a2bad..d6dfd9de69 100644 --- a/pkg/kn/commands/trigger/trigger.go +++ b/pkg/kn/commands/trigger/trigger.go @@ -27,8 +27,9 @@ func NewTriggerCommand(p *commands.KnParams) *cobra.Command { Short: "Trigger command group", } triggerCmd.AddCommand(NewTriggerCreateCommand(p)) - triggerCmd.AddCommand(NewTriggerDeleteCommand(p)) + triggerCmd.AddCommand(NewTriggerUpdateCommand(p)) triggerCmd.AddCommand(NewTriggerDescribeCommand(p)) triggerCmd.AddCommand(NewTriggerListCommand(p)) + triggerCmd.AddCommand(NewTriggerDeleteCommand(p)) return triggerCmd } diff --git a/pkg/kn/commands/trigger/update.go b/pkg/kn/commands/trigger/update.go index 246d03c1e0..0b60a0d733 100644 --- a/pkg/kn/commands/trigger/update.go +++ b/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 'knative.dev.bar' in a trigger 'mytrigger' kn trigger update mytrigger --filter type=knative.dev.bar + # Remove 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,26 @@ 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 { diff --git a/pkg/kn/commands/trigger/update_flags.go b/pkg/kn/commands/trigger/update_flags.go index 1cec4b01f9..a8847ed282 100644 --- a/pkg/kn/commands/trigger/update_flags.go +++ b/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.") diff --git a/pkg/kn/commands/trigger/update_flags_test.go b/pkg/kn/commands/trigger/update_flags_test.go index 18e6cf9e84..d383d815e6 100644 --- a/pkg/kn/commands/trigger/update_flags_test.go +++ b/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) + }) +} diff --git a/pkg/kn/commands/trigger/update_test.go b/pkg/kn/commands/trigger/update_test.go index 0e98315afa..0fba1db8a0 100644 --- a/pkg/kn/commands/trigger/update_test.go +++ b/pkg/kn/commands/trigger/update_test.go @@ -13,3 +13,67 @@ // limitations under the License. package trigger + +import ( + "fmt" + "testing" + + "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + eventing_client "knative.dev/client/pkg/eventing/v1alpha1" + knserving_client "knative.dev/client/pkg/serving/v1alpha1" + "knative.dev/client/pkg/util" + serving_v1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" +) + +func TestTriggerUpdate(t *testing.T) { + eventingClient := eventing_client.NewMockKnEventingClient(t) + servingClient := knserving_client.NewMockKnServiceClient(t) + + servingRecorder := servingClient.Recorder() + servingRecorder.GetService("mysvc", &serving_v1alpha1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Name: "mysvc"}, + }, nil) + + eventingRecorder := eventingClient.Recorder() + present := createTrigger("default", triggerName, map[string]string{"type": "dev.knative.foo"}, "mybroker", "mysvc") + updated := createTrigger("default", triggerName, map[string]string{"type": "dev.knative.new"}, "mybroker", "mysvc") + eventingRecorder.GetTrigger(triggerName, present, nil) + eventingRecorder.UpdateTrigger(updated, nil) + + out, err := executeTriggerCommand(eventingClient, servingClient, "update", triggerName, + "--filter", "type=dev.knative.new", "--sink", "svc:mysvc") + assert.NilError(t, err, "Trigger should be updated") + util.ContainsAll(out, "Trigger", triggerName, "updated", "namespace", "default") + + eventingRecorder.Validate() + servingRecorder.Validate() +} + +func TestTriggerUpdateWithError(t *testing.T) { + eventingClient := eventing_client.NewMockKnEventingClient(t) + eventingRecorder := eventingClient.Recorder() + eventingRecorder.GetTrigger(triggerName, nil, fmt.Errorf("trigger not found")) + + out, err := executeTriggerCommand(eventingClient, nil, "update", triggerName, + "--filter", "type=dev.knative.new", "--sink", "svc:newsvc") + assert.ErrorContains(t, err, "trigger not found") + util.ContainsAll(out, "Usage", triggerName) + + eventingRecorder.Validate() +} + +func TestTriggerUpdateInvalidBroker(t *testing.T) { + eventingClient := eventing_client.NewMockKnEventingClient(t) + eventingRecorder := eventingClient.Recorder() + present := createTrigger("default", triggerName, map[string]string{"type": "dev.knative.new"}, "mybroker", "newsvc") + eventingRecorder.GetTrigger(triggerName, present, nil) + + out, err := executeTriggerCommand(eventingClient, nil, "update", triggerName, + "--broker", "newbroker") + assert.ErrorContains(t, err, "broker is immutable") + util.ContainsAll(out, "Usage", triggerName) + + eventingRecorder.Validate() +}