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

Pre-parse platform string with StringSliceVar #551

Merged
merged 5 commits into from
Jan 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/ko_apply.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,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.
Expand Down
2 changes: 1 addition & 1 deletion doc/ko_build.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,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")
Expand Down
2 changes: 1 addition & 1 deletion doc/ko_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,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.
Expand Down
2 changes: 1 addition & 1 deletion doc/ko_resolve.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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.
Expand Down
2 changes: 1 addition & 1 deletion doc/ko_run.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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")
Expand Down
18 changes: 9 additions & 9 deletions pkg/build/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/build/gobuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 14 additions & 4 deletions pkg/build/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package build

import (
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"
)

Expand Down Expand Up @@ -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
}
}
Expand Down
18 changes: 11 additions & 7 deletions pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
imjasonh marked this conversation as resolved.
Show resolved Hide resolved

// 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]
}
Expand All @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/options/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type BuildOptions struct {
ConcurrentBuilds int
DisableOptimizations bool
SBOM string
Platform string
Platforms []string
halvards marked this conversation as resolved.
Show resolved Hide resolved
Labels []string
// UserAgent enables overriding the default value of the `User-Agent` HTTP
// request header used when retrieving the base image.
Expand All @@ -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]*")
halvards marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringSliceVar(&bo.Labels, "image-label", []string{},
"Which labels (key=value) to add to the image.")
Expand Down
12 changes: 7 additions & 5 deletions pkg/commands/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"} {
Expand All @@ -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 {
Expand Down