Implement pflag slice value interface for image types

cmd/skaffold/app/cmd/deploy.go

@@ -43,7 +43,7 @@ func NewCmdDeploy() *cobra.Command {
 		WithExample("Deploy without first rendering the manifests", "deploy --skip-render").
 		WithCommonFlags().
 		WithFlags([]*Flag{
-			{Value: &preBuiltImages, Name: "images", Shorthand: "i", Usage: "A list of pre-built images to deploy"},
+			{Value: &preBuiltImages, Name: "images", Shorthand: "i", DefValue: nil, Usage: "A list of pre-built images to deploy"},
 			{Value: &opts.SkipRender, Name: "skip-render", DefValue: false, Usage: "Don't render the manifests, just deploy them", IsEnum: true},
 		}).
 		WithHouseKeepingMessages().

cmd/skaffold/app/flags/image.go

@@ -37,22 +37,46 @@ type image struct {
 }
 
 // String Implements String() method for pflag interface and
-// returns a comma separated list of images.
+// returns a placeholder for the help text.
 func (i *Images) String() string {
+	return strings.Join(i.GetSlice(), ",")
+}
+
+// Type Implements Type() method for pflag interface
+func (i *Images) Type() string {
+	return fmt.Sprintf("%T", i)
+}
+
+// SetNil Implements SetNil() method for our Nillable interface
+func (i *Images) SetNil() error {
+	i.images = []image{}
+	return nil
+}
+
+// Set Implements Set() method for pflag interface.  We append values
+// to preserve compatibility with previous behaviour where each image
+// required a separate `-i` flag.
+func (i *Images) Set(csv string) error {
+	for _, split := range strings.Split(csv, ",") {
+		if err := i.Append(split); err != nil {
+			return fmt.Errorf("%s: %w", split, err)
+		}
+	}
+	return nil
+}
+
+// GetSlice Implements GetSlice() method for pflag SliceValue interface and
+// returns a slice of image names.
+func (i *Images) GetSlice() []string {
 	names := make([]string, len(i.images))
 	for i, image := range i.images {
 		names[i] = image.name
 	}
-	return strings.Join(names, ",")
-}
-
-// Usage Implements Usage() method for pflag interface
-func (i *Images) Usage() string {
-	return i.usage
+	return names
 }
 
-// Set Implements Set() method for pflag interface
-func (i *Images) Set(value string) error {
+// Append Implements Append() method for pflag SliceValue interface
+func (i *Images) Append(value string) error {
 	a, err := convertImageToArtifact(value)
 	if err != nil {
 		return err
@@ -61,9 +85,18 @@ func (i *Images) Set(value string) error {
 	return nil
 }
 
-// Type Implements Type() method for pflag interface
-func (i *Images) Type() string {
-	return fmt.Sprintf("%T", i)
+// Replace Implements Replace() method for pflag SliceValue interface
+func (i *Images) Replace(images []string) error {
+	newImages := make([]image, 0, len(images))
+	for _, value := range images {
+		a, err := convertImageToArtifact(value)
+		if err != nil {
+			return err
+		}
+		newImages = append(newImages, image{name: value, artifact: a})
+	}
+	i.images = newImages
+	return nil
 }
 
 // Artifacts returns an artifact representation for the corresponding image

cmd/skaffold/app/flags/image_test.go

@@ -32,6 +32,7 @@ func TestNewEmptyImage(t *testing.T) {
 }
 
 func TestImagesFlagSet(t *testing.T) {
+	// These tests only check Set() with a single value.
 	tests := []struct {
 		description      string
 		setValue         string
@@ -98,14 +99,30 @@ func TestImagesFlagSet(t *testing.T) {
 	}
 }
 
+func TestImagesSetCSV(t *testing.T) {
+	flag := NewEmptyImages("input image flag")
+	flag.Set("test-image1:test,test-image2:latest")
+	testutil.CheckDeepEqual(t, 2, len(flag.GetSlice()))
+	testutil.CheckDeepEqual(t, "test-image1:test,test-image2:latest", flag.String())
+
+	// check repeated calls to Set accumulate values.  This is backwards compatibility
+	// with the old behaviour that required `-i` for each image.
+	flag.Set("test-image3:test")
+	testutil.CheckDeepEqual(t, 3, len(flag.GetSlice()))
+	testutil.CheckDeepEqual(t, "test-image1:test,test-image2:latest,test-image3:test", flag.String())
+}
+
 func TestImagesString(t *testing.T) {
 	flag := NewEmptyImages("input image flag")
-	flag.Set("gcr.io/test/test-image:test")
-	flag.Set("gcr.io/test/test-image-1:test")
-	str := "gcr.io/test/test-image:test,gcr.io/test/test-image-1:test"
-	if str != flag.String() {
-		t.Errorf("Flag String() does not match. Expected %s, Actual %s", str, flag.String())
-	}
+	flag.Append("gcr.io/test/test-image:test")
+	flag.Append("gcr.io/test/test-image-1:test")
+	testutil.CheckDeepEqual(t, "gcr.io/test/test-image:test,gcr.io/test/test-image-1:test", flag.String())
+
+	flag.SetNil()
+	testutil.CheckDeepEqual(t, "", flag.String())
+
+	flag.Set("gcr.io/test/test-image:test,gcr.io/test/test-image-1:test")
+	testutil.CheckDeepEqual(t, "gcr.io/test/test-image:test,gcr.io/test/test-image-1:test", flag.String())
 }
 
 func TestImagesType(t *testing.T) {