From ca9eaab5bb10dae65e4aaaa5ce1bcee3cc190df0 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Sirot Date: Wed, 23 Oct 2019 10:05:03 +0200 Subject: [PATCH 1/2] Large refactoring of the bundle store in order to handle ids in commands Signed-off-by: Jean-Christophe Sirot --- internal/cnab/cnab.go | 5 +- internal/store/app.go | 3 +- internal/store/bundle.go | 196 ++++++++++++++++++++++++++------- internal/store/digest.go | 12 +- internal/store/installation.go | 2 +- 5 files changed, 175 insertions(+), 43 deletions(-) diff --git a/internal/cnab/cnab.go b/internal/cnab/cnab.go index 9634607dd..dcb651932 100644 --- a/internal/cnab/cnab.go +++ b/internal/cnab/cnab.go @@ -10,7 +10,6 @@ import ( "github.com/docker/app/internal" "github.com/docker/app/internal/log" "github.com/docker/app/internal/packager" - "github.com/docker/app/internal/store" appstore "github.com/docker/app/internal/store" "github.com/docker/cli/cli/command" "github.com/docker/cnab-to-oci/remotes" @@ -90,7 +89,9 @@ 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) + //ref, err := store.StringToRef(name) + ref, err := bundleStore.LookUp(name) + fmt.Println(ref) if err != nil { return nil, nil, err } diff --git a/internal/store/app.go b/internal/store/app.go index c026f23bb..0d2473a3b 100644 --- a/internal/store/app.go +++ b/internal/store/app.go @@ -74,7 +74,8 @@ 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 &bundleStore{path: path}, nil + return NewBundleStore(path) } func makeDigestedDirectory(context string) string { diff --git a/internal/store/bundle.go b/internal/store/bundle.go index 8546b8ad3..5d831577e 100644 --- a/internal/store/bundle.go +++ b/internal/store/bundle.go @@ -11,7 +11,6 @@ import ( "github.com/deislabs/cnab-go/bundle" "github.com/docker/distribution/reference" - "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -22,12 +21,27 @@ type BundleStore interface { Read(ref reference.Reference) (*bundle.Bundle, error) List() ([]reference.Reference, error) Remove(ref reference.Reference) error + LookUp(refOrID string) (reference.Reference, error) } var _ BundleStore = &bundleStore{} type bundleStore struct { - path string + path string + referenceMap map[ID][]reference.Reference +} + +func NewBundleStore(path string) (BundleStore, error) { + bundleStore := &bundleStore{ + path: path, + referenceMap: make(map[ID][]reference.Reference), + } + err := bundleStore.scanAllBundles() + if err != nil { + return nil, err + } + fmt.Println(bundleStore.referenceMap) + return bundleStore, nil } // We store bundles either by image:tags, image:digest or by unique ID (actually, bundle's sha256). @@ -72,56 +86,27 @@ func (b *bundleStore) Store(ref reference.Reference, bndle *bundle.Bundle) (refe } func (b *bundleStore) Read(ref reference.Reference) (*bundle.Bundle, error) { - path, err := b.storePath(ref) + paths, err := b.storePaths(ref) + fmt.Println(paths) if err != nil { return nil, errors.Wrapf(err, "failed to read bundle %q", ref) } - data, err := ioutil.ReadFile(filepath.Join(path, "bundle.json")) + bndl, err := b.fetchBundleJSON(filepath.Join(paths[0], "bundle.json")) if err != nil { return nil, errors.Wrapf(err, "failed to read bundle %q", ref) } - var bndle bundle.Bundle - if err := json.Unmarshal(data, &bndle); err != nil { - return nil, errors.Wrapf(err, "failed to read bundle %q", ref) - } - return &bndle, nil + return bndl, nil } // Returns the list of all bundles present in the bundle store func (b *bundleStore) List() ([]reference.Reference, error) { var references []reference.Reference - digests := filepath.Join(b.path, "_ids") - if err := filepath.Walk(b.path, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - if info.IsDir() { - return nil - } - - if !strings.HasSuffix(info.Name(), ".json") { - return nil - } - - if strings.HasPrefix(path, digests) { - rel := path[len(digests)+1:] - dg := strings.Split(filepath.ToSlash(rel), "/")[0] - references = append(references, ID{digest.NewDigestFromEncoded(digest.SHA256, dg)}) - return nil - } - ref, err := b.pathToReference(path) - if err != nil { - return err + for _, refAliases := range b.referenceMap { + for _, ref := range refAliases { + references = append(references, ref) } - - references = append(references, ref) - - return nil - }); err != nil { - return nil, err } sort.Slice(references, func(i, j int) bool { @@ -143,6 +128,75 @@ func (b *bundleStore) Remove(ref reference.Reference) error { return os.RemoveAll(path) } +func (b *bundleStore) LookUp(refOrID string) (reference.Reference, error) { + ref, err := FromString(refOrID) + if err == nil { + return ref, nil + } + if isShortID(refOrID) { + ref, err := b.matchShortID(refOrID) + if err == nil { + return ref, nil + } + } + return reference.ParseNormalizedNamed(refOrID) +} + +func (b *bundleStore) matchShortID(shortID string) (reference.Reference, error) { + var found reference.Reference + for id := range b.referenceMap { + if strings.HasPrefix(id.String(), shortID) { + if found != nil && found != id { + return nil, fmt.Errorf("Ambiguous reference found") + } + found = id + } + } + if found == nil { + return nil, fmt.Errorf("Could not find reference") + } + fmt.Printf("found = %s\n", found) + return found, nil +} + +func (b *bundleStore) referenceToID(ref reference.Reference) (ID, error) { + if id, ok := ref.(ID); ok { + return id, nil + } + for id, refs := range b.referenceMap { + for _, r := range refs { + if r == ref { + return id, nil + } + } + } + return ID{}, fmt.Errorf("%s: reference not found", ref.String()) +} + +func (b *bundleStore) storePaths(ref reference.Reference) ([]string, error) { + var paths []string + + id, err := b.referenceToID(ref) + if err != nil { + return nil, err + } + + if refs, exist := b.referenceMap[id]; exist { + for _, rf := range refs { + path, err := b.storePath(rf) + if err != nil { + continue + } + paths = append(paths, path) + } + } + + if len(paths) == 0 { + return nil, fmt.Errorf("%s: reference not found", ref.String()) + } + return paths, nil +} + func (b *bundleStore) storePath(ref reference.Reference) (string, error) { named, ok := ref.(reference.Named) if !ok { @@ -175,6 +229,72 @@ func (b *bundleStore) storePath(ref reference.Reference) (string, error) { return storeDir, nil } +// Returns the list of all bundles present in the bundle store +func (b *bundleStore) scanAllBundles() error { + digests := filepath.Join(b.path, "_ids") + if err := filepath.Walk(b.path, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + return nil + } + + if !strings.HasSuffix(info.Name(), ".json") { + return nil + } + + if strings.HasPrefix(path, digests) { + rel := path[len(digests)+1:] + dg := strings.Split(filepath.ToSlash(rel), "/")[0] + //references = append(references, ID{dg}) + id := ID{dg} + if _, ok := b.referenceMap[id]; !ok { + b.referenceMap[id] = []reference.Reference{id} + } else { + b.referenceMap[id] = append(b.referenceMap[id], id) + } + return nil + } + + ref, err := b.pathToReference(path) + if err != nil { + return err + } + bndl, err := b.fetchBundleJSON(path) + if err != nil { + return err + } + id, err := FromBundle(bndl) + if err != nil { + return err + } + if _, ok := b.referenceMap[id]; !ok { + b.referenceMap[id] = []reference.Reference{ref} + } else { + b.referenceMap[id] = append(b.referenceMap[id], ref) + } + + return nil + }); err != nil { + return err + } + return nil +} + +func (b *bundleStore) fetchBundleJSON(bundlePath string) (*bundle.Bundle, error) { + data, err := ioutil.ReadFile(bundlePath) + if err != nil { + return nil, errors.Wrapf(err, "failed to read file %s", bundlePath) + } + var bndle bundle.Bundle + if err := json.Unmarshal(data, &bndle); err != nil { + return nil, errors.Wrapf(err, "failed to read file %s", bundlePath) + } + return &bndle, nil +} + func (b *bundleStore) pathToReference(path string) (reference.Named, error) { // Clean the path and remove the local bundle store path cleanpath := filepath.ToSlash(path) diff --git a/internal/store/digest.go b/internal/store/digest.go index e244c27da..7d5c9d92e 100644 --- a/internal/store/digest.go +++ b/internal/store/digest.go @@ -11,6 +11,16 @@ import ( "github.com/opencontainers/go-digest" ) +var ( + identifierRegexp = regexp.MustCompile(`^([a-f0-9]{64})$`) + + shortIdentifierRegexp = regexp.MustCompile(`^([a-f0-9]{1,64})$`) +) + +func isShortID(ref string) bool { + return shortIdentifierRegexp.MatchString(ref) +} + // ComputeDigest takes a bundle and produce a unigue reference.Digested func ComputeDigest(bundle io.WriterTo) (digest.Digest, error) { b := bytes.Buffer{} @@ -29,7 +39,7 @@ func StringToRef(s string) (reference.Reference, error) { } func FromString(s string) (ID, error) { - if ok, _ := regexp.MatchString("[a-z0-9]{64}", s); !ok { + if ok := identifierRegexp.MatchString(s); !ok { return ID{}, fmt.Errorf("could not parse '%s' as a valid reference", s) } digest := digest.NewDigestFromEncoded(digest.SHA256, s) diff --git a/internal/store/installation.go b/internal/store/installation.go index 64fae3b09..2c38362cc 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -34,7 +34,7 @@ func NewInstallation(name string, reference string) (*Installation, error) { }, nil } -// SetParameters sets the parameter value if the installation bundle has +// SetParameter sets the parameter value if the installation bundle has // a defined parameter with that name. func (i Installation) SetParameter(name string, value string) { if _, ok := i.Bundle.Parameters[name]; ok { From 280f562c9ecf3860fe502512b11b3228ca1caef3 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Sirot Date: Fri, 25 Oct 2019 18:10:38 +0200 Subject: [PATCH 2/2] Add support for short ID. It required a large refactoring of the bundle store. Now the bundle store scans the bundle directory and creates a map of existing bundle references. All subsequent operations on the bundle store are using this map as the reference for stored bundle. Signed-off-by: Jean-Christophe Sirot --- e2e/images_test.go | 10 +- internal/cnab/cnab.go | 35 +++-- internal/commands/image/list_test.go | 4 + internal/commands/image/rm.go | 2 +- internal/commands/image/tag.go | 15 ++- internal/commands/image/tag_test.go | 32 ++++- internal/store/app.go | 1 - internal/store/bundle.go | 193 +++++++++++++++++---------- internal/store/bundle_test.go | 192 ++++++++++++++++++++++++++ internal/store/digest.go | 13 +- 10 files changed, 393 insertions(+), 104 deletions(-) diff --git a/e2e/images_test.go b/e2e/images_test.go index b48d12eb3..9fc4d0f7e 100644 --- a/e2e/images_test.go +++ b/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 diff --git a/internal/cnab/cnab.go b/internal/cnab/cnab.go index dcb651932..142b2278e 100644 --- a/internal/cnab/cnab.go +++ b/internal/cnab/cnab.go @@ -10,10 +10,12 @@ import ( "github.com/docker/app/internal" "github.com/docker/app/internal/log" "github.com/docker/app/internal/packager" + "github.com/docker/app/internal/store" appstore "github.com/docker/app/internal/store" "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 @@ -89,25 +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) - fmt.Println(ref) 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 } diff --git a/internal/commands/image/list_test.go b/internal/commands/image/list_test.go index 3d114b4d7..7193ef952 100644 --- a/internal/commands/image/list_test.go +++ b/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) diff --git a/internal/commands/image/rm.go b/internal/commands/image/rm.go index 47d3a7698..c2c853009 100644 --- a/internal/commands/image/rm.go +++ b/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 } diff --git a/internal/commands/image/tag.go b/internal/commands/image/tag.go index e06896637..17b72f7d9 100644 --- a/internal/commands/image/tag.go +++ b/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 } diff --git a/internal/commands/image/tag_test.go b/internal/commands/image/tag_test.go index 8cd8338c2..bf5daf1f1 100644 --- a/internal/commands/image/tag_test.go +++ b/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 ( diff --git a/internal/store/app.go b/internal/store/app.go index 0d2473a3b..2a58aedc6 100644 --- a/internal/store/app.go +++ b/internal/store/app.go @@ -74,7 +74,6 @@ 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) } diff --git a/internal/store/bundle.go b/internal/store/bundle.go index 5d831577e..419fd66c6 100644 --- a/internal/store/bundle.go +++ b/internal/store/bundle.go @@ -11,6 +11,7 @@ import ( "github.com/deislabs/cnab-go/bundle" "github.com/docker/distribution/reference" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -26,21 +27,22 @@ type BundleStore interface { var _ BundleStore = &bundleStore{} +type referencesMap map[ID][]reference.Reference + type bundleStore struct { - path string - referenceMap map[ID][]reference.Reference + path string + refsMap referencesMap } +// NewBundleStore creates a new bundle store with the given path and initializes it func NewBundleStore(path string) (BundleStore, error) { bundleStore := &bundleStore{ - path: path, - referenceMap: make(map[ID][]reference.Reference), + path: path, + refsMap: make(referencesMap), } - err := bundleStore.scanAllBundles() - if err != nil { + if err := bundleStore.scanAllBundles(); err != nil { return nil, err } - fmt.Println(bundleStore.referenceMap) return bundleStore, nil } @@ -62,12 +64,10 @@ func NewBundleStore(path string) (BundleStore, error) { // func (b *bundleStore) Store(ref reference.Reference, bndle *bundle.Bundle) (reference.Digested, error) { - digest, err := ComputeDigest(bndle) + id, err := FromBundle(bndle) if err != nil { return nil, errors.Wrapf(err, "failed to store bundle %q", ref) } - id := ID{digest} - if ref == nil { ref = id } @@ -82,12 +82,12 @@ func (b *bundleStore) Store(ref reference.Reference, bndle *bundle.Bundle) (refe if err = bndle.WriteFile(path, 0644); err != nil { return id, errors.Wrapf(err, "failed to store bundle %q", ref) } + b.refsMap.appendRef(id, ref) return id, nil } func (b *bundleStore) Read(ref reference.Reference) (*bundle.Bundle, error) { paths, err := b.storePaths(ref) - fmt.Println(paths) if err != nil { return nil, errors.Wrapf(err, "failed to read bundle %q", ref) } @@ -103,10 +103,8 @@ func (b *bundleStore) Read(ref reference.Reference) (*bundle.Bundle, error) { func (b *bundleStore) List() ([]reference.Reference, error) { var references []reference.Reference - for _, refAliases := range b.referenceMap { - for _, ref := range refAliases { - references = append(references, ref) - } + for _, refAliases := range b.refsMap { + references = append(references, refAliases...) } sort.Slice(references, func(i, j int) bool { @@ -125,12 +123,16 @@ func (b *bundleStore) Remove(ref reference.Reference) error { if _, err := os.Stat(path); os.IsNotExist(err) { return errors.New("no such image " + reference.FamiliarString(ref)) } + b.refsMap.removeRef(ref) return os.RemoveAll(path) } func (b *bundleStore) LookUp(refOrID string) (reference.Reference, error) { ref, err := FromString(refOrID) if err == nil { + if _, found := b.refsMap[ref]; !found { + return nil, unknownReference(refOrID) + } return ref, nil } if isShortID(refOrID) { @@ -139,23 +141,29 @@ func (b *bundleStore) LookUp(refOrID string) (reference.Reference, error) { return ref, nil } } - return reference.ParseNormalizedNamed(refOrID) + named, err := StringToNamedRef(refOrID) + if err != nil { + return nil, err + } + if _, err = b.referenceToID(named); err != nil { + return nil, err + } + return named, nil } func (b *bundleStore) matchShortID(shortID string) (reference.Reference, error) { var found reference.Reference - for id := range b.referenceMap { + for id := range b.refsMap { if strings.HasPrefix(id.String(), shortID) { if found != nil && found != id { - return nil, fmt.Errorf("Ambiguous reference found") + return nil, fmt.Errorf("ambiguous reference found") } found = id } } if found == nil { - return nil, fmt.Errorf("Could not find reference") + return nil, unknownReference(shortID) } - fmt.Printf("found = %s\n", found) return found, nil } @@ -163,14 +171,14 @@ func (b *bundleStore) referenceToID(ref reference.Reference) (ID, error) { if id, ok := ref.(ID); ok { return id, nil } - for id, refs := range b.referenceMap { + for id, refs := range b.refsMap { for _, r := range refs { if r == ref { return id, nil } } } - return ID{}, fmt.Errorf("%s: reference not found", ref.String()) + return ID{}, unknownReference(reference.FamiliarString(ref)) } func (b *bundleStore) storePaths(ref reference.Reference) ([]string, error) { @@ -181,18 +189,18 @@ func (b *bundleStore) storePaths(ref reference.Reference) ([]string, error) { return nil, err } - if refs, exist := b.referenceMap[id]; exist { + if refs, exist := b.refsMap[id]; exist { for _, rf := range refs { path, err := b.storePath(rf) if err != nil { - continue + return nil, err } paths = append(paths, path) } } if len(paths) == 0 { - return nil, fmt.Errorf("%s: reference not found", ref.String()) + return nil, unknownReference(reference.FamiliarString(ref)) } return paths, nil } @@ -229,57 +237,51 @@ func (b *bundleStore) storePath(ref reference.Reference) (string, error) { return storeDir, nil } -// Returns the list of all bundles present in the bundle store +// scanAllBundles scans the bundle store directories and creates the internal map of App image +// references. This function must be called before any other public BundleStore interface method. func (b *bundleStore) scanAllBundles() error { - digests := filepath.Join(b.path, "_ids") - if err := filepath.Walk(b.path, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - if info.IsDir() { - return nil - } + if err := filepath.Walk(b.path, b.processBundleStoreFile); err != nil { + return err + } + return nil +} - if !strings.HasSuffix(info.Name(), ".json") { - return nil - } +func (b *bundleStore) processBundleStoreFile(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + idRefPath := filepath.Join(b.path, "_ids") - if strings.HasPrefix(path, digests) { - rel := path[len(digests)+1:] - dg := strings.Split(filepath.ToSlash(rel), "/")[0] - //references = append(references, ID{dg}) - id := ID{dg} - if _, ok := b.referenceMap[id]; !ok { - b.referenceMap[id] = []reference.Reference{id} - } else { - b.referenceMap[id] = append(b.referenceMap[id], id) - } - return nil - } + if info.IsDir() { + return nil + } - ref, err := b.pathToReference(path) - if err != nil { - return err - } - bndl, err := b.fetchBundleJSON(path) - if err != nil { - return err - } - id, err := FromBundle(bndl) - if err != nil { - return err - } - if _, ok := b.referenceMap[id]; !ok { - b.referenceMap[id] = []reference.Reference{ref} - } else { - b.referenceMap[id] = append(b.referenceMap[id], ref) - } + if !strings.HasSuffix(info.Name(), ".json") { + return nil + } + if strings.HasPrefix(path, idRefPath) { + rel := path[len(idRefPath)+1:] + dg := strings.Split(filepath.ToSlash(rel), "/")[0] + id := ID{digest.NewDigestFromEncoded(digest.SHA256, dg)} + b.refsMap.appendRef(id, id) return nil - }); err != nil { + } + + ref, err := b.pathToReference(path) + if err != nil { + return err + } + bndl, err := b.fetchBundleJSON(path) + if err != nil { + return err + } + id, err := FromBundle(bndl) + if err != nil { return err } + b.refsMap[id] = append(b.refsMap[id], ref) + return nil } @@ -288,11 +290,11 @@ func (b *bundleStore) fetchBundleJSON(bundlePath string) (*bundle.Bundle, error) if err != nil { return nil, errors.Wrapf(err, "failed to read file %s", bundlePath) } - var bndle bundle.Bundle - if err := json.Unmarshal(data, &bndle); err != nil { + var bndl bundle.Bundle + if err := json.Unmarshal(data, &bndl); err != nil { return nil, errors.Wrapf(err, "failed to read file %s", bundlePath) } - return &bndle, nil + return &bndl, nil } func (b *bundleStore) pathToReference(path string) (reference.Named, error) { @@ -322,6 +324,30 @@ func (b *bundleStore) pathToReference(path string) (reference.Named, error) { return reference.ParseNamed(name) } +func (rm referencesMap) appendRef(id ID, ref reference.Reference) { + if _, found := rm[id]; found { + if !containsRef(rm[id], ref) { + rm[id] = append(rm[id], ref) + } + } else { + rm[id] = []reference.Reference{ref} + } +} + +func (rm referencesMap) removeRef(ref reference.Reference) { + for id, refs := range rm { + for i, r := range refs { + if r == ref { + rm[id] = append(refs[:i], refs[i+1:]...) + if len(rm[id]) == 0 { + delete(rm, id) + } + return + } + } + } +} + func reconstructNamedReference(path string, paths []string) (string, error) { name, paths := strings.Replace(paths[0], "_", ":", 1), paths[1:] for i, p := range paths { @@ -342,3 +368,28 @@ func reconstructNamedReference(path string, paths []string) (string, error) { } return name, nil } + +func containsRef(list []reference.Reference, ref reference.Reference) bool { + for _, v := range list { + if v == ref { + return true + } + } + return false +} + +func unknownReference(ref string) *UnknownReferenceError { + return &UnknownReferenceError{ref} +} + +// UnknownReferenceError represents a reference not found in the bundle store +type UnknownReferenceError struct { + string +} + +func (e *UnknownReferenceError) Error() string { + return fmt.Sprintf("%s: reference not found", e.string) +} + +// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound +func (e *UnknownReferenceError) NotFound() {} diff --git a/internal/store/bundle_test.go b/internal/store/bundle_test.go index 7e55c6323..9b560cc37 100644 --- a/internal/store/bundle_test.go +++ b/internal/store/bundle_test.go @@ -1,6 +1,8 @@ package store import ( + "fmt" + "io/ioutil" "os" "path" "path/filepath" @@ -290,3 +292,193 @@ func TestRemove(t *testing.T) { assert.Equal(t, len(bundles), 0) }) } + +func TestLookUp(t *testing.T) { + dockerConfigDir := fs.NewDir(t, t.Name(), fs.WithMode(0755)) + defer dockerConfigDir.Remove() + appstore, err := NewApplicationStore(dockerConfigDir.Path()) + assert.NilError(t, err) + bundleStore, err := appstore.BundleStore() + assert.NilError(t, err) + bndl := &bundle.Bundle{Name: "bundle-name"} + // Adding the bundle referenced by id + id, err := bundleStore.Store(nil, bndl) + assert.NilError(t, err) + // Adding the same bundle referenced by a tag + ref := parseRefOrDie(t, "my-repo/a-bundle:my-tag") + _, err = bundleStore.Store(ref, bndl) + assert.NilError(t, err) + // Adding the same bundle referenced by tag prefixed by docker.io/library + dockerIoRef := parseRefOrDie(t, "docker.io/library/a-bundle:my-tag") + _, err = bundleStore.Store(dockerIoRef, bndl) + assert.NilError(t, err) + + for _, tc := range []struct { + Name string + refOrID string + ExpectedError string + ExpectedRef reference.Reference + }{ + { + Name: "Long Id", + refOrID: id.String(), + ExpectedError: "", + ExpectedRef: id, + }, + { + Name: "Short Id", + refOrID: id.String()[0:8], + ExpectedError: "", + ExpectedRef: id, + }, + { + Name: "Tagged Ref", + refOrID: "my-repo/a-bundle:my-tag", + ExpectedError: "", + ExpectedRef: ref, + }, + { + Name: "docker.io_library repository Tagged Ref", + refOrID: "a-bundle:my-tag", + ExpectedError: "", + ExpectedRef: dockerIoRef, + }, + { + Name: "Unknown Tag", + refOrID: "other-repo/a-bundle:other-tag", + ExpectedError: "other-repo/a-bundle:other-tag: reference not found", + ExpectedRef: nil, + }, + { + Name: "Unknown ID", + refOrID: "b4fcc3af16804e29d977918a3a322daf1eb6ab2992c3cc7cbfeae8c3d6ede8af", + ExpectedError: "b4fcc3af16804e29d977918a3a322daf1eb6ab2992c3cc7cbfeae8c3d6ede8af: reference not found", + ExpectedRef: nil, + }, + { + Name: "Unknown short ID", + refOrID: "b4fcc3af", + ExpectedError: "b4fcc3af:latest: reference not found", + ExpectedRef: nil, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + fmt.Println(tc.refOrID) + ref, err := bundleStore.LookUp(tc.refOrID) + + if tc.ExpectedError != "" { + assert.Equal(t, err.Error(), tc.ExpectedError) + } else { + assert.NilError(t, err) + } + + if tc.ExpectedRef != nil { + assert.Equal(t, ref, tc.ExpectedRef) + } + }) + } +} + +func TestScanBundles(t *testing.T) { + dockerConfigDir := fs.NewDir(t, t.Name(), fs.WithMode(0755)) + defer dockerConfigDir.Remove() + + // Adding a bundle which should be referenced by id only + bndl1 := &bundle.Bundle{Name: "bundle-1"} + id1, err := FromBundle(bndl1) + assert.NilError(t, err) + dir1 := dockerConfigDir.Join("app", "bundles", "_ids", id1.String()) + assert.NilError(t, os.MkdirAll(dir1, 0755)) + assert.NilError(t, ioutil.WriteFile(filepath.Join(dir1, "bundle.json"), []byte(`{"name": "bundle-1"}`), 0644)) + + // Adding a bundle which should be referenced by id and tag + bndl2 := &bundle.Bundle{Name: "bundle-2"} + id2, err := FromBundle(bndl2) + assert.NilError(t, err) + dir2 := dockerConfigDir.Join("app", "bundles", "_ids", id2.String()) + assert.NilError(t, os.MkdirAll(dir2, 0755)) + assert.NilError(t, ioutil.WriteFile(filepath.Join(dir2, "bundle.json"), []byte(`{"name": "bundle-2"}`), 0644)) + dir2 = dockerConfigDir.Join("app", "bundles", "docker.io", "my-repo", "my-bundle", "_tags", "my-tag") + assert.NilError(t, os.MkdirAll(dir2, 0755)) + assert.NilError(t, ioutil.WriteFile(filepath.Join(dir2, "bundle.json"), []byte(`{"name": "bundle-2"}`), 0644)) + + appstore, err := NewApplicationStore(dockerConfigDir.Path()) + assert.NilError(t, err) + bundleStore, err := appstore.BundleStore() + assert.NilError(t, err) + + // Ensure List() and Read() function returns expected bundles + refs, err := bundleStore.List() + assert.NilError(t, err) + expectedRefs := []string{id2.String(), "my-repo/my-bundle:my-tag", id1.String()} + refsAsString := func(references []reference.Reference) []string { + var rv []string + for _, r := range references { + rv = append(rv, reference.FamiliarString(r)) + } + return rv + } + assert.DeepEqual(t, refsAsString(refs), expectedRefs) + bndl, err := bundleStore.Read(id1) + assert.NilError(t, err) + assert.DeepEqual(t, bndl, bndl1) + bndl, err = bundleStore.Read(id2) + assert.NilError(t, err) + assert.DeepEqual(t, bndl, bndl2) + bndl, err = bundleStore.Read(parseRefOrDie(t, "my-repo/my-bundle:my-tag")) + assert.NilError(t, err) + assert.DeepEqual(t, bndl, bndl2) +} + +func TestAppendRemoveReference(t *testing.T) { + id1, err := FromString("68720b2db729794a3521bc83e3699ac629f26beba6862b6ec491cd0d677d02a0") + assert.NilError(t, err) + id2, err := FromString("b7244e15970354cceb75f417f1e98b3a340cff35576eeeac603d33afa73b0b4b") + assert.NilError(t, err) + initialize := func() referencesMap { + m := make(referencesMap) + m[id1] = []reference.Reference{id1, parseRefOrDie(t, "foo/bar:1.0")} + m[id2] = []reference.Reference{parseRefOrDie(t, "qix/qux:1.0")} + return m + } + + t.Run("Add new reference to existing ID", func(t *testing.T) { + m := initialize() + ref := parseRefOrDie(t, "zox/zox:1.0") + m.appendRef(id2, ref) + assert.Equal(t, len(m), 2) + assert.Equal(t, len(m[id2]), 2) + assert.Equal(t, m[id2][1], ref) + }) + + t.Run("Add new ID", func(t *testing.T) { + m := initialize() + id3, err := FromString("d19b0b0d9ac36a8198465bcd8bf816a45110bf26b731a4e299e771ca0b082a21") + assert.NilError(t, err) + ref := parseRefOrDie(t, "zox/zox:1.0") + m.appendRef(id3, ref) + assert.Equal(t, len(m), 3) + assert.Equal(t, len(m[id3]), 1) + assert.Equal(t, m[id3][0], ref) + }) + + t.Run("Remove reference", func(t *testing.T) { + m := initialize() + ref := parseRefOrDie(t, "foo/bar:1.0") + fmt.Println(m) + m.removeRef(ref) + assert.Equal(t, len(m), 2) + assert.Equal(t, len(m[id1]), 1) + assert.Equal(t, m[id1][0], id1) + }) + + t.Run("Remove reference and ID", func(t *testing.T) { + m := initialize() + ref := parseRefOrDie(t, "qix/qux:1.0") + fmt.Println(m) + m.removeRef(ref) + assert.Equal(t, len(m), 1) + _, exist := m[id2] + assert.Equal(t, exist, false) + }) +} diff --git a/internal/store/digest.go b/internal/store/digest.go index 7d5c9d92e..71640b719 100644 --- a/internal/store/digest.go +++ b/internal/store/digest.go @@ -9,6 +9,7 @@ import ( "github.com/deislabs/cnab-go/bundle" "github.com/docker/distribution/reference" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) var ( @@ -31,16 +32,18 @@ func ComputeDigest(bundle io.WriterTo) (digest.Digest, error) { return digest.SHA256.FromBytes(b.Bytes()), nil } -func StringToRef(s string) (reference.Reference, error) { - if named, err := reference.ParseNormalizedNamed(s); err == nil { - return reference.TagNameOnly(named), nil +// StringToNamedRef converts a string to a named reference +func StringToNamedRef(s string) (reference.Named, error) { + named, err := reference.ParseNormalizedNamed(s) + if err != nil { + return nil, errors.Wrapf(err, "could not parse %q as a valid reference", s) } - return FromString(s) + return reference.TagNameOnly(named), nil } func FromString(s string) (ID, error) { if ok := identifierRegexp.MatchString(s); !ok { - return ID{}, fmt.Errorf("could not parse '%s' as a valid reference", s) + return ID{}, fmt.Errorf("could not parse %q as a valid reference", s) } digest := digest.NewDigestFromEncoded(digest.SHA256, s) return ID{digest}, nil