Merge pull request #27178 from hashicorp/bugfix/provider-validation-f…

…or-subscription-id

bugfix: provider validation for `subscription_id`

internal/provider/framework/config.go

@@ -28,6 +28,12 @@ func (p *ProviderConfig) Load(ctx context.Context, data *ProviderModel, tfVersio
 	env := &environments.Environment{}
 	var err error
 
+	subscriptionId := getEnvStringOrDefault(data.SubscriptionId, "ARM_SUBSCRIPTION_ID", "")
+	if subscriptionId == "" {
+		diags.Append(diag.NewErrorDiagnostic("Configuring subscription", "`subscription_id` is a required provider property when performing a plan/apply operation"))
+		return
+	}
+
 	if metadataHost := getEnvStringOrDefault(data.MetaDataHost, "ARM_METADATA_HOSTNAME", ""); metadataHost != "" {
 		env, err = environments.FromEndpoint(ctx, metadataHost)
 		if err != nil {
@@ -98,7 +104,7 @@ func (p *ProviderConfig) Load(ctx context.Context, data *ProviderModel, tfVersio
 
 		CustomManagedIdentityEndpoint: getEnvStringOrDefault(data.MSIEndpoint, "ARM_MSI_ENDPOINT", ""),
 
-		AzureCliSubscriptionIDHint: getEnvStringOrDefault(data.SubscriptionId, "ARM_SUBSCRIPTION_ID", ""),
+		AzureCliSubscriptionIDHint: subscriptionId,
 
 		EnableAuthenticatingUsingClientCertificate: true,
 		EnableAuthenticatingUsingClientSecret:      true,
@@ -511,11 +517,11 @@ func (p *ProviderConfig) Load(ctx context.Context, data *ProviderModel, tfVersio
 		}
 	}
 
-	subscriptionId := commonids.NewSubscriptionID(client.Account.SubscriptionId)
+	subId := commonids.NewSubscriptionID(client.Account.SubscriptionId)
 	ctx2, cancel := context.WithTimeout(ctx, 30*time.Minute)
 	defer cancel()
 
-	if err = resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders); err != nil {
+	if err = resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subId, requiredResourceProviders); err != nil {
 		diags.AddError("registering resource providers", err.Error())
 		return
 	}

internal/provider/framework/provider.go

@@ -56,8 +56,7 @@ func (p *azureRmFrameworkProvider) Schema(_ context.Context, _ provider.SchemaRe
 			"subscription_id": schema.StringAttribute{
 				// Note: There is no equivalent of `DefaultFunc` in the provider schema package. This property is Required, but can be
 				// set via env var instead of provider config, so needs to be toggled in schema based on the presence of that env var.
-				Required:    getEnvStringOrDefault(types.StringUnknown(), "ARM_SUBSCRIPTION_ID", "") == "",
-				Optional:    getEnvStringOrDefault(types.StringUnknown(), "ARM_SUBSCRIPTION_ID", "") != "",
+				Optional:    true,
 				Description: "The Subscription ID which should be used.",
 			},
 

internal/provider/provider.go

@@ -143,7 +143,7 @@ func azureProvider(supportLegacyTestSuite bool) *schema.Provider {
 		Schema: map[string]*schema.Schema{
 			"subscription_id": {
 				Type:        schema.TypeString,
-				Required:    true,
+				Optional:    true,
 				DefaultFunc: schema.EnvDefaultFunc("ARM_SUBSCRIPTION_ID", nil),
 				Description: "The Subscription ID which should be used.",
 			},
@@ -385,6 +385,11 @@ func azureProvider(supportLegacyTestSuite bool) *schema.Provider {
 // This separation allows us to robustly test different authentication scenarios.
 func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc {
 	return func(ctx context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
+		subscriptionId := d.Get("subscription_id").(string)
+		if subscriptionId == "" {
+			return nil, diag.FromErr(fmt.Errorf("`subscription_id` is a required provider property when performing a plan/apply operation"))
+		}
+
 		var auxTenants []string
 		if v, ok := d.Get("auxiliary_tenant_ids").([]interface{}); ok && len(v) > 0 {
 			auxTenants = *utils.ExpandStringSlice(v)
@@ -467,7 +472,7 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc {
 
 			CustomManagedIdentityEndpoint: d.Get("msi_endpoint").(string),
 
-			AzureCliSubscriptionIDHint: d.Get("subscription_id").(string),
+			AzureCliSubscriptionIDHint: subscriptionId,
 
 			EnableAuthenticatingUsingClientCertificate: true,
 			EnableAuthenticatingUsingClientSecret:      true,

internal/provider/provider_test.go

@@ -177,45 +177,46 @@ func TestAccProvider_resourceProviders_deprecatedSkip(t *testing.T) {
 	}
 }
 
-// TODO - Test expected value needs updating, commenting out for now
-// func TestAccProvider_resourceProviders_legacyWithAdditional(t *testing.T) {
-//	if !features.FourPointOhBeta() {
-//		t.Skip("skipping 4.0 specific test")
-//	}
-//
-//	if os.Getenv("TF_ACC") == "" {
-//		t.Skip("TF_ACC not set")
-//	}
-//
-//	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
-//	defer cancel()
-//
-//	logging.SetOutput(t)
-//
-//	provider := TestAzureProvider()
-//	config := map[string]interface{}{
-//		"resource_providers_to_register": []interface{}{
-//			"Microsoft.ApiManagement",
-//			"Microsoft.ContainerService",
-//			"Microsoft.KeyVault",
-//			"Microsoft.Kubernetes",
-//		},
-//	}
-//
-//	if diags := provider.Configure(ctx, terraform.NewResourceConfigRaw(config)); diags != nil && diags.HasError() {
-//		t.Fatalf("provider failed to configure: %v", diags)
-//	}
-//
-//	expectedResourceProviders := resourceproviders.Legacy().Merge(resourceproviders.ResourceProviders{
-//		"Microsoft.ApiManagement": {},
-//		"Microsoft.KeyVault":      {},
-//	})
-//	registeredResourceProviders := provider.Meta().(*clients.Client).Account.RegisteredResourceProviders
-//
-//	if !reflect.DeepEqual(registeredResourceProviders, expectedResourceProviders) {
-//		t.Fatalf("unexpected value for RegisteredResourceProviders: %#v", registeredResourceProviders)
-//	}
-// }
+func TestAccProvider_resourceProviders_legacyWithAdditional(t *testing.T) {
+	if !features.FourPointOhBeta() {
+		t.Skip("skipping 4.0 specific test")
+	}
+
+	if os.Getenv("TF_ACC") == "" {
+		t.Skip("TF_ACC not set")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	logging.SetOutput(t)
+
+	provider := TestAzureProvider()
+	config := map[string]interface{}{
+		"resource_providers_to_register": []interface{}{
+			"Microsoft.ApiManagement",
+			"Microsoft.ContainerService",
+			"Microsoft.KeyVault",
+			"Microsoft.Kubernetes",
+		},
+	}
+
+	if diags := provider.Configure(ctx, terraform.NewResourceConfigRaw(config)); diags != nil && diags.HasError() {
+		t.Fatalf("provider failed to configure: %v", diags)
+	}
+
+	expectedResourceProviders := resourceproviders.Legacy().Merge(resourceproviders.ResourceProviders{
+		"Microsoft.ApiManagement":    {},
+		"Microsoft.ContainerService": {},
+		"Microsoft.KeyVault":         {},
+		"Microsoft.Kubernetes":       {},
+	})
+	registeredResourceProviders := provider.Meta().(*clients.Client).Account.RegisteredResourceProviders
+
+	if !reflect.DeepEqual(registeredResourceProviders, expectedResourceProviders) {
+		t.Fatalf("unexpected value for RegisteredResourceProviders: %#v", registeredResourceProviders)
+	}
+}
 
 func TestAccProvider_resourceProviders_core(t *testing.T) {
 	if !features.FourPointOhBeta() {

website/docs/index.html.markdown

@@ -103,14 +103,16 @@ The following arguments are supported:
 
 * `features` - (Required) A `features` block as defined below which can be used to customize the behaviour of certain Azure Provider resources.
 
+* `subscription_id` - (Required) The Subscription ID which should be used. This can also be sourced from the `ARM_SUBSCRIPTION_ID` Environment Variable.
+
+-> The `subscription_id` property is required when performing a plan or apply operation, but is not required to run `terraform validate`.
+
 * `client_id` - (Optional) The Client ID which should be used. This can also be sourced from the `ARM_CLIENT_ID` Environment Variable.
 
 * `client_id_file_path` (Optional) The path to a file containing the Client ID which should be used. This can also be sourced from the `ARM_CLIENT_ID_FILE_PATH` Environment Variable.
 
 * `environment` - (Optional) The Cloud Environment which should be used. Possible values are `public`, `usgovernment`, `german`, and `china`. Defaults to `public`. This can also be sourced from the `ARM_ENVIRONMENT` Environment Variable. Not used when `metadata_host` is specified.
 
-* `subscription_id` - (Optional) The Subscription ID which should be used. This can also be sourced from the `ARM_SUBSCRIPTION_ID` Environment Variable.
-
 * `tenant_id` - (Optional) The Tenant ID which should be used. This can also be sourced from the `ARM_TENANT_ID` Environment Variable.
 
 * `auxiliary_tenant_ids` - (Optional) List of auxiliary Tenant IDs required for multi-tenancy and cross-tenant scenarios. This can also be sourced from the `ARM_AUXILIARY_TENANT_IDS` Environment Variable.
@@ -203,10 +205,6 @@ For some advanced scenarios, such as where more granular permissions are necessa
 
 ~> **Note:** The Files Storage API does not support authenticating via AzureAD and will continue to use a SharedKey when AAD authentication is enabled.
 
-* `use_msal` - (Optional) When `true`, and when using service principal authentication, the provider will obtain [v2 authentication tokens](https://docs.microsoft.com/azure/active-directory/develop/access-tokens#token-formats-and-ownership) from the Microsoft Identity Platform. Has no effect when authenticating via Managed Identity or the Azure CLI. Can also be set via the `ARM_USE_MSAL` or `ARM_USE_MSGRAPH` environment variables.
-
--> **Note:** This will behaviour will be defaulted on in version 3.0 of the AzureRM (with no opt-out) due to [the deprecation of Azure Active Directory Graph](https://docs.microsoft.com/azure/active-directory/develop/msal-migration).
-
 It's also possible to use multiple Provider blocks within a single Terraform configuration, for example, to work with resources across multiple Subscriptions - more information can be found [in the documentation for Providers](https://www.terraform.io/docs/configuration/providers.html#multiple-provider-instances).
 
 ## Features
@@ -228,4 +226,4 @@ To view the latest specific Azure Resource Providers included in each set, pleas
 
 In addition to, or in place of, the sets described above, you can also configure the AzureRM Provider to register specific Azure Resource Providers, by setting the `resource_providers_to_register` provider property. This should be a list of strings, containing the exact names of Azure Resource Providers to register. For a list of all resource providers, please refer to [official Azure documentation](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-providers-and-types).
 
-->**Note on Permissions** The User, Service Principal or Managed Identity running Terraform should have permissions to register [Azure Resource Providers](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-providers-and-types). If the principal running Terraform has insufficient permissions to register Resource Providers then we recommend setting the property [`resource_provider_registrations`](#resource_provider_registrations) to `none` in the provider block to prevent auto-registration.
+-> **Note on Permissions** The User, Service Principal or Managed Identity running Terraform should have permissions to register [Azure Resource Providers](https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-providers-and-types). If the principal running Terraform has insufficient permissions to register Resource Providers then we recommend setting the property [`resource_provider_registrations`](#resource_provider_registrations) to `none` in the provider block to prevent auto-registration.