fix: OCI store resolve returns full descriptor (#468)

Based on discussion on #457, this PR returns a full descriptor on Resolve when the method it's called by tag and it returns a plain descriptor when the method it's called by digest. Signed-off-by: oanatmaria <[email protected]>
oras-project · Mar 23, 2023 · e8225cb · e8225cb
1 parent c1bf59e
commit e8225cb
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 9 deletions.
diff --git a/content/oci/oci.go b/content/oci/oci.go
@@ -165,7 +165,11 @@ func (s *Store) tag(ctx context.Context, desc ocispec.Descriptor, reference stri
 	return nil
 }
 
-// Resolve resolves a reference to a descriptor.
+// Resolve resolves a reference to a descriptor. If the reference to be resolved
+// is a tag, the returned descriptor will be a full descriptor declared by
+// github.com/opencontainers/image-spec/specs-go/v1. If the reference is a
+// digest the returned descriptor will be a plain descriptor (containing only
+// the digest, media type and size).
 func (s *Store) Resolve(ctx context.Context, reference string) (ocispec.Descriptor, error) {
 	if reference == "" {
 		return ocispec.Descriptor{}, errdef.ErrMissingReference
@@ -180,7 +184,12 @@ func (s *Store) Resolve(ctx context.Context, reference string) (ocispec.Descript
 		}
 		return ocispec.Descriptor{}, err
 	}
-	return descriptor.Plain(desc), nil
+
+	if reference == desc.Digest.String() {
+		return descriptor.Plain(desc), nil
+	}
+
+	return desc, nil
 }
 
 // Predecessors returns the nodes directly pointing to the current node.

diff --git a/content/oci/oci_test.go b/content/oci/oci_test.go
@@ -39,6 +39,7 @@ import (
 	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras-go/v2/errdef"
 	"oras.land/oras-go/v2/internal/cas"
+	"oras.land/oras-go/v2/internal/descriptor"
 	"oras.land/oras-go/v2/registry"
 )
 
@@ -452,6 +453,82 @@ func TestStore_ContentBadPush(t *testing.T) {
 	}
 }
 
+func TestStore_ResolveByTagReturnsFullDescriptor(t *testing.T) {
+	content := []byte("hello world")
+	ref := "hello-world:0.0.1"
+	annotations := map[string]string{"name": "Hello"}
+	desc := ocispec.Descriptor{
+		MediaType:   "test",
+		Digest:      digest.FromBytes(content),
+		Size:        int64(len(content)),
+		Annotations: annotations,
+	}
+
+	tempDir := t.TempDir()
+	s, err := New(tempDir)
+	if err != nil {
+		t.Fatal("New() error =", err)
+	}
+	ctx := context.Background()
+
+	err = s.Push(ctx, desc, bytes.NewReader(content))
+	if err != nil {
+		t.Errorf("Store.Push() error = %v, wantErr %v", err, false)
+	}
+
+	err = s.Tag(ctx, desc, ref)
+	if err != nil {
+		t.Errorf("error tagging descriptor error = %v, wantErr %v", err, false)
+	}
+
+	resolvedDescr, err := s.Resolve(ctx, ref)
+	if err != nil {
+		t.Errorf("error resolving descriptor error = %v, wantErr %v", err, false)
+	}
+
+	if !reflect.DeepEqual(resolvedDescr, desc) {
+		t.Errorf("Store.Resolve() = %v, want %v", resolvedDescr, desc)
+	}
+}
+
+func TestStore_ResolveByDigestReturnsPlainDescriptor(t *testing.T) {
+	content := []byte("hello world")
+	ref := "hello-world:0.0.1"
+	desc := ocispec.Descriptor{
+		MediaType:   "test",
+		Digest:      digest.FromBytes(content),
+		Size:        int64(len(content)),
+		Annotations: map[string]string{"name": "Hello"},
+	}
+	plainDescriptor := descriptor.Plain(desc)
+
+	tempDir := t.TempDir()
+	s, err := New(tempDir)
+	if err != nil {
+		t.Fatal("New() error =", err)
+	}
+	ctx := context.Background()
+
+	err = s.Push(ctx, desc, bytes.NewReader(content))
+	if err != nil {
+		t.Errorf("Store.Push() error = %v, wantErr %v", err, false)
+	}
+
+	err = s.Tag(ctx, desc, ref)
+	if err != nil {
+		t.Errorf("error tagging descriptor error = %v, wantErr %v", err, false)
+	}
+
+	resolvedDescr, err := s.Resolve(ctx, string(desc.Digest))
+	if err != nil {
+		t.Errorf("error resolving descriptor error = %v, wantErr %v", err, false)
+	}
+
+	if !reflect.DeepEqual(resolvedDescr, plainDescriptor) {
+		t.Errorf("Store.Resolve() = %v, want %v", resolvedDescr, plainDescriptor)
+	}
+}
+
 func TestStore_TagNotFound(t *testing.T) {
 	ref := "foobar"
 
@@ -1092,7 +1169,7 @@ func TestStore_ExistingStore(t *testing.T) {
 	if err != nil {
 		t.Fatal("Store: Resolve() error =", err)
 	}
-	if !reflect.DeepEqual(gotDesc, indexRoot) {
+	if !content.Equal(gotDesc, indexRoot) {
 		t.Errorf("Store.Resolve() = %v, want %v", gotDesc, indexRoot)
 	}
 

diff --git a/content/oci/readonlyoci.go b/content/oci/readonlyoci.go
@@ -83,7 +83,11 @@ func (s *ReadOnlyStore) Exists(ctx context.Context, target ocispec.Descriptor) (
 	return s.storage.Exists(ctx, target)
 }
 
-// Resolve resolves a reference to a descriptor.
+// Resolve resolves a reference to a descriptor. If the reference to be resolved
+// is a tag, the returned descriptor will be a full descriptor declared by
+// github.com/opencontainers/image-spec/specs-go/v1. If the reference is a
+// digest the returned descriptor will be a plain descriptor (containing only
+// the digest, media type and size).
 func (s *ReadOnlyStore) Resolve(ctx context.Context, reference string) (ocispec.Descriptor, error) {
 	if reference == "" {
 		return ocispec.Descriptor{}, errdef.ErrMissingReference
@@ -98,7 +102,12 @@ func (s *ReadOnlyStore) Resolve(ctx context.Context, reference string) (ocispec.
 		}
 		return ocispec.Descriptor{}, err
 	}
-	return descriptor.Plain(desc), nil
+
+	if reference == desc.Digest.String() {
+		return descriptor.Plain(desc), nil
+	}
+
+	return desc, nil
 }
 
 // Predecessors returns the nodes directly pointing to the current node.

diff --git a/content/oci/readonlyoci_test.go b/content/oci/readonlyoci_test.go
@@ -34,6 +34,7 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"golang.org/x/sync/errgroup"
 	"oras.land/oras-go/v2"
+	"oras.land/oras-go/v2/content"
 	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras-go/v2/internal/docker"
 	"oras.land/oras-go/v2/registry"
@@ -148,10 +149,17 @@ func TestReadOnlyStore(t *testing.T) {
 	if err != nil {
 		t.Error("ReadOnlyStore.Resolve() error =", err)
 	}
-	if want := descs[2]; !reflect.DeepEqual(gotDesc, want) {
+	if want := descs[2]; !content.Equal(gotDesc, want) {
 		t.Errorf("ReadOnlyStore.Resolve() = %v, want %v", gotDesc, want)
 	}
 
+	// descriptor resolved by tag should have annotations
+	if gotDesc.Annotations[ocispec.AnnotationRefName] != subjectTag {
+		t.Errorf("ReadOnlyStore.Resolve() returned descriptor without annotations %v, want %v",
+			gotDesc.Annotations,
+			map[string]string{ocispec.AnnotationRefName: subjectTag})
+	}
+
 	// test resolving artifact by digest
 	gotDesc, err = s.Resolve(ctx, descs[3].Digest.String())
 	if err != nil {
@@ -318,7 +326,7 @@ func TestReadOnlyStore_DirFS(t *testing.T) {
 	if err != nil {
 		t.Fatal("ReadOnlyStore: Resolve() error =", err)
 	}
-	if !reflect.DeepEqual(gotDesc, indexRoot) {
+	if !content.Equal(gotDesc, indexRoot) {
 		t.Errorf("ReadOnlyStore.Resolve() = %v, want %v", gotDesc, indexRoot)
 	}
 
@@ -613,7 +621,7 @@ func TestReadOnlyStore_Copy_OCIToMemory(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Copy() error = %v, wantErr %v", err, false)
 	}
-	if !reflect.DeepEqual(gotDesc, root) {
+	if !content.Equal(gotDesc, root) {
 		t.Errorf("Copy() = %v, want %v", gotDesc, root)
 	}
 
@@ -633,7 +641,7 @@ func TestReadOnlyStore_Copy_OCIToMemory(t *testing.T) {
 	if err != nil {
 		t.Fatal("dst.Resolve() error =", err)
 	}
-	if !reflect.DeepEqual(gotDesc, root) {
+	if !content.Equal(gotDesc, root) {
 		t.Errorf("dst.Resolve() = %v, want %v", gotDesc, root)
 	}
 }