Fix --image flag to only allow single occurence (#647)

* Fix --image flag to only allow single occurence

* Fix changelog link

* Reflect feedback in error message and type name

* Add unit tests

* Reflect unit tests improvement from feedback

* Fix unit tests due to changes in master

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%"]

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
 		}

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)

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()