Pre-parse platform string with StringSliceVar (ko-build#551)

* Pre-parse platform string with StringSliceVar

This allows users to declare --platform multiple times and have the
values appended, i.e.:
  ko build --platform=linux/amd64 --platform=linux/arm64
is equivalent to
  ko build --platform=linux/amd64,linux/arm64

As a side effect, platformMatcher.spec and gobuildOpener.platforms are
now of type []string (instead of string) to maintain structure of
information from flag parsing.

* Adjust comments and styling for clarity.

* The flag --platform is now of type strings.

Internally cobra/pflag defines StringSliceVar as "strings" whereas
StringVar is defined as "string".

This change is updated by running hack/update-codegen.sh script.

* Add backwards compatibility for WithPlatforms function signature

Update comments to reflect implementation as well.

* Fix syntax failure on unit test

doc/ko_apply.md

@@ -68,7 +68,7 @@ ko apply -f FILENAME [flags]
   -n, --namespace string               If present, the namespace scope for this CLI request (DEPRECATED)
       --oci-layout-path string         Path to save the OCI image layout of the built images
       --password string                Password for basic authentication to the API server (DEPRECATED)
-      --platform string                Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
+      --platform strings               Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
   -P, --preserve-import-paths          Whether to preserve the full import path after KO_DOCKER_REPO.
       --push                           Push images to KO_DOCKER_REPO (default true)
   -R, --recursive                      Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.

doc/ko_build.md

@@ -53,7 +53,7 @@ ko build IMPORTPATH... [flags]
   -j, --jobs int                 The maximum number of concurrent builds (default GOMAXPROCS)
   -L, --local                    Load into images to local docker daemon.
       --oci-layout-path string   Path to save the OCI image layout of the built images
-      --platform string          Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
+      --platform strings         Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
   -P, --preserve-import-paths    Whether to preserve the full import path after KO_DOCKER_REPO.
       --push                     Push images to KO_DOCKER_REPO (default true)
       --sbom string              The SBOM media type to use (none will disable SBOM synthesis and upload, also supports: spdx, go.version-m). (default "spdx")

doc/ko_create.md

@@ -68,7 +68,7 @@ ko create -f FILENAME [flags]
   -n, --namespace string               If present, the namespace scope for this CLI request (DEPRECATED)
       --oci-layout-path string         Path to save the OCI image layout of the built images
       --password string                Password for basic authentication to the API server (DEPRECATED)
-      --platform string                Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
+      --platform strings               Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
   -P, --preserve-import-paths          Whether to preserve the full import path after KO_DOCKER_REPO.
       --push                           Push images to KO_DOCKER_REPO (default true)
   -R, --recursive                      Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.

doc/ko_resolve.md

@@ -49,7 +49,7 @@ ko resolve -f FILENAME [flags]
   -j, --jobs int                 The maximum number of concurrent builds (default GOMAXPROCS)
   -L, --local                    Load into images to local docker daemon.
       --oci-layout-path string   Path to save the OCI image layout of the built images
-      --platform string          Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
+      --platform strings         Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
   -P, --preserve-import-paths    Whether to preserve the full import path after KO_DOCKER_REPO.
       --push                     Push images to KO_DOCKER_REPO (default true)
   -R, --recursive                Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.

doc/ko_run.md

@@ -40,7 +40,7 @@ ko run IMPORTPATH [flags]
   -j, --jobs int                 The maximum number of concurrent builds (default GOMAXPROCS)
   -L, --local                    Load into images to local docker daemon.
       --oci-layout-path string   Path to save the OCI image layout of the built images
-      --platform string          Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
+      --platform strings         Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*
   -P, --preserve-import-paths    Whether to preserve the full import path after KO_DOCKER_REPO.
       --push                     Push images to KO_DOCKER_REPO (default true)
       --sbom string              The SBOM media type to use (none will disable SBOM synthesis and upload, also supports: spdx, go.version-m). (default "spdx")

pkg/build/gobuild.go

@@ -65,7 +65,7 @@ type builder func(context.Context, string, string, v1.Platform, Config) (string,
 type sbomber func(context.Context, string, string, v1.Image) ([]byte, types.MediaType, error)
 
 type platformMatcher struct {
-	spec      string
+	spec      []string
 	platforms []v1.Platform
 }
 
@@ -100,7 +100,7 @@ type gobuildOpener struct {
 	disableOptimizations bool
 	trimpath             bool
 	buildConfigs         map[string]Config
-	platform             string
+	platforms            []string
 	labels               map[string]string
 	dir                  string
 	jobs                 int
@@ -110,7 +110,7 @@ func (gbo *gobuildOpener) Open() (Interface, error) {
 	if gbo.getBase == nil {
 		return nil, errors.New("a way of providing base images must be specified, see build.WithBaseImages")
 	}
-	matcher, err := parseSpec(gbo.platform)
+	matcher, err := parseSpec(gbo.platforms)
 	if err != nil {
 		return nil, err
 	}
@@ -964,15 +964,15 @@ func (g *gobuild) buildAll(ctx context.Context, ref string, baseIndex v1.ImageIn
 	return idx, nil
 }
 
-func parseSpec(spec string) (*platformMatcher, error) {
+func parseSpec(spec []string) (*platformMatcher, error) {
 	// Don't bother parsing "all".
-	// "" should never happen because we default to linux/amd64.
-	platforms := []v1.Platform{}
-	if spec == "all" || spec == "" {
+	// Empty slice should never happen because we default to linux/amd64 (or GOOS/GOARCH).
+	if len(spec) == 0 || spec[0] == "all" {
 		return &platformMatcher{spec: spec}, nil
 	}
 
-	for _, platform := range strings.Split(spec, ",") {
+	platforms := []v1.Platform{}
+	for _, platform := range spec {
 		var p v1.Platform
 		parts := strings.Split(strings.TrimSpace(platform), "/")
 		if len(parts) > 0 {
@@ -993,7 +993,7 @@ func parseSpec(spec string) (*platformMatcher, error) {
 }
 
 func (pm *platformMatcher) matches(base *v1.Platform) bool {
-	if pm.spec == "all" {
+	if len(pm.spec) > 0 && pm.spec[0] == "all" {
 		return true
 	}
 

pkg/build/gobuild_test.go

@@ -980,57 +980,57 @@ func TestGoarm(t *testing.T) {
 func TestMatchesPlatformSpec(t *testing.T) {
 	for _, tc := range []struct {
 		platform *v1.Platform
-		spec     string
+		spec     []string
 		result   bool
 		err      bool
 	}{{
 		platform: nil,
-		spec:     "all",
+		spec:     []string{"all"},
 		result:   true,
 	}, {
 		platform: nil,
-		spec:     "linux/amd64",
+		spec:     []string{"linux/amd64"},
 		result:   false,
 	}, {
 		platform: &v1.Platform{
 			Architecture: "amd64",
 			OS:           "linux",
 		},
-		spec:   "all",
+		spec:   []string{"all"},
 		result: true,
 	}, {
 		platform: &v1.Platform{
 			Architecture: "amd64",
 			OS:           "windows",
 		},
-		spec:   "linux",
+		spec:   []string{"linux"},
 		result: false,
 	}, {
 		platform: &v1.Platform{
 			Architecture: "arm64",
 			OS:           "linux",
 			Variant:      "v3",
 		},
-		spec:   "linux/amd64,linux/arm64",
+		spec:   []string{"linux/amd64", "linux/arm64"},
 		result: true,
 	}, {
 		platform: &v1.Platform{
 			Architecture: "arm64",
 			OS:           "linux",
 			Variant:      "v3",
 		},
-		spec:   "linux/amd64,linux/arm64/v4",
+		spec:   []string{"linux/amd64", "linux/arm64/v4"},
 		result: false,
 	}, {
 		platform: &v1.Platform{
 			Architecture: "arm64",
 			OS:           "linux",
 			Variant:      "v3",
 		},
-		spec: "linux/amd64,linux/arm64/v3/z5",
+		spec: []string{"linux/amd64", "linux/arm64/v3/z5"},
 		err:  true,
 	}, {
-		spec: "",
+		spec: []string{},
 		platform: &v1.Platform{
 			Architecture: "amd64",
 			OS:           "linux",

pkg/build/options.go

@@ -15,6 +15,8 @@
 package build
 
 import (
+	"strings"
+
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
@@ -85,13 +87,21 @@ func WithConfig(buildConfigs map[string]Config) Option {
 
 // WithPlatforms is a functional option for building certain platforms for
 // multi-platform base images. To build everything from the base, use "all",
-// otherwise use a comma-separated list of platform specs, i.e.:
+// otherwise use a list of platform specs, i.e.:
 //
 // platform = <os>[/<arch>[/<variant>]]
-// allowed = all | platform[,platform]*
-func WithPlatforms(platforms string) Option {
+// allowed = "all" | []string{platform[,platform]*}
+//
+// Note: a string of comma-separated platforms (i.e. "platform[,platform]*")
+// has been deprecated and only exist for backwards compatibility reasons,
+// which will be removed in the future.
+func WithPlatforms(platforms ...string) Option {
 	return func(gbo *gobuildOpener) error {
-		gbo.platform = platforms
+		if len(platforms) == 1 {
+			// TODO: inform users that they are using deprecated flow?
+			platforms = strings.Split(platforms[0], ",")
+		}
+		gbo.platforms = platforms
 		return nil
 	}
 }

pkg/commands/config.go

@@ -58,14 +58,18 @@ func getBaseImage(bo *options.BuildOptions) build.GetBase {
 
 		// Using --platform=all will use an image index for the base,
 		// otherwise we'll resolve it to the appropriate platform.
-		//
-		// Platforms can be comma-separated if we only want a subset of the base
-		// image.
-		multiplatform := bo.Platform == "all" || strings.Contains(bo.Platform, ",")
-		if bo.Platform != "" && !multiplatform {
+		allPlatforms := len(bo.Platforms) == 1 && bo.Platforms[0] == "all"
+
+		// Platforms can be listed in a slice if we only want a subset of the base image.
+		selectiveMultiplatform := len(bo.Platforms) > 1
+
+		multiplatform := allPlatforms || selectiveMultiplatform
+		if !multiplatform {
 			var p v1.Platform
 
-			parts := strings.Split(bo.Platform, ":")
+			// There is _at least_ one platform specified at this point,
+			// because receiving "" means we would infer from GOOS/GOARCH.
+			parts := strings.Split(bo.Platforms[0], ":")
 			if len(parts) == 2 {
 				p.OSVersion = parts[1]
 			}
@@ -81,7 +85,7 @@ func getBaseImage(bo *options.BuildOptions) build.GetBase {
 				p.Variant = parts[2]
 			}
 			if len(parts) > 3 {
-				return nil, fmt.Errorf("too many slashes in platform spec: %s", bo.Platform)
+				return nil, fmt.Errorf("too many slashes in platform spec: %s", bo.Platforms[0])
 			}
 			ropt = append(ropt, remote.WithPlatform(p))
 		}

pkg/commands/config_test.go

@@ -41,7 +41,7 @@ func TestOverrideDefaultBaseImageUsingBuildOption(t *testing.T) {
 	wantImage := fmt.Sprintf("%s@%s", baseImage, wantDigest)
 	bo := &options.BuildOptions{
 		BaseImage: wantImage,
-		Platform:  "all",
+		Platforms: []string{"all"},
 	}
 
 	baseFn := getBaseImage(bo)

pkg/commands/options/build.go

@@ -51,7 +51,7 @@ type BuildOptions struct {
 	ConcurrentBuilds     int
 	DisableOptimizations bool
 	SBOM                 string
-	Platform             string
+	Platforms            []string
 	Labels               []string
 	// UserAgent enables overriding the default value of the `User-Agent` HTTP
 	// request header used when retrieving the base image.
@@ -76,7 +76,7 @@ func AddBuildOptions(cmd *cobra.Command, bo *BuildOptions) {
 		"Disable optimizations when building Go code. Useful when you want to interactively debug the created container.")
 	cmd.Flags().StringVar(&bo.SBOM, "sbom", "spdx",
 		"The SBOM media type to use (none will disable SBOM synthesis and upload, also supports: spdx, go.version-m).")
-	cmd.Flags().StringVar(&bo.Platform, "platform", "",
+	cmd.Flags().StringSliceVar(&bo.Platforms, "platform", []string{},
 		"Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]][,platform]*")
 	cmd.Flags().StringSliceVar(&bo.Labels, "image-label", []string{},
 		"Which labels (key=value) to add to the image.")

pkg/commands/resolver.go

@@ -61,20 +61,22 @@ func gobuildOptions(bo *options.BuildOptions) ([]build.Option, error) {
 		return nil, err
 	}
 
-	if bo.Platform == "" {
-		bo.Platform = "linux/amd64"
+	if len(bo.Platforms) == 0 {
+		envPlatform := "linux/amd64"
 
 		goos, goarch, goarm := os.Getenv("GOOS"), os.Getenv("GOARCH"), os.Getenv("GOARM")
 
 		// Default to linux/amd64 unless GOOS and GOARCH are set.
 		if goos != "" && goarch != "" {
-			bo.Platform = path.Join(goos, goarch)
+			envPlatform = path.Join(goos, goarch)
 		}
 
 		// Use GOARM for variant if it's set and GOARCH is arm.
 		if strings.Contains(goarch, "arm") && goarm != "" {
-			bo.Platform = path.Join(bo.Platform, "v"+goarm)
+			envPlatform = path.Join(envPlatform, "v"+goarm)
 		}
+
+		bo.Platforms = []string{envPlatform}
 	} else {
 		// Make sure these are all unset
 		for _, env := range []string{"GOOS", "GOARCH", "GOARM"} {
@@ -86,7 +88,7 @@ func gobuildOptions(bo *options.BuildOptions) ([]build.Option, error) {
 
 	opts := []build.Option{
 		build.WithBaseImages(getBaseImage(bo)),
-		build.WithPlatforms(bo.Platform),
+		build.WithPlatforms(bo.Platforms...),
 		build.WithJobs(bo.ConcurrentBuilds),
 	}
 	if creationTime != nil {