From 1c3c01e3a4cfa989518c000b56f073b06208ca53 Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Fri, 2 Jul 2021 19:21:03 +1000 Subject: [PATCH] Enable overriding docker client 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: https://github.com/GoogleContainerTools/skaffold/pull/6054#discussion_r662230195 --- pkg/commands/options/publish.go | 6 ++ pkg/commands/resolver.go | 4 +- pkg/commands/resolver_test.go | 109 +++++++++++++++++++++++++------- pkg/publish/daemon.go | 30 ++++++--- pkg/publish/daemon_test.go | 15 +---- 5 files changed, 120 insertions(+), 44 deletions(-) diff --git a/pkg/commands/options/publish.go b/pkg/commands/options/publish.go index ffbc82042c..4efced5da6 100644 --- a/pkg/commands/options/publish.go +++ b/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 diff --git a/pkg/commands/resolver.go b/pkg/commands/resolver.go index 1e0442fe42..84b5a2aba2 100644 --- a/pkg/commands/resolver.go +++ b/pkg/commands/resolver.go @@ -159,7 +159,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 diff --git a/pkg/commands/resolver_test.go b/pkg/commands/resolver_test.go index ff1fd2feeb..53a4e67a59 100644 --- a/pkg/commands/resolver_test.go +++ b/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) + } } } diff --git a/pkg/publish/daemon.go b/pkg/publish/daemon.go index 1b0818a656..c58a628d69 100644 --- a/pkg/publish/daemon.go +++ b/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) diff --git a/pkg/publish/daemon_test.go b/pkg/publish/daemon_test.go index dfc5975fe0..1209c758aa 100644 --- a/pkg/publish/daemon_test.go +++ b/pkg/publish/daemon_test.go @@ -46,15 +46,6 @@ 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) @@ -62,7 +53,7 @@ func TestDaemon(t *testing.T) { 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) }