Skip to content

fix(ko): Fall back to build configs in .ko.yaml #6821

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

Merged
merged 2 commits into from
Nov 10, 2021
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
15 changes: 12 additions & 3 deletions docs/content/en/docs/pipeline-stages/builders/ko.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,18 @@ Useful tips for existing `ko` users:
[`default-repo` functionality]({{< relref "/docs/environment/image-registries" >}}).
The ko builder does _not_ read the `KO_DOCKER_REPO` environment variable.

- The ko builder supports reading base image configuration from the `.ko.yaml`
file. If you already configure your base images using this file, you do not
need to repeat this configuration in `skaffold.yaml`.
- The ko builder supports reading
[base image configuration](https://github.com/google/ko#overriding-base-images)
from the `.ko.yaml` file. If you already configure your base images using
this file, you do not need to specify the `fromImage` field for the
artifact in `skaffold.yaml`.

- The ko builder supports reading
[build configs](https://github.com/google/ko#overriding-go-build-settings)
from the `.ko.yaml` file if `skaffold.yaml` does not specify any of the build
config fields (`dir`, `main`, `env`, `flags`, and `ldflags`). If you already
specify these fields in `.ko.yaml`, you do not need to repeat them in
`skaffold.yaml`.

- When you use the Skaffold ko builder, Skaffold takes care of replacing the
image placeholder name in your Kubernetes manifest files using its
Expand Down
61 changes: 47 additions & 14 deletions pkg/skaffold/build/ko/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,13 @@ func (b *Builder) newKoBuilder(ctx context.Context, a *latestV1.Artifact) (build
}

func buildOptions(a *latestV1.Artifact, runMode config.RunMode) (*options.BuildOptions, error) {
koImportpath, err := getImportPath(a)
buildconfig, err := buildConfig(a)
if err != nil {
return nil, fmt.Errorf("could not determine import path: %v", err)
return nil, fmt.Errorf("could not create ko build config: %v", err)
}
importpath := strings.TrimPrefix(koImportpath, build.StrictScheme)
return &options.BuildOptions{
BaseImage: a.KoArtifact.BaseImage,
BuildConfigs: map[string]build.Config{
importpath: {
ID: a.ImageName,
Dir: ".",
Env: a.KoArtifact.Env,
Flags: a.KoArtifact.Flags,
Ldflags: a.KoArtifact.Ldflags,
Main: a.KoArtifact.Main,
},
},
BaseImage: a.KoArtifact.BaseImage,
BuildConfigs: buildconfig,
ConcurrentBuilds: 1, // we could plug in Skaffold's max builds here, but it'd be incorrect if users build more than one artifact
DisableOptimizations: runMode == config.RunModes.Debug,
Labels: labels(a),
Expand All @@ -66,6 +56,49 @@ func buildOptions(a *latestV1.Artifact, runMode config.RunMode) (*options.BuildO
}, nil
}

// buildConfig creates the ko build config map based on the artifact config.
// A map entry is only required if the artifact config specifies fields that need to be part of ko build configs.
// If none of these are specified, we can provide an empty `BuildConfigs` map.
// In this case, ko falls back to build configs provided in `.ko.yaml`, or to the default zero config.
func buildConfig(a *latestV1.Artifact) (map[string]build.Config, error) {
buildconfigs := map[string]build.Config{}
if koArtifactSpecifiesBuildConfig(*a.KoArtifact) {
koImportpath, err := getImportPath(a)
if err != nil {
return nil, fmt.Errorf("could not determine import path of image %s: %v", a.ImageName, err)
}
importpath := strings.TrimPrefix(koImportpath, build.StrictScheme)
buildconfigs[importpath] = build.Config{
ID: a.ImageName,
Dir: ".",
Env: a.KoArtifact.Env,
Flags: a.KoArtifact.Flags,
Ldflags: a.KoArtifact.Ldflags,
Main: a.KoArtifact.Main,
}
}
return buildconfigs, nil
}

func koArtifactSpecifiesBuildConfig(k latestV1.KoArtifact) bool {
if k.Dir != "" && k.Dir != "." {
return true
}
if k.Main != "" && k.Main != "." {
return true
}
if len(k.Env) != 0 {
return true
}
if len(k.Flags) != 0 {
return true
}
if len(k.Ldflags) != 0 {
return true
}
return false
}

func labels(a *latestV1.Artifact) []string {
var labels []string
for k, v := range a.KoArtifact.Labels {
Expand Down
15 changes: 3 additions & 12 deletions pkg/skaffold/build/ko/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import (
"github.com/GoogleContainerTools/skaffold/testutil"
)

// Most of the test cases set the artifact image name to a `ko://`-prefixed import path.
// This speeds up the tests, as the code under test doesn't need to repeatedly determine the import path of this package.
func TestBuildOptions(t *testing.T) {
tests := []struct {
description string
Expand All @@ -48,7 +46,6 @@ func TestBuildOptions(t *testing.T) {
KoArtifact: &latestV1.KoArtifact{},
},
},
wantImportPath: "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko", // this package
},
{
description: "base image",
Expand All @@ -60,7 +57,6 @@ func TestBuildOptions(t *testing.T) {
},
ImageName: "ko://example.com/foo",
},
wantImportPath: "example.com/foo",
},
{
description: "empty platforms",
Expand All @@ -72,7 +68,6 @@ func TestBuildOptions(t *testing.T) {
},
ImageName: "ko://example.com/foo",
},
wantImportPath: "example.com/foo",
},
{
description: "multiple platforms",
Expand All @@ -84,8 +79,7 @@ func TestBuildOptions(t *testing.T) {
},
ImageName: "ko://example.com/foo",
},
wantPlatform: "linux/amd64,linux/arm64",
wantImportPath: "example.com/foo",
wantPlatform: "linux/amd64,linux/arm64",
},
{
description: "workspace",
Expand All @@ -97,7 +91,6 @@ func TestBuildOptions(t *testing.T) {
Workspace: "my-app-subdirectory",
},
wantWorkingDirectory: "my-app-subdirectory",
wantImportPath: "example.com/foo",
},
{
description: "source dir",
Expand Down Expand Up @@ -136,7 +129,6 @@ func TestBuildOptions(t *testing.T) {
},
runMode: config.RunModes.Debug,
wantDisableOptimizations: true,
wantImportPath: "example.com/foo",
},
{
description: "labels",
Expand All @@ -151,8 +143,7 @@ func TestBuildOptions(t *testing.T) {
},
ImageName: "ko://example.com/foo",
},
wantLabels: []string{"foo=bar", "frob=baz"},
wantImportPath: "example.com/foo",
wantLabels: []string{"foo=bar", "frob=baz"},
},
}
for _, test := range tests {
Expand All @@ -170,7 +161,7 @@ func TestBuildOptions(t *testing.T) {
t.CheckDeepEqual(test.wantLabels, bo.Labels,
cmpopts.SortSlices(func(x, y string) bool { return x < y }),
cmpopts.EquateEmpty())
if len(bo.BuildConfigs) != 1 {
if test.wantImportPath != "" && len(bo.BuildConfigs) != 1 {
t.Fatalf("expected exactly one build config, got %d", len(bo.BuildConfigs))
}
for importpath := range bo.BuildConfigs {
Expand Down