Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move some trust-related code to trust package #5894

Merged
merged 3 commits into from
Mar 6, 2025
Merged

move some trust-related code to trust package #5894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6f456b
cli/command/image: deprecate and internalize TrustedPush
thaJeztah Mar 5, 2025
d804360
cli/command/image: deprecate PushTrustedReference, move to trust
thaJeztah Mar 5, 2025
e37d814
cli/command/image: deprecate TagTrusted, move to cli/trust
thaJeztah Mar 5, 2025
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
3 changes: 2 additions & 1 deletion cli/command/container/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ 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 @@ -242,7 +243,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
return err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
return image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef)
return trust.TagTrusted(ctx, dockerCli.Client(), dockerCli.Err(), trustedRef, taggedRef)
}
Comment on lines 245 to 247
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like this introduced a regression; most likely because before this dockerCli.Client() would be initialised lazily, and it's possible some settings were loaded elsewhere.

I opened a PR that reverts some of the changes;

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like reverting those changes (PR above) still makes things fail occasionally.

I think it's just a bad test and/or multiple tests affecting each other (perhaps using same port or otherwise)

return nil
}
Expand Down
3 changes: 2 additions & 1 deletion cli/command/image/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ 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 @@ -406,7 +407,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 := TagTrusted(ctx, dockerCli, resolved.digestRef, resolved.tagRef); err != nil {
if err := trust.TagTrusted(ctx, dockerCli.Client(), dockerCli.Err(), resolved.digestRef, resolved.tagRef); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/command/image/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ To push the complete multi-platform image, remove the --platform flag.

defer responseBody.Close()
if !opts.untrusted {
// TODO PushTrustedReference currently doesn't respect `--quiet`
return PushTrustedReference(ctx, dockerCli, repoInfo, ref, authConfig, responseBody)
// TODO pushTrustedReference currently doesn't respect `--quiet`
return pushTrustedReference(ctx, dockerCli, repoInfo, ref, authConfig, responseBody)
}

if opts.quiet {
Expand Down
142 changes: 21 additions & 121 deletions cli/command/image/trust.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@ package image
import (
"context"
"encoding/hex"
"encoding/json"
"fmt"
"io"
"sort"

"github.com/distribution/reference"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/internal/jsonstream"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/cli/trust"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/image"
registrytypes "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/registry"
Expand Down Expand Up @@ -44,7 +41,9 @@ func newNotaryClient(cli command.Streams, imgRefAndAuth trust.ImageRefAndAuth) (
return trust.GetNotaryRepository(cli.In(), cli.Out(), command.UserAgent(), imgRefAndAuth.RepoInfo(), imgRefAndAuth.AuthConfig(), "pull")
}

// TrustedPush handles content trust pushing of an image
// TrustedPush handles content trust pushing of an image.
//
// Deprecated: this function was only used internally and will be removed in the next release.
func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, options image.PushOptions) error {
responseBody, err := cli.Client().ImagePush(ctx, reference.FamiliarString(ref), options)
if err != nil {
Expand All @@ -53,120 +52,19 @@ func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi

defer responseBody.Close()

return PushTrustedReference(ctx, cli, repoInfo, ref, authConfig, responseBody)
return trust.PushTrustedReference(ctx, cli, repoInfo, ref, authConfig, responseBody, command.UserAgent())
}

// PushTrustedReference pushes a canonical reference to the trust server.
//
//nolint:gocyclo
// Deprecated: use [trust.PushTrustedReference] instead. this function was only used internally and will be removed in the next release.
func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
// If it is a trusted push we would like to find the target entry which match the
// tag provided in the function and then do an AddTarget later.
notaryTarget := &client.Target{}
// Count the times of calling for handleTarget,
// if it is called more that once, that should be considered an error in a trusted push.
cnt := 0
handleTarget := func(msg jsonstream.JSONMessage) {
cnt++
if cnt > 1 {
// handleTarget should only be called once. This will be treated as an error.
return
}

var pushResult types.PushResult
err := json.Unmarshal(*msg.Aux, &pushResult)
if err == nil && pushResult.Tag != "" {
if dgst, err := digest.Parse(pushResult.Digest); err == nil {
h, err := hex.DecodeString(dgst.Hex())
if err != nil {
notaryTarget = nil
return
}
notaryTarget.Name = pushResult.Tag
notaryTarget.Hashes = data.Hashes{string(dgst.Algorithm()): h}
notaryTarget.Length = int64(pushResult.Size)
}
}
}

var tag string
switch x := ref.(type) {
case reference.Canonical:
return errors.New("cannot push a digest reference")
case reference.NamedTagged:
tag = x.Tag()
default:
// We want trust signatures to always take an explicit tag,
// otherwise it will act as an untrusted push.
if err := jsonstream.Display(ctx, in, ioStreams.Out()); err != nil {
return err
}
_, _ = fmt.Fprintln(ioStreams.Err(), "No tag specified, skipping trust metadata push")
return nil
}

if err := jsonstream.Display(ctx, in, ioStreams.Out(), jsonstream.WithAuxCallback(handleTarget)); err != nil {
return err
}

if cnt > 1 {
return errors.Errorf("internal error: only one call to handleTarget expected")
}

if notaryTarget == nil {
return errors.Errorf("no targets found, provide a specific tag in order to sign it")
}

_, _ = fmt.Fprintln(ioStreams.Out(), "Signing and pushing trust metadata")

repo, err := trust.GetNotaryRepository(ioStreams.In(), ioStreams.Out(), command.UserAgent(), repoInfo, &authConfig, "push", "pull")
if err != nil {
return errors.Wrap(err, "error establishing connection to trust repository")
}

// get the latest repository metadata so we can figure out which roles to sign
_, err = repo.ListTargets()

switch err.(type) {
case client.ErrRepoNotInitialized, client.ErrRepositoryNotExist:
keys := repo.GetCryptoService().ListKeys(data.CanonicalRootRole)
var rootKeyID string
// always select the first root key
if len(keys) > 0 {
sort.Strings(keys)
rootKeyID = keys[0]
} else {
rootPublicKey, err := repo.GetCryptoService().Create(data.CanonicalRootRole, "", data.ECDSAKey)
if err != nil {
return err
}
rootKeyID = rootPublicKey.ID()
}

// Initialize the notary repository with a remotely managed snapshot key
if err := repo.Initialize([]string{rootKeyID}, data.CanonicalSnapshotRole); err != nil {
return trust.NotaryError(repoInfo.Name.Name(), err)
}
_, _ = fmt.Fprintf(ioStreams.Out(), "Finished initializing %q\n", repoInfo.Name.Name())
err = repo.AddTarget(notaryTarget, data.CanonicalTargetsRole)
case nil:
// already initialized and we have successfully downloaded the latest metadata
err = trust.AddToAllSignableRoles(repo, notaryTarget)
default:
return trust.NotaryError(repoInfo.Name.Name(), err)
}

if err == nil {
err = repo.Publish()
}

if err != nil {
err = errors.Wrapf(err, "failed to sign %s:%s", repoInfo.Name.Name(), tag)
return trust.NotaryError(repoInfo.Name.Name(), err)
}
return pushTrustedReference(ctx, ioStreams, repoInfo, ref, authConfig, in)
}

_, _ = fmt.Fprintf(ioStreams.Out(), "Successfully signed %s:%s\n", repoInfo.Name.Name(), tag)
return nil
// pushTrustedReference pushes a canonical reference to the trust server.
func pushTrustedReference(ctx context.Context, ioStreams command.Streams, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig registrytypes.AuthConfig, in io.Reader) error {
return trust.PushTrustedReference(ctx, ioStreams, repoInfo, ref, authConfig, in, command.UserAgent())
}

// trustedPull handles content trust pulling of an image
Expand Down Expand Up @@ -206,7 +104,11 @@ func trustedPull(ctx context.Context, cli command.Cli, imgRefAndAuth trust.Image
return err
}

if err := TagTrusted(ctx, cli, trustedRef, tagged); err != nil {
// 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 {
Comment on lines +107 to +111
Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW; there's a couple of changes I want to start making; I just had a look at the image references and formats we're using, and, even slightly simplified in my test, we're literally converting the image name 11 times, including juggling between a string, to a reference (various permutations), back to a string, before we even make a request with the engine 🫠

=== RUN   TestJuggleFormats
     STEP 1:  busybox (string)
     STEP 2:  docker.io/library/busybox (reference.repository)
     STEP 3:  docker.io/library/busybox:latest (reference.taggedReference)
     STEP 4:  docker.io/library/busybox:latest (string)
     STEP 5:  docker.io/library/busybox:latest (reference.taggedReference)
     STEP 6A: docker.io/library/busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 (reference.canonicalReference)
     STEP 6B: docker.io/library/busybox:latest (reference.taggedReference)
     --> Tagging busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 as busybox:latest
     STEP 7A: busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 (string)
     STEP 7B: busybox:latest (string)
     STEP 8:  docker.io/library/busybox:latest (reference.taggedReference)
     STEP 9:  docker.io/library/busybox:latest (reference.taggedReference)
     --> SENDING TO DAEMON:  /images/busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0/tag?repo=busybox&tag=latest
     STEP 10: docker.io/library/busybox (reference.repository)
     STEP 11: docker.io/library/busybox:latest (reference.taggedReference)
     STEP 12: busybox (string)
     --> backend.GetImage
     --> backend.TagImage

return err
}
}
Expand Down Expand Up @@ -327,15 +229,13 @@ func convertTarget(t client.Target) (target, error) {
}, nil
}

// TagTrusted tags a trusted 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.
//
// Deprecated: 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 {
// Use familiar references when interacting with client and output
familiarRef := reference.FamiliarString(ref)
trustedFamiliarRef := reference.FamiliarString(trustedRef)

_, _ = fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef)

return cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef)
return trust.TagTrusted(ctx, cli.Client(), cli.Err(), trustedRef, ref)
}

// AuthResolver returns an auth resolver function from a command.Cli
Expand Down
4 changes: 2 additions & 2 deletions cli/command/plugin/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
"github.com/distribution/reference"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/image"
"github.com/docker/cli/cli/internal/jsonstream"
"github.com/docker/cli/cli/trust"
registrytypes "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/registry"
"github.com/pkg/errors"
Expand Down Expand Up @@ -66,7 +66,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOptions) error
defer responseBody.Close()

if !opts.untrusted {
return image.PushTrustedReference(ctx, dockerCli, repoInfo, named, authConfig, responseBody)
return trust.PushTrustedReference(ctx, dockerCli, repoInfo, named, authConfig, responseBody, command.UserAgent())
}

return jsonstream.Display(ctx, responseBody, dockerCli.Out())
Expand Down
8 changes: 7 additions & 1 deletion cli/command/trust/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sort"
"strings"

"github.com/distribution/reference"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/image"
Expand Down Expand Up @@ -98,10 +99,15 @@ func runSignImage(ctx context.Context, dockerCLI command.Cli, options signOption
if err != nil {
return err
}
return image.TrustedPush(ctx, dockerCLI, imgRefAndAuth.RepoInfo(), imgRefAndAuth.Reference(), *imgRefAndAuth.AuthConfig(), imagetypes.PushOptions{
responseBody, err := dockerCLI.Client().ImagePush(ctx, reference.FamiliarString(imgRefAndAuth.Reference()), imagetypes.PushOptions{
RegistryAuth: encodedAuth,
PrivilegeFunc: requestPrivilege,
})
if err != nil {
return err
}
defer responseBody.Close()
return trust.PushTrustedReference(ctx, dockerCLI, imgRefAndAuth.RepoInfo(), imgRefAndAuth.Reference(), authConfig, responseBody, command.UserAgent())
default:
return err
}
Expand Down
Loading
Loading