ociindex: fix handling multiple names per descriptor

Previous implementation mixed tags and names and added invalid comma-separated reference annotation. Signed-off-by: Tonis Tiigi <[email protected]>
moby · Dec 12, 2024 · 5906c86 · 5906c86
1 parent 3e5a563
commit 5906c86
Show file tree

Hide file tree

Showing 4 changed files with 277 additions and 16 deletions.
diff --git a/client/ociindex/ociindex.go b/client/ociindex/ociindex.go
@@ -3,10 +3,12 @@ package ociindex
 import (
 	"encoding/json"
 	"io"
+	"maps"
 	"os"
 	"path"
 	"syscall"
 
+	"github.com/containerd/containerd/reference"
 	"github.com/gofrs/flock"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -15,6 +17,8 @@ import (
 const (
 	// lockFileSuffix is the suffix of the lock file
 	lockFileSuffix = ".lock"
+
+	annotationImageName = "io.containerd.image.name"
 )
 
 type StoreIndex struct {
@@ -23,6 +27,19 @@ type StoreIndex struct {
 	layoutPath string
 }
 
+type NameOrTag struct {
+	isTag bool
+	value string
+}
+
+func Name(name string) NameOrTag {
+	return NameOrTag{value: name}
+}
+
+func Tag(tag string) NameOrTag {
+	return NameOrTag{isTag: true, value: tag}
+}
+
 func NewStoreIndex(storePath string) StoreIndex {
 	indexPath := path.Join(storePath, ocispecs.ImageIndexFile)
 	layoutPath := path.Join(storePath, ocispecs.ImageLayoutFile)
@@ -61,7 +78,7 @@ func (s StoreIndex) Read() (*ocispecs.Index, error) {
 	return &idx, nil
 }
 
-func (s StoreIndex) Put(tag string, desc ocispecs.Descriptor) error {
+func (s StoreIndex) Put(desc ocispecs.Descriptor, names ...NameOrTag) error {
 	// lock the store to prevent concurrent access
 	lock := flock.New(s.lockPath)
 	locked, err := lock.TryLock()
@@ -107,8 +124,19 @@ func (s StoreIndex) Put(tag string, desc ocispecs.Descriptor) error {
 	}
 
 	setOCIIndexDefaults(&idx)
-	if err = insertDesc(&idx, desc, tag); err != nil {
-		return err
+
+	namesp := make([]*NameOrTag, 0, len(names))
+	for _, n := range names {
+		namesp = append(namesp, &n)
+	}
+	if len(names) == 0 {
+		namesp = append(namesp, nil)
+	}
+
+	for _, name := range namesp {
+		if err = insertDesc(&idx, desc, name); err != nil {
+			return err
+		}
 	}
 
 	idxData, err = json.Marshal(idx)
@@ -130,6 +158,12 @@ func (s StoreIndex) Get(tag string) (*ocispecs.Descriptor, error) {
 		return nil, err
 	}
 
+	for _, m := range idx.Manifests {
+		if t, ok := m.Annotations[annotationImageName]; ok && t == tag {
+			return &m, nil
+		}
+	}
+
 	for _, m := range idx.Manifests {
 		if t, ok := m.Annotations[ocispecs.AnnotationRefName]; ok && t == tag {
 			return &m, nil
@@ -165,20 +199,34 @@ func setOCIIndexDefaults(index *ocispecs.Index) {
 
 // insertDesc puts desc to index with tag.
 // Existing manifests with the same tag will be removed from the index.
-func insertDesc(index *ocispecs.Index, desc ocispecs.Descriptor, tag string) error {
+func insertDesc(index *ocispecs.Index, in ocispecs.Descriptor, name *NameOrTag) error {
 	if index == nil {
 		return nil
 	}
 
-	if tag != "" {
+	// make a copy to not modify the input descriptor
+	desc := in
+	desc.Annotations = maps.Clone(in.Annotations)
+
+	if name != nil {
 		if desc.Annotations == nil {
 			desc.Annotations = make(map[string]string)
 		}
-		desc.Annotations[ocispecs.AnnotationRefName] = tag
-		// remove existing manifests with the same tag
+		imgName, refName := name.value, name.value
+		if name.isTag {
+			imgName = ""
+		} else {
+			refName = ociReferenceName(imgName)
+		}
+
+		if imgName != "" {
+			desc.Annotations[annotationImageName] = imgName
+		}
+		desc.Annotations[ocispecs.AnnotationRefName] = refName
+		// remove existing manifests with the same tag/name
 		var manifests []ocispecs.Descriptor
 		for _, m := range index.Manifests {
-			if m.Annotations[ocispecs.AnnotationRefName] != tag {
+			if m.Annotations[ocispecs.AnnotationRefName] != refName || m.Annotations[annotationImageName] != imgName {
 				manifests = append(manifests, m)
 			}
 		}
@@ -187,3 +235,20 @@ func insertDesc(index *ocispecs.Index, desc ocispecs.Descriptor, tag string) err
 	index.Manifests = append(index.Manifests, desc)
 	return nil
 }
+
+// ociReferenceName takes the loosely defined reference name same way as
+// containerd tar exporter does.
+func ociReferenceName(name string) string {
+	// OCI defines the reference name as only a tag excluding the
+	// repository. The containerd annotation contains the full image name
+	// since the tag is insufficient for correctly naming and referring to an
+	// image
+	var ociRef string
+	if spec, err := reference.Parse(name); err == nil {
+		ociRef = spec.Object
+	} else {
+		ociRef = name
+	}
+
+	return ociRef
+}
diff --git a/client/ociindex/ociindex_test.go b/client/ociindex/ociindex_test.go
@@ -6,7 +6,7 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/opencontainers/go-digest"
+	digest "github.com/opencontainers/go-digest"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -85,7 +85,7 @@ func TestWriteSingleDescriptor(t *testing.T) {
 	store := NewStoreIndex(dir)
 
 	desc := randDescriptor("foo")
-	err := store.Put("", desc)
+	err := store.Put(desc)
 	require.NoError(t, err)
 
 	readDesc, err := store.GetSingle()
@@ -113,7 +113,7 @@ func TestAddDescriptor(t *testing.T) {
 
 	store := NewStoreIndex(dir)
 	three := randDescriptor("baz")
-	err = store.Put("", three)
+	err = store.Put(three)
 	require.NoError(t, err)
 
 	readIdx, err := store.Read()
@@ -149,7 +149,7 @@ func TestAddDescriptorWithTag(t *testing.T) {
 
 	store := NewStoreIndex(dir)
 	three := randDescriptor("baz")
-	err = store.Put("ver1", three)
+	err = store.Put(three, Tag("ver1"))
 	require.NoError(t, err)
 
 	desc, err := store.Get("ver1")
@@ -170,6 +170,101 @@ func TestAddDescriptorWithTag(t *testing.T) {
 	assert.Equal(t, *desc, readIdx.Manifests[2])
 }
 
+func TestAddMultipleNames(t *testing.T) {
+	dir := t.TempDir()
+
+	store := NewStoreIndex(dir)
+
+	one := randDescriptor("foo")
+	err := store.Put(one, Name("app/name:v1"), Name("app/name:v1.0"), Name("app/other:latest"))
+	require.NoError(t, err)
+
+	var idx ocispecs.Index
+	dt, err := os.ReadFile(filepath.Join(dir, "index.json"))
+	require.NoError(t, err)
+
+	err = json.Unmarshal(dt, &idx)
+	require.NoError(t, err)
+
+	require.Len(t, idx.Manifests, 3)
+
+	require.Equal(t, one.Digest, idx.Manifests[0].Digest)
+	require.Equal(t, one.Size, idx.Manifests[0].Size)
+	require.Equal(t, one.MediaType, idx.Manifests[0].MediaType)
+
+	require.Equal(t, "v1", idx.Manifests[0].Annotations["org.opencontainers.image.ref.name"])
+	require.Equal(t, "app/name:v1", idx.Manifests[0].Annotations["io.containerd.image.name"])
+
+	require.Equal(t, one.Digest, idx.Manifests[1].Digest)
+	require.Equal(t, one.Size, idx.Manifests[1].Size)
+	require.Equal(t, one.MediaType, idx.Manifests[1].MediaType)
+
+	require.Equal(t, "v1.0", idx.Manifests[1].Annotations["org.opencontainers.image.ref.name"])
+	require.Equal(t, "app/name:v1.0", idx.Manifests[1].Annotations["io.containerd.image.name"])
+
+	require.Equal(t, one.Digest, idx.Manifests[2].Digest)
+	require.Equal(t, one.Size, idx.Manifests[2].Size)
+	require.Equal(t, one.MediaType, idx.Manifests[1].MediaType)
+
+	require.Equal(t, "latest", idx.Manifests[2].Annotations["org.opencontainers.image.ref.name"])
+	require.Equal(t, "app/other:latest", idx.Manifests[2].Annotations["io.containerd.image.name"])
+
+	desc, err := store.Get("app/name:v1")
+	require.NoError(t, err)
+	require.NotNil(t, desc)
+
+	require.Equal(t, one.Digest, desc.Digest)
+	require.Equal(t, one.Size, desc.Size)
+	require.Equal(t, one.MediaType, desc.MediaType)
+
+	require.Equal(t, "v1", desc.Annotations["org.opencontainers.image.ref.name"])
+	require.Equal(t, "app/name:v1", desc.Annotations["io.containerd.image.name"])
+}
+
+func TestReplaceByImageName(t *testing.T) {
+	dir := t.TempDir()
+
+	strore := NewStoreIndex(dir)
+	one := randDescriptor("foo")
+	two := randDescriptor("bar")
+	three := randDescriptor("baz")
+
+	err := strore.Put(one)
+	require.NoError(t, err)
+
+	err = strore.Put(two, Name("app/name:v1"))
+	require.NoError(t, err)
+
+	err = strore.Put(three, Name("app/name:v2"))
+	require.NoError(t, err)
+
+	// replace by image name
+	four := randDescriptor("qux")
+	err = strore.Put(four, Name("app/name:v1"))
+	require.NoError(t, err)
+
+	readIdx, err := strore.Read()
+	require.NoError(t, err)
+
+	assert.Len(t, readIdx.Manifests, 3)
+
+	assert.Equal(t, one, readIdx.Manifests[0])
+
+	assert.Equal(t, three.Digest, readIdx.Manifests[1].Digest)
+	assert.Equal(t, three.Size, readIdx.Manifests[1].Size)
+	assert.Equal(t, three.MediaType, readIdx.Manifests[1].MediaType)
+
+	assert.Equal(t, "v2", readIdx.Manifests[1].Annotations["org.opencontainers.image.ref.name"])
+	assert.Equal(t, "app/name:v2", readIdx.Manifests[1].Annotations["io.containerd.image.name"])
+
+	assert.Equal(t, four.Digest, readIdx.Manifests[2].Digest)
+	assert.Equal(t, four.Size, readIdx.Manifests[2].Size)
+	assert.Equal(t, four.MediaType, readIdx.Manifests[2].MediaType)
+
+	assert.Equal(t, "v1", readIdx.Manifests[2].Annotations["org.opencontainers.image.ref.name"])
+	assert.Equal(t, "app/name:v1", readIdx.Manifests[2].Annotations["io.containerd.image.name"])
+}
+
 func randDescriptor(seed string) ocispecs.Descriptor {
 	dgst := digest.FromBytes([]byte(seed))
 	return ocispecs.Descriptor{

diff --git a/client/solve.go b/client/solve.go
@@ -345,7 +345,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 		}
 		for storePath, tag := range cacheOpt.storesToUpdate {
 			idx := ociindex.NewStoreIndex(storePath)
-			if err := idx.Put(tag, manifestDesc); err != nil {
+			if err := idx.Put(manifestDesc, ociindex.Tag(tag)); err != nil {
 				return nil, err
 			}
 		}
@@ -360,12 +360,16 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 			return nil, err
 		}
 		for _, storePath := range storesToUpdate {
-			tag := "latest"
+			names := []ociindex.NameOrTag{ociindex.Tag("latest")}
 			if t, ok := res.ExporterResponse["image.name"]; ok {
-				tag = t
+				inp := strings.Split(t, ",")
+				names = make([]ociindex.NameOrTag, len(inp))
+				for i, n := range inp {
+					names[i] = ociindex.Name(n)
+				}
 			}
 			idx := ociindex.NewStoreIndex(storePath)
-			if err := idx.Put(tag, manifestDesc); err != nil {
+			if err := idx.Put(manifestDesc, names...); err != nil {
 				return nil, err
 			}
 		}