fix(trigger): Make --filter flag truly optional (#745)

* fix(trigger): Make --filter flag truly optional * fix(trigger): Update trigger docs * chore: Update changelog
knative · Mar 18, 2020 · 58ee9c4 · 58ee9c4
1 parent a858a25
commit 58ee9c4
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 10 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -12,6 +12,16 @@
 | https://github.com/knative/client/pull/[#]
 ////
 
+## v0.14.0 (unreleased)
+
+[cols="1,10,3", options="header", width="100%"]
+|===
+| | Description | PR
+
+| 🐛
+| Fix trigger create --filter flag to be optional
+| https://github.com/knative/client/pull/735[#745]
+
 ## v0.13.0 (2020-03-11)
 
 [cols="1,10,3", options="header", width="100%"]

diff --git a/docs/cmd/kn_trigger_create.md b/docs/cmd/kn_trigger_create.md
@@ -7,14 +7,17 @@ Create a trigger
 Create a trigger
 
 ```
-kn trigger create NAME --broker BROKER --filter KEY=VALUE --sink SINK [flags]
+kn trigger create NAME --broker BROKER --sink SINK [flags]
 ```
 
 ### Examples
 
 ```
 
-  # Create a trigger 'mytrigger' to declare a subscription to events with attribute 'type=dev.knative.foo' from default broker. The subscriber is service 'mysvc'
+  # Create a trigger 'mytrigger' to declare a subscription to events from default broker. The subscriber is service 'mysvc'
+  kn trigger create mytrigger --broker default --sink svc:mysvc
+
+  # Create a trigger to filter events with attribute 'type=dev.knative.foo'
   kn trigger create mytrigger --broker default --filter type=dev.knative.foo --sink svc:mysvc
 ```
 

diff --git a/pkg/eventing/v1alpha1/client.go b/pkg/eventing/v1alpha1/client.go
@@ -180,6 +180,10 @@ func (b *TriggerBuilder) InjectBroker(inject bool) *TriggerBuilder {
 }
 
 func (b *TriggerBuilder) Filters(filters map[string]string) *TriggerBuilder {
+	if len(filters) == 0 {
+		b.trigger.Spec.Filter = &v1alpha1.TriggerFilter{}
+		return b
+	}
 	filter := b.trigger.Spec.Filter
 	if filter == nil {
 		filter = &v1alpha1.TriggerFilter{}

diff --git a/pkg/eventing/v1alpha1/client_test.go b/pkg/eventing/v1alpha1/client_test.go
@@ -155,6 +155,17 @@ func TestTriggerBuilder(t *testing.T) {
 		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
 	})
 
+	t.Run("update filters to remove filters", func(t *testing.T) {
+		b := NewTriggerBuilderFromExisting(a.Build())
+		assert.DeepEqual(t, b.Build(), a.Build())
+		b.Filters(nil)
+		expected := &v1alpha1.TriggerFilter{}
+		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
+
+		b.Filters((make(map[string]string)))
+		assert.DeepEqual(t, expected, b.Build().Spec.Filter)
+	})
+
 	t.Run("add and remove inject annotation", func(t *testing.T) {
 		b := NewTriggerBuilder("broker-trigger")
 		b.InjectBroker(true)

diff --git a/pkg/kn/commands/trigger/create.go b/pkg/kn/commands/trigger/create.go
@@ -33,10 +33,13 @@ func NewTriggerCreateCommand(p *commands.KnParams) *cobra.Command {
 	var sinkFlags flags.SinkFlags
 
 	cmd := &cobra.Command{
-		Use:   "create NAME --broker BROKER --filter KEY=VALUE --sink SINK",
+		Use:   "create NAME --broker BROKER --sink SINK",
 		Short: "Create a trigger",
 		Example: `
-  # Create a trigger 'mytrigger' to declare a subscription to events with attribute 'type=dev.knative.foo' from default broker. The subscriber is service 'mysvc'
+  # Create a trigger 'mytrigger' to declare a subscription to events from default broker. The subscriber is service 'mysvc'
+  kn trigger create mytrigger --broker default --sink svc:mysvc
+
+  # Create a trigger to filter events with attribute 'type=dev.knative.foo'
   kn trigger create mytrigger --broker default --filter type=dev.knative.foo --sink svc:mysvc`,
 
 		RunE: func(cmd *cobra.Command, args []string) (err error) {

diff --git a/pkg/kn/commands/trigger/describe.go b/pkg/kn/commands/trigger/describe.go
@@ -112,8 +112,10 @@ func writeSink(dw printers.PrefixWriter, sink *duckv1.Destination) {
 func writeTrigger(dw printers.PrefixWriter, trigger *v1alpha1.Trigger, printDetails bool) {
 	commands.WriteMetadata(dw, &trigger.ObjectMeta, printDetails)
 	dw.WriteAttribute("Broker", trigger.Spec.Broker)
-	subWriter := dw.WriteAttribute("Filter", "")
-	for key, value := range *trigger.Spec.Filter.Attributes {
-		subWriter.WriteAttribute(key, value)
+	if trigger.Spec.Filter != nil && trigger.Spec.Filter.Attributes != nil {
+		subWriter := dw.WriteAttribute("Filter", "")
+		for key, value := range *trigger.Spec.Filter.Attributes {
+			subWriter.WriteAttribute(key, value)
+		}
 	}
 }
diff --git a/test/e2e/trigger_test.go b/test/e2e/trigger_test.go
@@ -46,7 +46,7 @@ func TestBrokerTrigger(t *testing.T) {
 	test.serviceCreate(t, r, "sinksvc1")
 
 	t.Log("create triggers and list them")
-	test.triggerCreate(t, r, "trigger1", "sinksvc0", []string{"a=b"})
+	test.triggerCreate(t, r, "trigger1", "sinksvc0", nil)
 	test.triggerCreate(t, r, "trigger2", "sinksvc1", []string{"type=knative.dev.bar", "source=ping"})
 	test.verifyTriggerList(t, r, "trigger1", "trigger2")
 	test.triggerDelete(t, r, "trigger1")
@@ -93,8 +93,10 @@ func (test *e2eTest) lableNamespaceForDefaultBroker(t *testing.T) error {
 
 func (test *e2eTest) triggerCreate(t *testing.T, r *KnRunResultCollector, name string, sinksvc string, filters []string) {
 	args := []string{"trigger", "create", name, "--broker", "default", "--sink", "svc:" + sinksvc}
-	for _, v := range filters {
-		args = append(args, "--filter", v)
+	if len(filters) > 0 {
+		for _, v := range filters {
+			args = append(args, "--filter", v)
+		}
 	}
 	out := test.kn.Run(args...)
 	r.AssertNoError(out)