Repository: containers/image
Pull request #960 (2020-06-15 19:05:10)

oci: don't overwrite tags pointing at the same manifest
In ca5fe04cb38a ("Add manifest list support"), the code was changed from a
single step (match on the name annotation) to a two step process: 1. untag
the existing name, and 2. match on the manifest's digest and change the tag
of this match to the new tag.

However, this means that if there were two tags:

    "foo": sha256:c0ffee55
    "bar": sha256:deadbeef

And we update "bar" to point to c0ffee55, we'd end up doing:

1. remove any "bar" tags:

    "foo": sha256:c0ffee55
    "": sha256:deadbeef

2. re-tag the first c0ffee55 thing:

    "bar": sha256:c0ffee55
    "": sha256:deadbeef

Thus losing the "foo" tag entirely.

Instead, let's make sure that we only re-tag something with the empty
string, so we don't delete existing tags.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>


Patch:
diff --git a/go.mod b/go.mod
index bdaa2a95..f8b3cb36 100644
--- a/go.mod
+++ b/go.mod
@@ -18,7 +18,7 @@ require (
 	github.com/ghodss/yaml v1.0.0
 	github.com/gorilla/mux v1.7.4 // indirect
 	github.com/imdario/mergo v0.3.9
-	github.com/klauspost/compress v1.10.7
+	github.com/klauspost/compress v1.10.8
 	github.com/klauspost/pgzip v1.2.4
 	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/mtrmac/gpgme v0.1.2
diff --git a/go.sum b/go.sum
index 6b5b4b75..5bbab87e 100644
--- a/go.sum
+++ b/go.sum
@@ -127,6 +127,8 @@ github.com/klauspost/compress v1.10.6 h1:SP6zavvTG3YjOosWePXFDlExpKIWMTO4SE/Y8MZ
 github.com/klauspost/compress v1.10.6/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.10.7 h1:7rix8v8GpI3ZBb0nSozFRgbtXKv+hOe+qfEpZqybrAg=
 github.com/klauspost/compress v1.10.7/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/klauspost/compress v1.10.8 h1:eLeJ3dr/Y9+XRfJT4l+8ZjmtB5RPJhucH2HeCV5+IZY=
+github.com/klauspost/compress v1.10.8/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/pgzip v1.2.3 h1:Ce2to9wvs/cuJ2b86/CKQoTYr9VHfpanYosZ0UBJqdw=
 github.com/klauspost/pgzip v1.2.3/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
 github.com/klauspost/pgzip v1.2.4 h1:TQ7CNpYKovDOmqzRHKxJh0BeaBI7UdQZYc6p7pMQh1A=
diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go
index fb0449ca..48a32315 100644
--- a/oci/layout/oci_dest.go
+++ b/oci/layout/oci_dest.go
@@ -279,7 +279,7 @@ func (d *ociImageDestination) addManifest(desc *imgspecv1.Descriptor) {
 	// If it has the same digest as another entry in the index, we already overwrote the file,
 	// so just pick up the other information.
 	for i, manifest := range d.index.Manifests {
-		if manifest.Digest == desc.Digest {
+		if manifest.Digest == desc.Digest && manifest.Annotations[imgspecv1.AnnotationRefName] == "" {
 			// Replace it completely.
 			d.index.Manifests[i] = *desc
 			return


Test patch:
diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go
index f6feefc6..77f470c6 100644
--- a/oci/layout/oci_dest_test.go
+++ b/oci/layout/oci_dest_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/containers/image/v5/pkg/blobinfocache/memory"
 	"github.com/containers/image/v5/types"
 	digest "github.com/opencontainers/go-digest"
+	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -105,7 +106,31 @@ func TestPutManifestTwice(t *testing.T) {
 
 	index, err := ociRef.getIndex()
 	assert.NoError(t, err)
-	assert.Equal(t, 2, len(index.Manifests), "Unexpected number of manifests")
+	assert.Len(t, index.Manifests, 2, "Unexpected number of manifests")
+}
+
+func TestPutTwoDifferentTags(t *testing.T) {
+	ref, tmpDir := refToTempOCI(t)
+	defer os.RemoveAll(tmpDir)
+
+	ociRef, ok := ref.(ociReference)
+	require.True(t, ok)
+
+	putTestConfig(t, ociRef, tmpDir)
+	putTestManifest(t, ociRef, tmpDir)
+
+	// add the same manifest with a different tag; it shouldn't get overwritten
+	ref, err := NewReference(tmpDir, "zomg")
+	assert.NoError(t, err)
+	ociRef, ok = ref.(ociReference)
+	require.True(t, ok)
+	putTestManifest(t, ociRef, tmpDir)
+
+	index, err := ociRef.getIndex()
+	assert.NoError(t, err)
+	assert.Len(t, index.Manifests, 3, "Unexpected number of manifests")
+	assert.Equal(t, "imageValue", index.Manifests[1].Annotations[imgspecv1.AnnotationRefName])
+	assert.Equal(t, "zomg", index.Manifests[2].Annotations[imgspecv1.AnnotationRefName])
 }
 
 func putTestConfig(t *testing.T, ociRef ociReference, tmpDir string) {