From d956386b2d0cc28f2f9732aee5ba3c5ab2036d6c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 26 Sep 2017 12:33:35 -0400 Subject: [PATCH 1/2] Update gometalinter The update includes bug fixes in gometalinter and updates to linters, which discovered more linter problems. Signed-off-by: Daniel Nephin --- cli/command/container/hijack.go | 1 + cli/command/container/opts_test.go | 1 + cli/command/image/trust_test.go | 2 ++ cli/command/stack/deploy.go | 5 ++--- cli/command/stack/ps_test.go | 1 + cli/command/swarm/unlock.go | 8 ++------ cli/command/trust/revoke_test.go | 2 ++ cli/command/trust/sign_test.go | 17 ++++++++++++++--- cli/command/trust/view.go | 4 +++- cli/trust/trust_test.go | 2 ++ dockerfiles/Dockerfile.lint | 2 +- docs/yaml/yaml.go | 9 ++++----- gometalinter.json | 8 ++++++-- 13 files changed, 41 insertions(+), 21 deletions(-) diff --git a/cli/command/container/hijack.go b/cli/command/container/hijack.go index 3653f1f9b4..b3ca2e6ad5 100644 --- a/cli/command/container/hijack.go +++ b/cli/command/container/hijack.go @@ -185,6 +185,7 @@ func setRawTerminal(streams command.Streams) error { return streams.Out().SetRawTerminal() } +// nolint: unparam func restoreTerminal(streams command.Streams, in io.Closer) error { streams.In().RestoreTerminal() streams.Out().RestoreTerminal() diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 08c1d07da3..0aa043839c 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -43,6 +43,7 @@ func TestValidateAttach(t *testing.T) { } } +// nolint: unparam func parseRun(args []string) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) { flags := pflag.NewFlagSet("run", pflag.ContinueOnError) flags.SetOutput(ioutil.Discard) diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go index d173973d88..6356438a3a 100644 --- a/cli/command/image/trust_test.go +++ b/cli/command/image/trust_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/notary/passphrase" "github.com/docker/notary/trustpinning" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func unsetENV() { @@ -67,6 +68,7 @@ func TestAddTargetToAllSignableRolesError(t *testing.T) { defer os.RemoveAll(tmpDir) notaryRepo, err := client.NewFileCachedRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + require.NoError(t, err) target := client.Target{} err = AddTargetToAllSignableRoles(notaryRepo, &target) assert.EqualError(t, err, "client is offline") diff --git a/cli/command/stack/deploy.go b/cli/command/stack/deploy.go index 8e14b70d2f..a6f9ef6377 100644 --- a/cli/command/stack/deploy.go +++ b/cli/command/stack/deploy.go @@ -104,13 +104,12 @@ func checkDaemonIsSwarmManager(ctx context.Context, dockerCli command.Cli) error } // pruneServices removes services that are no longer referenced in the source -func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, services map[string]struct{}) bool { +func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, services map[string]struct{}) { client := dockerCli.Client() oldServices, err := getServices(ctx, client, namespace.Name()) if err != nil { fmt.Fprintf(dockerCli.Err(), "Failed to list services: %s", err) - return true } pruneServices := []swarm.Service{} @@ -119,5 +118,5 @@ func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert pruneServices = append(pruneServices, service) } } - return removeServices(ctx, dockerCli, pruneServices) + removeServices(ctx, dockerCli, pruneServices) } diff --git a/cli/command/stack/ps_test.go b/cli/command/stack/ps_test.go index 3bb22609b5..fb7055f09c 100644 --- a/cli/command/stack/ps_test.go +++ b/cli/command/stack/ps_test.go @@ -59,6 +59,7 @@ func TestStackPsEmptyStack(t *testing.T) { }) cmd := newPsCommand(fakeCli) cmd.SetArgs([]string{"foo"}) + cmd.SetOutput(ioutil.Discard) assert.Error(t, cmd.Execute()) assert.EqualError(t, cmd.Execute(), "nothing found in stack: foo") diff --git a/cli/command/swarm/unlock.go b/cli/command/swarm/unlock.go index dd999a7f4b..700a8ed533 100644 --- a/cli/command/swarm/unlock.go +++ b/cli/command/swarm/unlock.go @@ -15,24 +15,20 @@ import ( "golang.org/x/net/context" ) -type unlockOptions struct{} - func newUnlockCommand(dockerCli command.Cli) *cobra.Command { - opts := unlockOptions{} - cmd := &cobra.Command{ Use: "unlock", Short: "Unlock swarm", Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return runUnlock(dockerCli, opts) + return runUnlock(dockerCli) }, } return cmd } -func runUnlock(dockerCli command.Cli, opts unlockOptions) error { +func runUnlock(dockerCli command.Cli) error { client := dockerCli.Client() ctx := context.Background() diff --git a/cli/command/trust/revoke_test.go b/cli/command/trust/revoke_test.go index 43c68a30d1..5a6e105b16 100644 --- a/cli/command/trust/revoke_test.go +++ b/cli/command/trust/revoke_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/notary/passphrase" "github.com/docker/notary/trustpinning" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTrustRevokeCommandErrors(t *testing.T) { @@ -140,6 +141,7 @@ func TestGetSignableRolesForTargetAndRemoveError(t *testing.T) { defer os.RemoveAll(tmpDir) notaryRepo, err := client.NewFileCachedRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + require.NoError(t, err) target := client.Target{} err = getSignableRolesForTargetAndRemove(target, notaryRepo) assert.EqualError(t, err, "client is offline") diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index d75e212a91..66ed800ba9 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -17,6 +17,7 @@ import ( "github.com/docker/notary/trustpinning" "github.com/docker/notary/tuf/data" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const passwd = "password" @@ -151,6 +152,7 @@ func TestAddStageSigners(t *testing.T) { NewThreshold: notary.MinThreshold, AddKeys: data.KeyList([]data.PublicKey{userKey}), }) + require.NoError(t, err) expectedChange := changelist.NewTUFChange( changelist.ActionCreate, userRole, @@ -165,6 +167,7 @@ func TestAddStageSigners(t *testing.T) { expectedJSON, err = json.Marshal(&changelist.TUFDelegation{ AddPaths: []string{""}, }) + require.NoError(t, err) expectedChange = changelist.NewTUFChange( changelist.ActionCreate, userRole, @@ -182,6 +185,7 @@ func TestAddStageSigners(t *testing.T) { NewThreshold: notary.MinThreshold, AddKeys: data.KeyList([]data.PublicKey{userKey}), }) + require.NoError(t, err) expectedChange = changelist.NewTUFChange( changelist.ActionCreate, releasesRole, @@ -196,6 +200,7 @@ func TestAddStageSigners(t *testing.T) { expectedJSON, err = json.Marshal(&changelist.TUFDelegation{ AddPaths: []string{""}, }) + require.NoError(t, err) expectedChange = changelist.NewTUFChange( changelist.ActionCreate, releasesRole, @@ -268,18 +273,24 @@ func TestPrettyPrintExistingSignatureInfo(t *testing.T) { assert.Contains(t, fakeCli.OutBuffer().String(), "Existing signatures for tag tagName digest abc123 from:\nAlice, Bob, Carol") } -func TestChangeList(t *testing.T) { +func TestSignCommandChangeListIsCleanedOnError(t *testing.T) { tmpDir, err := ioutil.TempDir("", "docker-sign-test-") assert.NoError(t, err) defer os.RemoveAll(tmpDir) + config.SetDir(tmpDir) - cmd := newSignCommand( - test.NewFakeCli(&fakeClient{})) + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getLoadedNotaryRepository) + cmd := newSignCommand(cli) cmd.SetArgs([]string{"ubuntu:latest"}) cmd.SetOutput(ioutil.Discard) + err = cmd.Execute() + require.Error(t, err) + notaryRepo, err := client.NewFileCachedRepository(tmpDir, "docker.io/library/ubuntu", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) assert.NoError(t, err) cl, err := notaryRepo.GetChangelist() + require.NoError(t, err) assert.Equal(t, len(cl.List()), 0) } diff --git a/cli/command/trust/view.go b/cli/command/trust/view.go index 18d07585eb..383c03d1ae 100644 --- a/cli/command/trust/view.go +++ b/cli/command/trust/view.go @@ -113,7 +113,9 @@ func lookupTrustInfo(cli command.Cli, remote string) error { // If we do not have additional signers, do not display if len(signerRoleToKeyIDs) > 0 { fmt.Fprintf(cli.Out(), "\nList of signers and their keys for %s:\n\n", strings.Split(remote, ":")[0]) - printSignerInfo(cli.Out(), signerRoleToKeyIDs) + if err := printSignerInfo(cli.Out(), signerRoleToKeyIDs); err != nil { + return err + } } // This will always have the root and targets information diff --git a/cli/trust/trust_test.go b/cli/trust/trust_test.go index 5cd339d358..da9c3a8688 100644 --- a/cli/trust/trust_test.go +++ b/cli/trust/trust_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/notary/trustpinning" digest "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetTag(t *testing.T) { @@ -53,6 +54,7 @@ func TestGetSignableRolesError(t *testing.T) { defer os.RemoveAll(tmpDir) notaryRepo, err := client.NewFileCachedRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + require.NoError(t, err) target := client.Target{} _, err = GetSignableRoles(notaryRepo, &target) assert.EqualError(t, err, "client is offline") diff --git a/dockerfiles/Dockerfile.lint b/dockerfiles/Dockerfile.lint index 4b3cccf085..9cef6ea1a9 100644 --- a/dockerfiles/Dockerfile.lint +++ b/dockerfiles/Dockerfile.lint @@ -2,7 +2,7 @@ FROM golang:1.8.3-alpine RUN apk add -U git -ARG GOMETALINTER_SHA=4306381615a2ba2a207f8fcea02c08c6b2b0803f +ARG GOMETALINTER_SHA=8eca55135021737bbc65ed68b548b3336853274c RUN go get -d github.com/alecthomas/gometalinter && \ cd /go/src/github.com/alecthomas/gometalinter && \ git checkout -q "$GOMETALINTER_SHA" && \ diff --git a/docs/yaml/yaml.go b/docs/yaml/yaml.go index 2dbe406bdb..528eefbbb3 100644 --- a/docs/yaml/yaml.go +++ b/docs/yaml/yaml.go @@ -10,7 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" ) type cmdOption struct { @@ -46,18 +46,17 @@ type cmdDoc struct { // GenYamlTree creates yaml structured ref files func GenYamlTree(cmd *cobra.Command, dir string) error { - identity := func(s string) string { return s } emptyStr := func(s string) string { return "" } - return GenYamlTreeCustom(cmd, dir, emptyStr, identity) + return GenYamlTreeCustom(cmd, dir, emptyStr) } // GenYamlTreeCustom creates yaml structured ref files -func GenYamlTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) error { +func GenYamlTreeCustom(cmd *cobra.Command, dir string, filePrepender func(string) string) error { for _, c := range cmd.Commands() { if !c.IsAvailableCommand() || c.IsHelpCommand() { continue } - if err := GenYamlTreeCustom(c, dir, filePrepender, linkHandler); err != nil { + if err := GenYamlTreeCustom(c, dir, filePrepender); err != nil { return err } } diff --git a/gometalinter.json b/gometalinter.json index b5956dca60..1f2131dd4d 100644 --- a/gometalinter.json +++ b/gometalinter.json @@ -1,8 +1,12 @@ { "Vendor": true, "Deadline": "2m", - "Sort": ["linter", "severity", "path"], - "Exclude": ["cli/compose/schema/bindata.go"], + "Sort": ["linter", "severity", "path", "line"], + "Exclude": [ + "cli/compose/schema/bindata.go", + "parameter .* always receives" + ], + "EnableGC": true, "DisableAll": true, "Enable": [ From 4203b4943154ad09917702dbae2f2b414f70dc5d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 26 Sep 2017 12:53:21 -0400 Subject: [PATCH 2/2] 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 }