Move creation of bearerToken to obtainBearerToken

Instead of having getBearerToken* construct a new bearerToken
object, have the caller provide one.

This will allow us to record that a token is being obtained,
so that others can wait for it.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>

Author: mtrmac

docker/docker_client.go

@@ -141,7 +141,8 @@ const (
 	noAuth
 )
 
-func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) {
+// readFromJSONBlob sets token data in dest from the provided JSON.
+func (bt *bearerToken) readFromJSONBlob(blob []byte) error {
 	var token struct {
 		Token          string    `json:"token"`
 		AccessToken    string    `json:"access_token"`
@@ -150,14 +151,12 @@ func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) {
 		expirationTime time.Time
 	}
 	if err := json.Unmarshal(blob, &token); err != nil {
-		return nil, err
+		return err
 	}
 
-	res := &bearerToken{
-		token: token.Token,
-	}
-	if res.token == "" {
-		res.token = token.AccessToken
+	bt.token = token.Token
+	if bt.token == "" {
+		bt.token = token.AccessToken
 	}
 
 	if token.ExpiresIn < minimumTokenLifetimeSeconds {
@@ -167,8 +166,8 @@ func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) {
 	if token.IssuedAt.IsZero() {
 		token.IssuedAt = time.Now().UTC()
 	}
-	res.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second)
-	return res, nil
+	bt.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second)
+	return nil
 }
 
 // this is cloned from docker/go-connections because upstream docker has changed
@@ -712,20 +711,18 @@ func (c *dockerClient) obtainBearerToken(ctx context.Context, challenge challeng
 		token, inCache = c.tokenCache[cacheKey]
 	}()
 	if !inCache || time.Now().After(token.expirationTime) {
-		var (
-			t   *bearerToken
-			err error
-		)
+		token = &bearerToken{}
+
+		var err error
 		if c.auth.IdentityToken != "" {
-			t, err = c.getBearerTokenOAuth2(ctx, challenge, scopes)
+			err = c.getBearerTokenOAuth2(ctx, token, challenge, scopes)
 		} else {
-			t, err = c.getBearerToken(ctx, challenge, scopes)
+			err = c.getBearerToken(ctx, token, challenge, scopes)
 		}
 		if err != nil {
 			return "", err
 		}
 
-		token = t
 		func() { // A scope for defer
 			c.tokenCacheLock.Lock()
 			defer c.tokenCacheLock.Unlock()
@@ -735,16 +732,19 @@ func (c *dockerClient) obtainBearerToken(ctx context.Context, challenge challeng
 	return token.token, nil
 }
 
-func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge challenge,
-	scopes []authScope) (*bearerToken, error) {
+// getBearerTokenOAuth2 obtains an "Authorization: Bearer" token using a pre-existing identity token per
+// https://github.com/distribution/distribution/blob/main/docs/spec/auth/oauth.md for challenge and scopes,
+// and writes it into dest.
+func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, dest *bearerToken, challenge challenge,
+	scopes []authScope) error {
 	realm, ok := challenge.Parameters["realm"]
 	if !ok {
-		return nil, errors.New("missing realm in bearer auth challenge")
+		return errors.New("missing realm in bearer auth challenge")
 	}
 
 	authReq, err := http.NewRequestWithContext(ctx, http.MethodPost, realm, nil)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// Make the form data required against the oauth2 authentication
@@ -769,31 +769,34 @@ func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge chall
 	logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted())
 	res, err := c.client.Do(authReq)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	defer res.Body.Close()
 	if err := httpResponseToError(res, "Trying to obtain access token"); err != nil {
-		return nil, err
+		return err
 	}
 
 	tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	return newBearerTokenFromJSONBlob(tokenBlob)
+	return dest.readFromJSONBlob(tokenBlob)
 }
 
-func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge,
-	scopes []authScope) (*bearerToken, error) {
+// getBearerToken obtains an "Authorization: Bearer" token using a GET request, per
+// https://github.com/distribution/distribution/blob/main/docs/spec/auth/token.md for challenge and scopes,
+// and writes it into dest.
+func (c *dockerClient) getBearerToken(ctx context.Context, dest *bearerToken, challenge challenge,
+	scopes []authScope) error {
 	realm, ok := challenge.Parameters["realm"]
 	if !ok {
-		return nil, errors.New("missing realm in bearer auth challenge")
+		return errors.New("missing realm in bearer auth challenge")
 	}
 
 	authReq, err := http.NewRequestWithContext(ctx, http.MethodGet, realm, nil)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	params := authReq.URL.Query()
@@ -821,18 +824,18 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge,
 	logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted())
 	res, err := c.client.Do(authReq)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	defer res.Body.Close()
 	if err := httpResponseToError(res, "Requesting bearer token"); err != nil {
-		return nil, err
+		return err
 	}
 	tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	return newBearerTokenFromJSONBlob(tokenBlob)
+	return dest.readFromJSONBlob(tokenBlob)
 }
 
 // detectPropertiesHelper performs the work of detectProperties which executes

docker/docker_client_test.go

@@ -92,7 +92,7 @@ func TestDockerCertDir(t *testing.T) {
 	}
 }
 
-func TestNewBearerTokenFromJSONBlob(t *testing.T) {
+func TestBearerTokenReadFromJSONBlob(t *testing.T) {
 	for _, c := range []struct {
 		input    string
 		expected *bearerToken // or nil on failure
@@ -111,7 +111,8 @@ func TestNewBearerTokenFromJSONBlob(t *testing.T) {
 			&bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+60, 0)},
 		},
 	} {
-		token, err := newBearerTokenFromJSONBlob([]byte(c.input))
+		token := &bearerToken{}
+		err := token.readFromJSONBlob([]byte(c.input))
 		if c.expected == nil {
 			assert.Error(t, err, c.input)
 		} else {
@@ -123,11 +124,12 @@ func TestNewBearerTokenFromJSONBlob(t *testing.T) {
 	}
 }
 
-func TestNewBearerTokenIssuedAtZeroFromJsonBlob(t *testing.T) {
+func TestBearerTokenReadFromJSONBlobIssuedAtZeroFrom(t *testing.T) {
 	zeroTime := time.Time{}.Format(time.RFC3339)
 	now := time.Now()
 	tokenBlob := []byte(fmt.Sprintf(`{"token":"IAmAToken","expires_in":100,"issued_at":"%s"}`, zeroTime))
-	token, err := newBearerTokenFromJSONBlob(tokenBlob)
+	token := &bearerToken{}
+	err := token.readFromJSONBlob(tokenBlob)
 	require.NoError(t, err)
 	expectedExpiration := now.Add(time.Duration(100) * time.Second)
 	require.False(t, token.expirationTime.Before(expectedExpiration),