From 7c1309bac7f4bec595ae6108a832d532e936e5c7 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 11 Dec 2024 14:13:26 -0800 Subject: [PATCH] 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 --- client/ociindex/ociindex.go | 74 ++++++++++++++++++++++++++++---- client/ociindex/ociindex_test.go | 6 +-- client/solve.go | 12 ++++-- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/client/ociindex/ociindex.go b/client/ociindex/ociindex.go index e427f6550806..954f687b025c 100644 --- a/client/ociindex/ociindex.go +++ b/client/ociindex/ociindex.go @@ -7,6 +7,7 @@ import ( "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 +16,8 @@ import ( const ( // lockFileSuffix is the suffix of the lock file lockFileSuffix = ".lock" + + annotationImageName = "io.containerd.image.name" ) type StoreIndex struct { @@ -23,6 +26,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 +77,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 +123,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) @@ -165,21 +192,33 @@ 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, desc ocispecs.Descriptor, name *NameOrTag) error { if index == nil { return nil } - if tag != "" { + 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 { - manifests = append(manifests, m) + if m.Annotations[ocispecs.AnnotationRefName] != refName || m.Annotations[annotationImageName] != imgName { + if imgName == "" || m.Annotations[annotationImageName] != imgName { + manifests = append(manifests, m) + } } } index.Manifests = manifests @@ -187,3 +226,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 index ff68dcf793f1..3571be06a625 100644 --- a/client/ociindex/ociindex_test.go +++ b/client/ociindex/ociindex_test.go @@ -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") diff --git a/client/solve.go b/client/solve.go index df2cd16a5eb1..5fb4eb697196 100644 --- 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 } }