From 759c61c410fb09eb7acd887f2c97be35719f2ed0 Mon Sep 17 00:00:00 2001 From: Hardy Ferentschik Date: Thu, 19 Oct 2017 21:52:44 +0200 Subject: [PATCH] Issue #155 Making sure existing OCI index gets not overwritten Signed-off-by: Hardy Ferentschik --- .gitignore | 7 ++++ oci/layout/oci_dest.go | 67 ++++++++++++++++++++++++++++--------- oci/layout/oci_dest_test.go | 62 ++++++++++++++++++++++++++++++++++ oci/layout/oci_transport.go | 19 ++++++++--- 4 files changed, 135 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index aa95175891..a085e43162 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,9 @@ vendor tools.timestamp + +# Idea IDE +*.iml +.idea + +# Visual Studio Code +.vscode/* diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index ce1e0c3e2b..4c6d349edb 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -27,12 +27,23 @@ func newImageDestination(ref ociReference) (types.ImageDestination, error) { if ref.image == "" { return nil, errors.Errorf("cannot save image with empty image.ref.name") } - index := imgspecv1.Index{ - Versioned: imgspec.Versioned{ - SchemaVersion: 2, - }, + + var index *imgspecv1.Index + if indexExists(ref) { + var err error + index, err = ref.getIndex() + if err != nil { + return nil, err + } + } else { + index = &imgspecv1.Index{ + Versioned: imgspec.Versioned{ + SchemaVersion: 2, + }, + } } - return &ociImageDestination{ref: ref, index: index}, nil + + return &ociImageDestination{ref: ref, index: *index}, nil } // Reference returns the reference used to set up this destination. Note that this should directly correspond to user's intent, @@ -191,23 +202,20 @@ func (d *ociImageDestination) PutManifest(m []byte) error { Architecture: runtime.GOARCH, OS: runtime.GOOS, } - d.index.Manifests = append(d.index.Manifests, desc) + d.addManifest(&desc) return nil } -func ensureDirectoryExists(path string) error { - if _, err := os.Stat(path); err != nil && os.IsNotExist(err) { - if err := os.MkdirAll(path, 0755); err != nil { - return err +func (d *ociImageDestination) addManifest(desc *imgspecv1.Descriptor) { + for i, manifest := range d.index.Manifests { + if manifest.Annotations["org.opencontainers.image.ref.name"] == desc.Annotations["org.opencontainers.image.ref.name"] { + // TODO Should there first be a cleanup based on the descriptor we are going to replace? + d.index.Manifests[i] = *desc + return } } - return nil -} - -// ensureParentDirectoryExists ensures the parent of the supplied path exists. -func ensureParentDirectoryExists(path string) error { - return ensureDirectoryExists(filepath.Dir(path)) + d.index.Manifests = append(d.index.Manifests, *desc) } func (d *ociImageDestination) PutSignatures(signatures [][]byte) error { @@ -231,3 +239,30 @@ func (d *ociImageDestination) Commit() error { } return ioutil.WriteFile(d.ref.indexPath(), indexJSON, 0644) } + +func ensureDirectoryExists(path string) error { + if _, err := os.Stat(path); err != nil && os.IsNotExist(err) { + if err := os.MkdirAll(path, 0755); err != nil { + return err + } + } + return nil +} + +// ensureParentDirectoryExists ensures the parent of the supplied path exists. +func ensureParentDirectoryExists(path string) error { + return ensureDirectoryExists(filepath.Dir(path)) +} + +// indexExists checks whether the index location specified in the OCI reference exists. +// The implementation is opinionated, since in case of unexpected errors false is returned +func indexExists(ref ociReference) bool { + _, err := os.Stat(ref.indexPath()) + if err == nil { + return true + } + if os.IsNotExist(err) { + return false + } + return true +} diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index 9767f94f3a..0e20e240c0 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "path/filepath" ) // readerFromFunc allows implementing Reader by any function, e.g. a closure. @@ -59,3 +60,64 @@ func TestPutBlobDigestFailure(t *testing.T) { require.Error(t, err) require.True(t, os.IsNotExist(err)) } + +// TestPutManifestAppendsToExistingManifest tests that new manifests are getting added to existing index. +func TestPutManifestAppendsToExistingManifest(t *testing.T) { + ref, tmpDir := refToTempOCI(t) + defer os.RemoveAll(tmpDir) + + ociRef, ok := ref.(ociReference) + require.True(t, ok) + + // iniitally we have one manifest + index, err := ociRef.getIndex() + assert.NoError(t, err) + assert.Equal(t, 1, len(index.Manifests), "Unexpected number of manifests") + + // create a new test reference + ociRef2, err := NewReference(tmpDir, "new-image") + assert.NoError(t, err) + + putTestManifest(t, ociRef2.(ociReference), tmpDir) + + index, err = ociRef.getIndex() + assert.NoError(t, err) + assert.Equal(t, 2, len(index.Manifests), "Unexpected number of manifests") +} + +// TestPutManifestTwice tests that existing manifest gets updated and not appended. +func TestPutManifestTwice(t *testing.T) { + ref, tmpDir := refToTempOCI(t) + defer os.RemoveAll(tmpDir) + + ociRef, ok := ref.(ociReference) + require.True(t, ok) + + putTestManifest(t, ociRef, tmpDir) + putTestManifest(t, ociRef, tmpDir) + + index, err := ociRef.getIndex() + assert.NoError(t, err) + assert.Equal(t, 1, len(index.Manifests), "Unexpected number of manifests") +} + +func putTestManifest(t *testing.T, ociRef ociReference, tmpDir string) { + imageDest, err := newImageDestination(ociRef) + assert.NoError(t, err) + + data := []byte("abc") + err = imageDest.PutManifest(data) + assert.NoError(t, err) + + err = imageDest.Commit() + assert.NoError(t, err) + + paths := []string{} + filepath.Walk(tmpDir, func(path string, info os.FileInfo, err error) error { + paths = append(paths, path) + return nil + }) + + digest := "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + assert.Contains(t, paths, filepath.Join(tmpDir, "blobs", "sha256", digest), "The OCI directory does not contain the new manifest data") +} diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index 312bc0e4eb..77730f3900 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -189,14 +189,25 @@ func (ref ociReference) NewImage(ctx *types.SystemContext) (types.Image, error) return image.FromSource(src) } -func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { +// getIndex returns a pointer to the index references by this ociReference. If an error occurs opening an index nil is returned together +// with an error. +func (ref ociReference) getIndex() (*imgspecv1.Index, error) { indexJSON, err := os.Open(ref.indexPath()) if err != nil { - return imgspecv1.Descriptor{}, err + return nil, err } defer indexJSON.Close() - index := imgspecv1.Index{} - if err := json.NewDecoder(indexJSON).Decode(&index); err != nil { + + index := &imgspecv1.Index{} + if err := json.NewDecoder(indexJSON).Decode(index); err != nil { + return nil, err + } + return index, nil +} + +func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { + index, err := ref.getIndex() + if err != nil { return imgspecv1.Descriptor{}, err }