Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement pflag slice value interface for image types #5575

Merged
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
51 changes: 39 additions & 12 deletions cmd/skaffold/app/flags/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,40 @@ 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
func (p *Images) Set(csv string) error {
split := strings.Split(csv, ",")
return p.Replace(split)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that this should be .Append() to maintain compatibility with our previous behaviour where we required used to provide multiple -i flags, one for for each image

}

// 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, ",")
return names
}

// Usage Implements Usage() method for pflag interface
func (i *Images) Usage() string {
return i.usage
}

// 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
Expand All @@ -61,9 +79,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
Expand Down
15 changes: 9 additions & 6 deletions cmd/skaffold/app/flags/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,15 @@ func TestImagesFlagSet(t *testing.T) {

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) {
Expand Down