Don't use global credentials in low-level methods (#1309)

* Extract credentials fields into config.Credentials struct

These are now retrieved using config.GlobalCredentials(), which will
eventually be pushed further and further up the call stack, to make
implementing flexible auth easier.

* Remove GlobalCredentials usage from pkg/resourcemanager/iam

All functions that used the global credentials now require creds
passed in - all callers are updated to pass in the global credentials.

Eventually this will push all creds-getting up to the level of the
reconcile loop so that we can change it to get them from somewhere
else.

* Redefine config.Credentials as an interface for testing

This allows tests to pass their own credentials without having to set
the global ones.

Remove GlobalCredentials usage from:
* pkg/secrets/keyvault
* pkg/resourcemanager/*

* Eliminate global KeyVaultManager used in tests - now it's constructed
with credentials in the affected tests instead.

* Remove GlobalCredentials usage from pkg/resourcemanager/resourcegroups

Credentials are now passed in when constructing a manager.

Converted package level functions ListGroups, GetGroup,
DeleteAllGroupsWithPrefix and WaitForDeleteCompletion into methods on
AzureResourceGroupManager. There didn't seem to be a reason that they
were functions when they would now need credentials.
* Rename useMI to useManagedIdentity

MI is too cryptic.

* Incorporate code review comments

Thanks @matthchr!

controllers/async_controller.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager"
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	"github.com/Azure/azure-service-operator/pkg/secrets"
 	keyvaultsecretlib "github.com/Azure/azure-service-operator/pkg/secrets/keyvault"
 	telemetry "github.com/Azure/azure-service-operator/pkg/telemetry"
@@ -75,7 +76,7 @@ func (r *AsyncReconciler) Reconcile(req ctrl.Request, obj runtime.Object) (resul
 
 	if len(KeyVaultName) != 0 {
 		// Instantiate the KeyVault Secret Client
-		keyvaultSecretClient = keyvaultsecretlib.New(KeyVaultName)
+		keyvaultSecretClient = keyvaultsecretlib.New(KeyVaultName, config.GlobalCredentials())
 
 		r.Telemetry.LogInfoByInstance("status", "ensuring vault", req.String())
 

controllers/azuresql_combined_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	helpers "github.com/Azure/azure-service-operator/pkg/helpers"
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	kvsecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -300,7 +301,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 			EnsureInstance(ctx, t, tc, kvSqlUser1)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(keyVaultName)
+			keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 
 			assert.Eventually(func() bool {
 				keyNamespace := "azuresqluser-" + sqlServerName + "-" + sqlDatabaseName1
@@ -342,7 +343,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 			EnsureInstance(ctx, t, tc, kvSqlUser2)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(keyVaultName)
+			keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 
 			assert.Eventually(func() bool {
 				keyNamespace := "azuresqluser-" + sqlServerName + "-" + sqlDatabaseName1
@@ -362,7 +363,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 		key := types.NamespacedName{Name: kvSqlUser1.ObjectMeta.Name, Namespace: keyNamespace}
 
 		keyVaultName := tc.keyvaultName
-		keyVaultSecretClient := kvsecrets.New(keyVaultName)
+		keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 		var oldSecret, _ = keyVaultSecretClient.Get(ctx, key)
 
 		sqlActionName := GenerateTestResourceNameWithRandom("azuresqlaction-dev", 10)
@@ -409,7 +410,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 			EnsureDelete(ctx, t, tc, kvSqlUser2)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(tc.keyvaultName)
+			keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials())
 
 			assert.Eventually(func() bool {
 				key := types.NamespacedName{Name: sqlUser.ObjectMeta.Name, Namespace: sqlUser.ObjectMeta.Namespace}

controllers/eventhub_storageaccount_controller_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/Azure/azure-service-operator/pkg/errhelp"
 
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	kvhelper "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults"
 	kvsecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault"
 
@@ -127,7 +128,8 @@ func TestEventHubControllerCreateAndDeleteCustomKeyVault(t *testing.T) {
 	keyVaultNameForSecrets := tc.keyvaultName
 
 	// Instantiate a KV client for the Keyvault that was created during test suite setup
-	_, err := kvhelper.AzureKeyVaultManager.GetVault(ctx, rgName, keyVaultNameForSecrets)
+	kvManager := kvhelper.NewAzureKeyVaultManager(config.GlobalCredentials(), nil)
+	_, err := kvManager.GetVault(ctx, rgName, keyVaultNameForSecrets)
 	assert.Equal(nil, err, "wait for keyvault to be available")
 
 	// Create EventhubNamespace instance as prereq
@@ -170,7 +172,7 @@ func TestEventHubControllerCreateAndDeleteCustomKeyVault(t *testing.T) {
 	EnsureInstance(ctx, t, tc, eventhubInstance)
 
 	// Check that the secret is added to KeyVault
-	keyvaultSecretClient := kvsecrets.New(keyVaultNameForSecrets)
+	keyvaultSecretClient := kvsecrets.New(keyVaultNameForSecrets, config.GlobalCredentials())
 
 	EnsureSecrets(ctx, t, tc, eventhubInstance, keyvaultSecretClient, eventhubName, eventhubInstance.Namespace)
 

controllers/eventhubnamespace_controller_test.go

@@ -76,7 +76,7 @@ func TestEventHubNamespaceControllerNetworkRules(t *testing.T) {
 	EnsureInstance(ctx, t, tc, VNetInstance)
 
 	// Create EventhubNamespace network rule using the above VNET
-	subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName
+	subnetID := "/subscriptions/" + config.GlobalCredentials().SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName
 	vnetRules := []azurev1alpha1.VirtualNetworkRules{
 		{
 			SubnetID:                     subnetID,

controllers/keyvault_controller_test.go

@@ -75,8 +75,8 @@ func TestKeyvaultControllerWithAccessPolicies(t *testing.T) {
 	keyVaultLocation := tc.resourceGroupLocation
 	accessPolicies := []azurev1alpha1.AccessPolicyEntry{
 		{
-			TenantID: config.TenantID(),
-			ClientID: config.ClientID(),
+			TenantID: config.GlobalCredentials().TenantID(),
+			ClientID: config.GlobalCredentials().ClientID(),
 
 			Permissions: &azurev1alpha1.Permissions{
 				Keys: &[]string{
@@ -122,7 +122,7 @@ func TestKeyvaultControllerWithAccessPolicies(t *testing.T) {
 
 	//Add code to set secret and get secret from this keyvault using secretclient
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName)
+	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 	secretName := "test-key"
 	key := types.NamespacedName{Name: secretName, Namespace: "default"}
 	datanew := map[string][]byte{
@@ -163,8 +163,8 @@ func TestKeyvaultControllerWithLimitedAccessPoliciesAndUpdate(t *testing.T) {
 			ResourceGroup: tc.resourceGroupName,
 			AccessPolicies: &[]azurev1alpha1.AccessPolicyEntry{
 				{
-					TenantID: config.TenantID(),
-					ClientID: config.ClientID(),
+					TenantID: config.GlobalCredentials().TenantID(),
+					ClientID: config.GlobalCredentials().ClientID(),
 					Permissions: &azurev1alpha1.Permissions{
 						Secrets: &[]string{"backup"},
 					},
@@ -182,7 +182,7 @@ func TestKeyvaultControllerWithLimitedAccessPoliciesAndUpdate(t *testing.T) {
 	}, tc.timeout, tc.retry, "wait for keyVaultInstance to be ready in azure")
 	//Add code to set secret and get secret from this keyvault using secretclient
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName)
+	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 	key := types.NamespacedName{Name: "test-key", Namespace: "default"}
 	datanew := map[string][]byte{
 		"test1": []byte("test2"),
@@ -296,8 +296,8 @@ func TestKeyvaultControllerWithVirtualNetworkRulesAndUpdate(t *testing.T) {
 	keyVaultLocation := tc.resourceGroupLocation
 	accessPolicies := []azurev1alpha1.AccessPolicyEntry{
 		{
-			TenantID: config.TenantID(),
-			ClientID: config.ClientID(),
+			TenantID: config.GlobalCredentials().TenantID(),
+			ClientID: config.GlobalCredentials().ClientID(),
 
 			Permissions: &azurev1alpha1.Permissions{
 				Secrets: &[]string{
@@ -329,7 +329,7 @@ func TestKeyvaultControllerWithVirtualNetworkRulesAndUpdate(t *testing.T) {
 		return result.Response.StatusCode == http.StatusOK
 	}, tc.timeout, tc.retry, "wait for keyVaultInstance to be ready in azure")
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName)
+	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials())
 	secretName := "test-key"
 	key := types.NamespacedName{Name: secretName, Namespace: "default"}
 	datanew := map[string][]byte{

controllers/keyvaultkey_controller_test.go

@@ -36,8 +36,8 @@ func TestKeyvaultKeyControllerHappyPath(t *testing.T) {
 	keyPermissions := []string{"get", "list", "update", "delete", "recover", "backup", "restore", "create", "import"}
 	accessPolicies := []azurev1alpha1.AccessPolicyEntry{
 		{
-			TenantID: config.TenantID(),
-			ClientID: config.ClientID(),
+			TenantID: config.GlobalCredentials().TenantID(),
+			ClientID: config.GlobalCredentials().ClientID(),
 			Permissions: &azurev1alpha1.Permissions{
 				Keys: &keyPermissions,
 			},
@@ -86,7 +86,7 @@ func TestKeyvaultKeyControllerHappyPath(t *testing.T) {
 	// create key
 	EnsureInstance(ctx, t, tc, keyVaultKey)
 
-	kvopsclient := resourcemanagerkeyvaults.NewOpsClient(keyVaultName)
+	kvopsclient := resourcemanagerkeyvaults.NewOpsClient(config.GlobalCredentials(), keyVaultName)
 
 	assert.Eventually(func() bool {
 		kvault, err := tc.keyVaultManager.GetVault(ctx, tc.resourceGroupName, keyVaultInstance.Name)