Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,20 @@ type Cli interface {
DefaultVersion() string
ManifestStore() manifeststore.Store
RegistryClient(bool) registryclient.RegistryClient
ContentTrustEnabled() bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

way cleaner! 👍

}

// DockerCli is an instance the docker command line client.
// Instances of the client can be returned from NewDockerCli.
type DockerCli struct {
configFile *configfile.ConfigFile
in *InStream
out *OutStream
err io.Writer
client client.APIClient
serverInfo ServerInfo
clientInfo ClientInfo
configFile *configfile.ConfigFile
in *InStream
out *OutStream
err io.Writer
client client.APIClient
serverInfo ServerInfo
clientInfo ClientInfo
contentTrust bool
}

// DefaultVersion returns api.defaultVersion or DOCKER_API_VERSION if specified.
Expand Down Expand Up @@ -121,6 +123,12 @@ func (cli *DockerCli) ClientInfo() ClientInfo {
return cli.clientInfo
}

// ContentTrustEnabled returns if content trust has been enabled by an
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: reads a bit odd, perhaps could be returns whether trust has been enabled..

// environment variable.
func (cli *DockerCli) ContentTrustEnabled() bool {
return cli.contentTrust
}

// ManifestStore returns a store for local manifests
func (cli *DockerCli) ManifestStore() manifeststore.Store {
// TODO: support override default location from config file
Expand Down Expand Up @@ -237,8 +245,8 @@ func (c ClientInfo) HasKubernetes() bool {
}

// NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
func NewDockerCli(in io.ReadCloser, out, err io.Writer) *DockerCli {
return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err}
func NewDockerCli(in io.ReadCloser, out, err io.Writer, isTrusted bool) *DockerCli {
return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err, contentTrust: isTrusted}
}

// NewAPIClientFromFlags creates a new APIClient from command line flags
Expand Down
19 changes: 10 additions & 9 deletions cli/command/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import (
)

type createOptions struct {
name string
platform string
name string
platform string
untrusted bool
}

// NewCreateCommand creates a new cobra.Command for `docker create`
Expand Down Expand Up @@ -53,7 +54,7 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
flags.Bool("help", false, "Print usage")

command.AddPlatformFlag(flags, &opts.platform)
command.AddTrustVerificationFlags(flags)
command.AddTrustVerificationFlags(flags, &opts.untrusted, dockerCli.ContentTrustEnabled())
copts = addFlags(flags)
return cmd
}
Expand All @@ -64,7 +65,7 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, opts *createOptions,
reportError(dockerCli.Err(), "create", err.Error(), true)
return cli.StatusError{StatusCode: 125}
}
response, err := createContainer(context.Background(), dockerCli, containerConfig, opts.name, opts.platform)
response, err := createContainer(context.Background(), dockerCli, containerConfig, opts)
if err != nil {
return err
}
Expand Down Expand Up @@ -158,7 +159,7 @@ func newCIDFile(path string) (*cidFile, error) {
return &cidFile{path: path, file: f}, nil
}

func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig *containerConfig, name string, platform string) (*container.ContainerCreateCreatedBody, error) {
func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig *containerConfig, opts *createOptions) (*container.ContainerCreateCreatedBody, error) {
config := containerConfig.Config
hostConfig := containerConfig.HostConfig
networkingConfig := containerConfig.NetworkingConfig
Expand All @@ -182,7 +183,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
if named, ok := ref.(reference.Named); ok {
namedRef = reference.TagNameOnly(named)

if taggedRef, ok := namedRef.(reference.NamedTagged); ok && command.IsTrusted() {
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && !opts.untrusted {
var err error
trustedRef, err = image.TrustedReference(ctx, dockerCli, taggedRef, nil)
if err != nil {
Expand All @@ -193,15 +194,15 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
}

//create the container
response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)

//if image not found try to pull it
if err != nil {
if apiclient.IsErrNotFound(err) && namedRef != nil {
fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef))

// we don't want to write to stdout anything apart from container.ID
if err := pullImage(ctx, dockerCli, config.Image, platform, stderr); err != nil {
if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil {
return nil, err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
Expand All @@ -211,7 +212,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
}
// Retry
var retryErr error
response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)
if retryErr != nil {
return nil, retryErr
}
Expand Down
6 changes: 5 additions & 1 deletion cli/command/container/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,11 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) {
},
HostConfig: &container.HostConfig{},
}
body, err := createContainer(context.Background(), cli, config, "name", runtime.GOOS)
body, err := createContainer(context.Background(), cli, config, &createOptions{
name: "name",
platform: runtime.GOOS,
untrusted: true,
})
assert.NilError(t, err)
expected := container.ContainerCreateCreatedBody{ID: containerID}
assert.Check(t, is.DeepEqual(expected, *body))
Expand Down
7 changes: 3 additions & 4 deletions cli/command/container/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ import (
)

type runOptions struct {
createOptions
detach bool
sigProxy bool
name string
detachKeys string
platform string
}

// NewRunCommand create a new `docker run` command
Expand Down Expand Up @@ -64,7 +63,7 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
flags.Bool("help", false, "Print usage")

command.AddPlatformFlag(flags, &opts.platform)
command.AddTrustVerificationFlags(flags)
command.AddTrustVerificationFlags(flags, &opts.untrusted, dockerCli.ContentTrustEnabled())
copts = addFlags(flags)
return cmd
}
Expand Down Expand Up @@ -162,7 +161,7 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio

ctx, cancelFun := context.WithCancel(context.Background())

createResponse, err := createContainer(ctx, dockerCli, containerConfig, opts.name, opts.platform)
createResponse, err := createContainer(ctx, dockerCli, containerConfig, &opts.createOptions)
if err != nil {
reportError(stderr, cmdPath, err.Error(), true)
return runStartContainerErr(err)
Expand Down
26 changes: 13 additions & 13 deletions cli/command/image/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type buildOptions struct {
imageIDFile string
stream bool
platform string
untrusted bool
}

// dockerfileFromStdin returns true when the user specified that the Dockerfile
Expand Down Expand Up @@ -137,7 +138,7 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
flags.StringVar(&options.target, "target", "", "Set the target build stage to build.")
flags.StringVar(&options.imageIDFile, "iidfile", "", "Write the image ID to the file")

command.AddTrustVerificationFlags(flags)
command.AddTrustVerificationFlags(flags, &options.untrusted, dockerCli.ContentTrustEnabled())
command.AddPlatformFlag(flags, &options.platform)

flags.BoolVar(&options.squash, "squash", false, "Squash newly built layers into a single new layer")
Expand Down Expand Up @@ -285,18 +286,18 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
defer cancel()

var resolvedTags []*resolvedTag
if command.IsTrusted() {
if !options.untrusted {
translator := func(ctx context.Context, ref reference.NamedTagged) (reference.Canonical, error) {
return TrustedReference(ctx, dockerCli, ref, nil)
}
// if there is a tar wrapper, the dockerfile needs to be replaced inside it
if buildCtx != nil {
// Wrap the tar archive to replace the Dockerfile entry with the rewritten
// Dockerfile which uses trusted pulls.
buildCtx = replaceDockerfileTarWrapper(ctx, buildCtx, relDockerfile, translator, &resolvedTags)
buildCtx = replaceDockerfileForContentTrust(ctx, buildCtx, relDockerfile, translator, &resolvedTags)
} else if dockerfileCtx != nil {
// if there was not archive context still do the possible replacements in Dockerfile
newDockerfile, _, err := rewriteDockerfileFrom(ctx, dockerfileCtx, translator)
newDockerfile, _, err := rewriteDockerfileFrom(ctx, dockerfileCtx, translator, !options.untrusted)
if err != nil {
return err
}
Expand Down Expand Up @@ -460,7 +461,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
return err
}
}
if command.IsTrusted() {
if !options.untrusted {
// Since the build was successful, now we must tag any of the resolved
// images from the above Dockerfile rewrite.
for _, resolved := range resolvedTags {
Expand Down Expand Up @@ -503,7 +504,7 @@ type resolvedTag struct {
// "FROM <image>" instructions to a digest reference. `translator` is a
// function that takes a repository name and tag reference and returns a
// trusted digest reference.
func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator translatorFunc) (newDockerfile []byte, resolvedTags []*resolvedTag, err error) {
func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator translatorFunc, istrusted bool) (newDockerfile []byte, resolvedTags []*resolvedTag, err error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the whole reason this rewriteDockerfileFrom() file is there, is for docker content trust. Instead of passing this boolean, can we just skip the whole call to this function?

So, actually, looks like that's already what's done; it's only called in that case, so we can just remove the boolean (and check). This is the only location this function is called:

if command.IsTrusted() {
translator := func(ctx context.Context, ref reference.NamedTagged) (reference.Canonical, error) {
return TrustedReference(ctx, dockerCli, ref, nil)
}
// if there is a tar wrapper, the dockerfile needs to be replaced inside it
if buildCtx != nil {
// Wrap the tar archive to replace the Dockerfile entry with the rewritten
// Dockerfile which uses trusted pulls.
buildCtx = replaceDockerfileTarWrapper(ctx, buildCtx, relDockerfile, translator, &resolvedTags)
} else if dockerfileCtx != nil {
// if there was not archive context still do the possible replacements in Dockerfile
newDockerfile, _, err := rewriteDockerfileFrom(ctx, dockerfileCtx, translator)
if err != nil {
return err
}
dockerfileCtx = ioutil.NopCloser(bytes.NewBuffer(newDockerfile))
}
}

scanner := bufio.NewScanner(dockerfile)
buf := bytes.NewBuffer(nil)

Expand All @@ -520,7 +521,7 @@ func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator
return nil, nil, err
}
ref = reference.TagNameOnly(ref)
if ref, ok := ref.(reference.NamedTagged); ok && command.IsTrusted() {
if ref, ok := ref.(reference.NamedTagged); ok && istrusted {
trustedRef, err := translator(ctx, ref)
if err != nil {
return nil, nil, err
Expand All @@ -543,11 +544,10 @@ func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator
return buf.Bytes(), resolvedTags, scanner.Err()
}

// replaceDockerfileTarWrapper wraps the given input tar archive stream and
// replaces the entry with the given Dockerfile name with the contents of the
// new Dockerfile. Returns a new tar archive stream with the replaced
// Dockerfile.
func replaceDockerfileTarWrapper(ctx context.Context, inputTarStream io.ReadCloser, dockerfileName string, translator translatorFunc, resolvedTags *[]*resolvedTag) io.ReadCloser {
// replaceDockerfileForContentTrust wraps the given input tar archive stream and
// uses the translator to replace the Dockerfile which uses a trusted reference.
// Returns a new tar archive stream with the replaced Dockerfile.
func replaceDockerfileForContentTrust(ctx context.Context, inputTarStream io.ReadCloser, dockerfileName string, translator translatorFunc, resolvedTags *[]*resolvedTag) io.ReadCloser {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could rename these functions to, e.g. resolveDigestsYadaYada or something else (it's just that we use them for "content trust" as it requires all images to be referenced by digest)

pipeReader, pipeWriter := io.Pipe()
go func() {
tarReader := tar.NewReader(inputTarStream)
Expand All @@ -574,7 +574,7 @@ func replaceDockerfileTarWrapper(ctx context.Context, inputTarStream io.ReadClos
// generated from a directory on the local filesystem, the
// Dockerfile will only appear once in the archive.
var newDockerfile []byte
newDockerfile, *resolvedTags, err = rewriteDockerfileFrom(ctx, content, translator)
newDockerfile, *resolvedTags, err = rewriteDockerfileFrom(ctx, content, translator, true)
if err != nil {
pipeWriter.CloseWithError(err)
return
Expand Down
1 change: 1 addition & 0 deletions cli/command/image/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ COPY data /data
options := newBuildOptions()
options.context = dir.Path()
options.dockerfileName = df.Path()
options.untrusted = true

err = runBuild(cli, options)
assert.NilError(t, err)
Expand Down
11 changes: 6 additions & 5 deletions cli/command/image/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
)

type pullOptions struct {
remote string
all bool
platform string
remote string
all bool
platform string
untrusted bool
}

// NewPullCommand creates a new `docker pull` command
Expand All @@ -38,7 +39,7 @@ func NewPullCommand(dockerCli command.Cli) *cobra.Command {
flags.BoolVarP(&opts.all, "all-tags", "a", false, "Download all tagged images in the repository")

command.AddPlatformFlag(flags, &opts.platform)
command.AddTrustVerificationFlags(flags)
command.AddTrustVerificationFlags(flags, &opts.untrusted, dockerCli.ContentTrustEnabled())

return cmd
}
Expand All @@ -65,7 +66,7 @@ func runPull(cli command.Cli, opts pullOptions) error {

// Check if reference has a digest
_, isCanonical := distributionRef.(reference.Canonical)
if command.IsTrusted() && !isCanonical {
if !opts.untrusted && !isCanonical {
err = trustedPull(ctx, cli, imgRefAndAuth, opts.platform)
} else {
err = imagePullPrivileged(ctx, cli, imgRefAndAuth, opts.all, opts.platform)
Expand Down
42 changes: 42 additions & 0 deletions cli/command/image/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/docker/cli/internal/test"
"github.com/docker/cli/internal/test/notary"
"github.com/docker/docker/api/types"
"github.com/gotestyourself/gotestyourself/assert"
is "github.com/gotestyourself/gotestyourself/assert/cmp"
Expand Down Expand Up @@ -77,3 +78,44 @@ func TestNewPullCommandSuccess(t *testing.T) {
golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("pull-command-success.%s.golden", tc.name))
}
}

func TestNewPullCommandWithContentTrustErrors(t *testing.T) {
testCases := []struct {
name string
args []string
expectedError string
notaryFunc test.NotaryClientFuncType
}{
{
name: "offline-notary-server",
notaryFunc: notary.GetOfflineNotaryRepository,
expectedError: "client is offline",
args: []string{"image:tag"},
},
{
name: "empty-notary-server",
notaryFunc: notary.GetUninitializedNotaryRepository,
expectedError: "remote trust data does not exist",
args: []string{"image:tag"},
},
{
name: "empty-notary-server",
notaryFunc: notary.GetEmptyTargetsNotaryRepository,
expectedError: "No valid trust data for tag",
args: []string{"image:tag"},
},
}
for _, tc := range testCases {
cli := test.NewFakeCli(&fakeClient{
imagePullFunc: func(ref string, options types.ImagePullOptions) (io.ReadCloser, error) {
return ioutil.NopCloser(strings.NewReader("")), fmt.Errorf("shouldn't try to pull image")
},
}, test.EnableContentTrust)
cli.SetNotaryClient(tc.notaryFunc)
cmd := NewPullCommand(cli)
cmd.SetOutput(ioutil.Discard)
cmd.SetArgs(tc.args)
err := cmd.Execute()
assert.ErrorContains(t, err, tc.expectedError)
}
}
Loading