Merge pull request #776 from fluxcd/cache-authn

[OCI] Cache Login credentials

oci/auth/aws/auth.go

@@ -24,6 +24,7 @@ import (
 	"regexp"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
@@ -78,11 +79,7 @@ func (c *Client) WithConfig(cfg *aws.Config) {
 // be the case if it's running in EKS, and may need additional setup
 // otherwise (visit https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/
 // as a starting point).
-func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.AuthConfig, error) {
-	// No caching of tokens is attempted; the quota for getting an
-	// auth token is high enough that getting a token every time you
-	// scan an image is viable for O(500) images per region. See
-	// https://docs.aws.amazon.com/general/latest/gr/ecr.html.
+func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.AuthConfig, time.Time, error) {
 	var authConfig authn.AuthConfig
 	var cfg aws.Config
 
@@ -94,7 +91,7 @@ func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.A
 		cfg, err = config.LoadDefaultConfig(ctx, config.WithRegion(awsEcrRegion))
 		if err != nil {
 			c.mu.Unlock()
-			return authConfig, fmt.Errorf("failed to load default configuration: %w", err)
+			return authConfig, time.Time{}, fmt.Errorf("failed to load default configuration: %w", err)
 		}
 		c.config = &cfg
 	}
@@ -105,61 +102,74 @@ func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.A
 	// pass nil input.
 	ecrToken, err := ecrService.GetAuthorizationToken(ctx, nil)
 	if err != nil {
-		return authConfig, err
+		return authConfig, time.Time{}, err
 	}
 
 	// Validate the authorization data.
 	if len(ecrToken.AuthorizationData) == 0 {
-		return authConfig, errors.New("no authorization data")
+		return authConfig, time.Time{}, errors.New("no authorization data")
 	}
 	if ecrToken.AuthorizationData[0].AuthorizationToken == nil {
-		return authConfig, fmt.Errorf("no authorization token")
+		return authConfig, time.Time{}, fmt.Errorf("no authorization token")
 	}
 	token, err := base64.StdEncoding.DecodeString(*ecrToken.AuthorizationData[0].AuthorizationToken)
 	if err != nil {
-		return authConfig, err
+		return authConfig, time.Time{}, err
 	}
 
 	tokenSplit := strings.Split(string(token), ":")
 	// Validate the tokens.
 	if len(tokenSplit) != 2 {
-		return authConfig, fmt.Errorf("invalid authorization token, expected the token to have two parts separated by ':', got %d parts", len(tokenSplit))
+		return authConfig, time.Time{}, fmt.Errorf("invalid authorization token, expected the token to have two parts separated by ':', got %d parts", len(tokenSplit))
 	}
 	authConfig = authn.AuthConfig{
 		Username: tokenSplit[0],
 		Password: tokenSplit[1],
 	}
-	return authConfig, nil
+	expiresAt := ecrToken.AuthorizationData[0].ExpiresAt
+	if expiresAt == nil {
+		expiresAt = &time.Time{}
+	}
+	return authConfig, *expiresAt, nil
 }
 
-// Login attempts to get the authentication material for ECR.
-func (c *Client) Login(ctx context.Context, autoLogin bool, image string) (authn.Authenticator, error) {
+// LoginWithExpiry attempts to get the authentication material for ECR.
+// It returns the authentication material and the expiry time of the token.
+func (c *Client) LoginWithExpiry(ctx context.Context, autoLogin bool, image string) (authn.Authenticator, time.Time, error) {
 	if autoLogin {
 		log.FromContext(ctx).Info("logging in to AWS ECR for " + image)
 		_, awsEcrRegion, ok := ParseRegistry(image)
 		if !ok {
-			return nil, errors.New("failed to parse AWS ECR image, invalid ECR image")
+			return nil, time.Time{}, errors.New("failed to parse AWS ECR image, invalid ECR image")
 		}
 
-		authConfig, err := c.getLoginAuth(ctx, awsEcrRegion)
+		authConfig, expiresAt, err := c.getLoginAuth(ctx, awsEcrRegion)
 		if err != nil {
-			return nil, err
+			return nil, time.Time{}, err
 		}
 
 		auth := authn.FromConfig(authConfig)
-		return auth, nil
+		return auth, expiresAt, nil
 	}
-	return nil, fmt.Errorf("ECR authentication failed: %w", oci.ErrUnconfiguredProvider)
+	return nil, time.Time{}, fmt.Errorf("ECR authentication failed: %w", oci.ErrUnconfiguredProvider)
+}
+
+// Login attempts to get the authentication material for ECR.
+func (c *Client) Login(ctx context.Context, autoLogin bool, image string) (authn.Authenticator, error) {
+	auth, _, err := c.LoginWithExpiry(ctx, autoLogin, image)
+	return auth, err
 }
 
 // OIDCLogin attempts to get the authentication material for ECR.
+//
+// Deprecated: Use LoginWithExpiry instead.
 func (c *Client) OIDCLogin(ctx context.Context, registryURL string) (authn.Authenticator, error) {
 	_, awsEcrRegion, ok := ParseRegistry(registryURL)
 	if !ok {
 		return nil, errors.New("failed to parse AWS ECR image, invalid ECR image")
 	}
 
-	authConfig, err := c.getLoginAuth(ctx, awsEcrRegion)
+	authConfig, _, err := c.getLoginAuth(ctx, awsEcrRegion)
 	if err != nil {
 		return nil, err
 	}

oci/auth/aws/auth_test.go

@@ -18,9 +18,11 @@ package aws
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/credentials"
@@ -99,6 +101,7 @@ func TestParseRegistry(t *testing.T) {
 }
 
 func TestGetLoginAuth(t *testing.T) {
+	authorizationData := fmt.Sprintf(`{"authorizationData": [{"authorizationToken": "c29tZS1rZXk6c29tZS1zZWNyZXQ=","expiresAt": %d}]}`, time.Now().Add(1*time.Hour).Unix())
 	tests := []struct {
 		name           string
 		responseBody   []byte
@@ -108,15 +111,9 @@ func TestGetLoginAuth(t *testing.T) {
 	}{
 		{
 			// NOTE: The authorizationToken is base64 encoded.
-			name: "success",
-			responseBody: []byte(`{
-	"authorizationData": [
-		{
-			"authorizationToken": "c29tZS1rZXk6c29tZS1zZWNyZXQ="
-		}
-	]
-}`),
-			statusCode: http.StatusOK,
+			name:         "success",
+			responseBody: []byte(authorizationData),
+			statusCode:   http.StatusOK,
 			wantAuthConfig: authn.AuthConfig{
 				Username: "some-key",
 				Password: "some-secret",
@@ -183,8 +180,11 @@ func TestGetLoginAuth(t *testing.T) {
 			cfg.Credentials = credentials.NewStaticCredentialsProvider("x", "y", "z")
 			ec.WithConfig(cfg)
 
-			a, err := ec.getLoginAuth(context.TODO(), "us-east-1")
+			a, expiresAt, err := ec.getLoginAuth(context.TODO(), "us-east-1")
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+			if !tt.wantErr {
+				g.Expect(expiresAt).To(BeTemporally("~", time.Now().Add(1*time.Hour), time.Second))
+			}
 			if tt.statusCode == http.StatusOK {
 				g.Expect(a).To(Equal(tt.wantAuthConfig))
 			}

oci/auth/azure/auth.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	_ "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
@@ -33,6 +34,11 @@ import (
 	"github.com/fluxcd/pkg/oci"
 )
 
+// Default cache expiration time in seconds for ACR refresh token.
+// TODO @souleb: This is copied from https://github.com/Azure/msi-acrpull/blob/0ca921a7740e561c7204d9c3b3b55c4e0b9bd7b9/pkg/authorizer/token_retriever.go#L21C2-L21C39
+// as it is not provided by the Azure SDK. Check with the Azure SDK team to see if there is a better way to get this value.
+const defaultCacheExpirationInSeconds = 600
+
 // Client is an Azure ACR client which can log into the registry and return
 // authorization information.
 type Client struct {
@@ -60,7 +66,7 @@ func (c *Client) WithScheme(scheme string) *Client {
 // getLoginAuth returns authentication for ACR. The details needed for authentication
 // are gotten from environment variable so there is no need to mount a host path.
 // The endpoint is the registry server and will be queried for OAuth authorization token.
-func (c *Client) getLoginAuth(ctx context.Context, registryURL string) (authn.AuthConfig, error) {
+func (c *Client) getLoginAuth(ctx context.Context, registryURL string) (authn.AuthConfig, time.Time, error) {
 	var authConfig authn.AuthConfig
 
 	// Use default credentials if no token credential is provided.
@@ -69,7 +75,7 @@ func (c *Client) getLoginAuth(ctx context.Context, registryURL string) (authn.Au
 	if c.credential == nil {
 		cred, err := azidentity.NewDefaultAzureCredential(nil)
 		if err != nil {
-			return authConfig, err
+			return authConfig, time.Time{}, err
 		}
 		c.credential = cred
 	}
@@ -80,22 +86,24 @@ func (c *Client) getLoginAuth(ctx context.Context, registryURL string) (authn.Au
 		Scopes: []string{configurationEnvironment.Services[cloud.ResourceManager].Endpoint + "/" + ".default"},
 	})
 	if err != nil {
-		return authConfig, err
+		return authConfig, time.Time{}, err
 	}
 
 	// Obtain ACR access token using exchanger.
 	ex := newExchanger(registryURL)
 	accessToken, err := ex.ExchangeACRAccessToken(string(armToken.Token))
 	if err != nil {
-		return authConfig, fmt.Errorf("error exchanging token: %w", err)
+		return authConfig, time.Time{}, fmt.Errorf("error exchanging token: %w", err)
 	}
 
+	expiresAt := time.Now().Add(defaultCacheExpirationInSeconds * time.Second)
+
 	return authn.AuthConfig{
 		// This is the acr username used by Azure
 		// See documentation: https://docs.microsoft.com/en-us/azure/container-registry/container-registry-authentication?tabs=azure-cli#az-acr-login-with---expose-token
 		Username: "00000000-0000-0000-0000-000000000000",
 		Password: accessToken,
-	}, nil
+	}, expiresAt, nil
 }
 
 // getCloudConfiguration returns the cloud configuration based on the registry URL.
@@ -122,32 +130,42 @@ func ValidHost(host string) bool {
 	return false
 }
 
-// Login attempts to get the authentication material for ACR. The caller can
-// ensure that the passed image is a valid ACR image using ValidHost().
-func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, error) {
+// LoginWithExpiry attempts to get the authentication material for ACR.
+// It returns the authentication material and the expiry time of the token.
+// The caller can ensure that the passed image is a valid ACR image using ValidHost().
+func (c *Client) LoginWithExpiry(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, time.Time, error) {
 	if autoLogin {
 		log.FromContext(ctx).Info("logging in to Azure ACR for " + image)
 		// get registry host from image
 		strArr := strings.SplitN(image, "/", 2)
 		endpoint := fmt.Sprintf("%s://%s", c.scheme, strArr[0])
-		authConfig, err := c.getLoginAuth(ctx, endpoint)
+		authConfig, expiresAt, err := c.getLoginAuth(ctx, endpoint)
 		if err != nil {
 			log.FromContext(ctx).Info("error logging into ACR " + err.Error())
-			return nil, err
+			return nil, time.Time{}, err
 		}
 
 		auth := authn.FromConfig(authConfig)
-		return auth, nil
+		return auth, expiresAt, nil
 	}
-	return nil, fmt.Errorf("ACR authentication failed: %w", oci.ErrUnconfiguredProvider)
+	return nil, time.Time{}, fmt.Errorf("ACR authentication failed: %w", oci.ErrUnconfiguredProvider)
+}
+
+// Login attempts to get the authentication material for ACR. The caller can
+// ensure that the passed image is a valid ACR image using ValidHost().
+func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, error) {
+	auth, _, err := c.LoginWithExpiry(ctx, autoLogin, image, ref)
+	return auth, err
 }
 
 // OIDCLogin attempts to get an Authenticator for the provided ACR registry URL endpoint.
 //
 // If you want to construct an Authenticator based on an image reference,
 // you may want to use Login instead.
+//
+// Deprecated: Use LoginWithExpiry instead.
 func (c *Client) OIDCLogin(ctx context.Context, registryUrl string) (authn.Authenticator, error) {
-	authConfig, err := c.getLoginAuth(ctx, registryUrl)
+	authConfig, _, err := c.getLoginAuth(ctx, registryUrl)
 	if err != nil {
 		log.FromContext(ctx).Info("error logging into ACR " + err.Error())
 		return nil, err

oci/auth/azure/auth_test.go

@@ -24,6 +24,7 @@ import (
 	"net/url"
 	"path"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
@@ -84,8 +85,11 @@ func TestGetAzureLoginAuth(t *testing.T) {
 				WithTokenCredential(tt.tokenCredential).
 				WithScheme("http")
 
-			auth, err := c.getLoginAuth(context.TODO(), srv.URL)
+			auth, expiresAt, err := c.getLoginAuth(context.TODO(), srv.URL)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+			if !tt.wantErr {
+				g.Expect(expiresAt).To(BeTemporally("~", time.Now().Add(defaultCacheExpirationInSeconds*time.Second), time.Second))
+			}
 			if tt.statusCode == http.StatusOK {
 				g.Expect(auth).To(Equal(tt.wantAuthConfig))
 			}