Repository: knative/client
Pull request #726 (2020-03-10 11:00:19)

feat(trigger): add --inject-broker flag to trigger create command

## Description

Allow user to add default broker when creating trigger.

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

* Add flag `--inject-broker` to trigger create commnad


knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/726#" title="Author self-approved">dsimansk</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mattmoor**
You can assign the PR to them by writing `/assign @mattmoor` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative/client/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["mattmoor"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/eventing/v1alpha1/client.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/726/pull-knative-client-go-coverage/1237332570576785409/artifacts/line-cov.html#file0) | 81.5% | 82.1% | 0.7

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/eventing/v1alpha1/client.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/726/pull-knative-client-go-coverage/1237365035920003074/artifacts/line-cov.html#file0) | 81.5% | 80.4% | -1.1

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/eventing/v1alpha1/client.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/726/pull-knative-client-go-coverage/1237416375115321345/artifacts/line-cov.html#file0) | 81.5% | 82.1% | 0.7
[pkg/kn/commands/trigger/create.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/726/pull-knative-client-go-coverage/1237416375115321345/artifacts/line-cov.html#file1) | 81.2% | 82.4% | 1.1


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 46bee13d..b0fdb78b 100644
--- a/CHANGELOG.adoc
+++ b/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)
diff --git a/docs/cmd/kn_service_export.md b/docs/cmd/kn_service_export.md
index f9491301..5c2b7266 100644
--- a/docs/cmd/kn_service_export.md
+++ b/docs/cmd/kn_service_export.md
@@ -25,10 +25,10 @@ kn service export NAME [flags]
 ```
       --allow-missing-template-keys   If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
   -h, --help                          help for export
-  -r, --history                       Export all active revisions
   -n, --namespace string              Specify the namespace to operate in.
   -o, --output string                 Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file.
       --template string               Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
+      --with-revisions                Export all routed revisions (experimental)
 ```
 
 ### Options inherited from parent commands
diff --git a/docs/cmd/kn_trigger_create.md b/docs/cmd/kn_trigger_create.md
index f0211d7c..62510d41 100644
--- a/docs/cmd/kn_trigger_create.md
+++ b/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
 ```
diff --git a/docs/cmd/kn_trigger_update.md b/docs/cmd/kn_trigger_update.md
index 3ac46d6b..4dab1adb 100644
--- a/docs/cmd/kn_trigger_update.md
+++ b/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
 ```
diff --git a/pkg/eventing/v1alpha1/client.go b/pkg/eventing/v1alpha1/client.go
index de62a9c5..49bee43e 100644
--- a/pkg/eventing/v1alpha1/client.go
+++ b/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
 }
 
diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go
index a2cf1aa7..1b34abb9 100644
--- a/pkg/kn/commands/service/export.go
+++ b/pkg/kn/commands/service/export.go
@@ -70,7 +70,7 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			history, err := cmd.Flags().GetBool("history")
+			withRevisions, err := cmd.Flags().GetBool("with-revisions")
 			if err != nil {
 				return err
 			}
@@ -80,8 +80,8 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			if history {
-				if svcList, err := exportServicewithActiveRevisions(service, client); err != nil {
+			if withRevisions {
+				if svcList, err := exportServiceWithActiveRevisions(service, client); err != nil {
 					return err
 				} else {
 					return printer.PrintObj(svcList, cmd.OutOrStdout())
@@ -92,7 +92,7 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command {
 	}
 	flags := command.Flags()
 	commands.AddNamespaceFlags(flags, false)
-	flags.BoolP("history", "r", false, "Export all active revisions")
+	flags.Bool("with-revisions", false, "Export all routed revisions (experimental)")
 	machineReadablePrintFlags.AddFlags(command)
 	return command
 }
@@ -135,22 +135,19 @@ func constructServicefromRevision(latestSvc *servingv1.Service, revision serving
 	return exportedSvc
 }
 
-func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) {
+func exportServiceWithActiveRevisions(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) {
 	var exportedSvcItems []servingv1.Service
 
 	//get revisions to export from traffic
 	revsMap := getRevisionstoExport(latestSvc)
 
-	var params []clientservingv1.ListConfig
-	params = append(params, clientservingv1.WithService(latestSvc.ObjectMeta.Name))
-
 	// Query for list with filters
-	revisionList, err := client.ListRevisions(params...)
+	revisionList, err := client.ListRevisions(clientservingv1.WithService(latestSvc.ObjectMeta.Name))
 	if err != nil {
 		return nil, err
 	}
 	if len(revisionList.Items) == 0 {
-		return nil, fmt.Errorf("No revisions found for the service %s", latestSvc.ObjectMeta.Name)
+		return nil, fmt.Errorf("no revisions found for the service %s", latestSvc.ObjectMeta.Name)
 	}
 
 	// sort revisions to main the order of generations
@@ -163,6 +160,10 @@ func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clien
 		}
 	}
 
+	if len(exportedSvcItems) == 0 {
+		return nil, fmt.Errorf("no revisions found for service %s", latestSvc.ObjectMeta.Name)
+	}
+
 	//set traffic in the latest revision
 	exportedSvcItems[len(exportedSvcItems)-1] = setTrafficSplit(latestSvc, exportedSvcItems[len(exportedSvcItems)-1])
 
@@ -181,7 +182,6 @@ func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clien
 func setTrafficSplit(latestSvc *servingv1.Service, exportedSvc servingv1.Service) servingv1.Service {
 
 	exportedSvc.Spec.RouteSpec = latestSvc.Spec.RouteSpec
-
 	return exportedSvc
 }
 
diff --git a/pkg/kn/commands/trigger/create.go b/pkg/kn/commands/trigger/create.go
index 065d3b91..ecf0523f 100644
--- a/pkg/kn/commands/trigger/create.go
+++ b/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 {
diff --git a/pkg/kn/commands/trigger/update_flags.go b/pkg/kn/commands/trigger/update_flags.go
index 061f2836..0825c487 100644
--- a/pkg/kn/commands/trigger/update_flags.go
+++ b/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 patch:
diff --git a/pkg/eventing/v1alpha1/client_test.go b/pkg/eventing/v1alpha1/client_test.go
index ce2ef5ad..351e6fc2 100644
--- a/pkg/eventing/v1alpha1/client_test.go
+++ 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 {
diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go
index 6b9be103..59662281 100644
--- a/pkg/kn/commands/service/export_test.go
+++ b/pkg/kn/commands/service/export_test.go
@@ -171,7 +171,7 @@ func callServiceExportHistoryTest(t *testing.T, expectedService *servingv1.Servi
 
 	r.ListRevisions(mock.Any(), revs, nil)
 
-	output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "-r", "-o", "json")
+	output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "--with-revisions", "-o", "json")
 
 	assert.NilError(t, err)
 
diff --git a/pkg/kn/commands/trigger/create_test.go b/pkg/kn/commands/trigger/create_test.go
index 102c66f3..bc715006 100644
--- a/pkg/kn/commands/trigger/create_test.go
+++ b/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")
diff --git a/pkg/kn/commands/trigger/trigger_test.go b/pkg/kn/commands/trigger/trigger_test.go
index 89e5dff4..7cda60ad 100644
--- a/pkg/kn/commands/trigger/trigger_test.go
+++ b/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{
diff --git a/test/e2e/service_options_test.go b/test/e2e/service_options_test.go
index 6d26efd4..011ac7d1 100644
--- a/test/e2e/service_options_test.go
+++ b/test/e2e/service_options_test.go
@@ -18,11 +18,16 @@
 package e2e
 
 import (
+	"encoding/json"
 	"fmt"
+	"os"
+	"strconv"
+	"strings"
 	"testing"
 
 	"gotest.tools/assert"
 	"knative.dev/client/pkg/util"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )
 
 func TestServiceOptions(t *testing.T) {
@@ -93,10 +98,15 @@ func TestServiceOptions(t *testing.T) {
 	test.validateContainerField(t, r, "svc5", "args", "[myArg1]")
 
 	t.Log("create, update and validate service with user defined")
-	test.serviceCreateWithOptions(t, r, "svc6", "--user", "1001")
-	test.validateContainerField(t, r, "svc6", "securityContext.runAsUser", "1001")
-	test.serviceUpdate(t, r, "svc6", "--user", "1002")
-	test.validateContainerField(t, r, "svc6", "securityContext.runAsUser", "1002")
+	var uid int64 = 1000
+	if uids, ok := os.LookupEnv("TEST_RUN_AS_UID"); ok {
+		uid, err = strconv.ParseInt(uids, 10, 64)
+		assert.NilError(t, err)
+	}
+	test.serviceCreateWithOptions(t, r, "svc6", "--user", strconv.FormatInt(uid, 10))
+	test.validateUserId(t, r, "svc6", uid)
+	test.serviceUpdate(t, r, "svc6", "--user", strconv.FormatInt(uid+1, 10))
+	test.validateUserId(t, r, "svc6", uid+1)
 }
 
 func (test *e2eTest) serviceCreateWithOptions(t *testing.T, r *KnRunResultCollector, serviceName string, options ...string) {
@@ -163,3 +173,13 @@ func (test *e2eTest) validateContainerField(t *testing.T, r *KnRunResultCollecto
 	assert.Equal(t, out.Stdout, expected)
 	r.AssertNoError(out)
 }
+
+func (test *e2eTest) validateUserId(t *testing.T, r *KnRunResultCollector, serviceName string, uid int64) {
+	out := test.kn.Run("service", "describe", serviceName, "-ojson")
+	data := json.NewDecoder(strings.NewReader(out.Stdout))
+	data.UseNumber()
+	var service servingv1.Service
+	err := data.Decode(&service)
+	assert.NilError(t, err)
+	assert.Equal(t, *service.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser, uid)
+}
diff --git a/test/e2e/trigger_inject_broker_test.go b/test/e2e/trigger_inject_broker_test.go
new file mode 100644
index 00000000..234d9661
--- /dev/null
+++ b/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))
+}