From 86c2280e10e7fa0c57123065e50026f31d8906f8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:07:31 +0100 Subject: [PATCH 1/4] simplify mocks Embed the interface that we're mocking; calling any of it's methods that are not implemented will panic, so should give the same result as before. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 2cd52d5c0c2952e7cce05683ae48ab3d573434fc) Signed-off-by: Sebastiaan van Stijn --- manifest/ocischema/builder_test.go | 17 +---------------- manifest/schema1/config_builder_test.go | 17 +---------------- manifest/schema2/builder_test.go | 17 +---------------- registry/proxy/proxytagservice_test.go | 7 +------ 4 files changed, 4 insertions(+), 54 deletions(-) diff --git a/manifest/ocischema/builder_test.go b/manifest/ocischema/builder_test.go index 7332f3a4a3..1e7021b075 100644 --- a/manifest/ocischema/builder_test.go +++ b/manifest/ocischema/builder_test.go @@ -12,6 +12,7 @@ import ( type mockBlobService struct { descriptors map[digest.Digest]distribution.Descriptor + distribution.BlobService } func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { @@ -21,14 +22,6 @@ func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distri return distribution.Descriptor{}, distribution.ErrBlobUnknown } -func (bs *mockBlobService) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { - panic("not implemented") -} - func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { d := distribution.Descriptor{ Digest: digest.FromBytes(p), @@ -39,14 +32,6 @@ func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) return d, nil } -func (bs *mockBlobService) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { - panic("not implemented") -} - func TestBuilder(t *testing.T) { imgJSON := []byte(`{ "created": "2015-10-31T22:22:56.015925234Z", diff --git a/manifest/schema1/config_builder_test.go b/manifest/schema1/config_builder_test.go index 03b0d2fac3..4f4db83ceb 100644 --- a/manifest/schema1/config_builder_test.go +++ b/manifest/schema1/config_builder_test.go @@ -17,6 +17,7 @@ import ( type mockBlobService struct { descriptors map[digest.Digest]distribution.Descriptor + distribution.BlobService } func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { @@ -26,14 +27,6 @@ func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distri return distribution.Descriptor{}, distribution.ErrBlobUnknown } -func (bs *mockBlobService) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { - panic("not implemented") -} - func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { d := distribution.Descriptor{ Digest: digest.FromBytes(p), @@ -44,14 +37,6 @@ func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) return d, nil } -func (bs *mockBlobService) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { - panic("not implemented") -} - func TestEmptyTar(t *testing.T) { // Confirm that gzippedEmptyTar expands to 1024 NULL bytes. var decompressed [2048]byte diff --git a/manifest/schema2/builder_test.go b/manifest/schema2/builder_test.go index cde73242e6..7621901bd6 100644 --- a/manifest/schema2/builder_test.go +++ b/manifest/schema2/builder_test.go @@ -11,6 +11,7 @@ import ( type mockBlobService struct { descriptors map[digest.Digest]distribution.Descriptor + distribution.BlobService } func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { @@ -20,14 +21,6 @@ func (bs *mockBlobService) Stat(ctx context.Context, dgst digest.Digest) (distri return distribution.Descriptor{}, distribution.ErrBlobUnknown } -func (bs *mockBlobService) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { - panic("not implemented") -} - func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { d := distribution.Descriptor{ Digest: digest.FromBytes(p), @@ -38,14 +31,6 @@ func (bs *mockBlobService) Put(ctx context.Context, mediaType string, p []byte) return d, nil } -func (bs *mockBlobService) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) { - panic("not implemented") -} - -func (bs *mockBlobService) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { - panic("not implemented") -} - func TestBuilder(t *testing.T) { imgJSON := []byte(`{ "architecture": "amd64", diff --git a/registry/proxy/proxytagservice_test.go b/registry/proxy/proxytagservice_test.go index 1314121e63..c1e103e1d2 100644 --- a/registry/proxy/proxytagservice_test.go +++ b/registry/proxy/proxytagservice_test.go @@ -13,10 +13,9 @@ import ( type mockTagStore struct { mapping map[string]distribution.Descriptor sync.Mutex + distribution.TagService } -var _ distribution.TagService = &mockTagStore{} - func (m *mockTagStore) Get(ctx context.Context, tag string) (distribution.Descriptor, error) { m.Lock() defer m.Unlock() @@ -58,10 +57,6 @@ func (m *mockTagStore) All(ctx context.Context) ([]string, error) { return tags, nil } -func (m *mockTagStore) Lookup(ctx context.Context, digest distribution.Descriptor) ([]string, error) { - panic("not implemented") -} - func testProxyTagService(local, remote map[string]distribution.Descriptor) *proxyTagService { if local == nil { local = make(map[string]distribution.Descriptor) From e22b5550892554c62c4d3a7d02a7ff62f154e01d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:15:53 +0100 Subject: [PATCH 2/4] deprecate ReadSeekCloser in favor of io.ReadSeekCloser Go's io package in stdlib now defines this interface, so we can switch to using that instead. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 019ead86f5603e5b1b3a7afd4bb7cbcdab2613e9) Signed-off-by: Sebastiaan van Stijn --- blobs.go | 14 ++++++-------- notifications/listener.go | 3 ++- registry/client/repository.go | 15 +++++++-------- registry/proxy/proxyblobstore.go | 2 +- registry/proxy/proxyblobstore_test.go | 3 ++- registry/storage/blobstore.go | 3 ++- registry/storage/linkedblobstore.go | 3 ++- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/blobs.go b/blobs.go index 671372abf4..fb49cfcfc9 100644 --- a/blobs.go +++ b/blobs.go @@ -142,20 +142,18 @@ type BlobDescriptorServiceFactory interface { // ReadSeekCloser is the primary reader type for blob data, combining // io.ReadSeeker with io.Closer. -type ReadSeekCloser interface { - io.ReadSeeker - io.Closer -} +// +// Deprecated: use [io.ReadSeekCloser]. +type ReadSeekCloser = io.ReadSeekCloser // BlobProvider describes operations for getting blob data. type BlobProvider interface { // Get returns the entire blob identified by digest along with the descriptor. Get(ctx context.Context, dgst digest.Digest) ([]byte, error) - // Open provides a ReadSeekCloser to the blob identified by the provided - // descriptor. If the blob is not known to the service, an error will be - // returned. - Open(ctx context.Context, dgst digest.Digest) (ReadSeekCloser, error) + // Open provides an [io.ReadSeekCloser] to the blob identified by the provided + // descriptor. If the blob is not known to the service, an error is returned. + Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) } // BlobServer can serve blobs via http. diff --git a/notifications/listener.go b/notifications/listener.go index 51d833d4dd..c8fb44f9a8 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -2,6 +2,7 @@ package notifications import ( "context" + "io" "net/http" "github.com/docker/distribution" @@ -147,7 +148,7 @@ func (bsl *blobServiceListener) Get(ctx context.Context, dgst digest.Digest) ([] return p, err } -func (bsl *blobServiceListener) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (bsl *blobServiceListener) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { rc, err := bsl.BlobStore.Open(ctx, dgst) if err == nil { if desc, err := bsl.Stat(ctx, dgst); err != nil { diff --git a/registry/client/repository.go b/registry/client/repository.go index fd42a1e66f..c2bcca8975 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -645,7 +645,7 @@ func (bs *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { return ioutil.ReadAll(reader) } -func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { ref, err := reference.WithDigest(bs.name, dgst) if err != nil { return nil, err @@ -655,13 +655,12 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.Rea return nil, err } - return transport.NewHTTPReadSeeker(bs.client, blobURL, - func(resp *http.Response) error { - if resp.StatusCode == http.StatusNotFound { - return distribution.ErrBlobUnknown - } - return HandleErrorResponse(resp) - }), nil + return transport.NewHTTPReadSeeker(bs.client, blobURL, func(resp *http.Response) error { + if resp.StatusCode == http.StatusNotFound { + return distribution.ErrBlobUnknown + } + return HandleErrorResponse(resp) + }), nil } func (bs *blobs) ServeBlob(ctx context.Context, w http.ResponseWriter, r *http.Request, dgst digest.Digest) error { diff --git a/registry/proxy/proxyblobstore.go b/registry/proxy/proxyblobstore.go index a9219957f3..5605e0774e 100644 --- a/registry/proxy/proxyblobstore.go +++ b/registry/proxy/proxyblobstore.go @@ -212,7 +212,7 @@ func (pbs *proxyBlobStore) Mount(ctx context.Context, sourceRepo reference.Named return distribution.Descriptor{}, distribution.ErrUnsupported } -func (pbs *proxyBlobStore) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (pbs *proxyBlobStore) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { return nil, distribution.ErrUnsupported } diff --git a/registry/proxy/proxyblobstore_test.go b/registry/proxy/proxyblobstore_test.go index b491d555be..8141b8e7e9 100644 --- a/registry/proxy/proxyblobstore_test.go +++ b/registry/proxy/proxyblobstore_test.go @@ -2,6 +2,7 @@ package proxy import ( "context" + "io" "io/ioutil" "math/rand" "net/http" @@ -60,7 +61,7 @@ func (sbs statsBlobStore) Resume(ctx context.Context, id string) (distribution.B return sbs.blobs.Resume(ctx, id) } -func (sbs statsBlobStore) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (sbs statsBlobStore) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { sbsMu.Lock() sbs.stats["open"]++ sbsMu.Unlock() diff --git a/registry/storage/blobstore.go b/registry/storage/blobstore.go index 1008aad8c0..d70911a1ff 100644 --- a/registry/storage/blobstore.go +++ b/registry/storage/blobstore.go @@ -2,6 +2,7 @@ package storage import ( "context" + "io" "path" "github.com/docker/distribution" @@ -41,7 +42,7 @@ func (bs *blobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte, error return p, nil } -func (bs *blobStore) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (bs *blobStore) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { desc, err := bs.statter.Stat(ctx, dgst) if err != nil { return nil, err diff --git a/registry/storage/linkedblobstore.go b/registry/storage/linkedblobstore.go index 32e9a4898f..c96d149805 100644 --- a/registry/storage/linkedblobstore.go +++ b/registry/storage/linkedblobstore.go @@ -3,6 +3,7 @@ package storage import ( "context" "fmt" + "io" "net/http" "path" "time" @@ -59,7 +60,7 @@ func (lbs *linkedBlobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte return lbs.blobStore.Get(ctx, canonical.Digest) } -func (lbs *linkedBlobStore) Open(ctx context.Context, dgst digest.Digest) (distribution.ReadSeekCloser, error) { +func (lbs *linkedBlobStore) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekCloser, error) { canonical, err := lbs.Stat(ctx, dgst) // access check if err != nil { return nil, err From 1be402c953a5fead8b620c09275da6d692a125bc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:23:11 +0100 Subject: [PATCH 3/4] transport.NewHTTPReadSeeker: return concrete type, deprecate ReadSeekCloser General convention is to define interfaces on the receiver side, and to return concrete types. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit d71ad5b3a6be14e002d130db9b9703732eee42e8) Signed-off-by: Sebastiaan van Stijn --- registry/client/transport/http_reader.go | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/registry/client/transport/http_reader.go b/registry/client/transport/http_reader.go index 9120dbed66..d93957ca38 100644 --- a/registry/client/transport/http_reader.go +++ b/registry/client/transport/http_reader.go @@ -19,24 +19,25 @@ var ( ) // ReadSeekCloser combines io.ReadSeeker with io.Closer. -type ReadSeekCloser interface { - io.ReadSeeker - io.Closer -} +// +// Deprecated: use [io.ReadSeekCloser]. +type ReadSeekCloser = io.ReadSeekCloser // NewHTTPReadSeeker handles reading from an HTTP endpoint using a GET // request. When seeking and starting a read from a non-zero offset // the a "Range" header will be added which sets the offset. +// // TODO(dmcgowan): Move this into a separate utility package -func NewHTTPReadSeeker(client *http.Client, url string, errorHandler func(*http.Response) error) ReadSeekCloser { - return &httpReadSeeker{ +func NewHTTPReadSeeker(client *http.Client, url string, errorHandler func(*http.Response) error) *HTTPReadSeeker { + return &HTTPReadSeeker{ client: client, url: url, errorHandler: errorHandler, } } -type httpReadSeeker struct { +// HTTPReadSeeker implements an [io.ReadSeekCloser]. +type HTTPReadSeeker struct { client *http.Client url string @@ -60,7 +61,7 @@ type httpReadSeeker struct { err error } -func (hrs *httpReadSeeker) Read(p []byte) (n int, err error) { +func (hrs *HTTPReadSeeker) Read(p []byte) (n int, err error) { if hrs.err != nil { return 0, hrs.err } @@ -89,7 +90,7 @@ func (hrs *httpReadSeeker) Read(p []byte) (n int, err error) { return n, err } -func (hrs *httpReadSeeker) Seek(offset int64, whence int) (int64, error) { +func (hrs *HTTPReadSeeker) Seek(offset int64, whence int) (int64, error) { if hrs.err != nil { return 0, hrs.err } @@ -132,7 +133,7 @@ func (hrs *httpReadSeeker) Seek(offset int64, whence int) (int64, error) { return hrs.seekOffset, err } -func (hrs *httpReadSeeker) Close() error { +func (hrs *HTTPReadSeeker) Close() error { if hrs.err != nil { return hrs.err } @@ -149,7 +150,7 @@ func (hrs *httpReadSeeker) Close() error { return nil } -func (hrs *httpReadSeeker) reset() { +func (hrs *HTTPReadSeeker) reset() { if hrs.err != nil { return } @@ -159,7 +160,7 @@ func (hrs *httpReadSeeker) reset() { } } -func (hrs *httpReadSeeker) reader() (io.Reader, error) { +func (hrs *HTTPReadSeeker) reader() (io.Reader, error) { if hrs.err != nil { return nil, hrs.err } From bb33a43c749b5b319e11f4ef57cd4111b06cdcff Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:30:21 +0100 Subject: [PATCH 4/4] registry/client: use struct literals Remove some intermediate variables, and use struct literals instead. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 1d8cd5e443cad5e6d1fdee8bfdfd94b840030eee) Signed-off-by: Sebastiaan van Stijn --- registry/client/repository.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/registry/client/repository.go b/registry/client/repository.go index c2bcca8975..969994420f 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -135,16 +135,14 @@ func NewRepository(name reference.Named, baseURL string, transport http.RoundTri return nil, err } - client := &http.Client{ - Transport: transport, - CheckRedirect: checkHTTPRedirect, - // TODO(dmcgowan): create cookie jar - } - return &repository{ - client: client, - ub: ub, - name: name, + client: &http.Client{ + Transport: transport, + CheckRedirect: checkHTTPRedirect, + // TODO(dmcgowan): create cookie jar + }, + ub: ub, + name: name, }, nil } @@ -159,16 +157,15 @@ func (r *repository) Named() reference.Named { } func (r *repository) Blobs(ctx context.Context) distribution.BlobStore { - statter := &blobStatter{ + return &blobs{ name: r.name, ub: r.ub, client: r.client, - } - return &blobs{ - name: r.name, - ub: r.ub, - client: r.client, - statter: cache.NewCachedBlobStatter(memory.NewInMemoryBlobDescriptorCacheProvider(), statter), + statter: cache.NewCachedBlobStatter(memory.NewInMemoryBlobDescriptorCacheProvider(), &blobStatter{ + name: r.name, + ub: r.ub, + client: r.client, + }), } }