Address review comments

Signed-off-by: Dipti Pai <[email protected]>

auth/azure/token_provider.go → auth/azure/client.go

@@ -36,55 +36,27 @@ const (
 	AzureDevOpsRestApiScope = "499b84ac-1321-427f-aa17-267ca6975798/.default"
 )
 
-// Provider is an authentication provider for Azure.
-type Provider struct {
+// Client is an authentication provider for Azure.
+type Client struct {
 	credential azcore.TokenCredential
 	scopes     []string
 	proxyURL   *url.URL
 }
 
-// ProviderOptFunc enables specifying options for the provider.
-type ProviderOptFunc func(*Provider)
+// OptFunc enables specifying options for the provider.
+type OptFunc func(*Client)
 
-// NewProvider returns a new authentication provider for Azure.
-func NewProvider(opts ...ProviderOptFunc) *Provider {
-	p := &Provider{}
+// New returns a new authentication provider for Azure. It configures
+// credentials using a default credential chain with options.
+// https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#NewDefaultAzureCredential
+// The default scope is to ARM endpoint in Azure Cloud. The scope is overridden
+// using OptFunc.
+func New(opts ...OptFunc) (*Client, error) {
+	p := &Client{}
 	for _, opt := range opts {
 		opt(p)
 	}
-	return p
-}
 
-// WithCredential configures the credential to use to fetch the resource manager
-// token.
-func WithCredential(cred azcore.TokenCredential) ProviderOptFunc {
-	return func(p *Provider) {
-		p.credential = cred
-	}
-}
-
-// WithAzureDevOpsScope() configures the scope to access Azure DevOps Rest API
-// needed to access Azure DevOps Repositories
-func WithAzureDevOpsScope() ProviderOptFunc {
-	return func(p *Provider) {
-		p.scopes = []string{AzureDevOpsRestApiScope}
-	}
-}
-
-// WithProxyURL sets the proxy URL to use with NewDefaultAzureCredential.
-func WithProxyURL(proxyURL *url.URL) ProviderOptFunc {
-	return func(p *Provider) {
-		p.proxyURL = proxyURL
-	}
-}
-
-// GetToken requests an access token from Microsoft Entra ID by using a default
-// credential chain.
-// https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#NewDefaultAzureCredential
-// The default scope is to ARM endpoint in Azure Cloud. The scope is overridden
-// using ProviderOptFunc.
-func (p *Provider) GetToken(ctx context.Context) (azcore.AccessToken, error) {
-	var accessToken azcore.AccessToken
 	clientOpts := &azidentity.DefaultAzureCredentialOptions{}
 
 	if p.proxyURL != nil {
@@ -96,7 +68,7 @@ func (p *Provider) GetToken(ctx context.Context) (azcore.AccessToken, error) {
 	if p.credential == nil {
 		cred, err := azidentity.NewDefaultAzureCredential(clientOpts)
 		if err != nil {
-			return accessToken, err
+			return nil, err
 		}
 		p.credential = cred
 	}
@@ -105,6 +77,34 @@ func (p *Provider) GetToken(ctx context.Context) (azcore.AccessToken, error) {
 		p.scopes = []string{cloud.AzurePublic.Services[cloud.ResourceManager].Endpoint + "/" + ".default"}
 	}
 
+	return p, nil
+}
+
+// WithCredential configures the credential to use to fetch the resource manager
+// token.
+func WithCredential(cred azcore.TokenCredential) OptFunc {
+	return func(p *Client) {
+		p.credential = cred
+	}
+}
+
+// WithAzureDevOpsScope() configures the scope to access Azure DevOps Rest API
+// needed to access Azure DevOps Repositories
+func WithAzureDevOpsScope() OptFunc {
+	return func(p *Client) {
+		p.scopes = []string{AzureDevOpsRestApiScope}
+	}
+}
+
+// WithProxyURL sets the proxy URL to use with NewDefaultAzureCredential.
+func WithProxyURL(proxyURL *url.URL) OptFunc {
+	return func(p *Client) {
+		p.proxyURL = proxyURL
+	}
+}
+
+// GetToken
+func (p *Client) GetToken(ctx context.Context) (azcore.AccessToken, error) {
 	return p.credential.GetToken(ctx, policy.TokenRequestOptions{
 		Scopes: p.scopes,
 	})

auth/azure/token_provider_test.go → auth/azure/client_test.go

@@ -30,7 +30,7 @@ func TestGetProviderToken(t *testing.T) {
 	tests := []struct {
 		name      string
 		tokenCred azcore.TokenCredential
-		opts      []ProviderOptFunc
+		opts      []OptFunc
 		wantToken string
 		wantScope string
 		wantErr   error
@@ -40,7 +40,7 @@ func TestGetProviderToken(t *testing.T) {
 			tokenCred: &FakeTokenCredential{
 				Token: "foo",
 			},
-			opts:      []ProviderOptFunc{WithAzureDevOpsScope()},
+			opts:      []OptFunc{WithAzureDevOpsScope()},
 			wantScope: AzureDevOpsRestApiScope,
 			wantToken: "foo",
 		},
@@ -64,11 +64,12 @@ func TestGetProviderToken(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
-			provider := NewProvider(tt.opts...)
-			provider.credential = tt.tokenCred
+			tt.opts = append(tt.opts, WithCredential(tt.tokenCred))
+			client, err := New(tt.opts...)
+			g.Expect(err).ToNot(HaveOccurred())
 			str := ""
 			ctx := context.WithValue(context.TODO(), "scope", &str)
-			token, err := provider.GetToken(ctx)
+			token, err := client.GetToken(ctx)
 
 			if tt.wantErr != nil {
 				g.Expect(err).To(HaveOccurred())

git/credentials.go

@@ -31,7 +31,7 @@ const (
 // Credentials contains authentication data needed in order to access a Git
 // repository.
 type Credentials struct {
-	BearerToken string `json:"bearerToken,omitempty"`
+	BearerToken string
 }
 
 // GetCredentials returns authentication credentials for accessing the provided
@@ -50,18 +50,23 @@ func GetCredentials(ctx context.Context, url string, providerOpts *ProviderOptio
 	case ProviderAzure:
 		opts := providerOpts.AzureOpts
 		if providerOpts.AzureOpts == nil {
-			opts = []azure.ProviderOptFunc{
+			opts = []azure.OptFunc{
 				azure.WithAzureDevOpsScope(),
 			}
 		}
-		azureProvider := azure.NewProvider(opts...)
+		azureProvider, err := azure.New(opts...)
+		if err != nil {
+			return nil, expiresOn, err
+		}
 		accessToken, err := azureProvider.GetToken(ctx)
 		if err != nil {
 			return nil, expiresOn, err
 		}
+
 		creds = Credentials{
 			BearerToken: accessToken.Token,
 		}
+
 		return &creds, accessToken.ExpiresOn, nil
 	default:
 		return nil, expiresOn, fmt.Errorf("invalid provider")

git/credentials_test.go

@@ -40,7 +40,7 @@ func TestGetCredentials(t *testing.T) {
 			url:  "https://dev.azure.com/foo/bar/_git/baz",
 			provider: &ProviderOptions{
 				Name: ProviderAzure,
-				AzureOpts: []azure.ProviderOptFunc{
+				AzureOpts: []azure.OptFunc{
 					azure.WithCredential(&azure.FakeTokenCredential{
 						Token:     "ado-token",
 						ExpiresOn: expiresAt,
@@ -58,7 +58,7 @@ func TestGetCredentials(t *testing.T) {
 			url:  "https://dev.azure.com/foo/bar/_git/baz",
 			provider: &ProviderOptions{
 				Name: ProviderAzure,
-				AzureOpts: []azure.ProviderOptFunc{
+				AzureOpts: []azure.OptFunc{
 					azure.WithCredential(&azure.FakeTokenCredential{
 						Token:     "ado-token",
 						ExpiresOn: expiresAt,
@@ -71,7 +71,7 @@ func TestGetCredentials(t *testing.T) {
 			wantScope: cloud.AzurePublic.Services[cloud.ResourceManager].Endpoint + "/" + ".default",
 		},
 		{
-			name: "get credentials from azure",
+			name: "dummy provider",
 			url:  "https://dev.azure.com/foo/bar/_git/baz",
 			provider: &ProviderOptions{
 				Name: "dummy",

git/gogit/client.go

@@ -25,9 +25,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/fluxcd/pkg/auth/azure"
-	"github.com/fluxcd/pkg/git"
-	"github.com/fluxcd/pkg/git/repository"
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/go-git/go-billy/v5/osfs"
@@ -41,6 +38,10 @@ import (
 	"github.com/go-git/go-git/v5/storage"
 	"github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/go-git/go-git/v5/storage/memory"
+
+	"github.com/fluxcd/pkg/auth/azure"
+	"github.com/fluxcd/pkg/git"
+	"github.com/fluxcd/pkg/git/repository"
 )
 
 func init() {
@@ -253,7 +254,7 @@ func (g *Client) Clone(ctx context.Context, url string, cfg repository.CloneConf
 
 	if g.authOpts != nil && g.authOpts.ProviderOpts != nil && g.authOpts.BearerToken == "" {
 		if g.proxy.URL != "" {
-			proxyURL, err := g.getProxyURL(ctx)
+			proxyURL, err := g.proxy.FullURL()
 			if err != nil {
 				return nil, err
 			}
@@ -325,19 +326,6 @@ func (g *Client) validateUrl(u string) error {
 	return nil
 }
 
-// getProxyURL constructs the proxy URL from url address, username and password
-func (g *Client) getProxyURL(ctx context.Context) (*url.URL, error) {
-	proxyURL, err := url.Parse(g.proxy.URL)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse proxy address '%s': %w", g.proxy.URL, err)
-	}
-
-	if g.proxy.Username != "" || g.proxy.Password != "" {
-		proxyURL.User = url.UserPassword(g.proxy.Username, g.proxy.Password)
-	}
-	return proxyURL, nil
-}
-
 func (g *Client) writeFile(path string, reader io.Reader) error {
 	if g.repository == nil {
 		return git.ErrNoGitRepository

git/options.go

@@ -55,7 +55,7 @@ type AuthOptions struct {
 // providers.
 type ProviderOptions struct {
 	Name      string
-	AzureOpts []azure.ProviderOptFunc
+	AzureOpts []azure.OptFunc
 }
 
 // KexAlgos hosts the key exchange algorithms to be used for SSH connections.