Skip to content

Commit

Permalink
3.0: adding missing import validators (#15989)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tombuildsstuff
tombuildsstuff authored Mar 23, 2022
1 parent a8895c8 commit 1db46b9
Show file tree
Hide file tree
Showing 22 changed files with 1,335 additions and 113 deletions.
8 changes: 8 additions & 0 deletions internal/provider/provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package provider

import (
"fmt"
"log"
"testing"
"time"
)
Expand Down Expand Up @@ -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))
}
48 changes: 48 additions & 0 deletions internal/provider/services_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
110 changes: 42 additions & 68 deletions internal/services/keyvault/key_vault_access_policy_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -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())
}
}
}
Expand All @@ -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

Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand All @@ -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)
Expand Down
15 changes: 6 additions & 9 deletions internal/services/keyvault/key_vault_access_policy_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 1db46b9

Please sign in to comment.