Repository: knative/client (go)

Issue:
kn trigger create fails without --filter flag
<!-- If you need to report a security issue with Knative, send an email to knative-security@googlegroups.com. -->

### Bug report

There was a bug in Eventing 0.13.0 that rejected triggers without filters. However, even on version `0.13.1` the creation of such trigger will fail. There's a bug in our current impl that will initialize empty `trigger.Spec.Filter.Attributes` regardless of no flag provided that causes the error behaviour. 

Beware, that `kn trigger describe` has to be fixed for empty filters also. 

```bash
$ kn trigger create test --broker default --sink svc:svc1
cannot create trigger 'test' in namespace 'default' because: admission webhook "validation.webhook.eventing.knative.dev" denied the request: validation failed: At least one filtered attribute must be specified: spec.filter.attributes
```

### Expected behavior

Trigger can be created without `--filter` flag.

### Steps to reproduce the problem

1. kn trigger create test --broker default --sink svc:svc1


<!-- How can a maintainer reproduce this issue (please be detailed) -->

### kn version

HEAD 

### Knative (serving/eventing) version

0.13.1

/kind good-first-issue


Pull request:
fix(trigger): Make --filter flag truly optional
## Description

Fix issue with `kn trigger create` that always required `--filter` flag.

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

* `TriggerBuilder` adds only empty Spec.Filter for empty or nil arrays
  * The above should also allow builder to be used to clean up existing filters
* `kn trigger describe` doesn't print empty Filter

## Reference

<!-- Please add the corresponding issue number which this pull request is about to fix -->
Fixes #744 

<!--
Please add an entrty to CHANGELOG.adoc file, too, as part of your Pull Request.

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 New feature
- 🐛 Bug fix
- ✨ Feature Update
- 🐣 Refactoring
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example for how to add the entry, including a reference to the corresponding issue/PR

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->

<!--
To automatically lint go code in this pull request uncomment the line belpow. You get feedback as comments on your pull request then -->

<!--
/lint
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 4872db20..358ab7b6 100644
--- 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
index 62510d41..d480786b 100644
--- 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
index 49bee43e..2e6c910a 100644
--- 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/kn/commands/trigger/create.go b/pkg/kn/commands/trigger/create.go
index ecf0523f..d7ab59aa 100644
--- 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
index 6b56ebd8..f8083115 100644
--- 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)
+		}
 	}
 }


Test patch:
diff --git a/pkg/eventing/v1alpha1/client_test.go b/pkg/eventing/v1alpha1/client_test.go
index 351e6fc2..419824f9 100644
--- 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/test/e2e/trigger_test.go b/test/e2e/trigger_test.go
index 55c16867..3aaf445a 100644
--- 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)