Repository: knative/client (go)

Issue:
Does not reject multiple `--image` parameters
## What version of Knative Client?

> Paste output of 'kn version'

```
$ kn version
Version:      v0.10.0
Build Date:   2019-11-07 15:02:21
Git Revision: 22c65d6
Supported APIs:
- serving.knative.dev/v1alpha1 (knative-serving v0.10.0)
```

## What version of Knative Serving running on your cluster?

0.10

## Expected Behavior

When I type `kn update service --image example.com/first --image example.com/second`, I expect to be told that I cannot provide two images.

## Actual Behavior

`kn` accepts the command line without error or warning. It applies the last-provided argument. In the example above this means that the updated service has `example.com/second` as its image.

## Steps to Reproduce the Problem

1. Create a service.
1. Update the service, using two different `--image` parameters.
1. Observe that no error occurs.
1. Use describe to observe that second parameter is applied.

Pull request:
Fix --image flag to only allow single occurence
<!--
Request Prow to automatically lint any go code in this PR:

/lint
-->

Fixes #533 

## Proposed Changes

* Introduce new type `singletonString` for flag values that only allow single value assignment, otherwise return error
* Fix `--image` flag with the new type.

Example:
```bash
➜  client git:(issue-533) kn service create svc-test --image gcr.io/knative-samples/helloworld-go --image gcr.io/foo/bar:barz --no-wait 
invalid argument "gcr.io/foo/bar:barz" for "--image" flag: value is already set
```

<!--
Release Note:

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 Add new feature
- 🐛 Fix bug
- 🧽 Update or clean up current behaviour
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example.

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.
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 1d9d0a44..fd138675 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -45,6 +45,10 @@
 | remove unecessary `sync` parameter in setup call
 | https://github.com/knative/client/pull/656[#656]
 
+| 🐛
+| Fix `--image` flag to only allow single occurence
+| https://github.com/knative/client/pull/647[#647]
+
 ## v0.12.0 (2020-01-29)
 
 [cols="1,10,3", options="header", width="100%"]
diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go
index 25753a85..8731eec0 100644
--- a/pkg/kn/commands/service/configuration_edit_flags.go
+++ b/pkg/kn/commands/service/configuration_edit_flags.go
@@ -30,7 +30,7 @@ import (
 
 type ConfigurationEditFlags struct {
 	// Direct field manipulation
-	Image   string
+	Image   uniqueStringArg
 	Env     []string
 	EnvFrom []string
 	Mount   []string
@@ -67,6 +67,25 @@ type ResourceFlags struct {
 	Memory string
 }
 
+// -- uniqueStringArg Value
+// Custom implementation of flag.Value interface to prevent multiple value assignment.
+// Useful to enforce unique use of flags, e.g. --image.
+type uniqueStringArg string
+
+func (s *uniqueStringArg) Set(val string) error {
+	if len(*s) > 0 {
+		return errors.New("can be provided only once")
+	}
+	*s = uniqueStringArg(val)
+	return nil
+}
+
+func (s *uniqueStringArg) Type() string {
+	return "string"
+}
+
+func (s *uniqueStringArg) String() string { return string(*s) }
+
 // markFlagMakesRevision indicates that a flag will create a new revision if you
 // set it.
 func (p *ConfigurationEditFlags) markFlagMakesRevision(f string) {
@@ -75,7 +94,7 @@ func (p *ConfigurationEditFlags) markFlagMakesRevision(f string) {
 
 // addSharedFlags adds the flags common between create & update.
 func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
-	command.Flags().StringVar(&p.Image, "image", "", "Image to run.")
+	command.Flags().VarP(&p.Image, "image", "", "Image to run.")
 	p.markFlagMakesRevision("image")
 	command.Flags().StringArrayVarP(&p.Env, "env", "e", []string{},
 		"Environment variable to set. NAME=value; you may provide this flag "+
@@ -251,7 +270,7 @@ func (p *ConfigurationEditFlags) Apply(
 	}
 	imageSet := false
 	if cmd.Flags().Changed("image") {
-		err = servinglib.UpdateImage(template, p.Image)
+		err = servinglib.UpdateImage(template, p.Image.String())
 		if err != nil {
 			return err
 		}


Test patch:
diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go
index 47f96a5d..6eb522ac 100644
--- a/pkg/kn/commands/service/create_test.go
+++ b/pkg/kn/commands/service/create_test.go
@@ -29,6 +29,7 @@ import (
 
 	"knative.dev/client/pkg/kn/commands"
 	servinglib "knative.dev/client/pkg/serving"
+	"knative.dev/client/pkg/util"
 	"knative.dev/client/pkg/wait"
 
 	corev1 "k8s.io/api/core/v1"
@@ -135,6 +136,13 @@ func TestServiceCreateImage(t *testing.T) {
 	}
 }
 
+func TestServiceCreateWithMultipleImages(t *testing.T) {
+	_, _, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--image", "gcr.io/bar/foo:baz", "--no-wait"}, false)
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "\"--image\"", "\"gcr.io/bar/foo:baz\"", "flag", "once"))
+}
+
 func TestServiceCreateImageSync(t *testing.T) {
 	action, created, output, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz"}, false)
diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go
index 868dc271..a04fd0e9 100644
--- a/pkg/kn/commands/service/update_test.go
+++ b/pkg/kn/commands/service/update_test.go
@@ -191,6 +191,14 @@ func TestServiceUpdateImage(t *testing.T) {
 	}
 }
 
+func TestServiceUpdateWithMultipleImages(t *testing.T) {
+	orig := newEmptyService()
+	_, _, _, err := fakeServiceUpdate(orig, []string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--image", "gcr.io/bar/foo:baz", "--no-wait"})
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "\"--image\"", "\"gcr.io/bar/foo:baz\"", "flag", "once"))
+}
+
 func TestServiceUpdateCommand(t *testing.T) {
 	orig := newEmptyService()