Skip to content

Commit

Permalink
fix(ko): Ko builder push vs load behavior (#8845)
Browse files Browse the repository at this point in the history
Ko v0.13.0 introduced a change to the handling of image names when
loading images into a Docker daemon (instead of pushing to a registry).

Specifically, ko v0.13.0 assumes that if the image repo name matches
the
[`LocalDomain` field](https://github.com/ko-build/ko/blob/v0.13.0/pkg/commands/options/publish.go#L37),
the build image should be loaded into the Docker daemon, regardless of
the values of the
[`Push` and `Local` fields](https://github.com/ko-build/ko/blob/v0.13.0/pkg/commands/options/publish.go#L52-L56).
The implementation of this behavior is in
[ko's `makePublisher() function](https://github.com/ko-build/ko/blob/v0.13.0/pkg/commands/resolver.go#L189-L190).

This change ensures that Skaffold only sets the `LocalDomain` field for
images that should be loaded.

Fixes: #8760
  • Loading branch information
halvards authored Jun 2, 2023
1 parent 3c6ed45 commit d219498
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
6 changes: 5 additions & 1 deletion pkg/skaffold/build/ko/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,18 @@ func publishOptions(ref string, pushImages bool, dockerClient daemon.Client, ins
return nil, err
}
imageNameWithoutTag := imageRef.Context().Name()
localDomain := ""
if !pushImages {
localDomain = imageNameWithoutTag
}

return &options.PublishOptions{
Bare: true,
DockerClient: dockerClient,
DockerRepo: imageNameWithoutTag,
InsecureRegistry: useInsecureRegistry(imageNameWithoutTag, insecureRegistries),
Local: !pushImages,
LocalDomain: imageNameWithoutTag,
LocalDomain: localDomain,
Push: pushImages,
Tags: []string{imageRef.Identifier()},
UserAgent: version.UserAgentWithClient(),
Expand Down
5 changes: 4 additions & 1 deletion pkg/skaffold/build/ko/publisher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ func TestPublishOptions(t *testing.T) {
t.Errorf("wanted InsecureRegistry (%v), got (%v)", test.wantInsecureRegistry, po.InsecureRegistry)
}
if !test.pushImages && po.LocalDomain != test.repo {
t.Errorf("wanted LocalDomain (%q), got (%q)", test.repo, po.DockerRepo)
t.Errorf("wanted LocalDomain (%q), got (%q)", test.repo, po.LocalDomain)
}
if test.pushImages && po.LocalDomain != "" {
t.Errorf("wanted zero value for LocalDomain, got (%q)", po.LocalDomain)
}
if test.pushImages == po.Local {
t.Errorf("Local (%v) should be the inverse of pushImages (%v)", po.Local, test.pushImages)
Expand Down

0 comments on commit d219498

Please sign in to comment.