diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 1d9d0a44d..fd138675d 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 25753a855..8731eec00 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 } diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index 47f96a5d0..6eb522acd 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 868dc271d..a04fd0e9a 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()