Merge pull request #25717 from hashicorp/remove-legacy-auth

Remove legacy authentication helpers

internal/acceptance/data.go

@@ -11,7 +11,6 @@ import (
 	"strconv"
 	"testing"
 
-	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/features"
 )
 
@@ -40,18 +39,14 @@ type TestData struct {
 	// RandomString is a random 5 character string is unique to this test case
 	RandomString string
 
-	// ResourceName is the fully qualified resource name, comprising of the
+	// ResourceName is the fully qualified resource name, comprising the
 	// resource type and then the resource label
 	// e.g. `azurerm_resource_group.test`
 	ResourceName string
 
 	// ResourceType is the Terraform Resource Type - `azurerm_resource_group`
 	ResourceType string
 
-	// Environment is a struct containing Details about the Azure Environment
-	// that we're running against
-	Environment azure.Environment
-
 	// EnvironmentName is the name of the Azure Environment where we're running
 	EnvironmentName string
 
@@ -64,16 +59,10 @@ type TestData struct {
 
 // BuildTestData generates some test data for the given resource
 func BuildTestData(t *testing.T, resourceType string, resourceLabel string) TestData {
-	env, err := Environment()
-	if err != nil {
-		t.Fatalf("Error retrieving Environment: %+v", err)
-	}
-
 	testData := TestData{
 		RandomInteger:   RandTimeInt(),
 		RandomString:    randString(5),
 		ResourceName:    fmt.Sprintf("%s.%s", resourceType, resourceLabel),
-		Environment:     *env,
 		EnvironmentName: EnvironmentName(),
 		MetadataURL:     os.Getenv("ARM_METADATA_HOSTNAME"),
 

internal/acceptance/testing.go

@@ -10,8 +10,6 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/Azure/go-autorest/autorest/azure"
-	"github.com/hashicorp/go-azure-helpers/authentication"
 	"github.com/hashicorp/go-azure-sdk/sdk/auth"
 	"github.com/hashicorp/go-azure-sdk/sdk/environments"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -45,12 +43,6 @@ func EnvironmentName() string {
 	return envName
 }
 
-func Environment() (*azure.Environment, error) {
-	envName := EnvironmentName()
-	metadataURL := os.Getenv("ARM_METADATA_HOSTNAME")
-	return authentication.AzureEnvironmentByNameFromEndpoint(context.TODO(), metadataURL, envName)
-}
-
 func GetAuthConfig(t *testing.T) *auth.Credentials {
 	if os.Getenv(resource.EnvTfAcc) == "" {
 		t.Skipf("Acceptance test skipped unless env '%s' set", resource.EnvTfAcc)

internal/clients/auth.go

@@ -10,7 +10,6 @@ import (
 	"net/http"
 	"strings"
 
-	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/hashicorp/go-azure-sdk/sdk/auth"
 	"github.com/hashicorp/go-azure-sdk/sdk/claims"
 	"github.com/hashicorp/go-azure-sdk/sdk/environments"
@@ -27,12 +26,9 @@ type ResourceManagerAccount struct {
 
 	AuthenticatedAsAServicePrincipal bool
 	SkipResourceProviderRegistration bool
-
-	// TODO: delete these when no longer needed by older clients
-	AzureEnvironment azure.Environment
 }
 
-func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, subscriptionId string, skipResourceProviderRegistration bool, azureEnvironment azure.Environment) (*ResourceManagerAccount, error) {
+func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, subscriptionId string, skipResourceProviderRegistration bool) (*ResourceManagerAccount, error) {
 	authorizer, err := auth.NewAuthorizerFromCredentials(ctx, config, config.Environment.MicrosoftGraph)
 	if err != nil {
 		return nil, fmt.Errorf("unable to build authorizer for Microsoft Graph API: %+v", err)
@@ -44,23 +40,23 @@ func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, sub
 		return nil, fmt.Errorf("could not acquire access token to parse claims: %+v", err)
 	}
 
-	claims, err := claims.ParseClaims(token)
+	tokenClaims, err := claims.ParseClaims(token)
 	if err != nil {
 		return nil, fmt.Errorf("parsing claims from access token: %+v", err)
 	}
 
 	authenticatedAsServicePrincipal := true
-	if strings.Contains(strings.ToLower(claims.Scopes), "openid") {
+	if strings.Contains(strings.ToLower(tokenClaims.Scopes), "openid") {
 		authenticatedAsServicePrincipal = false
 	}
 
-	clientId := claims.AppId
+	clientId := tokenClaims.AppId
 	if clientId == "" {
 		log.Printf("[DEBUG] Using user-supplied ClientID because the `appid` claim was missing from the access token")
 		clientId = config.ClientID
 	}
 
-	objectId := claims.ObjectId
+	objectId := tokenClaims.ObjectId
 	if objectId == "" {
 		if authenticatedAsServicePrincipal {
 			log.Printf("[DEBUG] Querying Microsoft Graph to discover authenticated service principal object ID because the `oid` claim was missing from the access token")
@@ -81,7 +77,7 @@ func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, sub
 		}
 	}
 
-	tenantId := claims.TenantId
+	tenantId := tokenClaims.TenantId
 	if tenantId == "" {
 		log.Printf("[DEBUG] Using user-supplied TenantID because the `tid` claim was missing from the access token")
 		tenantId = config.TenantID
@@ -136,9 +132,6 @@ func NewResourceManagerAccount(ctx context.Context, config auth.Credentials, sub
 
 		AuthenticatedAsAServicePrincipal: authenticatedAsServicePrincipal,
 		SkipResourceProviderRegistration: skipResourceProviderRegistration,
-
-		// TODO: delete these when no longer needed by older clients
-		AzureEnvironment: azureEnvironment,
 	}
 
 	return &account, nil

internal/clients/builder.go

@@ -9,7 +9,6 @@ import (
 	"log"
 	"time"
 
-	"github.com/hashicorp/go-azure-helpers/authentication"
 	"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
 	"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
 	"github.com/hashicorp/go-azure-sdk/sdk/auth"
@@ -98,14 +97,7 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) {
 		return authorizer, nil
 	})
 
-	// TODO: remove these when autorest clients are no longer used
-	azureEnvironment, err := authentication.AzureEnvironmentByNameFromEndpoint(ctx, builder.MetadataHost, builder.AuthConfig.Environment.Name)
-	if err != nil {
-		return nil, fmt.Errorf("unable to find environment %q from endpoint %q: %+v", builder.AuthConfig.Environment.Name, builder.MetadataHost, err)
-	}
-	resourceManagerEndpoint, _ := builder.AuthConfig.Environment.ResourceManager.Endpoint()
-
-	account, err := NewResourceManagerAccount(ctx, *builder.AuthConfig, builder.SubscriptionID, builder.SkipProviderRegistration, *azureEnvironment)
+	account, err := NewResourceManagerAccount(ctx, *builder.AuthConfig, builder.SubscriptionID, builder.SkipProviderRegistration)
 	if err != nil {
 		return nil, fmt.Errorf("building account: %+v", err)
 	}
@@ -120,6 +112,11 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) {
 		log.Printf("[DEBUG] Skipping building the Managed HSM Authorizer since this is not supported in the current Azure Environment")
 	}
 
+	resourceManagerEndpoint, ok := builder.AuthConfig.Environment.ResourceManager.Endpoint()
+	if !ok {
+		return nil, fmt.Errorf("unable to determine resource manager endpoint for the current environment")
+	}
+
 	client := Client{
 		Account: account,
 	}
@@ -156,8 +153,6 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) {
 		SkipProviderReg:             builder.SkipProviderRegistration,
 		StorageUseAzureAD:           builder.StorageUseAzureAD,
 
-		// TODO: remove when `Azure/go-autorest` is no longer used
-		AzureEnvironment:        *azureEnvironment,
 		ResourceManagerEndpoint: *resourceManagerEndpoint,
 	}
 

internal/common/client_options.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 
 	"github.com/Azure/go-autorest/autorest"
-	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/hashicorp/go-azure-helpers/sender"
 	"github.com/hashicorp/go-azure-sdk/sdk/auth"
 	"github.com/hashicorp/go-azure-sdk/sdk/client"
@@ -51,8 +50,6 @@ type ClientOptions struct {
 	SkipProviderReg           bool
 	StorageUseAzureAD         bool
 
-	// Keep these around for convenience with Autorest based clients, remove when we are no longer using autorest
-	AzureEnvironment        azure.Environment
 	ResourceManagerEndpoint string
 
 	// Legacy authorizers for go-autorest

internal/services/storage/migration/share.go

@@ -26,7 +26,7 @@ func (ShareV0ToV1) Schema() map[string]*pluginsdk.Schema {
 
 func (ShareV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc {
 	// this should have been applied from pre-0.12 migration system; backporting just in-case
-	return func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	return func(ctx context.Context, rawState map[string]interface{}, _ interface{}) (map[string]interface{}, error) {
 		shareName := rawState["name"].(string)
 		resourceGroup := rawState["resource_group_name"].(string)
 		accountName := rawState["storage_account_name"].(string)

internal/services/storage/migration/share_test.go

@@ -9,55 +9,38 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/hashicorp/go-azure-sdk/sdk/environments"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
 	storageClient "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/client"
 )
 
 func TestShareV0ToV1(t *testing.T) {
-	clouds := []azure.Environment{
-		azure.ChinaCloud,
-		azure.PublicCloud,
-		azure.USGovernmentCloud,
+	input := map[string]interface{}{
+		"id":                   "share1",
+		"name":                 "share1",
+		"resource_group_name":  "group1",
+		"storage_account_name": "account1",
+		"quota":                5120,
 	}
 
-	for _, cloud := range clouds {
-		t.Logf("[DEBUG] Testing with Cloud %q", cloud.Name)
-
-		input := map[string]interface{}{
-			"id":                   "share1",
-			"name":                 "share1",
-			"resource_group_name":  "group1",
-			"storage_account_name": "account1",
-			"quota":                5120,
-		}
-
-		meta := &clients.Client{
-			Account: &clients.ResourceManagerAccount{
-				AzureEnvironment: cloud,
-			},
-		}
-
-		expected := map[string]interface{}{
-			"id":                   "share1/group1/account1",
-			"name":                 "share1",
-			"resource_group_name":  "group1",
-			"storage_account_name": "account1",
-			"quota":                5120,
-		}
-
-		actual, err := ShareV0ToV1{}.UpgradeFunc()(context.TODO(), input, meta)
-		if err != nil {
-			t.Fatalf("Expected no error but got: %s", err)
-		}
+	expected := map[string]interface{}{
+		"id":                   "share1/group1/account1",
+		"name":                 "share1",
+		"resource_group_name":  "group1",
+		"storage_account_name": "account1",
+		"quota":                5120,
+	}
 
-		if !reflect.DeepEqual(expected, actual) {
-			t.Fatalf("Expected %+v. Got %+v. But expected them to be the same", expected, actual)
-		}
+	actual, err := ShareV0ToV1{}.UpgradeFunc()(context.TODO(), input, &clients.Client{})
+	if err != nil {
+		t.Fatalf("Expected no error but got: %s", err)
+	}
 
-		t.Logf("[DEBUG] Ok!")
+	if !reflect.DeepEqual(expected, actual) {
+		t.Fatalf("Expected %+v. Got %+v. But expected them to be the same", expected, actual)
 	}
+
+	t.Logf("[DEBUG] Ok!")
 }
 
 func TestShareV1ToV2(t *testing.T) {