Skip to content

Commit

Permalink
cli/trust: TagTrusted: lazily request APIClient
Browse files Browse the repository at this point in the history
Commit e37d814 moved the image.TagTrusted
function to the trust package, but changed the signature slightly to accept
an API client, instead of requiring the command.Cli. However, this could
result in situations where the Client obtained from the CLI was not correctly
initialized, resulting in failures in our e2e test;

    === FAIL: e2e/global TestPromptExitCode/plugin_upgrade (9.14s)
        cli_test.go:203: assertion failed:
            Command:  docker plugin push registry:5000/plugin-content-trust-upgrade:next
            ExitCode: 1
            Error:    exit status 1
            Stdout:   The push refers to repository [registry:5000/plugin-content-trust-upgrade]
            24ec5b45d59b: Preparing
            6a594992d358: Preparing
            224414d1b129: Preparing
            24ec5b45d59b: Preparing
            6a594992d358: Preparing
            224414d1b129: Preparing

            Stderr:   error pushing plugin: failed to do request: Head "https://registry:5000/v2/plugin-content-trust-upgrade/blobs/sha256:6a594992d358facbbc4ab134bbbba77cb91e0adee6ff0d6103403ff94a9b796c": http: server gave HTTP response to HTTPS client

            Failures:
            ExitCode was 1 expected 0
            Expected no error

This patch changes the signature to accept an "APIClientProvider" so that
the Client is obtained the moment when used.

We should look what exactly causes this situation, and if we can make
sure that requesting the `Client()` will always produce the client with
the expected configuration.

WARNING: looks like the test is still flaky after this change, so it
may just be a bad test, or tests affecting each-other (same port, but
different config?). That said; these changes may still be ok to
include.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Mar 8, 2025
1 parent 2eec746 commit 47e2011
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 13 deletions.
3 changes: 1 addition & 2 deletions cli/command/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/docker/cli/cli/command/image"
"github.com/docker/cli/cli/internal/jsonstream"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/cli/trust"
"github.com/docker/cli/opts"
"github.com/docker/docker/api/types/container"
imagetypes "github.com/docker/docker/api/types/image"
Expand Down Expand Up @@ -243,7 +242,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
return err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
return trust.TagTrusted(ctx, dockerCli.Client(), dockerCli.Err(), trustedRef, taggedRef)
return tagTrusted(ctx, dockerCli, trustedRef, taggedRef)
}
return nil
}
Expand Down
13 changes: 13 additions & 0 deletions cli/command/container/trust.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package container

import (
"context"

"github.com/distribution/reference"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/trust"
)

func tagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canonical, ref reference.NamedTagged) error {
return trust.TagTrusted(ctx, cli, cli.Err(), trustedRef, ref)
}
3 changes: 1 addition & 2 deletions cli/command/image/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/docker/cli/cli/command/image/build"
"github.com/docker/cli/cli/internal/jsonstream"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/cli/trust"
"github.com/docker/cli/opts"
"github.com/docker/docker/api"
"github.com/docker/docker/api/types"
Expand Down Expand Up @@ -407,7 +406,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
// Since the build was successful, now we must tag any of the resolved
// images from the above Dockerfile rewrite.
for _, resolved := range resolvedTags {
if err := trust.TagTrusted(ctx, dockerCli.Client(), dockerCli.Err(), resolved.digestRef, resolved.tagRef); err != nil {
if err := tagTrusted(ctx, dockerCli, resolved.digestRef, resolved.tagRef); err != nil {
return err
}
}
Expand Down
17 changes: 10 additions & 7 deletions cli/command/image/trust.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,7 @@ func trustedPull(ctx context.Context, cli command.Cli, imgRefAndAuth trust.Image
return err
}

// Use familiar references when interacting with client and output
familiarRef := reference.FamiliarString(tagged)
trustedFamiliarRef := reference.FamiliarString(trustedRef)
_, _ = fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef)
if err := cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef); err != nil {
if err := tagTrusted(ctx, cli, trustedRef, tagged); err != nil {
return err
}
}
Expand Down Expand Up @@ -233,9 +229,16 @@ func convertTarget(t client.Target) (target, error) {
// that updates the given image references to their familiar format for tagging
// and printing.
//
// Deprecated: this function was only used internally, and will be removed in the next release.
// Deprecated: use [trust.TagTrusted] instead. This function was only used internally, and will be removed in the next release.
func TagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canonical, ref reference.NamedTagged) error {
return trust.TagTrusted(ctx, cli.Client(), cli.Err(), trustedRef, ref)
return tagTrusted(ctx, cli, trustedRef, ref)
}

// tagTrusted tags a trusted ref. It is a shallow wrapper around APIClient.ImageTag
// that updates the given image references to their familiar format for tagging
// and printing.
func tagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canonical, ref reference.NamedTagged) error {
return trust.TagTrusted(ctx, cli, cli.Err(), trustedRef, ref)
}

// AuthResolver returns an auth resolver function from a command.Cli
Expand Down
8 changes: 6 additions & 2 deletions cli/trust/trust_tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ import (
"github.com/docker/docker/client"
)

type APIClientProvider interface {
Client() client.APIClient
}

// TagTrusted tags a trusted ref. It is a shallow wrapper around [client.Client.ImageTag]
// that updates the given image references to their familiar format for tagging
// and printing.
func TagTrusted(ctx context.Context, apiClient client.ImageAPIClient, out io.Writer, trustedRef reference.Canonical, ref reference.NamedTagged) error {
func TagTrusted(ctx context.Context, cli APIClientProvider, out io.Writer, trustedRef reference.Canonical, ref reference.NamedTagged) error {
// Use familiar references when interacting with client and output
familiarRef := reference.FamiliarString(ref)
trustedFamiliarRef := reference.FamiliarString(trustedRef)

_, _ = fmt.Fprintf(out, "Tagging %s as %s\n", trustedFamiliarRef, familiarRef)
return apiClient.ImageTag(ctx, trustedFamiliarRef, familiarRef)
return cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef)
}

0 comments on commit 47e2011

Please sign in to comment.