Update ociregistry and deal with the minor breaking changes

This also updates our `replace` to my new upstream fix that deals with `HEAD` on a Docker Hub digest failing when it shouldn't.

Author: tianon

go.mod

@@ -20,5 +20,5 @@ require (
 	google.golang.org/protobuf v1.28.1 // indirect
 )
 
-// https://github.com/cue-labs/oci/pull/27
-replace cuelabs.dev/go/oci/ociregistry => github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240216044210-8aa0c990bd77
+// https://github.com/cue-labs/oci/pull/29
+replace cuelabs.dev/go/oci/ociregistry => github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240322151419-7d3242933116

go.sum

@@ -32,8 +32,8 @@ github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVs
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240216044210-8aa0c990bd77 h1:9EPZm+sGlYHo6LleMXWR6s3P8SJEYA7/aovpJ76JSpw=
-github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240216044210-8aa0c990bd77/go.mod h1:ApHceQLLwcOkCEXM1+DyCXTHEJhNGDpJ2kmV6axsx24=
+github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240322151419-7d3242933116 h1:ZDy4uRAhzODJXRo4EoNpJTCiSeOs8wwrkfMJy3JyDps=
+github.com/tianon/cuelabs-oci/ociregistry v0.0.0-20240322151419-7d3242933116/go.mod h1:pK23AUVXuNzzTpfMCA06sxZGeVQ/75FdVtW249de9Uo=
 golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g=
 golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

registry/client.go

@@ -25,18 +25,24 @@ func Client(host string, opts *ociclient.Options) (ociregistry.Interface, error)
 		if opts != nil {
 			clientOptions = *opts
 		}
-		if clientOptions.HTTPClient == nil {
-			clientOptions.HTTPClient = http.DefaultClient
+		if clientOptions.Transport == nil {
+			clientOptions.Transport = http.DefaultTransport
 		}
 
 		// if we have a rate limiter configured for this registry, shim it in
 		if limiter, ok := registryRateLimiters[host]; ok {
-			clientOptions.HTTPClient = &rateLimitedRetryingDoer{
-				doer:    clientOptions.HTTPClient,
-				limiter: limiter,
+			clientOptions.Transport = &rateLimitedRetryingRoundTripper{
+				roundTripper: clientOptions.Transport,
+				limiter:      limiter,
 			}
 		}
 
+		// install the "authorization" wrapper/shim
+		clientOptions.Transport = ociauth.NewStdTransport(ociauth.StdTransportParams{
+			Config:    authConfig,
+			Transport: clientOptions.Transport,
+		})
+
 		connectHost := host
 		if host == dockerHubCanonical {
 			connectHost = dockerHubConnect
@@ -46,14 +52,6 @@ func Client(host string, opts *ociclient.Options) (ociregistry.Interface, error)
 			// TODO some way for callers to specify that their "localhost" *does* require TLS (maybe only do this if `opts == nil`, but then users cannot supply *any* options and still get help setting Insecure for localhost 🤔 -- at least this is a more narrow use case than the opposite of not having a way to have non-localhost insecure registries)
 		}
 
-		if clientOptions.Authorizer == nil {
-			// TODO https://github.com/cue-labs/oci/pull/28 -- ideally we'd set this sooner, but https://github.com/cue-labs/oci/blob/5ebe80b0a9a67ae83802d1fb1a189a8f0d089fb0/ociregistry/ociclient/client.go#L278-L282 means we have to do it late in case we installed a rate limiting HTTPClient (or the caller provided a custom one)
-			clientOptions.Authorizer = ociauth.NewStdAuthorizer(ociauth.StdAuthorizerParams{
-				Config:     authConfig,
-				HTTPClient: clientOptions.HTTPClient,
-			})
-		}
-
 		hostOptions := clientOptions // make a copy, since "ociclient.New" mutates it (such that sharing the object afterwards probably isn't the best idea -- they'll have the same DebugID if so, which isn't ideal)
 		client, err := ociclient.New(connectHost, &hostOptions)
 		if err != nil {
@@ -86,7 +84,7 @@ func Client(host string, opts *ociclient.Options) (ociregistry.Interface, error)
 				default:
 					return nil, fmt.Errorf("unsupported DOCKERHUB_PUBLIC_PROXY (with path)")
 				}
-				// TODO complain about other URL bits (unsupported by "ociclient" except via custom "HTTPClient" / "HTTPDoer")
+				// TODO complain about other URL bits (unsupported by "ociclient" except via custom "RoundTripper")
 			} else if proxy := os.Getenv("DOCKERHUB_PUBLIC_PROXY_HOST"); proxy != "" {
 				proxyHost = proxy
 			}

registry/rate-limits.go

@@ -4,7 +4,6 @@ import (
 	"net/http"
 	"time"
 
-	"cuelabs.dev/go/oci/ociregistry/ociclient"
 	"golang.org/x/time/rate"
 )
 
@@ -14,13 +13,13 @@ var (
 	}
 )
 
-// an implementation of [ociclient.HTTPDoer] that transparently adds a total requests rate limit and 429-retrying behavior
-type rateLimitedRetryingDoer struct {
-	doer    ociclient.HTTPDoer
-	limiter *rate.Limiter
+// an implementation of [net/http.RoundTripper] that transparently adds a total requests rate limit and 429-retrying behavior
+type rateLimitedRetryingRoundTripper struct {
+	roundTripper http.RoundTripper
+	limiter      *rate.Limiter
 }
 
-func (d *rateLimitedRetryingDoer) Do(req *http.Request) (*http.Response, error) {
+func (d *rateLimitedRetryingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	requestRetryLimiter := rate.NewLimiter(rate.Every(time.Second), 1) // cap request retries at once per second
 	firstTry := true
 	ctx := req.Context()
@@ -32,23 +31,32 @@ func (d *rateLimitedRetryingDoer) Do(req *http.Request) (*http.Response, error)
 			return nil, err
 		}
 
-		if !firstTry && req.GetBody != nil {
-			var err error
-			req.Body, err = req.GetBody()
-			if err != nil {
-				return nil, err
+		if !firstTry {
+			// https://pkg.go.dev/net/http#RoundTripper
+			// "RoundTrip should not modify the request, except for consuming and closing the Request's Body."
+			if req.Body != nil {
+				req.Body.Close()
+			}
+			req = req.Clone(ctx)
+			if req.GetBody != nil {
+				var err error
+				req.Body, err = req.GetBody()
+				if err != nil {
+					return nil, err
+				}
 			}
 		}
 		firstTry = false
 
-		res, err := d.doer.Do(req)
+		// in theory, this RoundTripper we're invoking should close req.Body (per the RoundTripper contract), so we shouldn't have to 🤞
+		res, err := d.roundTripper.RoundTrip(req)
 		if err != nil {
 			return nil, err
 		}
 
 		// TODO 503 should probably result in at least one or two auto-retries (especially with the automatic retry delay this injects)
 		if res.StatusCode == 429 {
-			// satisfy the big scary warning on https://pkg.go.dev/net/http#Client.Do about the downsides of failing to Close the response body
+			// satisfy the big scary warnings on https://pkg.go.dev/net/http#RoundTripper and https://pkg.go.dev/net/http#Client.Do about the downsides of failing to Close the response body
 			if err := res.Body.Close(); err != nil {
 				return nil, err
 			}