From e54886148182109f1bc5ab470770c7a13f6e020b Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 10 Oct 2017 15:57:16 -0400 Subject: [PATCH] Refactor runPull to remove second GetImageReferencesAndAuth Fix unit tests to catch the regression. Signed-off-by: Daniel Nephin --- cli/command/image/pull.go | 25 +++++++++++-------------- cli/command/image/pull_test.go | 25 ++++++++++++++++++------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index 9d77e04696..1824de2a9b 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -41,28 +41,25 @@ func NewPullCommand(dockerCli command.Cli) *cobra.Command { } func runPull(cli command.Cli, opts pullOptions) error { - ctx := context.Background() - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, AuthResolver(cli), opts.remote) - if err != nil { + distributionRef, err := reference.ParseNormalizedNamed(opts.remote) + switch { + case err != nil: return err - } - - distributionRef := imgRefAndAuth.Reference() - if opts.all && !reference.IsNameOnly(distributionRef) { + case opts.all && !reference.IsNameOnly(distributionRef): return errors.New("tag can't be used with --all-tags/-a") - } - - if !opts.all && reference.IsNameOnly(distributionRef) { + case !opts.all && reference.IsNameOnly(distributionRef): distributionRef = reference.TagNameOnly(distributionRef) - imgRefAndAuth, err = trust.GetImageReferencesAndAuth(ctx, AuthResolver(cli), distributionRef.String()) - if err != nil { - return err - } if tagged, ok := distributionRef.(reference.Tagged); ok { fmt.Fprintf(cli.Out(), "Using default tag: %s\n", tagged.Tag()) } } + ctx := context.Background() + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, AuthResolver(cli), distributionRef.String()) + if err != nil { + return err + } + // Check if reference has a digest _, isCanonical := distributionRef.(reference.Canonical) if command.IsTrusted() && !isCanonical { diff --git a/cli/command/image/pull_test.go b/cli/command/image/pull_test.go index ec83ba1f35..b0e9929abe 100644 --- a/cli/command/image/pull_test.go +++ b/cli/command/image/pull_test.go @@ -2,11 +2,14 @@ package image import ( "fmt" + "io" "io/ioutil" + "strings" "testing" "github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test/testutil" + "github.com/docker/docker/api/types" "github.com/gotestyourself/gotestyourself/golden" "github.com/stretchr/testify/assert" ) @@ -44,20 +47,28 @@ func TestNewPullCommandErrors(t *testing.T) { func TestNewPullCommandSuccess(t *testing.T) { testCases := []struct { - name string - args []string + name string + args []string + expectedTag string }{ { - name: "simple", - args: []string{"image:tag"}, + name: "simple", + args: []string{"image:tag"}, + expectedTag: "image:tag", }, { - name: "simple-no-tag", - args: []string{"image"}, + name: "simple-no-tag", + args: []string{"image"}, + expectedTag: "image:latest", }, } for _, tc := range testCases { - cli := test.NewFakeCli(&fakeClient{}) + cli := test.NewFakeCli(&fakeClient{ + imagePullFunc: func(ref string, options types.ImagePullOptions) (io.ReadCloser, error) { + assert.Equal(t, tc.expectedTag, ref, tc.name) + return ioutil.NopCloser(strings.NewReader("")), nil + }, + }) cmd := NewPullCommand(cli) cmd.SetOutput(ioutil.Discard) cmd.SetArgs(tc.args)