Skip to content

Commit

Permalink
Merge pull request #1622 from matejvasek/narrow-docker-iface
Browse files Browse the repository at this point in the history
cleanup: narrow docker interface
Signed-off-by: David Freilich <[email protected]>
  • Loading branch information
dfreilich authored Feb 7, 2023
2 parents 2e05ee8 + b7d11bd commit 202ea2d
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 32 deletions.
17 changes: 8 additions & 9 deletions internal/build/container_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/buildpacks/lifecycle/platform"
"github.com/docker/docker/api/types"
dcontainer "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
darchive "github.com/docker/docker/pkg/archive"
"github.com/pkg/errors"

Expand All @@ -22,11 +21,11 @@ import (
"github.com/buildpacks/pack/pkg/archive"
)

type ContainerOperation func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error
type ContainerOperation func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error

// CopyOut copies container directories to a handler function. The handler is responsible for closing the Reader.
func CopyOut(handler func(closer io.ReadCloser) error, srcs ...string) ContainerOperation {
return func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
return func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
for _, src := range srcs {
reader, _, err := ctrClient.CopyFromContainer(ctx, containerID, src)
if err != nil {
Expand Down Expand Up @@ -58,7 +57,7 @@ func CopyOutTo(src, dest string) ContainerOperation {
// CopyDir copies a local directory (src) to the destination on the container while filtering files and changing it's UID/GID.
// if includeRoot is set the UID/GID will be set on the dst directory.
func CopyDir(src, dst string, uid, gid int, os string, includeRoot bool, fileFilter func(string) bool) ContainerOperation {
return func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
return func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
tarPath := dst
if os == "windows" {
tarPath = paths.WindowsToSlash(dst)
Expand All @@ -77,7 +76,7 @@ func CopyDir(src, dst string, uid, gid int, os string, includeRoot bool, fileFil
}
}

func copyDir(ctx context.Context, ctrClient client.CommonAPIClient, containerID string, appReader io.Reader) error {
func copyDir(ctx context.Context, ctrClient DockerClient, containerID string, appReader io.Reader) error {
var clientErr, err error

doneChan := make(chan interface{})
Expand All @@ -104,7 +103,7 @@ func copyDir(ctx context.Context, ctrClient client.CommonAPIClient, containerID
// for Windows containers and does not work. Instead, we perform the copy from inside a container
// using xcopy.
// See: https://github.com/moby/moby/issues/40771
func copyDirWindows(ctx context.Context, ctrClient client.CommonAPIClient, containerID string, reader io.Reader, dst string, stdout, stderr io.Writer) error {
func copyDirWindows(ctx context.Context, ctrClient DockerClient, containerID string, reader io.Reader, dst string, stdout, stderr io.Writer) error {
info, err := ctrClient.ContainerInspect(ctx, containerID)
if err != nil {
return err
Expand Down Expand Up @@ -173,7 +172,7 @@ func findMount(info types.ContainerJSON, dst string) (types.MountPoint, error) {

// WriteProjectMetadata
func WriteProjectMetadata(p string, metadata platform.ProjectMetadata, os string) ContainerOperation {
return func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
return func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
buf := &bytes.Buffer{}
err := toml.NewEncoder(buf).Encode(metadata)
if err != nil {
Expand Down Expand Up @@ -202,7 +201,7 @@ func WriteProjectMetadata(p string, metadata platform.ProjectMetadata, os string

// WriteStackToml writes a `stack.toml` based on the StackMetadata provided to the destination path.
func WriteStackToml(dstPath string, stack builder.StackMetadata, os string) ContainerOperation {
return func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
return func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
buf := &bytes.Buffer{}
err := toml.NewEncoder(buf).Encode(stack)
if err != nil {
Expand Down Expand Up @@ -252,7 +251,7 @@ func createReader(src, dst string, uid, gid int, includeRoot bool, fileFilter fu
// Changing permissions on volumes through stopped containers does not work on Docker for Windows so we start the container and make change using icacls
// See: https://github.com/moby/moby/issues/40771
func EnsureVolumeAccess(uid, gid int, os string, volumeNames ...string) ContainerOperation {
return func(ctrClient client.CommonAPIClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
return func(ctrClient DockerClient, ctx context.Context, containerID string, stdout, stderr io.Writer) error {
if os != "windows" {
return nil
}
Expand Down
24 changes: 24 additions & 0 deletions internal/build/docker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package build

import (
"context"
"io"

"github.com/docker/docker/api/types"
containertypes "github.com/docker/docker/api/types/container"
networktypes "github.com/docker/docker/api/types/network"
specs "github.com/opencontainers/image-spec/specs-go/v1"
)

type DockerClient interface {
ImageRemove(ctx context.Context, image string, options types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error)
VolumeRemove(ctx context.Context, volumeID string, force bool) error
ContainerWait(ctx context.Context, container string, condition containertypes.WaitCondition) (<-chan containertypes.ContainerWaitOKBody, <-chan error)
ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error)
ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error
ContainerCreate(ctx context.Context, config *containertypes.Config, hostConfig *containertypes.HostConfig, networkingConfig *networktypes.NetworkingConfig, platform *specs.Platform, containerName string) (containertypes.ContainerCreateCreatedBody, error)
CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error)
ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error)
ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error
CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error
}
5 changes: 2 additions & 3 deletions internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/auth"
"github.com/docker/docker/client"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
"github.com/pkg/errors"
Expand All @@ -28,7 +27,7 @@ const (

type LifecycleExecution struct {
logger logging.Logger
docker client.CommonAPIClient
docker DockerClient
platformAPI *api.Version
layersVolume string
appVolume string
Expand All @@ -37,7 +36,7 @@ type LifecycleExecution struct {
opts LifecycleOptions
}

func NewLifecycleExecution(logger logging.Logger, docker client.CommonAPIClient, opts LifecycleOptions) (*LifecycleExecution, error) {
func NewLifecycleExecution(logger logging.Logger, docker DockerClient, opts LifecycleOptions) (*LifecycleExecution, error) {
latestSupportedPlatformAPI, err := FindLatestSupported(append(
opts.Builder.LifecycleDescriptor().APIs.Platform.Deprecated,
opts.Builder.LifecycleDescriptor().APIs.Platform.Supported...,
Expand Down
5 changes: 2 additions & 3 deletions internal/build/lifecycle_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/buildpacks/imgutil"
"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/platform"
"github.com/docker/docker/client"
"github.com/google/go-containerregistry/pkg/name"

"github.com/buildpacks/pack/internal/builder"
Expand Down Expand Up @@ -45,7 +44,7 @@ type Builder interface {

type LifecycleExecutor struct {
logger logging.Logger
docker client.CommonAPIClient
docker DockerClient
}

type Cache interface {
Expand Down Expand Up @@ -100,7 +99,7 @@ type LifecycleOptions struct {
CreationTime *time.Time
}

func NewLifecycleExecutor(logger logging.Logger, docker client.CommonAPIClient) *LifecycleExecutor {
func NewLifecycleExecutor(logger logging.Logger, docker DockerClient) *LifecycleExecutor {
return &LifecycleExecutor{logger: logger, docker: docker}
}

Expand Down
3 changes: 1 addition & 2 deletions internal/build/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/docker/docker/api/types"
dcontainer "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/pkg/errors"

"github.com/buildpacks/pack/internal/container"
Expand All @@ -16,7 +15,7 @@ type Phase struct {
name string
infoWriter io.Writer
errorWriter io.Writer
docker client.CommonAPIClient
docker DockerClient
handler container.Handler
ctrConf *dcontainer.Config
hostConf *dcontainer.HostConfig
Expand Down
6 changes: 2 additions & 4 deletions internal/cache/bind_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ package cache
import (
"context"
"os"

"github.com/docker/docker/client"
)

type BindCache struct {
docker client.CommonAPIClient
docker DockerClient
bind string
}

func NewBindCache(cacheType CacheInfo, dockerClient client.CommonAPIClient) *BindCache {
func NewBindCache(cacheType CacheInfo, dockerClient DockerClient) *BindCache {
return &BindCache{
bind: cacheType.Source,
docker: dockerClient,
Expand Down
9 changes: 7 additions & 2 deletions internal/cache/image_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ import (
)

type ImageCache struct {
docker client.CommonAPIClient
docker DockerClient
image string
}

func NewImageCache(imageRef name.Reference, dockerClient client.CommonAPIClient) *ImageCache {
type DockerClient interface {
ImageRemove(ctx context.Context, image string, options types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error)
VolumeRemove(ctx context.Context, volumeID string, force bool) error
}

func NewImageCache(imageRef name.Reference, dockerClient DockerClient) *ImageCache {
return &ImageCache{
image: imageRef.Name(),
docker: dockerClient,
Expand Down
4 changes: 2 additions & 2 deletions internal/cache/volume_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (
)

type VolumeCache struct {
docker client.CommonAPIClient
docker DockerClient
volume string
}

func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient client.CommonAPIClient) *VolumeCache {
func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient) *VolumeCache {
var volumeName string
if cacheType.Source == "" {
sum := sha256.Sum256([]byte(imageRef.Name()))
Expand Down
10 changes: 8 additions & 2 deletions internal/container/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,21 @@ import (
"io"

"github.com/docker/docker/api/types"
containertypes "github.com/docker/docker/api/types/container"
dcontainer "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/docker/docker/pkg/stdcopy"
"github.com/pkg/errors"
)

type Handler func(bodyChan <-chan dcontainer.ContainerWaitOKBody, errChan <-chan error, reader io.Reader) error

func RunWithHandler(ctx context.Context, docker client.CommonAPIClient, ctrID string, handler Handler) error {
type DockerClient interface {
ContainerWait(ctx context.Context, container string, condition dcontainer.WaitCondition) (<-chan containertypes.ContainerWaitOKBody, <-chan error)
ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error)
ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error
}

func RunWithHandler(ctx context.Context, docker DockerClient, ctrID string, handler Handler) error {
bodyChan, errChan := docker.ContainerWait(ctx, ctrID, dcontainer.WaitConditionNextExit)

resp, err := docker.ContainerAttach(ctx, ctrID, types.ContainerAttachOptions{
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type BuildpackDownloader interface {
// All settings on this object should be changed through ClientOption functions.
type Client struct {
logger logging.Logger
docker dockerClient.CommonAPIClient
docker DockerClient

keychain authn.Keychain
imageFactory ImageFactory
Expand Down Expand Up @@ -151,7 +151,7 @@ func WithCacheDir(path string) Option {
}

// WithDockerClient supply your own docker client.
func WithDockerClient(docker dockerClient.CommonAPIClient) Option {
func WithDockerClient(docker DockerClient) Option {
return func(c *Client) {
c.docker = docker
}
Expand Down Expand Up @@ -260,7 +260,7 @@ func (r *registryResolver) Resolve(registryName, bpName string) (string, error)
}

type imageFactory struct {
dockerClient dockerClient.CommonAPIClient
dockerClient local.DockerClient
keychain authn.Keychain
}

Expand Down
31 changes: 31 additions & 0 deletions pkg/client/docker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package client

import (
"context"
"io"

"github.com/docker/docker/api/types"
containertypes "github.com/docker/docker/api/types/container"
networktypes "github.com/docker/docker/api/types/network"
specs "github.com/opencontainers/image-spec/specs-go/v1"
)

// DockerClient is the subset of CommonAPIClient which required by this package
type DockerClient interface {
ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error)
ImageTag(ctx context.Context, image, ref string) error
ImageLoad(ctx context.Context, input io.Reader, quiet bool) (types.ImageLoadResponse, error)
ImageSave(ctx context.Context, images []string) (io.ReadCloser, error)
ImageRemove(ctx context.Context, image string, options types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error)
ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error)
Info(ctx context.Context) (types.Info, error)
VolumeRemove(ctx context.Context, volumeID string, force bool) error
ContainerCreate(ctx context.Context, config *containertypes.Config, hostConfig *containertypes.HostConfig, networkingConfig *networktypes.NetworkingConfig, platform *specs.Platform, containerName string) (containertypes.ContainerCreateCreatedBody, error)
CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error)
ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error)
ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error
CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error
ContainerWait(ctx context.Context, container string, condition containertypes.WaitCondition) (<-chan containertypes.ContainerWaitOKBody, <-chan error)
ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error)
ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error
}
9 changes: 7 additions & 2 deletions pkg/image/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,13 @@ func WithKeychain(keychain authn.Keychain) FetcherOption {
}
}

type DockerClient interface {
local.DockerClient
ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error)
}

type Fetcher struct {
docker client.CommonAPIClient
docker DockerClient
logger logging.Logger
registryMirrors map[string]string
keychain authn.Keychain
Expand All @@ -53,7 +58,7 @@ type FetchOptions struct {
PullPolicy PullPolicy
}

func NewFetcher(logger logging.Logger, docker client.CommonAPIClient, opts ...FetcherOption) *Fetcher {
func NewFetcher(logger logging.Logger, docker DockerClient, opts ...FetcherOption) *Fetcher {
fetcher := &Fetcher{
logger: logger,
docker: docker,
Expand Down

0 comments on commit 202ea2d

Please sign in to comment.