Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Store repo digest as app image is pushed #770

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Store repo digest as app image is pushed #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8e7433
Store repo digest as app image is pushed
ndeloof Nov 27, 2019
6d54f0c
Code cleanup
ndeloof Nov 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/cnab/cnab.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func PullBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, tagRef
return nil, err
}
relocatedBundle := &relocated.Bundle{Bundle: bndl, RelocationMap: relocationMap}
if _, err := bundleStore.Store(tagRef, relocatedBundle); err != nil {
if _, err := bundleStore.Store(relocatedBundle, tagRef); err != nil {
return nil, err
}
return relocatedBundle, nil
Expand Down
5 changes: 2 additions & 3 deletions internal/commands/build/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,12 @@ func runBuild(dockerCli command.Cli, contextPath string, opt buildOptions) error
return err
}

var ref reference.Reference
ref, err = packager.GetNamedTagged(opt.tag)
ref, err := packager.GetNamedTagged(opt.tag)
if err != nil {
return err
}

id, err := packager.PersistInBundleStore(ref, bundle)
id, err := packager.PersistInBundleStore(bundle, ref)
if err != nil {
return err
}
Expand Down
7 changes: 2 additions & 5 deletions internal/commands/image/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ func getImageDesc(bundle *relocated.Bundle, ref reference.Reference) imageDesc {
if t, ok := ref.(reference.Tagged); ok {
tag = t.Tag()
}
var digest string
if t, ok := ref.(reference.Digested); ok {
digest = t.Digest().String()
}
digest := bundle.RepoDigest
var created time.Time
if payload, err := packager.CustomPayload(bundle.Bundle); err == nil {
if createdPayload, ok := payload.(packager.CustomPayloadCreated); ok {
Expand All @@ -141,7 +138,7 @@ func getImageDesc(bundle *relocated.Bundle, ref reference.Reference) imageDesc {
Name: bundle.Name,
Repository: repository,
Tag: tag,
Digest: digest,
Digest: digest.String(),
Created: created,
}
}
80 changes: 22 additions & 58 deletions internal/commands/image/list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,59 +3,24 @@ package image
import (
"bufio"
"bytes"
"fmt"
"testing"
"time"

"github.com/docker/app/internal/relocated"

"gotest.tools/assert"
"github.com/docker/app/internal/store"
"gotest.tools/fs"

"github.com/deislabs/cnab-go/bundle"
"github.com/docker/app/internal/store"
"github.com/docker/app/internal/relocated"
"github.com/docker/cli/cli/command"
"github.com/docker/distribution/reference"
"gotest.tools/assert"
)

type bundleStoreStubForListCmd struct {
refMap map[reference.Reference]*relocated.Bundle
// in order to keep the reference in the same order between tests
refList []reference.Reference
}

func (b *bundleStoreStubForListCmd) Store(ref reference.Reference, bndl *relocated.Bundle) (reference.Digested, error) {
b.refMap[ref] = bndl
b.refList = append(b.refList, ref)
return store.FromBundle(bndl)
}

func (b *bundleStoreStubForListCmd) Read(ref reference.Reference) (*relocated.Bundle, error) {
bndl, ok := b.refMap[ref]
if ok {
return bndl, nil
}
return nil, fmt.Errorf("Bundle not found")
}

func (b *bundleStoreStubForListCmd) List() ([]reference.Reference, error) {
return b.refList, nil
}

func (b *bundleStoreStubForListCmd) Remove(ref reference.Reference, force bool) error {
return nil
}

func (b *bundleStoreStubForListCmd) LookUp(refOrID string) (reference.Reference, error) {
return nil, nil
}

func TestListCmd(t *testing.T) {
ref, err := store.FromString("a855ac937f2ed375ba4396bbc49c4093e124da933acd2713fb9bc17d7562a087")
assert.NilError(t, err)
refs := []reference.Reference{
refs := []reference.Named{
parseReference(t, "foo/bar@sha256:b59492bb814012ca3d2ce0b6728242d96b4af41687cc82166a4b5d7f2d9fb865"),
parseReference(t, "foo/bar:1.0"),
ref,
nil,
}
bundles := []relocated.Bundle{
{
Expand Down Expand Up @@ -85,36 +50,36 @@ func TestListCmd(t *testing.T) {
{
name: "TestList",
expectedOutput: `REPOSITORY TAG APP IMAGE ID APP NAME CREATED
foo/bar <none> 3f825b2d0657 Digested App N/A
<none> <none> ad2828ea5653 Quiet App N/A
foo/bar 1.0 9aae408ee04f Foo App N/A
<none> <none> a855ac937f2e Quiet App N/A
foo/bar <none> 3f825b2d0657 Digested App N/A
`,
options: imageListOption{format: "table"},
},
{
name: "TestTemplate",
expectedOutput: `APP IMAGE ID DIGEST
3f825b2d0657 sha256:b59492bb814012ca3d2ce0b6728242d96b4af41687cc82166a4b5d7f2d9fb865
ad2828ea5653 <none>
9aae408ee04f <none>
a855ac937f2e sha256:a855ac937f2ed375ba4396bbc49c4093e124da933acd2713fb9bc17d7562a087
3f825b2d0657 sha256:b59492bb814012ca3d2ce0b6728242d96b4af41687cc82166a4b5d7f2d9fb865
`,
options: imageListOption{format: "table {{.ID}}", digests: true},
},
{
name: "TestListWithDigests",
//nolint:lll
expectedOutput: `REPOSITORY TAG DIGEST APP IMAGE ID APP NAME CREATED
foo/bar <none> sha256:b59492bb814012ca3d2ce0b6728242d96b4af41687cc82166a4b5d7f2d9fb865 3f825b2d0657 Digested App N/A
<none> <none> <none> ad2828ea5653 Quiet App N/A
foo/bar 1.0 <none> 9aae408ee04f Foo App N/A
<none> <none> sha256:a855ac937f2ed375ba4396bbc49c4093e124da933acd2713fb9bc17d7562a087 a855ac937f2e Quiet App N/A
foo/bar <none> sha256:b59492bb814012ca3d2ce0b6728242d96b4af41687cc82166a4b5d7f2d9fb865 3f825b2d0657 Digested App N/A
`,
options: imageListOption{format: "table", digests: true},
},
{
name: "TestListWithQuiet",
expectedOutput: `3f825b2d0657
expectedOutput: `ad2828ea5653
9aae408ee04f
a855ac937f2e
3f825b2d0657
`,
options: imageListOption{format: "table", quiet: true},
},
Expand Down Expand Up @@ -143,27 +108,26 @@ func TestSortImages(t *testing.T) {
assert.Equal(t, "3", images[4].ID)
}

func parseReference(t *testing.T, s string) reference.Reference {
ref, err := reference.Parse(s)
func parseReference(t *testing.T, s string) reference.Named {
ref, err := reference.ParseNormalizedNamed(s)
assert.NilError(t, err)
return ref
}

func testRunList(t *testing.T, refs []reference.Reference, bundles []relocated.Bundle, options imageListOption, expectedOutput string) {
func testRunList(t *testing.T, refs []reference.Named, bundles []relocated.Bundle, options imageListOption, expectedOutput string) {
var buf bytes.Buffer
w := bufio.NewWriter(&buf)
dockerCli, err := command.NewDockerCli(command.WithOutputStream(w))
assert.NilError(t, err)
bundleStore := &bundleStoreStubForListCmd{
refMap: make(map[reference.Reference]*relocated.Bundle),
refList: []reference.Reference{},
}
bundleStore, err := store.NewBundleStore(fs.NewDir(t, "store").Path())
assert.NilError(t, err)
for i, ref := range refs {
_, err = bundleStore.Store(ref, &bundles[i])
_, err = bundleStore.Store(&bundles[i], ref)
assert.NilError(t, err)
}
err = runList(dockerCli, options, bundleStore)
assert.NilError(t, err)
w.Flush()
assert.Equal(t, buf.String(), expectedOutput)
actualOutput := buf.String()
assert.Equal(t, actualOutput, expectedOutput)
}
4 changes: 2 additions & 2 deletions internal/commands/image/tag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func runTag(bundleStore store.BundleStore, srcAppImage, destAppImage string) err
if err != nil {
return err
}

srcRef.RepoDigest = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand that correctly, tagging a pulled app image means we drop its repo digest? So it won't be displayed in the docker app image ls --digest column?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong, but IIUC repo digest won't be the same if you push under a distinct tag. So keeing this "pulled digest" for a tagged image would be wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be the same repo digest, as tagging is just pushing a new descriptor, pointing to the same manifest/manifest list, but the repo digest should be the digest of the pointed object, not the pointer. cc @eunomie / @thaJeztah any idea?

return storeBundle(srcRef, destAppImage, bundleStore)
}

Expand Down Expand Up @@ -73,6 +73,6 @@ func storeBundle(bundle *relocated.Bundle, name string, bundleStore store.Bundle
if err != nil {
return err
}
_, err = bundleStore.Store(cnabRef, bundle)
_, err = bundleStore.Store(bundle, cnabRef)
return err
}
6 changes: 4 additions & 2 deletions internal/commands/image/tag_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/docker/app/internal/store"

"github.com/docker/app/internal/relocated"

"gotest.tools/assert"
Expand All @@ -23,12 +25,12 @@ type bundleStoreStub struct {
ReadError error
StoredBundle string
StoredError error
StoredID reference.Digested
StoredID store.ID
LookUpRef reference.Reference
LookUpError error
}

func (b *bundleStoreStub) Store(ref reference.Reference, bndle *relocated.Bundle) (reference.Digested, error) {
func (b *bundleStoreStub) Store(bndl *relocated.Bundle, ref reference.Named) (store.ID, error) {
defer func() {
b.StoredError = nil
}()
Expand Down
28 changes: 20 additions & 8 deletions internal/commands/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"os"
"strings"

"github.com/opencontainers/go-digest"

"github.com/docker/app/internal/relocated"
"github.com/docker/app/internal/store"

Expand Down Expand Up @@ -69,10 +71,20 @@ func runPush(dockerCli command.Cli, name string) error {
return err
}

cnabRef := reference.TagNameOnly(ref)

// Push the bundle
return pushBundle(dockerCli, bndl, cnabRef)
dg, err := pushBundle(dockerCli, bndl, ref)
if err != nil {
return errors.Wrapf(err, "could not push %q", ref)
}

// we can't just re-use bndl var here, as fixup did rewrite the bundle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in the specs, https://github.com/cnabio/cnab-spec/blob/master/101-bundle-json.md#invocation-images

During bundle development, it may be ideal to omit the contentDigest field and/or skip validation. Once a bundle is ready to be transmitted as a thick or thin bundle, it must have a contentDigest field.

the contentDigest can be omitted, but needs to be filled before pushing. I think that's exactly what we do here, that's why the bundle is mutated by the push.

bndl, err = resolveReferenceAndBundle(bundleStore, ref)
if err != nil {
return err
}
bndl.RepoDigest = dg
_, err = bundleStore.Store(bndl, ref)
return err
}

func resolveReferenceAndBundle(bundleStore store.BundleStore, ref reference.Reference) (*relocated.Bundle, error) {
Expand All @@ -88,10 +100,10 @@ func resolveReferenceAndBundle(bundleStore store.BundleStore, ref reference.Refe
return bndl, err
}

func pushBundle(dockerCli command.Cli, bndl *relocated.Bundle, cnabRef reference.Named) error {
func pushBundle(dockerCli command.Cli, bndl *relocated.Bundle, cnabRef reference.Named) (digest.Digest, error) {
insecureRegistries, err := internal.InsecureRegistriesFromEngine(dockerCli)
if err != nil {
return errors.Wrap(err, "could not retrieve insecure registries")
return "", errors.Wrap(err, "could not retrieve insecure registries")
}
resolver := remotes.CreateResolver(dockerCli.ConfigFile(), insecureRegistries...)
var display fixupDisplay = &plainDisplay{out: os.Stdout}
Expand All @@ -107,17 +119,17 @@ func pushBundle(dockerCli command.Cli, bndl *relocated.Bundle, cnabRef reference
// bundle fixup
relocationMap, err := remotes.FixupBundle(context.Background(), bndl.Bundle, cnabRef, resolver, fixupOptions...)
if err != nil {
return errors.Wrapf(err, "fixing up %q for push", cnabRef)
return "", errors.Wrapf(err, "fixing up %q for push", cnabRef)
}
bndl.RelocationMap = relocationMap
// push bundle manifest
logrus.Debugf("Pushing the bundle %q", cnabRef)
descriptor, err := remotes.Push(log.WithLogContext(context.Background()), bndl.Bundle, bndl.RelocationMap, cnabRef, resolver, true, withAppAnnotations)
if err != nil {
return errors.Wrapf(err, "pushing to %q", cnabRef)
return "", errors.Wrapf(err, "pushing to %q", cnabRef)
}
fmt.Fprintf(os.Stdout, "Successfully pushed bundle to %s. Digest is %s.\n", cnabRef, descriptor.Digest)
return nil
return descriptor.Digest, nil
}

func withAppAnnotations(index *ocischemav1.Index) error {
Expand Down
8 changes: 4 additions & 4 deletions internal/packager/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,16 @@ func MakeCNABImageName(appName, appVersion, suffix string) (string, error) {
}

// PersistInBundleStore do store a bundle with optional reference and return it's ID
func PersistInBundleStore(ref reference.Reference, bndl *bundle.Bundle) (reference.Digested, error) {
func PersistInBundleStore(bndl *bundle.Bundle, ref reference.Named) (store.ID, error) {
appstore, err := store.NewApplicationStore(config.Dir())
if err != nil {
return nil, err
return "", err
}
bundleStore, err := appstore.BundleStore()
if err != nil {
return nil, err
return "", err
}
return bundleStore.Store(ref, relocated.FromBundle(bndl))
return bundleStore.Store(relocated.FromBundle(bndl), ref)
}

func GetNamedTagged(tag string) (reference.NamedTagged, error) {
Expand Down
Loading