Merge pull request #842 from terraform-providers/auth-fixes

Pulling the Environment from the Azure CLI config
hashicorp · Feb 15, 2018 · 401c8a3 · 401c8a3
2 parents d3a12d3 + d42caca
commit 401c8a3
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 94 deletions.
diff --git a/azurerm/helpers/authentication/config.go b/azurerm/helpers/authentication/config.go
@@ -54,12 +54,12 @@ func (c *Config) LoadTokensFromAzureCLI() error {
 		}
 	}
 
-	// find the Tenant ID and Environment for that subscription if they're not specified
-	if c.TenantID == "" || c.Environment == "" {
-		err := c.populateTenantAndEnvironmentFromCLIProfile(cliProfile)
+	// find the Tenant ID for that subscription if they're not specified
+	if c.TenantID == "" {
+		err := c.populateTenantFromCLIProfile(cliProfile)
 		if err != nil {
 			// we want to expose a more friendly error to the user, but this is useful for debug purposes
-			log.Printf("Error Populating the Tenant and Environment from the CLI Profile: %s", err)
+			log.Printf("Error Populating the Tenant from the CLI Profile: %s", err)
 		}
 	}
 
@@ -89,6 +89,13 @@ func (c *Config) LoadTokensFromAzureCLI() error {
 		return fmt.Errorf("No valid (unexpired) Azure CLI Auth Tokens found. Please run `az login`.")
 	}
 
+	// always pull the Environment from the CLI
+	err = c.populateEnvironmentFromCLIProfile(cliProfile)
+	if err != nil {
+		// we want to expose a more friendly error to the user, but this is useful for debug purposes
+		log.Printf("Error Populating the Environment from the CLI Profile: %s", err)
+	}
+
 	return nil
 }
 
@@ -102,7 +109,7 @@ func (c *Config) populateSubscriptionFromCLIProfile(cliProfile AzureCLIProfile)
 	return nil
 }
 
-func (c *Config) populateTenantAndEnvironmentFromCLIProfile(cliProfile AzureCLIProfile) error {
+func (c *Config) populateTenantFromCLIProfile(cliProfile AzureCLIProfile) error {
 	subscription, err := cliProfile.FindSubscription(c.SubscriptionID)
 	if err != nil {
 		return err
@@ -112,10 +119,17 @@ func (c *Config) populateTenantAndEnvironmentFromCLIProfile(cliProfile AzureCLIP
 		c.TenantID = subscription.TenantID
 	}
 
-	if c.Environment == "" {
-		c.Environment = normalizeEnvironmentName(subscription.EnvironmentName)
+	return nil
+}
+
+func (c *Config) populateEnvironmentFromCLIProfile(cliProfile AzureCLIProfile) error {
+	subscription, err := cliProfile.FindSubscription(c.SubscriptionID)
+	if err != nil {
+		return err
 	}
 
+	c.Environment = normalizeEnvironmentName(subscription.EnvironmentName)
+
 	return nil
 }
 

diff --git a/azurerm/helpers/authentication/config_test.go b/azurerm/helpers/authentication/config_test.go
@@ -64,21 +64,21 @@ func TestAzurePopulateSubscriptionFromCLIProfile_Default(t *testing.T) {
 	}
 }
 
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Empty(t *testing.T) {
+func TestAzurePopulateTenantFromCLIProfile_Empty(t *testing.T) {
 	config := Config{}
 	profiles := AzureCLIProfile{
 		Profile: cli.Profile{
 			Subscriptions: []cli.Subscription{},
 		},
 	}
 
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
+	err := config.populateEnvironmentFromCLIProfile(profiles)
 	if err == nil {
 		t.Fatalf("Expected an error to be returned - but didn't get one")
 	}
 }
 
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription(t *testing.T) {
+func TestAzurePopulateTenantFromCLIProfile_MissingSubscription(t *testing.T) {
 	config := Config{
 		SubscriptionID: "bcd234",
 	}
@@ -93,85 +93,15 @@ func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription(t *
 		},
 	}
 
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
+	err := config.populateTenantFromCLIProfile(profiles)
 	if err == nil {
 		t.Fatalf("Expected an error to be returned - but didn't get one")
 	}
 }
 
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateEnvironment(t *testing.T) {
+func TestAzurePopulateTenantFromCLIProfile_PopulateTenantId(t *testing.T) {
 	config := Config{
 		SubscriptionID: "abc123",
-		TenantID:       "bcd234",
-	}
-	profiles := AzureCLIProfile{
-		Profile: cli.Profile{
-			Subscriptions: []cli.Subscription{
-				{
-					IsDefault:       false,
-					ID:              "abc123",
-					EnvironmentName: "public",
-				},
-			},
-		},
-	}
-
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
-	if err != nil {
-		t.Fatalf("Expected no error to be returned - but got: %+v", err)
-	}
-
-	if config.SubscriptionID != "abc123" {
-		t.Fatalf("Expected Subscription ID to be 'abc123' - got %q", config.SubscriptionID)
-	}
-
-	if config.TenantID != "bcd234" {
-		t.Fatalf("Expected Tenant ID to be 'bcd234' - got %q", config.TenantID)
-	}
-
-	if config.Environment != "public" {
-		t.Fatalf("Expected Tenant ID to be 'public' - got %q", config.Environment)
-	}
-}
-
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_NormaliseAndPopulateEnvironment(t *testing.T) {
-	config := Config{
-		SubscriptionID: "abc123",
-		TenantID:       "bcd234",
-	}
-	profiles := AzureCLIProfile{
-		Profile: cli.Profile{
-			Subscriptions: []cli.Subscription{
-				{
-					IsDefault: false,
-					ID:        "abc123",
-				},
-			},
-		},
-	}
-
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
-	if err != nil {
-		t.Fatalf("Expected no error to be returned - but got: %+v", err)
-	}
-
-	if config.SubscriptionID != "abc123" {
-		t.Fatalf("Expected Subscription ID to be 'abc123' - got %q", config.SubscriptionID)
-	}
-
-	if config.TenantID != "bcd234" {
-		t.Fatalf("Expected Tenant ID to be 'bcd234' - got %q", config.TenantID)
-	}
-
-	if config.Environment != "public" {
-		t.Fatalf("Expected Tenant ID to be 'public' - got %q", config.Environment)
-	}
-}
-
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId(t *testing.T) {
-	config := Config{
-		SubscriptionID: "abc123",
-		Environment:    "public",
 	}
 	profiles := AzureCLIProfile{
 		Profile: cli.Profile{
@@ -185,7 +115,7 @@ func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId(t *tes
 		},
 	}
 
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
+	err := config.populateTenantFromCLIProfile(profiles)
 	if err != nil {
 		t.Fatalf("Expected no error to be returned - but got: %+v", err)
 	}
@@ -197,17 +127,12 @@ func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId(t *tes
 	if config.TenantID != "bcd234" {
 		t.Fatalf("Expected Tenant ID to be 'bcd234' - got %q", config.TenantID)
 	}
-
-	if config.Environment != "public" {
-		t.Fatalf("Expected Tenant ID to be 'public' - got %q", config.Environment)
-	}
 }
 
-func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete(t *testing.T) {
+func TestAzurePopulateTenantFromCLIProfile_Complete(t *testing.T) {
 	config := Config{
 		SubscriptionID: "abc123",
 		TenantID:       "bcd234",
-		Environment:    "public",
 	}
 	profiles := AzureCLIProfile{
 		Profile: cli.Profile{
@@ -220,7 +145,7 @@ func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete(t *testing.T)
 		},
 	}
 
-	err := config.populateTenantAndEnvironmentFromCLIProfile(profiles)
+	err := config.populateTenantFromCLIProfile(profiles)
 	if err != nil {
 		t.Fatalf("Expected no error to be returned - but got: %+v", err)
 	}
@@ -232,10 +157,6 @@ func TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete(t *testing.T)
 	if config.TenantID != "bcd234" {
 		t.Fatalf("Expected Tenant ID to be 'bcd234' - got %q", config.TenantID)
 	}
-
-	if config.Environment != "public" {
-		t.Fatalf("Expected Tenant ID to be 'public' - got %q", config.Environment)
-	}
 }
 
 func TestAzurePopulateFromAccessToken_Missing(t *testing.T) {