From 4203b4943154ad09917702dbae2f2b414f70dc5d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 26 Sep 2017 12:53:21 -0400 Subject: [PATCH] Refactor image commands to make use of the new trust struct for trusted pull Signed-off-by: Daniel Nephin --- cli/command/image/pull.go | 27 +++----- cli/command/image/pull_test.go | 5 -- cli/command/image/trust.go | 120 +++++++++++++++++++-------------- cli/command/trust/revoke.go | 17 ++--- cli/command/trust/sign.go | 49 ++++++++------ cli/command/trust/sign_test.go | 9 +-- cli/command/trust/view.go | 10 +-- cli/trust/trust.go | 29 +++++--- gometalinter.json | 2 +- 9 files changed, 139 insertions(+), 129 deletions(-) diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index e60e5a4348..0cf3df609d 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -6,8 +6,8 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/trust" "github.com/docker/distribution/reference" - "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" "golang.org/x/net/context" @@ -40,11 +40,14 @@ func NewPullCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runPull(dockerCli command.Cli, opts pullOptions) error { - distributionRef, err := reference.ParseNormalizedNamed(opts.remote) +func runPull(cli command.Cli, opts pullOptions) error { + ctx := context.Background() + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, AuthResolver(cli), opts.remote) if err != nil { return err } + + distributionRef := imgRefAndAuth.Reference() if opts.all && !reference.IsNameOnly(distributionRef) { return errors.New("tag can't be used with --all-tags/-a") } @@ -52,27 +55,16 @@ func runPull(dockerCli command.Cli, opts pullOptions) error { if !opts.all && reference.IsNameOnly(distributionRef) { distributionRef = reference.TagNameOnly(distributionRef) if tagged, ok := distributionRef.(reference.Tagged); ok { - fmt.Fprintf(dockerCli.Out(), "Using default tag: %s\n", tagged.Tag()) + fmt.Fprintf(cli.Out(), "Using default tag: %s\n", tagged.Tag()) } } - // Resolve the Repository name from fqn to RepositoryInfo - repoInfo, err := registry.ParseRepositoryInfo(distributionRef) - if err != nil { - return err - } - - ctx := context.Background() - - authConfig := command.ResolveAuthConfig(ctx, dockerCli, repoInfo.Index) - requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, repoInfo.Index, "pull") - // Check if reference has a digest _, isCanonical := distributionRef.(reference.Canonical) if command.IsTrusted() && !isCanonical { - err = trustedPull(ctx, dockerCli, repoInfo, distributionRef, authConfig, requestPrivilege) + err = trustedPull(ctx, cli, imgRefAndAuth) } else { - err = imagePullPrivileged(ctx, dockerCli, authConfig, reference.FamiliarString(distributionRef), requestPrivilege, opts.all) + err = imagePullPrivileged(ctx, cli, imgRefAndAuth, opts.all) } if err != nil { if strings.Contains(err.Error(), "when fetching 'plugin'") { @@ -80,6 +72,5 @@ func runPull(dockerCli command.Cli, opts pullOptions) error { } return err } - return nil } diff --git a/cli/command/image/pull_test.go b/cli/command/image/pull_test.go index 157b391fdd..ec83ba1f35 100644 --- a/cli/command/image/pull_test.go +++ b/cli/command/image/pull_test.go @@ -32,11 +32,6 @@ func TestNewPullCommandErrors(t *testing.T) { expectedError: "tag can't be used with --all-tags/-a", args: []string{"--all-tags", "image:tag"}, }, - { - name: "pull-error", - args: []string{"--disable-content-trust=false", "image:tag"}, - expectedError: "you are not authorized to perform this operation: server returned 401.", - }, } for _, tc := range testCases { cli := test.NewFakeCli(&fakeClient{}) diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index a6c2710dd3..f2847e954b 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -11,11 +11,12 @@ import ( "github.com/docker/cli/cli/trust" "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" + registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/registry" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" - "github.com/opencontainers/go-digest" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" @@ -181,57 +182,13 @@ func imagePushPrivileged(ctx context.Context, cli command.Cli, authConfig types. } // trustedPull handles content trust pulling of an image -func trustedPull(ctx context.Context, cli command.Cli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, requestPrivilege types.RequestPrivilegeFunc) error { - var refs []target - - notaryRepo, err := trust.GetNotaryRepository(cli.In(), cli.Out(), command.UserAgent(), repoInfo, &authConfig, "pull") +func trustedPull(ctx context.Context, cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth) error { + refs, err := getTrustedPullTargets(cli, imgRefAndAuth) if err != nil { - fmt.Fprintf(cli.Out(), "Error establishing connection to trust repository: %s\n", err) return err } - if tagged, isTagged := ref.(reference.NamedTagged); !isTagged { - // List all targets - targets, err := notaryRepo.ListTargets(trust.ReleasesRole, data.CanonicalTargetsRole) - if err != nil { - return trust.NotaryError(ref.Name(), err) - } - for _, tgt := range targets { - t, err := convertTarget(tgt.Target) - if err != nil { - fmt.Fprintf(cli.Out(), "Skipping target for %q\n", reference.FamiliarName(ref)) - continue - } - // Only list tags in the top level targets role or the releases delegation role - ignore - // all other delegation roles - if tgt.Role != trust.ReleasesRole && tgt.Role != data.CanonicalTargetsRole { - continue - } - refs = append(refs, t) - } - if len(refs) == 0 { - return trust.NotaryError(ref.Name(), errors.Errorf("No trusted tags for %s", ref.Name())) - } - } else { - t, err := notaryRepo.GetTargetByName(tagged.Tag(), trust.ReleasesRole, data.CanonicalTargetsRole) - if err != nil { - return trust.NotaryError(ref.Name(), err) - } - // Only get the tag if it's in the top level targets role or the releases delegation role - // ignore it if it's in any other delegation roles - if t.Role != trust.ReleasesRole && t.Role != data.CanonicalTargetsRole { - return trust.NotaryError(ref.Name(), errors.Errorf("No trust data for %s", tagged.Tag())) - } - - logrus.Debugf("retrieving target for %s role\n", t.Role) - r, err := convertTarget(t.Target) - if err != nil { - return err - - } - refs = append(refs, r) - } - + ref := imgRefAndAuth.Reference() for i, r := range refs { displayTag := r.name if displayTag != "" { @@ -243,7 +200,7 @@ func trustedPull(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi if err != nil { return err } - if err := imagePullPrivileged(ctx, cli, authConfig, reference.FamiliarString(trustedRef), requestPrivilege, false); err != nil { + if err := imagePullPrivileged(ctx, cli, imgRefAndAuth, false); err != nil { return err } @@ -259,13 +216,65 @@ func trustedPull(ctx context.Context, cli command.Cli, repoInfo *registry.Reposi return nil } -// imagePullPrivileged pulls the image and displays it to the output -func imagePullPrivileged(ctx context.Context, cli command.Cli, authConfig types.AuthConfig, ref string, requestPrivilege types.RequestPrivilegeFunc, all bool) error { +func getTrustedPullTargets(cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth) ([]target, error) { + notaryRepo, err := cli.NotaryClient(imgRefAndAuth, trust.ActionsPullOnly) + if err != nil { + fmt.Fprintf(cli.Out(), "Error establishing connection to trust repository: %s\n", err) + return nil, err + } - encodedAuth, err := command.EncodeAuthToBase64(authConfig) + ref := imgRefAndAuth.Reference() + tagged, isTagged := ref.(reference.NamedTagged) + if !isTagged { + // List all targets + targets, err := notaryRepo.ListTargets(trust.ReleasesRole, data.CanonicalTargetsRole) + if err != nil { + return nil, trust.NotaryError(ref.Name(), err) + } + var refs []target + for _, tgt := range targets { + t, err := convertTarget(tgt.Target) + if err != nil { + fmt.Fprintf(cli.Out(), "Skipping target for %q\n", reference.FamiliarName(ref)) + continue + } + // Only list tags in the top level targets role or the releases delegation role - ignore + // all other delegation roles + if tgt.Role != trust.ReleasesRole && tgt.Role != data.CanonicalTargetsRole { + continue + } + refs = append(refs, t) + } + if len(refs) == 0 { + return nil, trust.NotaryError(ref.Name(), errors.Errorf("No trusted tags for %s", ref.Name())) + } + return refs, nil + } + + t, err := notaryRepo.GetTargetByName(tagged.Tag(), trust.ReleasesRole, data.CanonicalTargetsRole) + if err != nil { + return nil, trust.NotaryError(ref.Name(), err) + } + // Only get the tag if it's in the top level targets role or the releases delegation role + // ignore it if it's in any other delegation roles + if t.Role != trust.ReleasesRole && t.Role != data.CanonicalTargetsRole { + return nil, trust.NotaryError(ref.Name(), errors.Errorf("No trust data for %s", tagged.Tag())) + } + + logrus.Debugf("retrieving target for %s role\n", t.Role) + r, err := convertTarget(t.Target) + return []target{r}, err +} + +// imagePullPrivileged pulls the image and displays it to the output +func imagePullPrivileged(ctx context.Context, cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth, all bool) error { + ref := reference.FamiliarString(imgRefAndAuth.Reference()) + + encodedAuth, err := command.EncodeAuthToBase64(*imgRefAndAuth.AuthConfig()) if err != nil { return err } + requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, imgRefAndAuth.RepoInfo().Index, "pull") options := types.ImagePullOptions{ RegistryAuth: encodedAuth, PrivilegeFunc: requestPrivilege, @@ -346,3 +355,10 @@ func TagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canon return cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef) } + +// AuthResolver returns an auth resolver function from a command.Cli +func AuthResolver(cli command.Cli) func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { + return func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { + return command.ResolveAuthConfig(ctx, cli, index) + } +} diff --git a/cli/command/trust/revoke.go b/cli/command/trust/revoke.go index c3aae10e1c..23b423a39a 100644 --- a/cli/command/trust/revoke.go +++ b/cli/command/trust/revoke.go @@ -5,15 +5,13 @@ import ( "fmt" "os" - "github.com/docker/docker/api/types" - registrytypes "github.com/docker/docker/api/types/registry" + "github.com/docker/cli/cli" + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/command/image" + "github.com/docker/cli/cli/trust" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" "github.com/pkg/errors" - - "github.com/docker/cli/cli" - "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/trust" "github.com/spf13/cobra" ) @@ -38,10 +36,7 @@ func newRevokeCommand(dockerCli command.Cli) *cobra.Command { func revokeTrust(cli command.Cli, remote string, options revokeOptions) error { ctx := context.Background() - authResolver := func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { - return command.ResolveAuthConfig(ctx, cli, index) - } - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, authResolver, remote) + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), remote) if err != nil { return err } @@ -57,7 +52,7 @@ func revokeTrust(cli command.Cli, remote string, options revokeOptions) error { } } - notaryRepo, err := cli.NotaryClient(*imgRefAndAuth, trust.ActionsPushAndPull) + notaryRepo, err := cli.NotaryClient(imgRefAndAuth, trust.ActionsPushAndPull) if err != nil { return err } diff --git a/cli/command/trust/sign.go b/cli/command/trust/sign.go index 68598b9878..631f5b64a5 100644 --- a/cli/command/trust/sign.go +++ b/cli/command/trust/sign.go @@ -3,6 +3,7 @@ package trust import ( "context" "fmt" + "io" "path" "sort" "strings" @@ -11,8 +12,6 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/image" "github.com/docker/cli/cli/trust" - "github.com/docker/docker/api/types" - registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" "github.com/pkg/errors" @@ -25,30 +24,23 @@ func newSignCommand(dockerCli command.Cli) *cobra.Command { Short: "Sign an image", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return signImage(dockerCli, args[0]) + return runSignImage(dockerCli, args[0]) }, } return cmd } -func signImage(cli command.Cli, imageName string) error { +func runSignImage(cli command.Cli, imageName string) error { ctx := context.Background() - authResolver := func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { - return command.ResolveAuthConfig(ctx, cli, index) - } - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, authResolver, imageName) + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), imageName) if err != nil { return err } - tag := imgRefAndAuth.Tag() - if tag == "" { - if imgRefAndAuth.Digest() != "" { - return fmt.Errorf("cannot use a digest reference for IMAGE:TAG") - } - return fmt.Errorf("No tag specified for %s", imageName) + if err := validateTag(imgRefAndAuth); err != nil { + return err } - notaryRepo, err := cli.NotaryClient(*imgRefAndAuth, trust.ActionsPushAndPull) + notaryRepo, err := cli.NotaryClient(imgRefAndAuth, trust.ActionsPushAndPull) if err != nil { return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } @@ -78,7 +70,7 @@ func signImage(cli command.Cli, imageName string) error { } } requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, imgRefAndAuth.RepoInfo().Index, "push") - target, err := createTarget(notaryRepo, tag) + target, err := createTarget(notaryRepo, imgRefAndAuth.Tag()) if err != nil { switch err := err.(type) { case client.ErrNoSuchTarget, client.ErrRepositoryNotExist: @@ -91,21 +83,36 @@ func signImage(cli command.Cli, imageName string) error { return err } } + return signAndPublishToTarget(cli.Out(), imgRefAndAuth, notaryRepo, target) +} - fmt.Fprintf(cli.Out(), "Signing and pushing trust metadata for %s\n", imageName) +func signAndPublishToTarget(out io.Writer, imgRefAndAuth trust.ImageRefAndAuth, notaryRepo client.Repository, target client.Target) error { + tag := imgRefAndAuth.Tag() + fmt.Fprintf(out, "Signing and pushing trust metadata for %s\n", imgRefAndAuth.Name()) existingSigInfo, err := getExistingSignatureInfoForReleasedTag(notaryRepo, tag) if err != nil { return err } err = image.AddTargetToAllSignableRoles(notaryRepo, &target) if err == nil { - prettyPrintExistingSignatureInfo(cli, existingSigInfo) + prettyPrintExistingSignatureInfo(out, existingSigInfo) err = notaryRepo.Publish() } if err != nil { return errors.Wrapf(err, "failed to sign %q:%s", imgRefAndAuth.RepoInfo().Name.Name(), tag) } - fmt.Fprintf(cli.Out(), "Successfully signed %q:%s\n", imgRefAndAuth.RepoInfo().Name.Name(), tag) + fmt.Fprintf(out, "Successfully signed %q:%s\n", imgRefAndAuth.RepoInfo().Name.Name(), tag) + return nil +} + +func validateTag(imgRefAndAuth trust.ImageRefAndAuth) error { + tag := imgRefAndAuth.Tag() + if tag == "" { + if imgRefAndAuth.Digest() != "" { + return fmt.Errorf("cannot use a digest reference for IMAGE:TAG") + } + return fmt.Errorf("No tag specified for %s", imgRefAndAuth.Name()) + } return nil } @@ -154,10 +161,10 @@ func getExistingSignatureInfoForReleasedTag(notaryRepo client.Repository, tag st return releasedTargetInfoList[0], nil } -func prettyPrintExistingSignatureInfo(cli command.Cli, existingSigInfo trustTagRow) { +func prettyPrintExistingSignatureInfo(out io.Writer, existingSigInfo trustTagRow) { sort.Strings(existingSigInfo.Signers) joinedSigners := strings.Join(existingSigInfo.Signers, ", ") - fmt.Fprintf(cli.Out(), "Existing signatures for tag %s digest %s from:\n%s\n", existingSigInfo.TagName, existingSigInfo.HashHex, joinedSigners) + fmt.Fprintf(out, "Existing signatures for tag %s digest %s from:\n%s\n", existingSigInfo.TagName, existingSigInfo.HashHex, joinedSigners) } func initNotaryRepoWithSigners(notaryRepo client.Repository, newSigner data.RoleName) error { diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index 66ed800ba9..ec9a5dc59f 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -6,6 +6,8 @@ import ( "os" "testing" + "bytes" + "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/trust" "github.com/docker/cli/internal/test" @@ -264,13 +266,12 @@ func TestGetExistingSignatureInfoForReleasedTag(t *testing.T) { } func TestPrettyPrintExistingSignatureInfo(t *testing.T) { - fakeCli := test.NewFakeCli(&fakeClient{}) - + buf := bytes.NewBuffer(nil) signers := []string{"Bob", "Alice", "Carol"} existingSig := trustTagRow{trustTagKey{"tagName", "abc123"}, signers} - prettyPrintExistingSignatureInfo(fakeCli, existingSig) + prettyPrintExistingSignatureInfo(buf, existingSig) - assert.Contains(t, fakeCli.OutBuffer().String(), "Existing signatures for tag tagName digest abc123 from:\nAlice, Bob, Carol") + assert.Contains(t, buf.String(), "Existing signatures for tag tagName digest abc123 from:\nAlice, Bob, Carol") } func TestSignCommandChangeListIsCleanedOnError(t *testing.T) { diff --git a/cli/command/trust/view.go b/cli/command/trust/view.go index 383c03d1ae..f4de65f87e 100644 --- a/cli/command/trust/view.go +++ b/cli/command/trust/view.go @@ -11,9 +11,8 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/formatter" + "github.com/docker/cli/cli/command/image" "github.com/docker/cli/cli/trust" - "github.com/docker/docker/api/types" - registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/notary" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" @@ -61,15 +60,12 @@ func newViewCommand(dockerCli command.Cli) *cobra.Command { func lookupTrustInfo(cli command.Cli, remote string) error { ctx := context.Background() - authResolver := func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { - return command.ResolveAuthConfig(ctx, cli, index) - } - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, authResolver, remote) + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), remote) if err != nil { return err } tag := imgRefAndAuth.Tag() - notaryRepo, err := cli.NotaryClient(*imgRefAndAuth, trust.ActionsPullOnly) + notaryRepo, err := cli.NotaryClient(imgRefAndAuth, trust.ActionsPullOnly) if err != nil { return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } diff --git a/cli/trust/trust.go b/cli/trust/trust.go index 90809db2a9..d392153e5c 100644 --- a/cli/trust/trust.go +++ b/cli/trust/trust.go @@ -1,7 +1,6 @@ package trust import ( - "context" "encoding/json" "io" "net" @@ -32,6 +31,7 @@ import ( digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/sirupsen/logrus" + "golang.org/x/net/context" ) var ( @@ -288,6 +288,7 @@ func GetSignableRoles(repo client.Repository, target *client.Target) ([]data.Rol // ImageRefAndAuth contains all reference information and the auth config for an image request type ImageRefAndAuth struct { + original string authConfig *types.AuthConfig reference reference.Named repoInfo *registry.RepositoryInfo @@ -295,27 +296,29 @@ type ImageRefAndAuth struct { digest digest.Digest } -// NewImageRefAndAuth creates a new ImageRefAndAuth struct -func NewImageRefAndAuth(authConfig *types.AuthConfig, reference reference.Named, repoInfo *registry.RepositoryInfo, tag string, digest digest.Digest) *ImageRefAndAuth { - return &ImageRefAndAuth{authConfig, reference, repoInfo, tag, digest} -} - // GetImageReferencesAndAuth retrieves the necessary reference and auth information for an image name // as a ImageRefAndAuth struct -func GetImageReferencesAndAuth(ctx context.Context, authResolver func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig, imgName string) (*ImageRefAndAuth, error) { +func GetImageReferencesAndAuth(ctx context.Context, authResolver func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig, imgName string) (ImageRefAndAuth, error) { ref, err := reference.ParseNormalizedNamed(imgName) if err != nil { - return nil, err + return ImageRefAndAuth{}, err } // Resolve the Repository name from fqn to RepositoryInfo repoInfo, err := registry.ParseRepositoryInfo(ref) if err != nil { - return nil, err + return ImageRefAndAuth{}, err } authConfig := authResolver(ctx, repoInfo.Index) - return NewImageRefAndAuth(&authConfig, ref, repoInfo, getTag(ref), getDigest(ref)), nil + return ImageRefAndAuth{ + original: imgName, + authConfig: &authConfig, + reference: ref, + repoInfo: repoInfo, + tag: getTag(ref), + digest: getDigest(ref), + }, nil } func getTag(ref reference.Named) string { @@ -364,3 +367,9 @@ func (imgRefAuth *ImageRefAndAuth) Tag() string { func (imgRefAuth *ImageRefAndAuth) Digest() digest.Digest { return imgRefAuth.digest } + +// Name returns the image name used to initialize the ImageRefAndAuth +func (imgRefAuth *ImageRefAndAuth) Name() string { + return imgRefAuth.original + +} diff --git a/gometalinter.json b/gometalinter.json index 1f2131dd4d..e3230994e3 100644 --- a/gometalinter.json +++ b/gometalinter.json @@ -26,6 +26,6 @@ "vet" ], - "Cyclo": 19, + "Cyclo": 16, "LineLength": 200 }