Skip to content

Commit

Permalink
fix: OCI store resolve returns full descriptor (#468)
Browse files Browse the repository at this point in the history
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 <oana@aserto.com>
  • Loading branch information
oanatmaria authored Mar 23, 2023
1 parent c1bf59e commit e8225cb
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 9 deletions.
13 changes: 11 additions & 2 deletions content/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
79 changes: 78 additions & 1 deletion content/oci/oci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}

Expand Down
13 changes: 11 additions & 2 deletions content/oci/readonlyoci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
16 changes: 12 additions & 4 deletions content/oci/readonlyoci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Expand Down

0 comments on commit e8225cb

Please sign in to comment.