3.0: adding missing import validators

internal/provider/provider_test.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"fmt"
+	"log"
 	"testing"
 	"time"
 )
@@ -89,3 +90,10 @@ func TestResourcesSupportCustomTimeouts(t *testing.T) {
 func TestProvider_impl(t *testing.T) {
 	_ = AzureProvider()
 }
+
+func TestProvider_counts(t *testing.T) {
+	// @tombuildsstuff: this is less a unit test and more a useful placeholder tbh
+	provider := TestAzureProvider()
+	log.Printf("Data Sources: %d", len(provider.DataSourcesMap))
+	log.Printf("Resources: %d", len(provider.ResourcesMap))
+}

internal/provider/services_test.go

@@ -31,3 +31,51 @@ func TestTypedResourcesContainValidModelObjects(t *testing.T) {
 		}
 	}
 }
+
+func TestTypedResourcesContainValidIDParsers(t *testing.T) {
+	// This test confirms that all of the Typed Resources return an ID Validation method
+	// which is used to ensure that each of the resources will validate the Resource ID
+	// during import time. Whilst this may seem unnecessary as it's an interface method
+	// since we could return nil, this test is double-checking.
+	//
+	// Untyped Resources are checked via TestUntypedResourcesContainImporters
+	for _, service := range SupportedTypedServices() {
+		t.Logf("Service %q..", service.Name())
+		for _, resource := range service.Resources() {
+			t.Logf("- Resource %q..", resource.ResourceType())
+			obj := resource.IDValidationFunc()
+			if obj == nil {
+				t.Fatalf("IDValidationFunc returns nil - all resources must return an ID Validation function")
+			}
+		}
+	}
+}
+
+func TestUntypedResourcesContainImporters(t *testing.T) {
+	// Typed Resources are checked via TestTypedResourcesContainValidIDParsers
+	// as if an ID Parser is returned it's automatically used (and it's a required
+	// method on the sdk.Resource interface)
+	for _, service := range SupportedUntypedServices() {
+		deprecatedResourcesWhichDontSupportImport := map[string]struct{}{
+			// @tombuildsstuff: new resources shouldn't be added to this list - instead add an Import function
+			// to the resource, for example:
+			//
+			//		Importer: pluginsdk.ImporterValidatingResourceId(func(id string) error {
+			//			_, err := ParseTheId(id)
+			//			return err
+			//		})
+			"azurerm_security_center_server_vulnerability_assessment": {},
+			"azurerm_template_deployment":                             {},
+		}
+		for k, v := range service.SupportedResources() {
+			if _, ok := deprecatedResourcesWhichDontSupportImport[k]; ok {
+				t.Logf("the resource %q doesn't support import but it's deprecated so we're skipping..", k)
+				continue
+			}
+
+			if v.Importer == nil {
+				t.Fatalf("all resources must support import, however the resource %q does not support import", k)
+			}
+		}
+	}
+}

internal/services/keyvault/key_vault_access_policy_resource.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse"
 
 	"github.com/Azure/azure-sdk-for-go/services/preview/keyvault/mgmt/2020-04-01-preview/keyvault"
 	"github.com/gofrs/uuid"
@@ -28,10 +28,10 @@ func resourceKeyVaultAccessPolicy() *pluginsdk.Resource {
 		Update: resourceKeyVaultAccessPolicyUpdate,
 		Delete: resourceKeyVaultAccessPolicyDelete,
 
-		// TODO: switch below to using an ID Parser and then replace this with an importer which validates the ID during import
-		Importer: &schema.ResourceImporter{
-			StateContext: schema.ImportStatePassthroughContext,
-		},
+		Importer: pluginsdk.ImporterValidatingResourceId(func(id string) error {
+			_, err := parse.AccessPolicyID(id)
+			return err
+		}),
 
 		Timeouts: &pluginsdk.ResourceTimeout{
 			Create: pluginsdk.DefaultTimeout(30 * time.Minute),
@@ -86,54 +86,40 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta
 	defer cancel()
 	log.Printf("[INFO] Preparing arguments for Key Vault Access Policy: %s.", action)
 
-	vaultId := d.Get("key_vault_id").(string)
+	vaultId, err := parse.VaultID(d.Get("key_vault_id").(string))
+	if err != nil {
+		return err
+	}
 
 	tenantIdRaw := d.Get("tenant_id").(string)
 	tenantId, err := uuid.FromString(tenantIdRaw)
 	if err != nil {
 		return fmt.Errorf("parsing Tenant ID %q as a UUID: %+v", tenantIdRaw, err)
 	}
 
-	applicationIdRaw := d.Get("application_id").(string)
 	objectId := d.Get("object_id").(string)
+	applicationIdRaw := d.Get("application_id").(string)
 
-	id, err := azure.ParseAzureResourceID(vaultId)
-	if err != nil {
-		return err
-	}
-
-	resourceGroup := id.ResourceGroup
-	vaultName, ok := id.Path["vaults"]
-	if !ok {
-		return fmt.Errorf("key_value_id does not contain `vaults`: %q", vaultId)
-	}
+	id := parse.NewAccessPolicyId(*vaultId, objectId, applicationIdRaw)
 
-	keyVault, err := client.Get(ctx, resourceGroup, vaultName)
+	keyVault, err := client.Get(ctx, vaultId.ResourceGroup, vaultId.Name)
 	if err != nil {
 		// If the key vault does not exist but this is not a new resource, the policy
 		// which previously existed was deleted with the key vault, so reflect that in
 		// state. If this is a new resource and key vault does not exist, it's likely
 		// a bad ID was given.
 		if utils.ResponseWasNotFound(keyVault.Response) && !d.IsNewResource() {
-			log.Printf("[DEBUG] Parent Key Vault %q was not found in Resource Group %q - removing from state!", vaultName, resourceGroup)
+			log.Printf("[DEBUG] Parent %s was not found - removing from state!", *vaultId)
 			d.SetId("")
 			return nil
 		}
 
-		return fmt.Errorf("retrieving Key Vault %q (Resource Group %q): %+v", vaultName, resourceGroup, err)
-	}
-
-	// This is because azure doesn't have an 'id' for a keyvault access policy
-	// In order to compensate for this and allow importing of this resource we are artificially
-	// creating an identity for a key vault policy object
-	resourceId := fmt.Sprintf("%s/objectId/%s", *keyVault.ID, objectId)
-	if applicationIdRaw != "" {
-		resourceId = fmt.Sprintf("%s/applicationId/%s", resourceId, applicationIdRaw)
+		return fmt.Errorf("retrieving parent %s: %+v", *vaultId, err)
 	}
 
 	// Locking to prevent parallel changes causing issues
-	locks.ByName(vaultName, keyVaultResourceName)
-	defer locks.UnlockByName(vaultName, keyVaultResourceName)
+	locks.ByName(vaultId.Name, keyVaultResourceName)
+	defer locks.UnlockByName(vaultId.Name, keyVaultResourceName)
 
 	if d.IsNewResource() {
 		props := keyVault.Properties
@@ -159,7 +145,7 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta
 			}
 			applicationIdMatches := appId == applicationIdRaw
 			if tenantIdMatches && objectIdMatches && applicationIdMatches {
-				return tf.ImportAsExistsError("azurerm_key_vault_access_policy", resourceId)
+				return tf.ImportAsExistsError("azurerm_key_vault_access_policy", id.ID())
 			}
 		}
 	}
@@ -169,23 +155,23 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta
 	case keyvault.Remove:
 		// To remove a policy correctly, we need to send it with all permissions in the correct case which may have drifted
 		// in config over time so we read it back from the vault by objectId
-		resp, err := client.Get(ctx, id.ResourceGroup, vaultName)
+		resp, err := client.Get(ctx, vaultId.ResourceGroup, vaultId.Name)
 		if err != nil {
 			if utils.ResponseWasNotFound(resp.Response) {
-				log.Printf("[ERROR] Key Vault %q (Resource Group %q) was not found - removing from state", vaultName, id.ResourceGroup)
+				log.Printf("[DEBUG] parent %s was not found - removing from state", *vaultId)
 				d.SetId("")
 				return nil
 			}
-			return fmt.Errorf("making Read request on Azure KeyVault %q (Resource Group %q): %+v", vaultName, id.ResourceGroup, err)
+			return fmt.Errorf("retrieving parent %s: %+v", vaultId, err)
 		}
 
 		if resp.Properties == nil || resp.Properties.AccessPolicies == nil {
-			return fmt.Errorf("failed reading Access Policies for %q (resource group %q)", vaultName, id.ResourceGroup)
+			return fmt.Errorf("retrieving parent %s: `accessPolicies` was nil", *vaultId)
 		}
 
 		accessPolicyRaw := FindKeyVaultAccessPolicy(resp.Properties.AccessPolicies, objectId, applicationIdRaw)
 		if accessPolicyRaw == nil {
-			return fmt.Errorf("failed finding this specific Access Policy on Azure KeyVault %q (resource group %q)", vaultName, id.ResourceGroup)
+			return fmt.Errorf("unable to find Access Policy (Object ID %q / Application ID %q) on %s", id.ObjectID(), id.ApplicationId(), *vaultId)
 		}
 		accessPolicy = *accessPolicyRaw
 
@@ -226,19 +212,19 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta
 	accessPolicies := []keyvault.AccessPolicyEntry{accessPolicy}
 
 	parameters := keyvault.VaultAccessPolicyParameters{
-		Name: &vaultName,
+		Name: utils.String(vaultId.Name),
 		Properties: &keyvault.VaultAccessPolicyProperties{
 			AccessPolicies: &accessPolicies,
 		},
 	}
 
-	if _, err = client.UpdateAccessPolicy(ctx, resourceGroup, vaultName, action, parameters); err != nil {
-		return fmt.Errorf("updating Access Policy (Object ID %q / Application ID %q) for Key Vault %q (Resource Group %q): %+v", objectId, applicationIdRaw, vaultName, resourceGroup, err)
+	if _, err = client.UpdateAccessPolicy(ctx, vaultId.ResourceGroup, vaultId.Name, action, parameters); err != nil {
+		return fmt.Errorf("updating Access Policy (Object ID %q / Application ID %q) for %s: %+v", objectId, applicationIdRaw, *vaultId, err)
 	}
 	stateConf := &pluginsdk.StateChangeConf{
 		Pending:                   []string{"notfound", "vaultnotfound"},
 		Target:                    []string{"found"},
-		Refresh:                   accessPolicyRefreshFunc(ctx, client, resourceGroup, vaultName, objectId, applicationIdRaw),
+		Refresh:                   accessPolicyRefreshFunc(ctx, client, vaultId.ResourceGroup, vaultId.Name, objectId, applicationIdRaw),
 		Delay:                     5 * time.Second,
 		ContinuousTargetOccurence: 3,
 		Timeout:                   d.Timeout(pluginsdk.TimeoutCreate),
@@ -258,17 +244,8 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta
 		return fmt.Errorf("failed waiting for Key Vault Access Policy (Object ID: %q) to apply: %+v", objectId, err)
 	}
 
-	read, err := client.Get(ctx, resourceGroup, vaultName)
-	if err != nil {
-		return fmt.Errorf("retrieving Key Vault %q (Resource Group %q): %+v", vaultName, resourceGroup, err)
-	}
-
-	if read.ID == nil {
-		return fmt.Errorf("Cannot read KeyVault %q (Resource Group %q) ID", vaultName, resourceGroup)
-	}
-
 	if d.IsNewResource() {
-		d.SetId(resourceId)
+		d.SetId(id.ID())
 	}
 
 	return nil
@@ -291,48 +268,45 @@ func resourceKeyVaultAccessPolicyRead(d *pluginsdk.ResourceData, meta interface{
 	ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
 	defer cancel()
 
-	id, err := azure.ParseAzureResourceID(d.Id())
+	id, err := parse.AccessPolicyID(d.Id())
 	if err != nil {
 		return err
 	}
-	resGroup := id.ResourceGroup
-	vaultName := id.Path["vaults"]
-	objectId := id.Path["objectId"]
-	applicationId := id.Path["applicationId"]
 
-	resp, err := client.Get(ctx, resGroup, vaultName)
+	vaultId := id.KeyVaultId()
+
+	resp, err := client.Get(ctx, vaultId.ResourceGroup, vaultId.Name)
 	if err != nil {
 		if utils.ResponseWasNotFound(resp.Response) {
-			log.Printf("[ERROR] Key Vault %q (Resource Group %q) was not found - removing from state", vaultName, resGroup)
+			log.Printf("[DEBUG] parent %q was not found - removing from state", vaultId)
 			d.SetId("")
 			return nil
 		}
 
-		return fmt.Errorf("making Read request on Azure KeyVault %q (Resource Group %q): %+v", vaultName, resGroup, err)
+		return fmt.Errorf("retrieving parent %s: %+v", vaultId, err)
 	}
 
 	if resp.Properties == nil || resp.Properties.AccessPolicies == nil {
-		return fmt.Errorf("failed reading Access Policies for %q (resource group %q)", vaultName, id.ResourceGroup)
+		return fmt.Errorf("retrieving parent %s: accessPolicies were nil", vaultId)
 	}
 
-	policy := FindKeyVaultAccessPolicy(resp.Properties.AccessPolicies, objectId, applicationId)
+	policy := FindKeyVaultAccessPolicy(resp.Properties.AccessPolicies, id.ObjectID(), id.ApplicationId())
 
 	if policy == nil {
-		log.Printf("[ERROR] Access Policy (Object ID %q / Application ID %q) was not found in Key Vault %q (Resource Group %q) - removing from state", objectId, applicationId, vaultName, resGroup)
+		log.Printf("[ERROR] Access Policy (Object ID %q / Application ID %q) was not found in %s - removing from state", id.ObjectID(), id.ApplicationId(), vaultId)
 		d.SetId("")
 		return nil
 	}
 
-	d.Set("key_vault_id", resp.ID)
-	d.Set("object_id", objectId)
+	d.Set("key_vault_id", id.KeyVaultId().ID())
+	d.Set("application_id", id.ApplicationId())
+	d.Set("object_id", id.ObjectID())
 
+	tenantId := ""
 	if tid := policy.TenantID; tid != nil {
-		d.Set("tenant_id", tid.String())
-	}
-
-	if aid := policy.ApplicationID; aid != nil {
-		d.Set("application_id", aid.String())
+		tenantId = tid.String()
 	}
+	d.Set("tenant_id", tenantId)
 
 	if permissions := policy.Permissions; permissions != nil {
 		certificatePermissions := flattenCertificatePermissions(permissions.Certificates)

internal/services/keyvault/key_vault_access_policy_resource_test.go

@@ -6,7 +6,8 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse"
+
 	"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
@@ -122,27 +123,23 @@ func TestAccKeyVaultAccessPolicy_nonExistentVault(t *testing.T) {
 		{
 			Config:             r.nonExistentVault(data),
 			ExpectNonEmptyPlan: true,
-			ExpectError:        regexp.MustCompile(`retrieving Key Vault`),
+			ExpectError:        regexp.MustCompile(`retrieving parent`),
 		},
 	})
 }
 
 func (t KeyVaultAccessPolicyResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
-	id, err := azure.ParseAzureResourceID(state.ID)
+	id, err := parse.AccessPolicyID(state.ID)
 	if err != nil {
 		return nil, err
 	}
-	resGroup := id.ResourceGroup
-	vaultName := id.Path["vaults"]
-	objectId := id.Path["objectId"]
-	applicationId := id.Path["applicationId"]
 
-	resp, err := clients.KeyVault.VaultsClient.Get(ctx, resGroup, vaultName)
+	resp, err := clients.KeyVault.VaultsClient.Get(ctx, id.KeyVaultId().ResourceGroup, id.KeyVaultId().Name)
 	if err != nil {
 		return nil, fmt.Errorf("reading Key Vault (%s): %+v", id, err)
 	}
 
-	return utils.Bool(keyvault.FindKeyVaultAccessPolicy(resp.Properties.AccessPolicies, objectId, applicationId) != nil), nil
+	return utils.Bool(keyvault.FindKeyVaultAccessPolicy(resp.Properties.AccessPolicies, id.ObjectID(), id.ApplicationId()) != nil), nil
 }
 
 func (r KeyVaultAccessPolicyResource) basic(data acceptance.TestData) string {