Skip to content

Commit

Permalink
Enable overriding docker client
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
halvards committed Jul 2, 2021
1 parent ee23538 commit 1c3c01e
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 44 deletions.
6 changes: 6 additions & 0 deletions pkg/commands/options/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion pkg/commands/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
109 changes: 86 additions & 23 deletions pkg/commands/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
}
}

Expand Down
30 changes: 22 additions & 8 deletions pkg/publish/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
15 changes: 3 additions & 12 deletions pkg/publish/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 1c3c01e

Please sign in to comment.