From 019ead86f5603e5b1b3a7afd4bb7cbcdab2613e9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:15:53 +0100 Subject: [PATCH 1/3] 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 --- 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 33273213b..d07299343 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 9c0d27085..57857e18c 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -2,6 +2,7 @@ package notifications import ( "context" + "io" "net/http" "github.com/distribution/distribution/v3" @@ -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 10c6fe3f0..532443f77 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -676,7 +676,7 @@ func (bs *blobs) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { return io.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 @@ -686,13 +686,12 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.Rea return nil, err } - return transport.NewHTTPReadSeeker(ctx, bs.client, blobURL, - func(resp *http.Response) error { - if resp.StatusCode == http.StatusNotFound { - return distribution.ErrBlobUnknown - } - return HandleErrorResponse(resp) - }), nil + return transport.NewHTTPReadSeeker(ctx, 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 43f58741c..af34fa294 100644 --- a/registry/proxy/proxyblobstore.go +++ b/registry/proxy/proxyblobstore.go @@ -213,7 +213,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 386bd41f2..b8ac992a6 100644 --- a/registry/proxy/proxyblobstore_test.go +++ b/registry/proxy/proxyblobstore_test.go @@ -2,6 +2,7 @@ package proxy import ( "context" + "io" "math/rand" "net/http" "net/http/httptest" @@ -58,7 +59,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 eb79f1c91..5f4344d08 100644 --- a/registry/storage/blobstore.go +++ b/registry/storage/blobstore.go @@ -2,6 +2,7 @@ package storage import ( "context" + "io" "path" "github.com/distribution/distribution/v3" @@ -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 94dee84bd..076f29f62 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" @@ -57,7 +58,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 d71ad5b3a6be14e002d130db9b9703732eee42e8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:23:11 +0100 Subject: [PATCH 2/3] 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 --- 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 a409e4943..71671158c 100644 --- a/registry/client/transport/http_reader.go +++ b/registry/client/transport/http_reader.go @@ -20,17 +20,17 @@ 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(ctx context.Context, client *http.Client, url string, errorHandler func(*http.Response) error) ReadSeekCloser { - return &httpReadSeeker{ +func NewHTTPReadSeeker(ctx context.Context, client *http.Client, url string, errorHandler func(*http.Response) error) *HTTPReadSeeker { + return &HTTPReadSeeker{ ctx: ctx, client: client, url: url, @@ -38,7 +38,8 @@ func NewHTTPReadSeeker(ctx context.Context, client *http.Client, url string, err } } -type httpReadSeeker struct { +// HTTPReadSeeker implements an [io.ReadSeekCloser]. +type HTTPReadSeeker struct { ctx context.Context client *http.Client url string @@ -63,7 +64,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 } @@ -92,7 +93,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 } @@ -135,7 +136,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 } @@ -152,7 +153,7 @@ func (hrs *httpReadSeeker) Close() error { return nil } -func (hrs *httpReadSeeker) reset() { +func (hrs *HTTPReadSeeker) reset() { if hrs.err != nil { return } @@ -162,7 +163,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 1d8cd5e443cad5e6d1fdee8bfdfd94b840030eee Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Nov 2022 17:30:21 +0100 Subject: [PATCH 3/3] registry/client: use struct literals Remove some intermediate variables, and use struct literals instead. 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 532443f77..0bc5b9086 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -138,16 +138,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 } @@ -162,16 +160,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(memory.UnlimitedSize), statter), + statter: cache.NewCachedBlobStatter(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize), &blobStatter{ + name: r.name, + ub: r.ub, + client: r.client, + }), } }