Rename TokenCredential variables to indicate credential used (#2950)

In most cases this will be one of: fpTokenCredential : ARO first-party application spTokenCredential : The cluster's AAD application msiTokenCredential : Managed service identity for the ARO-RP VM
Azure · Jun 26, 2023 · c262800 · c262800
1 parent 000562d
commit c262800
Show file tree

Hide file tree

Showing 13 changed files with 41 additions and 43 deletions.
diff --git a/pkg/api/validate/dynamic/dynamic.go b/pkg/api/validate/dynamic/dynamic.go
@@ -60,7 +60,7 @@ type Subnet struct {
 }
 
 type ServicePrincipalValidator interface {
-	ValidateServicePrincipal(ctx context.Context, tokenCredential azcore.TokenCredential) error
+	ValidateServicePrincipal(ctx context.Context, spTokenCredential azcore.TokenCredential) error
 }
 
 // Dynamic validate in the operator context.

diff --git a/pkg/api/validate/dynamic/serviceprincipal.go b/pkg/api/validate/dynamic/serviceprincipal.go
@@ -15,13 +15,13 @@ import (
 	"github.com/Azure/ARO-RP/pkg/util/azureclaim"
 )
 
-func (dv *dynamic) ValidateServicePrincipal(ctx context.Context, tokenCredential azcore.TokenCredential) error {
+func (dv *dynamic) ValidateServicePrincipal(ctx context.Context, spTokenCredential azcore.TokenCredential) error {
 	dv.log.Print("ValidateServicePrincipal")
 
 	tokenRequestOptions := policy.TokenRequestOptions{
 		Scopes: []string{dv.azEnv.MicrosoftGraphScope},
 	}
-	token, err := tokenCredential.GetToken(ctx, tokenRequestOptions)
+	token, err := spTokenCredential.GetToken(ctx, tokenRequestOptions)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/api/validate/openshiftcluster_validatedynamic.go b/pkg/api/validate/openshiftcluster_validatedynamic.go
@@ -57,7 +57,7 @@ type openShiftClusterDynamicValidator struct {
 	fpAuthorizer    autorest.Authorizer
 }
 
-func ensureAccessTokenClaims(ctx context.Context, tokenCredential *azidentity.ClientSecretCredential, scopes []string) error {
+func ensureAccessTokenClaims(ctx context.Context, spTokenCredential azcore.TokenCredential, scopes []string) error {
 	var err error
 
 	timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Minute)
@@ -68,7 +68,7 @@ func ensureAccessTokenClaims(ctx context.Context, tokenCredential *azidentity.Cl
 	// latest error to the user in case the wait exceeds the timeout.
 	_ = wait.PollImmediateUntil(10*time.Second, func() (bool, error) {
 		options := policy.TokenRequestOptions{Scopes: scopes}
-		token, err := tokenCredential.GetToken(ctx, options)
+		token, err := spTokenCredential.GetToken(ctx, options)
 		if err != nil {
 			return false, err
 		}
@@ -201,18 +201,18 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
 
 	tenantID := dv.subscriptionDoc.Subscription.Properties.TenantID
 	options := dv.env.Environment().ClientSecretCredentialOptions()
-	tokenCredential, err := azidentity.NewClientSecretCredential(
+	spTokenCredential, err := azidentity.NewClientSecretCredential(
 		tenantID, spp.ClientID, string(spp.ClientSecret), options)
 	if err != nil {
 		return err
 	}
 
 	scopes := []string{dv.env.Environment().ResourceManagerScope}
-	err = ensureAccessTokenClaims(ctx, tokenCredential, scopes)
+	err = ensureAccessTokenClaims(ctx, spTokenCredential, scopes)
 	if err != nil {
 		return err
 	}
-	spAuthorizer := azidext.NewTokenCredentialAdapter(tokenCredential, scopes)
+	spAuthorizer := azidext.NewTokenCredentialAdapter(spTokenCredential, scopes)
 
 	spDynamic := dynamic.NewValidator(
 		dv.log,
@@ -227,7 +227,7 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
 	)
 
 	// SP validation
-	err = spDynamic.ValidateServicePrincipal(ctx, tokenCredential)
+	err = spDynamic.ValidateServicePrincipal(ctx, spTokenCredential)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/cluster/serviceprincipal.go b/pkg/cluster/serviceprincipal.go
@@ -19,14 +19,14 @@ import (
 func (m *manager) initializeClusterSPClients(ctx context.Context) error {
 	spp := m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile
 	options := m.env.Environment().ClientSecretCredentialOptions()
-	tokenCredential, err := azidentity.NewClientSecretCredential(
+	spTokenCredential, err := azidentity.NewClientSecretCredential(
 		m.subscriptionDoc.Subscription.Properties.TenantID,
 		spp.ClientID, string(spp.ClientSecret), options)
 	if err != nil {
 		return err
 	}
 
-	m.spGraphClient, err = m.env.Environment().NewGraphServiceClient(tokenCredential)
+	m.spGraphClient, err = m.env.Environment().NewGraphServiceClient(spTokenCredential)
 
 	return err
 }

diff --git a/pkg/env/armhelper.go b/pkg/env/armhelper.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	mgmtauthorization "github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2018-09-01-preview/authorization"
 	"github.com/Azure/go-autorest/autorest"
@@ -74,7 +73,6 @@ func newARMHelper(ctx context.Context, log *logrus.Entry, env Interface) (ARMHel
 		return &noopARMHelper{}, nil
 	}
 
-	var tokenCredential azcore.TokenCredential
 	var err error
 
 	key, certs, err := env.ServiceKeyvault().GetCertificateSecret(ctx, RPDevARMSecretName)
@@ -83,21 +81,21 @@ func newARMHelper(ctx context.Context, log *logrus.Entry, env Interface) (ARMHel
 	}
 
 	options := env.Environment().ClientCertificateCredentialOptions()
-	tokenCredential, err = azidentity.NewClientCertificateCredential(env.TenantID(), os.Getenv("AZURE_ARM_CLIENT_ID"), certs, key, options)
+	armHelperTokenCredential, err := azidentity.NewClientCertificateCredential(env.TenantID(), os.Getenv("AZURE_ARM_CLIENT_ID"), certs, key, options)
 	if err != nil {
 		return nil, err
 	}
 
 	scopes := []string{env.Environment().ResourceManagerScope}
-	armAuthorizer := azidext.NewTokenCredentialAdapter(tokenCredential, scopes)
+	armHelperAuthorizer := azidext.NewTokenCredentialAdapter(armHelperTokenCredential, scopes)
 
 	// Graph service client uses the first party service principal.
-	tokenCredential, err = env.FPNewClientCertificateCredential(env.TenantID())
+	fpTokenCredential, err := env.FPNewClientCertificateCredential(env.TenantID())
 	if err != nil {
 		return nil, err
 	}
 
-	fpGraphClient, err := env.Environment().NewGraphServiceClient(tokenCredential)
+	fpGraphClient, err := env.Environment().NewGraphServiceClient(fpTokenCredential)
 	if err != nil {
 		return nil, err
 	}
@@ -107,7 +105,7 @@ func newARMHelper(ctx context.Context, log *logrus.Entry, env Interface) (ARMHel
 		env: env,
 
 		fpGraphClient:   fpGraphClient,
-		roleassignments: authorization.NewRoleAssignmentsClient(env.Environment(), env.SubscriptionID(), armAuthorizer),
+		roleassignments: authorization.NewRoleAssignmentsClient(env.Environment(), env.SubscriptionID(), armHelperAuthorizer),
 	}, nil
 }
 

diff --git a/pkg/env/dev.go b/pkg/env/dev.go
@@ -77,12 +77,12 @@ func (d *dev) Listen() (net.Listener, error) {
 }
 
 func (d *dev) FPAuthorizer(tenantID string, scopes ...string) (autorest.Authorizer, error) {
-	tokenCredential, err := d.FPNewClientCertificateCredential(tenantID)
+	fpTokenCredential, err := d.FPNewClientCertificateCredential(tenantID)
 	if err != nil {
 		return nil, err
 	}
 
-	return azidext.NewTokenCredentialAdapter(tokenCredential, scopes), nil
+	return azidext.NewTokenCredentialAdapter(fpTokenCredential, scopes), nil
 }
 
 func (d *dev) FPNewClientCertificateCredential(tenantID string) (*azidentity.ClientCertificateCredential, error) {

diff --git a/pkg/env/msiauthorizer.go b/pkg/env/msiauthorizer.go
@@ -22,12 +22,12 @@ const (
 func (c *core) NewMSIAuthorizer(msiContext MSIContext, scopes ...string) (autorest.Authorizer, error) {
 	if !c.IsLocalDevelopmentMode() {
 		options := c.Environment().ManagedIdentityCredentialOptions()
-		tokenCredential, err := azidentity.NewManagedIdentityCredential(options)
+		msiTokenCredential, err := azidentity.NewManagedIdentityCredential(options)
 		if err != nil {
 			return nil, err
 		}
 
-		return azidext.NewTokenCredentialAdapter(tokenCredential, scopes), nil
+		return azidext.NewTokenCredentialAdapter(msiTokenCredential, scopes), nil
 	}
 
 	tenantIdKey := "AZURE_TENANT_ID"
@@ -44,10 +44,10 @@ func (c *core) NewMSIAuthorizer(msiContext MSIContext, scopes ...string) (autore
 
 	options := c.Environment().ClientSecretCredentialOptions()
 
-	tokenCredential, err := azidentity.NewClientSecretCredential(tenantId, azureClientId, azureClientSecret, options)
+	clientSecretCredential, err := azidentity.NewClientSecretCredential(tenantId, azureClientId, azureClientSecret, options)
 	if err != nil {
 		return nil, err
 	}
 
-	return azidext.NewTokenCredentialAdapter(tokenCredential, scopes), nil
+	return azidext.NewTokenCredentialAdapter(clientSecretCredential, scopes), nil
 }
diff --git a/pkg/env/prod.go b/pkg/env/prod.go
@@ -315,12 +315,12 @@ func (p *prod) FeatureIsSet(f Feature) bool {
 }
 
 func (p *prod) FPAuthorizer(tenantID string, scopes ...string) (autorest.Authorizer, error) {
-	tokenCredential, err := p.FPNewClientCertificateCredential(tenantID)
+	fpTokenCredential, err := p.FPNewClientCertificateCredential(tenantID)
 	if err != nil {
 		return nil, err
 	}
 
-	return azidext.NewTokenCredentialAdapter(tokenCredential, scopes), nil
+	return azidext.NewTokenCredentialAdapter(fpTokenCredential, scopes), nil
 }
 
 func (p *prod) FPClientID() string {

diff --git a/pkg/operator/controllers/checkers/serviceprincipalchecker/checker.go b/pkg/operator/controllers/checkers/serviceprincipalchecker/checker.go
@@ -54,10 +54,10 @@ func (r *checker) Check(ctx context.Context, AZEnvironment string) error {
 
 	spDynamic := r.newSPValidator(&azEnv)
 
-	tokenCredential, err := r.getTokenCredential(&azEnv, azCred)
+	spTokenCredential, err := r.getTokenCredential(&azEnv, azCred)
 	if err != nil {
 		return err
 	}
 
-	return spDynamic.ValidateServicePrincipal(ctx, tokenCredential)
+	return spDynamic.ValidateServicePrincipal(ctx, spTokenCredential)
 }
diff --git a/pkg/util/cluster/cluster.go b/pkg/util/cluster/cluster.go
@@ -91,18 +91,18 @@ func New(log *logrus.Entry, environment env.Core, ci bool) (*Cluster, error) {
 	}
 
 	options := environment.Environment().EnvironmentCredentialOptions()
-	tokenCredential, err := azidentity.NewEnvironmentCredential(options)
+	spTokenCredential, err := azidentity.NewEnvironmentCredential(options)
 	if err != nil {
 		return nil, err
 	}
 
-	spGraphClient, err := environment.Environment().NewGraphServiceClient(tokenCredential)
+	spGraphClient, err := environment.Environment().NewGraphServiceClient(spTokenCredential)
 	if err != nil {
 		return nil, err
 	}
 
 	scopes := []string{environment.Environment().ResourceManagerScope}
-	authorizer := azidext.NewTokenCredentialAdapter(tokenCredential, scopes)
+	authorizer := azidext.NewTokenCredentialAdapter(spTokenCredential, scopes)
 
 	c := &Cluster{
 		log: log,

diff --git a/pkg/util/instancemetadata/prod.go b/pkg/util/instancemetadata/prod.go
@@ -45,15 +45,15 @@ func newProd(ctx context.Context) (InstanceMetadata, error) {
 
 func (p *prod) populateTenantIDFromMSI(ctx context.Context) error {
 	options := p.Environment().ManagedIdentityCredentialOptions()
-	tokenCredential, err := azidentity.NewManagedIdentityCredential(options)
+	msiTokenCredential, err := azidentity.NewManagedIdentityCredential(options)
 	if err != nil {
 		return err
 	}
 
 	tokenRequestOptions := policy.TokenRequestOptions{
 		Scopes: []string{p.Environment().ResourceManagerScope},
 	}
-	token, err := tokenCredential.GetToken(ctx, tokenRequestOptions)
+	token, err := msiTokenCredential.GetToken(ctx, tokenRequestOptions)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/util/mocks/dynamic/dynamic.go b/pkg/util/mocks/dynamic/dynamic.go
diff --git a/pkg/util/purge/purge.go b/pkg/util/purge/purge.go
@@ -37,13 +37,13 @@ type ResourceCleaner struct {
 // NewResourceCleaner instantiates the new RC object
 func NewResourceCleaner(log *logrus.Entry, env env.Core, shouldDelete checkFn, dryRun bool) (*ResourceCleaner, error) {
 	options := env.Environment().EnvironmentCredentialOptions()
-	tokenCredential, err := azidentity.NewEnvironmentCredential(options)
+	spTokenCredential, err := azidentity.NewEnvironmentCredential(options)
 	if err != nil {
 		return nil, err
 	}
 
 	scopes := []string{env.Environment().ResourceManagerScope}
-	authorizer := azidext.NewTokenCredentialAdapter(tokenCredential, scopes)
+	authorizer := azidext.NewTokenCredentialAdapter(spTokenCredential, scopes)
 
 	return &ResourceCleaner{
 		log:    log,