refactor: working through the todo's for 3.0

internal/services/apimanagement/schema.go

@@ -17,7 +17,7 @@ func apiManagementResourceHostnameSchema() map[string]*pluginsdk.Schema {
 		},
 
 		"key_vault_id": {
-			// TODO: should this become `key_vault_key_id` since that's what this is?
+			// TODO: 4.0 - should this become `key_vault_key_id` since that's what this is?
 			Type:         pluginsdk.TypeString,
 			Optional:     true,
 			ValidateFunc: keyVaultValidate.NestedItemIdWithOptionalVersion,

internal/services/appconfiguration/app_configuration_feature_resource.go

@@ -166,6 +166,9 @@ func (k FeatureResource) Create() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, model.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", model.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}
@@ -222,6 +225,10 @@ func (k FeatureResource) Read() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				// if the AppConfiguration is gone then all the data inside it is too
+				return metadata.MarkAsGone(resourceID)
+			}
 			if err != nil {
 				return err
 			}
@@ -290,6 +297,9 @@ func (k FeatureResource) Update() sdk.ResourceFunc {
 			featureKey := fmt.Sprintf("%s/%s", FeatureKeyPrefix, resourceID.Name)
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", resourceID.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}
@@ -327,6 +337,9 @@ func (k FeatureResource) Delete() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", resourceID.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}

internal/services/appconfiguration/app_configuration_feature_resource_test.go

@@ -137,6 +137,10 @@ func (t AppConfigurationFeatureResource) Exists(ctx context.Context, clients *cl
 	}
 
 	client, err := clients.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+	if client == nil {
+		// if the AppConfiguration is gone all the data is too
+		return utils.Bool(false), nil
+	}
 	if err != nil {
 		return nil, err
 	}

internal/services/appconfiguration/app_configuration_key_data_source.go

@@ -100,6 +100,9 @@ func (k KeyDataSource) Read() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, model.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("building data plane client: app configuration %q was not found", model.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}

internal/services/appconfiguration/app_configuration_key_resource.go

@@ -122,6 +122,9 @@ func (k KeyResource) Create() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, model.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", model.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}
@@ -202,6 +205,10 @@ func (k KeyResource) Read() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				// if the parent AppConfiguration is gone, all the data will be too
+				return metadata.MarkAsGone(resourceID)
+			}
 			if err != nil {
 				return err
 			}
@@ -267,6 +274,9 @@ func (k KeyResource) Update() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", resourceID.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}
@@ -326,6 +336,9 @@ func (k KeyResource) Delete() sdk.ResourceFunc {
 			}
 
 			client, err := metadata.Client.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+			if client == nil {
+				return fmt.Errorf("app configuration %q was not found", resourceID.ConfigurationStoreId)
+			}
 			if err != nil {
 				return err
 			}

internal/services/appconfiguration/app_configuration_key_resource_test.go

@@ -150,6 +150,10 @@ func (t AppConfigurationKeyResource) Exists(ctx context.Context, clients *client
 	}
 
 	client, err := clients.AppConfiguration.DataPlaneClient(ctx, resourceID.ConfigurationStoreId)
+	if client == nil {
+		// if the AppConfiguration is gone all the data will be too
+		return utils.Bool(false), nil
+	}
 	if err != nil {
 		return nil, err
 	}

internal/services/appconfiguration/client/client.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/hashicorp/go-azure-helpers/lang/response"
+
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/common"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/appconfiguration/sdk/1.0/appconfiguration"
@@ -25,7 +27,10 @@ func (c Client) DataPlaneClient(ctx context.Context, configurationStoreId string
 	// TODO: caching all of this
 	appConfig, err := c.ConfigurationStoresClient.Get(ctx, *appConfigId)
 	if err != nil {
-		// TODO: if not found etc
+		if response.WasNotFound(appConfig.HttpResponse) {
+			return nil, nil
+		}
+
 		return nil, err
 	}
 

internal/services/bot/bot_web_app_resource.go

@@ -191,7 +191,7 @@ func resourceBotWebAppCreate(d *pluginsdk.ResourceData, meta interface{}) error
 		return fmt.Errorf("creating Web App Bot %q (Resource Group %q): %+v", resourceId.Name, resourceId.ResourceGroup, err)
 	}
 
-	// TODO: in 3.0 we should remove the "Default Site" on the Directline resource at this point if we can
+	// TODO: in 4.0 we should remove the "Default Site" on the Directline resource at this point if we can
 
 	d.SetId(resourceId.ID())
 	return resourceBotWebAppRead(d, meta)

internal/services/cdn/cdn_endpoint_resource_test.go

@@ -169,7 +169,7 @@ func TestAccCdnEndpoint_fullFields(t *testing.T) {
 				check.That(data.ResourceName).Key("tags.environment").HasValue("Production"),
 			),
 		},
-		// TODO -- add import step. Import step now gives us an error complaining that `is_compression_enabled` is not imported
+		data.ImportStep(),
 	})
 }
 

internal/services/compute/managed_disk_resource_test.go

@@ -1257,10 +1257,6 @@ resource "azurerm_managed_disk" "import" {
 }
 
 func (ManagedDiskResource) diskEncryptionSetDependencies(data acceptance.TestData) string {
-	// whilst this is in Preview it's only supported in: West Central US, Canada Central, North Europe
-	// TODO: switch back to default location
-	location := "westus2"
-
 	return fmt.Sprintf(`
 provider "azurerm" {
   features {
@@ -1356,7 +1352,7 @@ resource "azurerm_role_assignment" "disk-encryption-read-keyvault" {
   role_definition_name = "Reader"
   principal_id         = azurerm_disk_encryption_set.test.identity.0.principal_id
 }
-`, data.RandomInteger, location, data.RandomString, data.RandomInteger)
+`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger)
 }
 
 func (r ManagedDiskResource) diskEncryptionSetEncrypted(data acceptance.TestData) string {

internal/services/compute/windows_virtual_machine_resource.go

@@ -28,8 +28,6 @@ import (
 	"github.com/hashicorp/terraform-provider-azurerm/utils"
 )
 
-// TODO: confirm locking as appropriate
-
 func resourceWindowsVirtualMachine() *pluginsdk.Resource {
 	return &pluginsdk.Resource{
 		Create: resourceWindowsVirtualMachineCreate,

internal/services/containers/container_registry_task_resource.go

@@ -551,7 +551,7 @@ func (r ContainerRegistryTaskResource) Arguments() map[string]*pluginsdk.Schema
 									ValidateFunc: validation.StringIsNotEmpty,
 								},
 								"identity": {
-									// TODO - 3.0: should this be `user_assigned_identity_id`?
+									// TODO - 4.0: this should be `user_assigned_identity_id`?
 									Type:         pluginsdk.TypeString,
 									Optional:     true,
 									ValidateFunc: validation.StringIsNotEmpty,

internal/services/containers/kubernetes_cluster_data_source.go

@@ -381,36 +381,7 @@ func dataSourceKubernetesCluster() *pluginsdk.Resource {
 				Computed: true,
 			},
 
-			"identity": func() *schema.Schema {
-				if !features.ThreePointOhBeta() {
-					return &schema.Schema{
-						Type:     pluginsdk.TypeList,
-						Computed: true,
-						Elem: &pluginsdk.Resource{
-							Schema: map[string]*pluginsdk.Schema{
-								"type": {
-									Type:     pluginsdk.TypeString,
-									Computed: true,
-								},
-								"user_assigned_identity_id": {
-									Type:     pluginsdk.TypeString,
-									Computed: true,
-								},
-								"principal_id": {
-									Type:     pluginsdk.TypeString,
-									Computed: true,
-								},
-								"tenant_id": {
-									Type:     pluginsdk.TypeString,
-									Computed: true,
-								},
-							},
-						},
-					}
-				}
-
-				return commonschema.SystemOrUserAssignedIdentityComputed()
-			}(),
+			"identity": commonschema.SystemOrUserAssignedIdentityComputed(),
 
 			"kubernetes_version": {
 				Type:     pluginsdk.TypeString,
@@ -928,7 +899,6 @@ func dataSourceKubernetesClusterRead(d *pluginsdk.ResourceData, meta interface{}
 		d.Set("kubernetes_version", props.KubernetesVersion)
 		d.Set("node_resource_group", props.NodeResourceGroup)
 
-		// TODO: 2.0 we should introduce a access_profile block to match the new API design,
 		if accessProfile := props.APIServerAccessProfile; accessProfile != nil {
 			apiServerAuthorizedIPRanges := utils.FlattenStringSlice(accessProfile.AuthorizedIPRanges)
 			if err := d.Set("api_server_authorized_ip_ranges", apiServerAuthorizedIPRanges); err != nil {
@@ -1778,44 +1748,6 @@ func flattenKubernetesClusterDataSourceKubeConfigAAD(config kubernetes.KubeConfi
 }
 
 func flattenClusterDataSourceIdentity(input *containerservice.ManagedClusterIdentity) (*[]interface{}, error) {
-	if !features.ThreePointOhBeta() {
-		// if it's none, omit the block
-		if input == nil || input.Type == containerservice.ResourceIdentityTypeNone {
-			return &[]interface{}{}, nil
-		}
-
-		identity := make(map[string]interface{})
-
-		identity["principal_id"] = ""
-		if input.PrincipalID != nil {
-			identity["principal_id"] = *input.PrincipalID
-		}
-
-		identity["tenant_id"] = ""
-		if input.TenantID != nil {
-			identity["tenant_id"] = *input.TenantID
-		}
-
-		identity["user_assigned_identity_id"] = ""
-		if input.UserAssignedIdentities != nil {
-			keys := []string{}
-			for key := range input.UserAssignedIdentities {
-				keys = append(keys, key)
-			}
-			if len(keys) > 0 {
-				parsedId, err := msiparse.UserAssignedIdentityIDInsensitively(keys[0])
-				if err != nil {
-					return nil, err
-				}
-				identity["user_assigned_identity_id"] = parsedId.ID()
-			}
-		}
-
-		identity["type"] = string(input.Type)
-
-		return &[]interface{}{identity}, nil
-	}
-
 	var transform *identity.SystemOrUserAssignedMap
 
 	if input != nil {

internal/services/containers/kubernetes_cluster_node_pool_data_source.go

@@ -159,7 +159,6 @@ func dataSourceKubernetesClusterNodePool() *pluginsdk.Resource {
 			}
 
 			if !features.ThreePointOhBeta() {
-				// TODO: document that this is replaced by `zones` in 3.0
 				s["availability_zones"] = &schema.Schema{
 					Type:     pluginsdk.TypeList,
 					Computed: true,