Merge pull request #717 from jcsirot/fix-handling-short-id

Fix handling short id in docker app commands

Author: jcsirot

e2e/images_test.go

@@ -97,7 +97,7 @@ Deleted: b-simple-app:latest`,
 		cmd.Command = dockerCli.Command("app", "image", "rm", "b-simple-app")
 		icmd.RunCmd(cmd).Assert(t, icmd.Expected{
 			ExitCode: 1,
-			Err:      `Error: no such image b-simple-app:latest`,
+			Err:      `b-simple-app:latest: reference not found`,
 		})
 
 		expectedOutput := "APP IMAGE APP NAME\n"
@@ -141,28 +141,28 @@ a-simple-app:latest simple
 		dockerAppImageTag("a-simple-app$2", "b-simple-app")
 		icmd.RunCmd(cmd).Assert(t, icmd.Expected{
 			ExitCode: 1,
-			Err:      `could not parse 'a-simple-app$2' as a valid reference`,
+			Err:      `could not parse "a-simple-app$2" as a valid reference`,
 		})
 
 		// with invalid target reference
 		dockerAppImageTag("a-simple-app", "b@simple-app")
 		icmd.RunCmd(cmd).Assert(t, icmd.Expected{
 			ExitCode: 1,
-			Err:      `could not parse 'b@simple-app' as a valid reference`,
+			Err:      `could not parse "b@simple-app" as a valid reference`,
 		})
 
 		// with unexisting source image
 		dockerAppImageTag("b-simple-app", "target")
 		icmd.RunCmd(cmd).Assert(t, icmd.Expected{
 			ExitCode: 1,
-			Err:      `could not tag 'b-simple-app': no such App image`,
+			Err:      `could not tag "b-simple-app": no such App image`,
 		})
 
 		// with unexisting source tag
 		dockerAppImageTag("a-simple-app:not-a-tag", "target")
 		icmd.RunCmd(cmd).Assert(t, icmd.Expected{
 			ExitCode: 1,
-			Err:      `could not tag 'a-simple-app:not-a-tag': no such App image`,
+			Err:      `could not tag "a-simple-app:not-a-tag": no such App image`,
 		})
 
 		// tag image with only names

internal/cnab/cnab.go

@@ -15,6 +15,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cnab-to-oci/remotes"
 	"github.com/docker/distribution/reference"
+	"github.com/sirupsen/logrus"
 )
 
 type nameKind uint
@@ -90,23 +91,34 @@ func ResolveBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name
 
 // GetBundle searches for the bundle locally and tries to pull it if not found
 func GetBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name string) (*bundle.Bundle, reference.Reference, error) {
-	ref, err := store.StringToRef(name)
+	bndl, ref, err := getBundleFromStore(bundleStore, name)
 	if err != nil {
+		named, err := store.StringToNamedRef(name)
+		if err != nil {
+			return nil, nil, err
+		}
+		fmt.Fprintf(dockerCli.Err(), "Unable to find App image %q locally\n", reference.FamiliarString(named))
+		fmt.Fprintf(dockerCli.Out(), "Pulling from registry...\n")
+		bndl, err = PullBundle(dockerCli, bundleStore, named)
+		if err != nil {
+			return nil, nil, err
+		}
+		ref = named
+	}
+	return bndl, ref, nil
+}
+
+func getBundleFromStore(bundleStore appstore.BundleStore, name string) (*bundle.Bundle, reference.Reference, error) {
+	ref, err := bundleStore.LookUp(name)
+	if err != nil {
+		logrus.Debugf("Unable to find reference %q in the bundle store", name)
 		return nil, nil, err
 	}
 	bndl, err := bundleStore.Read(ref)
 	if err != nil {
-		fmt.Fprintf(dockerCli.Err(), "Unable to find App image %q locally\n", reference.FamiliarString(ref))
-
-		fmt.Fprintf(dockerCli.Out(), "Pulling from registry...\n")
-		if named, ok := ref.(reference.Named); ok {
-			bndl, err = PullBundle(dockerCli, bundleStore, named)
-			if err != nil {
-				return nil, nil, err
-			}
-		}
+		logrus.Debugf("Unable to read bundle %q from store", reference.FamiliarString(ref))
+		return nil, nil, err
 	}
-
 	return bndl, ref, nil
 }
 

internal/commands/image/list_test.go

@@ -48,6 +48,10 @@ func (b *bundleStoreStubForListCmd) Remove(ref reference.Reference) error {
 	return nil
 }
 
+func (b *bundleStoreStubForListCmd) LookUp(refOrID string) (reference.Reference, error) {
+	return nil, nil
+}
+
 func TestListWithQuietFlag(t *testing.T) {
 	var buf bytes.Buffer
 	w := bufio.NewWriter(&buf)

internal/commands/image/rm.go

@@ -48,7 +48,7 @@ $ docker app image rm 34be4a0c5f50`,
 }
 
 func runRm(bundleStore store.BundleStore, app string) error {
-	ref, err := store.StringToRef(app)
+	ref, err := bundleStore.LookUp(app)
 	if err != nil {
 		return err
 	}

internal/commands/image/tag.go

@@ -7,6 +7,7 @@ import (
 	"github.com/docker/app/internal/store"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/config"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -46,20 +47,26 @@ func runTag(bundleStore store.BundleStore, srcAppImage, destAppImage string) err
 }
 
 func readBundle(name string, bundleStore store.BundleStore) (*bundle.Bundle, error) {
-	cnabRef, err := store.StringToRef(name)
+	cnabRef, err := bundleStore.LookUp(name)
 	if err != nil {
-		return nil, err
+		switch err.(type) {
+		case *store.UnknownReferenceError:
+			return nil, fmt.Errorf("could not tag %q: no such App image", name)
+		default:
+			return nil, errors.Wrapf(err, "could not tag %q", name)
+		}
+
 	}
 
 	bundle, err := bundleStore.Read(cnabRef)
 	if err != nil {
-		return nil, fmt.Errorf("could not tag '%s': no such App image", name)
+		return nil, errors.Wrapf(err, "could not tag %q: no such App image", name)
 	}
 	return bundle, nil
 }
 
 func storeBundle(bundle *bundle.Bundle, name string, bundleStore store.BundleStore) error {
-	cnabRef, err := store.StringToRef(name)
+	cnabRef, err := store.StringToNamedRef(name)
 	if err != nil {
 		return err
 	}

internal/commands/image/tag_test.go

@@ -10,12 +10,21 @@ import (
 	"github.com/docker/distribution/reference"
 )
 
+func parseRefOrDie(t *testing.T, ref string) reference.Named {
+	t.Helper()
+	named, err := reference.ParseNormalizedNamed(ref)
+	assert.NilError(t, err)
+	return named
+}
+
 type bundleStoreStub struct {
 	ReadBundle   *bundle.Bundle
 	ReadError    error
 	StoredBundle string
 	StoredError  error
 	StoredID     reference.Digested
+	LookUpRef    reference.Reference
+	LookUpError  error
 }
 
 func (b *bundleStoreStub) Store(ref reference.Reference, bndle *bundle.Bundle) (reference.Digested, error) {
@@ -44,15 +53,25 @@ func (b *bundleStoreStub) Remove(ref reference.Reference) error {
 	return nil
 }
 
+func (b *bundleStoreStub) LookUp(refOrID string) (reference.Reference, error) {
+	defer func() {
+		b.LookUpRef = nil
+		b.LookUpError = nil
+	}()
+	return b.LookUpRef, b.LookUpError
+}
+
 var mockedBundleStore = &bundleStoreStub{}
 
 func TestInvalidSourceReference(t *testing.T) {
 	// given a bad source image reference
 	const badRef = "b@d reference"
+	// and given bundle store will return an error on LookUp
+	mockedBundleStore.LookUpError = fmt.Errorf("error from bundleStore.LookUp")
 
 	err := runTag(mockedBundleStore, badRef, "")
 
-	assert.ErrorContains(t, err, fmt.Sprintf("could not parse '%s' as a valid reference", badRef))
+	assert.Assert(t, err != nil)
 }
 
 func TestUnexistingSource(t *testing.T) {
@@ -67,18 +86,20 @@ func TestUnexistingSource(t *testing.T) {
 }
 
 func TestInvalidDestinationReference(t *testing.T) {
-	// given a bundle is returned by bundleStore.Read
+	// given a reference and a bundle is returned by bundleStore.LookUp and bundleStore.Read
+	mockedBundleStore.LookUpRef = parseRefOrDie(t, "ref")
 	mockedBundleStore.ReadBundle = &bundle.Bundle{}
 	// and given a bad destination reference
 	const badRef = "b@d reference"
 
 	err := runTag(mockedBundleStore, "ref", badRef)
 
-	assert.ErrorContains(t, err, fmt.Sprintf("could not parse '%s' as a valid reference", badRef))
+	assert.ErrorContains(t, err, fmt.Sprintf("invalid reference format"))
 }
 
 func TestBundleNotStored(t *testing.T) {
-	// given a bundle is returned by bundleStore.Read
+	// given a reference and a bundle is returned by bundleStore.LookUp and bundleStore.Read
+	mockedBundleStore.LookUpRef = parseRefOrDie(t, "src-app")
 	mockedBundleStore.ReadBundle = &bundle.Bundle{}
 	// and given bundleStore.Store will return an error
 	mockedBundleStore.StoredError = fmt.Errorf("error from bundleStore.Store")
@@ -89,7 +110,8 @@ func TestBundleNotStored(t *testing.T) {
 }
 
 func TestSuccessfulyTag(t *testing.T) {
-	// given a bundle is returned by bundleStore.Read
+	// given a reference and a bundle is returned by bundleStore.LookUp and bundleStore.Read
+	mockedBundleStore.LookUpRef = parseRefOrDie(t, "src-app")
 	mockedBundleStore.ReadBundle = &bundle.Bundle{}
 	// and given valid source and output references
 	const (

internal/store/app.go

@@ -74,7 +74,7 @@ func (a ApplicationStore) BundleStore() (BundleStore, error) {
 	if err := os.MkdirAll(path, 0755); err != nil {
 		return nil, errors.Wrapf(err, "failed to create bundle store directory %q", path)
 	}
-	return &bundleStore{path: path}, nil
+	return NewBundleStore(path)
 }
 
 func makeDigestedDirectory(context string) string {