Remove ManifestBuilder interface

Defining an interface on the implementer side is generally not best
practice in Go code. There is no code in the distribution module which
consumes a ManifestBuilder value so there is no need to define the
interface in the distribution module. Export the concrete
ManifestBuilder types and modify the constructors to return concrete
values.

Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider 2023-05-02 12:09:48 -04:00 committed by Sebastiaan van Stijn
parent f22dd61860
commit 671184e910
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
5 changed files with 25 additions and 40 deletions

View file

@ -32,7 +32,7 @@ type Builder struct {
// NewManifestBuilder is used to build new manifests for the current schema // NewManifestBuilder is used to build new manifests for the current schema
// version. It takes a BlobService so it can publish the configuration blob // version. It takes a BlobService so it can publish the configuration blob
// as part of the Build process, and annotations. // 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{ mb := &Builder{
bs: bs, bs: bs,
configJSON: make([]byte, len(configJSON)), configJSON: make([]byte, len(configJSON)),

View file

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

View file

@ -26,27 +26,6 @@ type Manifest interface {
Payload() (mediaType string, payload []byte, err error) 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. // ManifestService describes operations on manifests.
type ManifestService interface { type ManifestService interface {
// Exists returns true if the manifest exists. // Exists returns true if the manifest exists.

View file

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

View file

@ -1,7 +1,6 @@
package testutil package testutil
import ( import (
"context"
"fmt" "fmt"
"github.com/distribution/distribution/v3" "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) return nil, fmt.Errorf("unexpected error storing content in blobstore: %v", err)
} }
builder := schema2.NewManifestBuilder(d, configJSON) 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) { 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 var configJSON []byte
builder := ocischema.NewManifestBuilder(blobStore, configJSON, make(map[string]string)) builder := ocischema.NewManifestBuilder(blobStore, configJSON, make(map[string]string))
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 OCI manifest: %v", err)
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)
} }
} }
mfst, err := builder.Build(ctx) mfst, err := builder.Build(ctx)
if err != nil { 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 return mfst, nil