Enable overriding docker client (#378)

When embedding ko, it may be necessary to override the docker client.

This adds a PublishOption to inject a docker client created elsewhere.
Ko will use this client to interact with the docker daemon.

Context: GoogleContainerTools/skaffold#6054 (comment)

Author: halvards

pkg/commands/options/publish.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"path"
 
+	"github.com/google/go-containerregistry/pkg/v1/daemon"
 	"github.com/google/ko/pkg/publish"
 	"github.com/spf13/cobra"
 )
@@ -39,6 +40,11 @@ type PublishOptions struct {
 	// request header used when pushing the built image to an image registry.
 	UserAgent string
 
+	// DockerClient enables overriding the default docker client when embedding
+	// ko as a module in other tools.
+	// If left as the zero value, ko uses github.com/docker/docker/client.FromEnv
+	DockerClient daemon.Client
+
 	Tags []string
 	// TagOnly resolves images into tag-only references.
 	TagOnly bool

pkg/commands/resolver.go

@@ -164,7 +164,9 @@ func makePublisher(po *options.PublishOptions) (publish.Interface, error) {
 			// use local with other publishers, but that might
 			// not be true.
 			return publish.NewDaemon(namer, po.Tags,
-				publish.WithLocalDomain(po.LocalDomain))
+				publish.WithDockerClient(po.DockerClient),
+				publish.WithLocalDomain(po.LocalDomain),
+			)
 		}
 		if repoName == publish.KindDomain {
 			return publish.NewKindPublisher(namer, po.Tags), nil

pkg/commands/resolver_test.go

@@ -22,13 +22,16 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"path"
 	"strings"
 	"testing"
 
+	"github.com/docker/docker/api/types"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/go-containerregistry/pkg/name"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/daemon"
 	"github.com/google/go-containerregistry/pkg/v1/empty"
 	"github.com/google/go-containerregistry/pkg/v1/random"
 	"github.com/google/ko/pkg/build"
@@ -52,8 +55,23 @@ var (
 		fooRef: fooHash,
 		barRef: barHash,
 	}
+
+	errImageLoad = fmt.Errorf("ImageLoad() error")
+	errImageTag  = fmt.Errorf("ImageTag() error")
 )
 
+type erroringClient struct {
+	daemon.Client
+}
+
+func (m *erroringClient) NegotiateAPIVersion(context.Context) {}
+func (m *erroringClient) ImageLoad(context.Context, io.Reader, bool) (types.ImageLoadResponse, error) {
+	return types.ImageLoadResponse{}, errImageLoad
+}
+func (m *erroringClient) ImageTag(_ context.Context, _ string, _ string) error {
+	return errImageTag
+}
+
 func TestResolveMultiDocumentYAMLs(t *testing.T) {
 	refs := []string{fooRef, barRef}
 	hashes := []v1.Hash{fooHash, barHash}
@@ -138,14 +156,14 @@ kind: Bar
 	}
 }
 
-func TestMakeBuilder(t *testing.T) {
+func TestNewBuilderCanBuild(t *testing.T) {
 	ctx := context.Background()
 	bo := &options.BuildOptions{
 		ConcurrentBuilds: 1,
 	}
 	builder, err := NewBuilder(ctx, bo)
 	if err != nil {
-		t.Fatalf("MakeBuilder(): %v", err)
+		t.Fatalf("NewBuilder(): %v", err)
 	}
 	res, err := builder.Build(ctx, "ko://github.com/google/ko/test")
 	if err != nil {
@@ -158,29 +176,74 @@ func TestMakeBuilder(t *testing.T) {
 	fmt.Println(gotDigest.String())
 }
 
-func TestMakePublisher(t *testing.T) {
-	repo := "registry.example.com/repository"
-	po := &options.PublishOptions{
-		DockerRepo:          repo,
-		PreserveImportPaths: true,
-	}
-	publisher, err := NewPublisher(po)
-	if err != nil {
-		t.Fatalf("MakePublisher(): %v", err)
-	}
-	defer publisher.Close()
-	ctx := context.Background()
+func TestNewPublisherCanPublish(t *testing.T) {
+	dockerRepo := "registry.example.com/repo"
+	localDomain := "localdomain.example.com/repo"
 	importpath := "github.com/google/ko/test"
-	importpathWithScheme := build.StrictScheme + importpath
-	buildResult := empty.Index
-	ref, err := publisher.Publish(ctx, buildResult, importpathWithScheme)
-	if err != nil {
-		t.Fatalf("publisher.Publish(): %v", err)
+	tests := []struct {
+		description   string
+		wantImageName string
+		po            *options.PublishOptions
+		shouldError   bool
+		wantError     error
+	}{
+		{
+			description:   "base import path",
+			wantImageName: fmt.Sprintf("%s/%s", dockerRepo, path.Base(importpath)),
+			po: &options.PublishOptions{
+				BaseImportPaths: true,
+				DockerRepo:      dockerRepo,
+			},
+		},
+		{
+			description:   "preserve import path",
+			wantImageName: fmt.Sprintf("%s/%s", dockerRepo, importpath),
+			po: &options.PublishOptions{
+				DockerRepo:          dockerRepo,
+				PreserveImportPaths: true,
+			},
+		},
+		{
+			description:   "override LocalDomain",
+			wantImageName: fmt.Sprintf("%s/%s", localDomain, importpath),
+			po: &options.PublishOptions{
+				Local:               true,
+				LocalDomain:         localDomain,
+				PreserveImportPaths: true,
+			},
+		},
+		{
+			description:   "override DockerClient",
+			wantImageName: strings.ToLower(fmt.Sprintf("%s/%s", localDomain, importpath)),
+			po: &options.PublishOptions{
+				DockerClient: &erroringClient{},
+				Local:        true,
+			},
+			shouldError: true,
+			wantError:   errImageLoad,
+		},
 	}
-	gotImageName := ref.Context().Name()
-	wantImageName := strings.ToLower(fmt.Sprintf("%s/%s", repo, importpath))
-	if gotImageName != wantImageName {
-		t.Errorf("got %s, wanted %s", gotImageName, wantImageName)
+	for _, test := range tests {
+		publisher, err := NewPublisher(test.po)
+		if err != nil {
+			t.Fatalf("%s: NewPublisher(): %v", test.description, err)
+		}
+		defer publisher.Close()
+		ref, err := publisher.Publish(context.Background(), empty.Image, build.StrictScheme+importpath)
+		if test.shouldError {
+			if err == nil || !strings.HasSuffix(err.Error(), test.wantError.Error()) {
+				t.Errorf("%s: got error %v, wanted %v", test.description, err, test.wantError)
+			}
+			continue
+		}
+		if err != nil {
+			t.Fatalf("%s: publisher.Publish(): %v", test.description, err)
+		}
+		fmt.Printf("ref: %+v\n", ref)
+		gotImageName := ref.Context().Name()
+		if gotImageName != test.wantImageName {
+			t.Errorf("%s: got %s, wanted %s", test.description, gotImageName, test.wantImageName)
+		}
 	}
 }
 

pkg/publish/daemon.go

@@ -35,10 +35,12 @@ const (
 )
 
 // demon is intentionally misspelled to avoid name collision (and drive Jon nuts).
+// [Narrator: Jon wasn't the only one driven nuts.]
 type demon struct {
-	base  string
-	namer Namer
-	tags  []string
+	base   string
+	client daemon.Client
+	namer  Namer
+	tags   []string
 }
 
 // DaemonOption is a functional option for NewDaemon.
@@ -54,6 +56,16 @@ func WithLocalDomain(domain string) DaemonOption {
 	}
 }
 
+// WithDockerClient is a functional option for overriding the docker client.
+func WithDockerClient(client daemon.Client) DaemonOption {
+	return func(i *demon) error {
+		if client != nil {
+			i.client = client
+		}
+		return nil
+	}
+}
+
 // NewDaemon returns a new publish.Interface that publishes images to a container daemon.
 func NewDaemon(namer Namer, tags []string, opts ...DaemonOption) (Interface, error) {
 	d := &demon{
@@ -69,9 +81,11 @@ func NewDaemon(namer Namer, tags []string, opts ...DaemonOption) (Interface, err
 	return d, nil
 }
 
-// getOpts returns daemon.Options. It's a var to allow it to be overridden during tests.
-var getOpts = func(ctx context.Context) []daemon.Option {
-	return []daemon.Option{daemon.WithContext(ctx)}
+func (d *demon) getOpts(ctx context.Context) []daemon.Option {
+	return []daemon.Option{
+		daemon.WithContext(ctx),
+		daemon.WithClient(d.client),
+	}
 }
 
 // Publish implements publish.Interface
@@ -131,7 +145,7 @@ func (d *demon) Publish(ctx context.Context, br build.Result, s string) (name.Re
 	}
 
 	log.Printf("Loading %v", digestTag)
-	if _, err := daemon.Write(digestTag, img, getOpts(ctx)...); err != nil {
+	if _, err := daemon.Write(digestTag, img, d.getOpts(ctx)...); err != nil {
 		return nil, err
 	}
 	log.Printf("Loaded %v", digestTag)
@@ -143,7 +157,7 @@ func (d *demon) Publish(ctx context.Context, br build.Result, s string) (name.Re
 			return nil, err
 		}
 
-		if err := daemon.Tag(digestTag, tag, getOpts(ctx)...); err != nil {
+		if err := daemon.Tag(digestTag, tag, d.getOpts(ctx)...); err != nil {
 			return nil, err
 		}
 		log.Printf("Added tag %v", tagName)

pkg/publish/daemon_test.go

@@ -46,23 +46,14 @@ func (m *mockClient) ImageTag(_ context.Context, _ string, tag string) error {
 
 var Tags []string
 
-func init() {
-	getOpts = func(ctx context.Context) []daemon.Option {
-		return []daemon.Option{
-			daemon.WithContext(ctx),
-			daemon.WithClient(&mockClient{}),
-		}
-	}
-}
-
 func TestDaemon(t *testing.T) {
 	importpath := "github.com/google/ko"
 	img, err := random.Image(1024, 1)
 	if err != nil {
 		t.Fatalf("random.Image() = %v", err)
 	}
 
-	def, err := NewDaemon(md5Hash, []string{})
+	def, err := NewDaemon(md5Hash, []string{}, WithDockerClient(&mockClient{}))
 	if err != nil {
 		t.Fatalf("NewDaemon() = %v", err)
 	}
@@ -83,7 +74,7 @@ func TestDaemonTags(t *testing.T) {
 		t.Fatalf("random.Image() = %v", err)
 	}
 
-	def, err := NewDaemon(md5Hash, []string{"v2.0.0", "v1.2.3", "production"})
+	def, err := NewDaemon(md5Hash, []string{"v2.0.0", "v1.2.3", "production"}, WithDockerClient(&mockClient{}))
 	if err != nil {
 		t.Fatalf("NewDaemon() = %v", err)
 	}
@@ -113,7 +104,7 @@ func TestDaemonDomain(t *testing.T) {
 	}
 
 	localDomain := "registry.example.com/repository"
-	def, err := NewDaemon(md5Hash, []string{}, WithLocalDomain(localDomain))
+	def, err := NewDaemon(md5Hash, []string{}, WithLocalDomain(localDomain), WithDockerClient(&mockClient{}))
 	if err != nil {
 		t.Fatalf("NewDaemon() = %v", err)
 	}