Descriptor: do not implement Describable interface (#3886)

blobs.go

@@ -85,15 +85,6 @@ type Descriptor struct {
 	// depend on the simplicity of this type.
 }
 
-// Descriptor returns the descriptor, to make it satisfy the Describable
-// interface. Note that implementations of Describable are generally objects
-// which can be described, not simply descriptors; this exception is in place
-// to make it more convenient to pass actual descriptors to functions that
-// expect Describable objects.
-func (d Descriptor) Descriptor() Descriptor {
-	return d
-}
-
 // BlobStatter makes blob descriptors available by digest. The service may
 // provide a descriptor of a different digest if the provided digest is not
 // canonical.

manifest/ocischema/builder.go

@@ -32,7 +32,7 @@ type Builder struct {
 // NewManifestBuilder is used to build new manifests for the current schema
 // version. It takes a BlobService so it can publish the configuration blob
 // as part of the Build process, and annotations.
-func NewManifestBuilder(bs distribution.BlobService, configJSON []byte, annotations map[string]string) distribution.ManifestBuilder {
+func NewManifestBuilder(bs distribution.BlobService, configJSON []byte, annotations map[string]string) *Builder {
 	mb := &Builder{
 		bs:          bs,
 		configJSON:  make([]byte, len(configJSON)),
@@ -96,8 +96,8 @@ func (mb *Builder) Build(ctx context.Context) (distribution.Manifest, error) {
 }
 
 // AppendReference adds a reference to the current ManifestBuilder.
-func (mb *Builder) AppendReference(d distribution.Describable) error {
-	mb.layers = append(mb.layers, d.Descriptor())
+func (mb *Builder) AppendReference(ref distribution.Descriptor) error {
+	mb.layers = append(mb.layers, ref)
 	return nil
 }
 

manifest/schema2/builder.go

@@ -6,8 +6,8 @@ import (
 	"github.com/distribution/distribution/v3"
 )
 
-// builder is a type for constructing manifests.
-type builder struct {
+// Builder is a type for constructing manifests.
+type Builder struct {
 	// configDescriptor is used to describe configuration
 	configDescriptor distribution.Descriptor
 
@@ -22,8 +22,8 @@ type builder struct {
 // NewManifestBuilder is used to build new manifests for the current schema
 // version. It takes a BlobService so it can publish the configuration blob
 // as part of the Build process.
-func NewManifestBuilder(configDescriptor distribution.Descriptor, configJSON []byte) distribution.ManifestBuilder {
-	mb := &builder{
+func NewManifestBuilder(configDescriptor distribution.Descriptor, configJSON []byte) *Builder {
+	mb := &Builder{
 		configDescriptor: configDescriptor,
 		configJSON:       make([]byte, len(configJSON)),
 	}
@@ -33,7 +33,7 @@ func NewManifestBuilder(configDescriptor distribution.Descriptor, configJSON []b
 }
 
 // Build produces a final manifest from the given references.
-func (mb *builder) Build(ctx context.Context) (distribution.Manifest, error) {
+func (mb *Builder) Build(ctx context.Context) (distribution.Manifest, error) {
 	m := Manifest{
 		Versioned: SchemaVersion,
 		Layers:    make([]distribution.Descriptor, len(mb.dependencies)),
@@ -46,12 +46,12 @@ func (mb *builder) Build(ctx context.Context) (distribution.Manifest, error) {
 }
 
 // AppendReference adds a reference to the current ManifestBuilder.
-func (mb *builder) AppendReference(d distribution.Describable) error {
-	mb.dependencies = append(mb.dependencies, d.Descriptor())
+func (mb *Builder) AppendReference(ref distribution.Descriptor) error {
+	mb.dependencies = append(mb.dependencies, ref)
 	return nil
 }
 
 // References returns the current references added to this builder.
-func (mb *builder) References() []distribution.Descriptor {
+func (mb *Builder) References() []distribution.Descriptor {
 	return mb.dependencies
 }

manifests.go

@@ -26,27 +26,6 @@ type Manifest interface {
 	Payload() (mediaType string, payload []byte, err error)
 }
 
-// ManifestBuilder creates a manifest allowing one to include dependencies.
-// Instances can be obtained from a version-specific manifest package.  Manifest
-// specific data is passed into the function which creates the builder.
-type ManifestBuilder interface {
-	// Build creates the manifest from his builder.
-	Build(ctx context.Context) (Manifest, error)
-
-	// References returns a list of objects which have been added to this
-	// builder. The dependencies are returned in the order they were added,
-	// which should be from base to head.
-	References() []Descriptor
-
-	// AppendReference includes the given object in the manifest after any
-	// existing dependencies. If the add fails, such as when adding an
-	// unsupported dependency, an error may be returned.
-	//
-	// The destination of the reference is dependent on the manifest type and
-	// the dependency type.
-	AppendReference(dependency Describable) error
-}
-
 // ManifestService describes operations on manifests.
 type ManifestService interface {
 	// Exists returns true if the manifest exists.
@@ -69,8 +48,12 @@ type ManifestEnumerator interface {
 	Enumerate(ctx context.Context, ingester func(digest.Digest) error) error
 }
 
-// Describable is an interface for descriptors
+// Describable is an interface for descriptors.
+//
+// Implementations of Describable are generally objects which can be
+// described, not simply descriptors.
 type Describable interface {
+	// Descriptor returns the descriptor.
 	Descriptor() Descriptor
 }
 

registry/storage/manifeststore_test.go

@@ -595,7 +595,7 @@ func TestIndexManifestStorageWithSelectivePlatforms(t *testing.T) {
 // createRandomImage builds an image manifest and store it and its layers in the registry
 func createRandomImage(t *testing.T, testname string, imageMediaType string, blobStore distribution.BlobStore) (distribution.Manifest, error) {
 	builder := ocischema.NewManifestBuilder(blobStore, []byte{}, map[string]string{})
-	err := builder.(*ocischema.Builder).SetMediaType(imageMediaType)
+	err := builder.SetMediaType(imageMediaType)
 	if err != nil {
 		t.Fatal(err)
 	}

testutil/manifests.go

@@ -1,7 +1,6 @@
 package testutil
 
 import (
-	"context"
 	"fmt"
 
 	"github.com/distribution/distribution/v3"
@@ -51,7 +50,18 @@ func MakeSchema2Manifest(repository distribution.Repository, digests []digest.Di
 		return nil, fmt.Errorf("unexpected error storing content in blobstore: %v", err)
 	}
 	builder := schema2.NewManifestBuilder(d, configJSON)
-	return makeManifest(ctx, builder, digests)
+	for _, dgst := range digests {
+		if err := builder.AppendReference(distribution.Descriptor{Digest: dgst}); err != nil {
+			return nil, fmt.Errorf("unexpected error building schema2 manifest: %v", err)
+		}
+	}
+
+	mfst, err := builder.Build(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("unexpected error generating schema2 manifest: %v", err)
+	}
+
+	return mfst, nil
 }
 
 func MakeOCIManifest(repository distribution.Repository, digests []digest.Digest) (distribution.Manifest, error) {
@@ -61,19 +71,15 @@ func MakeOCIManifest(repository distribution.Repository, digests []digest.Digest
 	var configJSON []byte
 
 	builder := ocischema.NewManifestBuilder(blobStore, configJSON, make(map[string]string))
-	return makeManifest(ctx, builder, digests)
-}
-
-func makeManifest(ctx context.Context, builder distribution.ManifestBuilder, digests []digest.Digest) (distribution.Manifest, error) {
-	for _, digest := range digests {
-		if err := builder.AppendReference(distribution.Descriptor{Digest: digest}); err != nil {
-			return nil, fmt.Errorf("unexpected error building manifest: %v", err)
+	for _, dgst := range digests {
+		if err := builder.AppendReference(distribution.Descriptor{Digest: dgst}); err != nil {
+			return nil, fmt.Errorf("unexpected error building OCI manifest: %v", err)
 		}
 	}
 
 	mfst, err := builder.Build(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("unexpected error generating manifest: %v", err)
+		return nil, fmt.Errorf("unexpected error generating OCI manifest: %v", err)
 	}
 
 	return mfst, nil