feat(trigger): add --inject-broker flag to trigger create command (#726)

* feat(trigger): annotate trigger to inject default broker

* chore: update changelog

* fix: reflect review feedback

* fix: code comment wording

* fix: reflect review feedback

CHANGELOG.adoc

@@ -89,6 +89,10 @@
 | 🎁
 | add kn service export command for exporting a service
 | https://github.com/knative/client/pull/669[#669]
+
+| 🎁
+| Add flag --inject-broker to trigger create command
+| https://github.com/knative/client/pull/726[#726]
 |===
 
 ## v0.12.0 (2020-01-29)

docs/cmd/kn_trigger_create.md

@@ -24,6 +24,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 strings     Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
   -h, --help               help for create
+      --inject-broker      Create new broker with name default through common annotation
   -n, --namespace string   Specify the namespace to operate in.
   -s, --sink string        Addressable sink for events
 ```

docs/cmd/kn_trigger_update.md

@@ -31,6 +31,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 strings     Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo
   -h, --help               help for update
+      --inject-broker      Create new broker with name default through common annotation
   -n, --namespace string   Specify the namespace to operate in.
   -s, --sink string        Addressable sink for events
 ```

pkg/eventing/v1alpha1/client.go

@@ -163,6 +163,19 @@ func (b *TriggerBuilder) Subscriber(subscriber *duckv1.Destination) *TriggerBuil
 // Broker to set the broker of trigger object
 func (b *TriggerBuilder) Broker(broker string) *TriggerBuilder {
 	b.trigger.Spec.Broker = broker
+
+	return b
+}
+
+// InjectBroker to add annotation to setup default broker
+func (b *TriggerBuilder) InjectBroker(inject bool) *TriggerBuilder {
+	if inject {
+		meta_v1.SetMetaDataAnnotation(&b.trigger.ObjectMeta, v1alpha1.InjectionAnnotation, "enabled")
+	} else {
+		if meta_v1.HasAnnotation(b.trigger.ObjectMeta, v1alpha1.InjectionAnnotation) {
+			delete(b.trigger.ObjectMeta.Annotations, v1alpha1.InjectionAnnotation)
+		}
+	}
 	return b
 }
 

pkg/eventing/v1alpha1/client_test.go

@@ -154,6 +154,23 @@ func TestTriggerBuilder(t *testing.T) {
 		}
 		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)
+		expected := &metav1.ObjectMeta{
+			Annotations: map[string]string{
+				v1alpha1.InjectionAnnotation: "enabled",
+			},
+		}
+		assert.DeepEqual(t, expected.Annotations, b.Build().ObjectMeta.Annotations)
+
+		b = NewTriggerBuilderFromExisting(b.Build())
+		b.InjectBroker(false)
+		assert.DeepEqual(t, make(map[string]string), b.Build().ObjectMeta.Annotations)
+
+	})
+
 }
 
 func newTrigger(name string) *v1alpha1.Trigger {

pkg/kn/commands/trigger/create.go

@@ -83,6 +83,15 @@ func NewTriggerCreateCommand(p *commands.KnParams) *cobra.Command {
 					Ref: objectRef.Ref,
 					URI: objectRef.URI,
 				})
+			// add inject annotation only if flag broker name is `default`
+			if triggerUpdateFlags.InjectBroker {
+				if triggerUpdateFlags.Broker == "default" {
+					triggerBuilder.InjectBroker(true)
+				} else {
+					return fmt.Errorf("cannot create trigger '%s' in namespace '%s' "+
+						"because broker name must be 'default' if '--inject-broker' flag is used", name, namespace)
+				}
+			}
 
 			err = eventingClient.CreateTrigger(triggerBuilder.Build())
 			if err != nil {

pkg/kn/commands/trigger/create_test.go

@@ -49,6 +49,36 @@ func TestTriggerCreate(t *testing.T) {
 	eventingRecorder.Validate()
 }
 
+func TestTriggerWithInjectCreate(t *testing.T) {
+	eventingClient := clienteventingv1alpha1.NewMockKnEventingClient(t)
+	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", &servingv1.Service{
+		TypeMeta:   metav1.TypeMeta{Kind: "Service", APIVersion: "serving.knative.dev/v1"},
+		ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "default"},
+	})
+
+	eventingRecorder := eventingClient.Recorder()
+	eventingRecorder.CreateTrigger(createTriggerWithInject("default", triggerName, map[string]string{"type": "dev.knative.foo"}, "default", "mysvc"), nil)
+
+	out, err := executeTriggerCommand(eventingClient, dynamicClient, "create", triggerName, "--broker", "default", "--inject-broker",
+		"--filter", "type=dev.knative.foo", "--sink", "svc:mysvc")
+	assert.NilError(t, err, "Trigger should be created")
+	util.ContainsAll(out, "Trigger", triggerName, "created", "namespace", "default")
+
+	eventingRecorder.Validate()
+}
+
+func TestTriggetWithInjecError(t *testing.T) {
+	eventingClient := clienteventingv1alpha1.NewMockKnEventingClient(t)
+	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", &servingv1.Service{
+		TypeMeta:   metav1.TypeMeta{Kind: "Service", APIVersion: "serving.knative.dev/v1"},
+		ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "default"},
+	})
+
+	_, err := executeTriggerCommand(eventingClient, dynamicClient, "create", triggerName, "--broker", "mybroker", "--inject-broker",
+		"--filter", "type=dev.knative.foo", "--sink", "svc:mysvc")
+	assert.ErrorContains(t, err, "broker", "name", "'default'", "--inject-broker", "flag")
+}
+
 func TestSinkNotFoundError(t *testing.T) {
 	eventingClient := clienteventingv1alpha1.NewMockKnEventingClient(t)
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default")

pkg/kn/commands/trigger/trigger_test.go

@@ -84,6 +84,11 @@ func createTrigger(namespace string, name string, filters map[string]string, bro
 		Build()
 }
 
+func createTriggerWithInject(namespace string, name string, filters map[string]string, broker string, svcname string) *v1alpha1.Trigger {
+	t := createTrigger(namespace, name, filters, broker, svcname)
+	return eventc_v1alpha1.NewTriggerBuilderFromExisting(t).InjectBroker(true).Build()
+}
+
 func createTriggerWithStatus(namespace string, name string, filters map[string]string, broker string, svcname string) *v1alpha1.Trigger {
 	wanted := createTrigger(namespace, name, filters, broker, svcname)
 	wanted.Status = v1alpha1.TriggerStatus{

pkg/kn/commands/trigger/update_flags.go

@@ -23,8 +23,9 @@ import (
 
 // TriggerUpdateFlags are flags for create and update a trigger
 type TriggerUpdateFlags struct {
-	Broker  string
-	Filters []string
+	Broker       string
+	InjectBroker bool
+	Filters      []string
 }
 
 // GetFilters to return a map type of filters
@@ -49,5 +50,6 @@ 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().BoolVar(&f.InjectBroker, "inject-broker", false, "Create new broker with name default through common annotation")
 	cmd.Flags().StringSliceVar(&f.Filters, "filter", nil, "Key-value pair for exact CloudEvent attribute matching against incoming events, e.g type=dev.knative.foo")
 }

test/e2e/trigger_inject_broker_test.go

@@ -0,0 +1,65 @@
+// Copyright 2020 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 im
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build e2e
+// +build !serving
+
+package e2e
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	"knative.dev/client/pkg/util"
+)
+
+func TestInjectBrokerTrigger(t *testing.T) {
+	t.Parallel()
+	test, err := NewE2eTest()
+	assert.NilError(t, err)
+	defer func() {
+		assert.NilError(t, test.Teardown())
+	}()
+
+	r := NewKnRunResultCollector(t)
+	defer r.DumpIfFailed()
+
+	assert.NilError(t, err)
+
+	test.serviceCreate(t, r, "sinksvc0")
+	test.serviceCreate(t, r, "sinksvc1")
+
+	t.Log("create triggers and list them")
+	test.triggerCreateWithInject(t, r, "trigger1", "sinksvc0", []string{"a=b"})
+	test.triggerCreateWithInject(t, r, "trigger2", "sinksvc1", []string{"type=knative.dev.bar", "source=ping"})
+	test.verifyTriggerList(t, r, "trigger1", "trigger2")
+	test.triggerDelete(t, r, "trigger1")
+	test.triggerDelete(t, r, "trigger2")
+
+	t.Log("create trigger with error")
+	out := test.kn.Run("trigger", "create", "errorTrigger", "--broker", "mybroker", "--inject-broker",
+		"--sink", "svc:sinksvc0", "--filter", "a=b")
+	r.AssertError(out)
+	assert.Check(t, util.ContainsAllIgnoreCase(out.Stderr, "broker", "name", "'default'", "--inject-broker", "flag"))
+}
+
+func (test *e2eTest) triggerCreateWithInject(t *testing.T, r *KnRunResultCollector, name string, sinksvc string, filters []string) {
+	args := []string{"trigger", "create", name, "--broker", "default", "--inject-broker", "--sink", "svc:" + sinksvc}
+	for _, v := range filters {
+		args = append(args, "--filter", v)
+	}
+	out := test.kn.Run(args...)
+	r.AssertNoError(out)
+	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "Trigger", name, "created", "namespace", test.kn.namespace))
+}