From c10a615fe51dd251d77b18b31187faf2eef4e21f Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 23 Mar 2022 21:22:20 +0100 Subject: [PATCH 1/3] keyvault: updating azurerm_key_vault_access_policy to use an ID Parser --- .../key_vault_access_policy_resource.go | 110 +++++-------- .../key_vault_access_policy_resource_test.go | 15 +- .../services/keyvault/parse/access_policy.go | 93 +++++++++++ .../parse/access_policy_application.go | 81 ++++++++++ .../parse/access_policy_application_test.go | 144 ++++++++++++++++++ .../keyvault/parse/access_policy_object.go | 75 +++++++++ .../parse/access_policy_object_test.go | 128 ++++++++++++++++ internal/services/keyvault/resourceids.go | 4 + .../validate/access_policy_application_id.go | 23 +++ .../access_policy_application_id_test.go | 100 ++++++++++++ .../validate/access_policy_object_id.go | 23 +++ .../validate/access_policy_object_id_test.go | 88 +++++++++++ 12 files changed, 807 insertions(+), 77 deletions(-) create mode 100644 internal/services/keyvault/parse/access_policy.go create mode 100644 internal/services/keyvault/parse/access_policy_application.go create mode 100644 internal/services/keyvault/parse/access_policy_application_test.go create mode 100644 internal/services/keyvault/parse/access_policy_object.go create mode 100644 internal/services/keyvault/parse/access_policy_object_test.go create mode 100644 internal/services/keyvault/validate/access_policy_application_id.go create mode 100644 internal/services/keyvault/validate/access_policy_application_id_test.go create mode 100644 internal/services/keyvault/validate/access_policy_object_id.go create mode 100644 internal/services/keyvault/validate/access_policy_object_id_test.go diff --git a/internal/services/keyvault/key_vault_access_policy_resource.go b/internal/services/keyvault/key_vault_access_policy_resource.go index b94184c47f3d..3c0db35ac9e3 100644 --- a/internal/services/keyvault/key_vault_access_policy_resource.go +++ b/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,7 +86,10 @@ 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) @@ -94,46 +97,29 @@ func resourceKeyVaultAccessPolicyCreateOrDelete(d *pluginsdk.ResourceData, meta 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) diff --git a/internal/services/keyvault/key_vault_access_policy_resource_test.go b/internal/services/keyvault/key_vault_access_policy_resource_test.go index 81f9ebea5d60..20d4bd02c760 100644 --- a/internal/services/keyvault/key_vault_access_policy_resource_test.go +++ b/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 { diff --git a/internal/services/keyvault/parse/access_policy.go b/internal/services/keyvault/parse/access_policy.go new file mode 100644 index 000000000000..da617c6531a2 --- /dev/null +++ b/internal/services/keyvault/parse/access_policy.go @@ -0,0 +1,93 @@ +package parse + +import ( + "fmt" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +// NOTE: @tombuildsstuff - this entire file is not a recommended pattern and is a workaround for a resource which is +// managing two types of Access Policy (an Object ID-only and with an Application ID) instead of being split into +// two separate resources. +// +// If you find yourself needing to emulate this pattern, consider splitting/specialising the resource as required. + +var _ resourceids.Id = AccessPolicyId{} + +type AccessPolicyId struct { + applicationId *AccessPolicyApplicationId + objectId *AccessPolicyObjectId +} + +func NewAccessPolicyId(keyVaultId VaultId, objectId, applicationId string) AccessPolicyId { + out := AccessPolicyId{} + if applicationId != "" { + id := NewAccessPolicyApplicationID(keyVaultId.SubscriptionId, keyVaultId.ResourceGroup, keyVaultId.Name, objectId, applicationId) + out.applicationId = &id + } else { + id := NewAccessPolicyObjectID(keyVaultId.SubscriptionId, keyVaultId.ResourceGroup, keyVaultId.Name, objectId) + out.objectId = &id + } + return out +} + +func AccessPolicyID(input string) (*AccessPolicyId, error) { + accessPolicyObjectId, _ := AccessPolicyObjectID(input) + if accessPolicyObjectId != nil { + return &AccessPolicyId{ + objectId: accessPolicyObjectId, + }, nil + } + accessPolicyApplicationId, _ := AccessPolicyApplicationID(input) + if accessPolicyApplicationId != nil { + return &AccessPolicyId{ + applicationId: accessPolicyApplicationId, + }, nil + } + + return nil, fmt.Errorf("%q didn't parse as either an Object ID ('{keyVaultId}/objectId/{objId}') or an Application ID ('{keyVaultId}/objectId/{objId}/applicationId/{appId}')", input) +} + +func (a AccessPolicyId) ID() string { + if a.applicationId != nil { + return a.applicationId.ID() + } + + // whilst this is a pointer, as it has to be either/or it's fine + return a.objectId.ID() +} + +func (a AccessPolicyId) String() string { + if a.applicationId != nil { + return a.applicationId.String() + } + + // whilst this is a pointer, as it has to be either/or it's fine + return a.objectId.String() +} + +func (a AccessPolicyId) ApplicationId() string { + if a.applicationId != nil { + return a.applicationId.ApplicationIdName + } + + return "" +} + +func (a AccessPolicyId) KeyVaultId() VaultId { + if a.applicationId != nil { + return NewVaultID(a.applicationId.SubscriptionId, a.applicationId.ResourceGroup, a.applicationId.VaultName) + } + + // whilst this is a pointer, as it has to be either/or it's fine + return NewVaultID(a.objectId.SubscriptionId, a.objectId.ResourceGroup, a.objectId.VaultName) +} + +func (a AccessPolicyId) ObjectID() string { + if a.applicationId != nil { + return a.applicationId.ObjectIdName + } + + // whilst this is a pointer, as it has to be either/or it's fine + return a.objectId.ObjectIdName +} diff --git a/internal/services/keyvault/parse/access_policy_application.go b/internal/services/keyvault/parse/access_policy_application.go new file mode 100644 index 000000000000..7ee3b9f4d06c --- /dev/null +++ b/internal/services/keyvault/parse/access_policy_application.go @@ -0,0 +1,81 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +type AccessPolicyApplicationId struct { + SubscriptionId string + ResourceGroup string + VaultName string + ObjectIdName string + ApplicationIdName string +} + +func NewAccessPolicyApplicationID(subscriptionId, resourceGroup, vaultName, objectIdName, applicationIdName string) AccessPolicyApplicationId { + return AccessPolicyApplicationId{ + SubscriptionId: subscriptionId, + ResourceGroup: resourceGroup, + VaultName: vaultName, + ObjectIdName: objectIdName, + ApplicationIdName: applicationIdName, + } +} + +func (id AccessPolicyApplicationId) String() string { + segments := []string{ + fmt.Sprintf("Application Id Name %q", id.ApplicationIdName), + fmt.Sprintf("Object Id Name %q", id.ObjectIdName), + fmt.Sprintf("Vault Name %q", id.VaultName), + fmt.Sprintf("Resource Group %q", id.ResourceGroup), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Access Policy Application", segmentsStr) +} + +func (id AccessPolicyApplicationId) ID() string { + fmtString := "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.KeyVault/vaults/%s/objectId/%s/applicationId/%s" + return fmt.Sprintf(fmtString, id.SubscriptionId, id.ResourceGroup, id.VaultName, id.ObjectIdName, id.ApplicationIdName) +} + +// AccessPolicyApplicationID parses a AccessPolicyApplication ID into an AccessPolicyApplicationId struct +func AccessPolicyApplicationID(input string) (*AccessPolicyApplicationId, error) { + id, err := resourceids.ParseAzureResourceID(input) + if err != nil { + return nil, err + } + + resourceId := AccessPolicyApplicationId{ + SubscriptionId: id.SubscriptionID, + ResourceGroup: id.ResourceGroup, + } + + if resourceId.SubscriptionId == "" { + return nil, fmt.Errorf("ID was missing the 'subscriptions' element") + } + + if resourceId.ResourceGroup == "" { + return nil, fmt.Errorf("ID was missing the 'resourceGroups' element") + } + + if resourceId.VaultName, err = id.PopSegment("vaults"); err != nil { + return nil, err + } + if resourceId.ObjectIdName, err = id.PopSegment("objectId"); err != nil { + return nil, err + } + if resourceId.ApplicationIdName, err = id.PopSegment("applicationId"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &resourceId, nil +} diff --git a/internal/services/keyvault/parse/access_policy_application_test.go b/internal/services/keyvault/parse/access_policy_application_test.go new file mode 100644 index 000000000000..1abd2be85400 --- /dev/null +++ b/internal/services/keyvault/parse/access_policy_application_test.go @@ -0,0 +1,144 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "testing" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +var _ resourceids.Id = AccessPolicyApplicationId{} + +func TestAccessPolicyApplicationIDFormatter(t *testing.T) { + actual := NewAccessPolicyApplicationID("12345678-1234-9876-4563-123456789012", "resGroup1", "vault1", "object1", "application1").ID() + expected := "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/application1" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} + +func TestAccessPolicyApplicationID(t *testing.T) { + testData := []struct { + Input string + Error bool + Expected *AccessPolicyApplicationId + }{ + + { + // empty + Input: "", + Error: true, + }, + + { + // missing SubscriptionId + Input: "/", + Error: true, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Error: true, + }, + + { + // missing ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/", + Error: true, + }, + + { + // missing value for ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/", + Error: true, + }, + + { + // missing VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/", + Error: true, + }, + + { + // missing value for VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/", + Error: true, + }, + + { + // missing ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/", + Error: true, + }, + + { + // missing value for ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/", + Error: true, + }, + + { + // missing ApplicationIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/", + Error: true, + }, + + { + // missing value for ApplicationIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/", + Error: true, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/application1", + Expected: &AccessPolicyApplicationId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + ResourceGroup: "resGroup1", + VaultName: "vault1", + ObjectIdName: "object1", + ApplicationIdName: "application1", + }, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/RESOURCEGROUPS/RESGROUP1/PROVIDERS/MICROSOFT.KEYVAULT/VAULTS/VAULT1/OBJECTID/OBJECT1/APPLICATIONID/APPLICATION1", + Error: true, + }, + } + + for _, v := range testData { + t.Logf("[DEBUG] Testing %q", v.Input) + + actual, err := AccessPolicyApplicationID(v.Input) + if err != nil { + if v.Error { + continue + } + + t.Fatalf("Expect a value but got an error: %s", err) + } + if v.Error { + t.Fatal("Expect an error but didn't get one") + } + + if actual.SubscriptionId != v.Expected.SubscriptionId { + t.Fatalf("Expected %q but got %q for SubscriptionId", v.Expected.SubscriptionId, actual.SubscriptionId) + } + if actual.ResourceGroup != v.Expected.ResourceGroup { + t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.ResourceGroup, actual.ResourceGroup) + } + if actual.VaultName != v.Expected.VaultName { + t.Fatalf("Expected %q but got %q for VaultName", v.Expected.VaultName, actual.VaultName) + } + if actual.ObjectIdName != v.Expected.ObjectIdName { + t.Fatalf("Expected %q but got %q for ObjectIdName", v.Expected.ObjectIdName, actual.ObjectIdName) + } + if actual.ApplicationIdName != v.Expected.ApplicationIdName { + t.Fatalf("Expected %q but got %q for ApplicationIdName", v.Expected.ApplicationIdName, actual.ApplicationIdName) + } + } +} diff --git a/internal/services/keyvault/parse/access_policy_object.go b/internal/services/keyvault/parse/access_policy_object.go new file mode 100644 index 000000000000..fcfba8ec3e32 --- /dev/null +++ b/internal/services/keyvault/parse/access_policy_object.go @@ -0,0 +1,75 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +type AccessPolicyObjectId struct { + SubscriptionId string + ResourceGroup string + VaultName string + ObjectIdName string +} + +func NewAccessPolicyObjectID(subscriptionId, resourceGroup, vaultName, objectIdName string) AccessPolicyObjectId { + return AccessPolicyObjectId{ + SubscriptionId: subscriptionId, + ResourceGroup: resourceGroup, + VaultName: vaultName, + ObjectIdName: objectIdName, + } +} + +func (id AccessPolicyObjectId) String() string { + segments := []string{ + fmt.Sprintf("Object Id Name %q", id.ObjectIdName), + fmt.Sprintf("Vault Name %q", id.VaultName), + fmt.Sprintf("Resource Group %q", id.ResourceGroup), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Access Policy Object", segmentsStr) +} + +func (id AccessPolicyObjectId) ID() string { + fmtString := "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.KeyVault/vaults/%s/objectId/%s" + return fmt.Sprintf(fmtString, id.SubscriptionId, id.ResourceGroup, id.VaultName, id.ObjectIdName) +} + +// AccessPolicyObjectID parses a AccessPolicyObject ID into an AccessPolicyObjectId struct +func AccessPolicyObjectID(input string) (*AccessPolicyObjectId, error) { + id, err := resourceids.ParseAzureResourceID(input) + if err != nil { + return nil, err + } + + resourceId := AccessPolicyObjectId{ + SubscriptionId: id.SubscriptionID, + ResourceGroup: id.ResourceGroup, + } + + if resourceId.SubscriptionId == "" { + return nil, fmt.Errorf("ID was missing the 'subscriptions' element") + } + + if resourceId.ResourceGroup == "" { + return nil, fmt.Errorf("ID was missing the 'resourceGroups' element") + } + + if resourceId.VaultName, err = id.PopSegment("vaults"); err != nil { + return nil, err + } + if resourceId.ObjectIdName, err = id.PopSegment("objectId"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &resourceId, nil +} diff --git a/internal/services/keyvault/parse/access_policy_object_test.go b/internal/services/keyvault/parse/access_policy_object_test.go new file mode 100644 index 000000000000..0460b840b792 --- /dev/null +++ b/internal/services/keyvault/parse/access_policy_object_test.go @@ -0,0 +1,128 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "testing" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +var _ resourceids.Id = AccessPolicyObjectId{} + +func TestAccessPolicyObjectIDFormatter(t *testing.T) { + actual := NewAccessPolicyObjectID("12345678-1234-9876-4563-123456789012", "resGroup1", "vault1", "object1").ID() + expected := "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} + +func TestAccessPolicyObjectID(t *testing.T) { + testData := []struct { + Input string + Error bool + Expected *AccessPolicyObjectId + }{ + + { + // empty + Input: "", + Error: true, + }, + + { + // missing SubscriptionId + Input: "/", + Error: true, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Error: true, + }, + + { + // missing ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/", + Error: true, + }, + + { + // missing value for ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/", + Error: true, + }, + + { + // missing VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/", + Error: true, + }, + + { + // missing value for VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/", + Error: true, + }, + + { + // missing ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/", + Error: true, + }, + + { + // missing value for ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/", + Error: true, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1", + Expected: &AccessPolicyObjectId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + ResourceGroup: "resGroup1", + VaultName: "vault1", + ObjectIdName: "object1", + }, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/RESOURCEGROUPS/RESGROUP1/PROVIDERS/MICROSOFT.KEYVAULT/VAULTS/VAULT1/OBJECTID/OBJECT1", + Error: true, + }, + } + + for _, v := range testData { + t.Logf("[DEBUG] Testing %q", v.Input) + + actual, err := AccessPolicyObjectID(v.Input) + if err != nil { + if v.Error { + continue + } + + t.Fatalf("Expect a value but got an error: %s", err) + } + if v.Error { + t.Fatal("Expect an error but didn't get one") + } + + if actual.SubscriptionId != v.Expected.SubscriptionId { + t.Fatalf("Expected %q but got %q for SubscriptionId", v.Expected.SubscriptionId, actual.SubscriptionId) + } + if actual.ResourceGroup != v.Expected.ResourceGroup { + t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.ResourceGroup, actual.ResourceGroup) + } + if actual.VaultName != v.Expected.VaultName { + t.Fatalf("Expected %q but got %q for VaultName", v.Expected.VaultName, actual.VaultName) + } + if actual.ObjectIdName != v.Expected.ObjectIdName { + t.Fatalf("Expected %q but got %q for ObjectIdName", v.Expected.ObjectIdName, actual.ObjectIdName) + } + } +} diff --git a/internal/services/keyvault/resourceids.go b/internal/services/keyvault/resourceids.go index 535dce9937b8..6979b1b609a8 100644 --- a/internal/services/keyvault/resourceids.go +++ b/internal/services/keyvault/resourceids.go @@ -2,3 +2,7 @@ package keyvault //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=Vault -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1 //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=ManagedHSM -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/managedHSMs/hsm1 + +// KeyVault Access Policies are Terraform specific, but can be either an Object ID or an Application ID +//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=AccessPolicyApplication -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/application1 +//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=AccessPolicyObject -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1 diff --git a/internal/services/keyvault/validate/access_policy_application_id.go b/internal/services/keyvault/validate/access_policy_application_id.go new file mode 100644 index 000000000000..75e53b7ea86e --- /dev/null +++ b/internal/services/keyvault/validate/access_policy_application_id.go @@ -0,0 +1,23 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse" +) + +func AccessPolicyApplicationID(input interface{}, key string) (warnings []string, errors []error) { + v, ok := input.(string) + if !ok { + errors = append(errors, fmt.Errorf("expected %q to be a string", key)) + return + } + + if _, err := parse.AccessPolicyApplicationID(v); err != nil { + errors = append(errors, err) + } + + return +} diff --git a/internal/services/keyvault/validate/access_policy_application_id_test.go b/internal/services/keyvault/validate/access_policy_application_id_test.go new file mode 100644 index 000000000000..11c39ee867dc --- /dev/null +++ b/internal/services/keyvault/validate/access_policy_application_id_test.go @@ -0,0 +1,100 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import "testing" + +func TestAccessPolicyApplicationID(t *testing.T) { + cases := []struct { + Input string + Valid bool + }{ + + { + // empty + Input: "", + Valid: false, + }, + + { + // missing SubscriptionId + Input: "/", + Valid: false, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Valid: false, + }, + + { + // missing ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/", + Valid: false, + }, + + { + // missing value for ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/", + Valid: false, + }, + + { + // missing VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/", + Valid: false, + }, + + { + // missing value for VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/", + Valid: false, + }, + + { + // missing ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/", + Valid: false, + }, + + { + // missing value for ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/", + Valid: false, + }, + + { + // missing ApplicationIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/", + Valid: false, + }, + + { + // missing value for ApplicationIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/", + Valid: false, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1/applicationId/application1", + Valid: true, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/RESOURCEGROUPS/RESGROUP1/PROVIDERS/MICROSOFT.KEYVAULT/VAULTS/VAULT1/OBJECTID/OBJECT1/APPLICATIONID/APPLICATION1", + Valid: false, + }, + } + for _, tc := range cases { + t.Logf("[DEBUG] Testing Value %s", tc.Input) + _, errors := AccessPolicyApplicationID(tc.Input, "test") + valid := len(errors) == 0 + + if tc.Valid != valid { + t.Fatalf("Expected %t but got %t", tc.Valid, valid) + } + } +} diff --git a/internal/services/keyvault/validate/access_policy_object_id.go b/internal/services/keyvault/validate/access_policy_object_id.go new file mode 100644 index 000000000000..37356aa0941a --- /dev/null +++ b/internal/services/keyvault/validate/access_policy_object_id.go @@ -0,0 +1,23 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse" +) + +func AccessPolicyObjectID(input interface{}, key string) (warnings []string, errors []error) { + v, ok := input.(string) + if !ok { + errors = append(errors, fmt.Errorf("expected %q to be a string", key)) + return + } + + if _, err := parse.AccessPolicyObjectID(v); err != nil { + errors = append(errors, err) + } + + return +} diff --git a/internal/services/keyvault/validate/access_policy_object_id_test.go b/internal/services/keyvault/validate/access_policy_object_id_test.go new file mode 100644 index 000000000000..f7a9194c125d --- /dev/null +++ b/internal/services/keyvault/validate/access_policy_object_id_test.go @@ -0,0 +1,88 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import "testing" + +func TestAccessPolicyObjectID(t *testing.T) { + cases := []struct { + Input string + Valid bool + }{ + + { + // empty + Input: "", + Valid: false, + }, + + { + // missing SubscriptionId + Input: "/", + Valid: false, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Valid: false, + }, + + { + // missing ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/", + Valid: false, + }, + + { + // missing value for ResourceGroup + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/", + Valid: false, + }, + + { + // missing VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/", + Valid: false, + }, + + { + // missing value for VaultName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/", + Valid: false, + }, + + { + // missing ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/", + Valid: false, + }, + + { + // missing value for ObjectIdName + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/", + Valid: false, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.KeyVault/vaults/vault1/objectId/object1", + Valid: true, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/RESOURCEGROUPS/RESGROUP1/PROVIDERS/MICROSOFT.KEYVAULT/VAULTS/VAULT1/OBJECTID/OBJECT1", + Valid: false, + }, + } + for _, tc := range cases { + t.Logf("[DEBUG] Testing Value %s", tc.Input) + _, errors := AccessPolicyObjectID(tc.Input, "test") + valid := len(errors) == 0 + + if tc.Valid != valid { + t.Fatalf("Expected %t but got %t", tc.Valid, valid) + } + } +} From c33b25963c66c25a7af18d6d45847c7a2af2a155 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 23 Mar 2022 21:22:31 +0100 Subject: [PATCH 2/3] r/security_center_auto_provisioning: adding an import validator --- .../migration/auto_provisioning_v0_to_v1.go | 42 ++++ .../parse/auto_provisioning_setting.go | 100 +++++++++ .../parse/auto_provisioning_setting_test.go | 194 ++++++++++++++++++ .../services/securitycenter/resourceids.go | 1 + ...urity_center_auto_provisioning_resource.go | 73 ++++--- ..._center_auto_provisioning_resource_test.go | 11 +- .../validate/auto_provisioning_setting_id.go | 23 +++ .../auto_provisioning_setting_id_test.go | 64 ++++++ 8 files changed, 472 insertions(+), 36 deletions(-) create mode 100644 internal/services/securitycenter/migration/auto_provisioning_v0_to_v1.go create mode 100644 internal/services/securitycenter/parse/auto_provisioning_setting.go create mode 100644 internal/services/securitycenter/parse/auto_provisioning_setting_test.go create mode 100644 internal/services/securitycenter/validate/auto_provisioning_setting_id.go create mode 100644 internal/services/securitycenter/validate/auto_provisioning_setting_id_test.go diff --git a/internal/services/securitycenter/migration/auto_provisioning_v0_to_v1.go b/internal/services/securitycenter/migration/auto_provisioning_v0_to_v1.go new file mode 100644 index 000000000000..436d10efaa68 --- /dev/null +++ b/internal/services/securitycenter/migration/auto_provisioning_v0_to_v1.go @@ -0,0 +1,42 @@ +package migration + +import ( + "context" + "fmt" + "log" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/parse" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" +) + +type AutoProvisioningV0ToV1 struct{} + +func (a AutoProvisioningV0ToV1) Schema() map[string]*pluginsdk.Schema { + return map[string]*pluginsdk.Schema{ + "auto_provision": { + Type: pluginsdk.TypeString, + Required: true, + }, + } +} + +func (a AutoProvisioningV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { + return func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + oldId := rawState["id"].(string) + + parsed, err := parse.AutoProvisioningSettingIDInsensitively(oldId) + if err != nil { + return nil, fmt.Errorf("parsing old ID %q: %+v", oldId, err) + } + + // potentially overkill, but the name is fixed (we can't guarantee the casing, however) + parsed.Name = "default" + + newId := parsed.ID() + log.Printf("[DEBUG] Updating the ID from %q to %q..", oldId, newId) + rawState["id"] = newId + log.Printf("[DEBUG] Updated the ID from %q to %q.", oldId, newId) + + return rawState, nil + } +} diff --git a/internal/services/securitycenter/parse/auto_provisioning_setting.go b/internal/services/securitycenter/parse/auto_provisioning_setting.go new file mode 100644 index 000000000000..21412bba671f --- /dev/null +++ b/internal/services/securitycenter/parse/auto_provisioning_setting.go @@ -0,0 +1,100 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +type AutoProvisioningSettingId struct { + SubscriptionId string + Name string +} + +func NewAutoProvisioningSettingID(subscriptionId, name string) AutoProvisioningSettingId { + return AutoProvisioningSettingId{ + SubscriptionId: subscriptionId, + Name: name, + } +} + +func (id AutoProvisioningSettingId) String() string { + segments := []string{ + fmt.Sprintf("Name %q", id.Name), + } + segmentsStr := strings.Join(segments, " / ") + return fmt.Sprintf("%s: (%s)", "Auto Provisioning Setting", segmentsStr) +} + +func (id AutoProvisioningSettingId) ID() string { + fmtString := "/subscriptions/%s/providers/Microsoft.Security/autoProvisioningSettings/%s" + return fmt.Sprintf(fmtString, id.SubscriptionId, id.Name) +} + +// AutoProvisioningSettingID parses a AutoProvisioningSetting ID into an AutoProvisioningSettingId struct +func AutoProvisioningSettingID(input string) (*AutoProvisioningSettingId, error) { + id, err := resourceids.ParseAzureResourceID(input) + if err != nil { + return nil, err + } + + resourceId := AutoProvisioningSettingId{ + SubscriptionId: id.SubscriptionID, + } + + if resourceId.SubscriptionId == "" { + return nil, fmt.Errorf("ID was missing the 'subscriptions' element") + } + + if resourceId.Name, err = id.PopSegment("autoProvisioningSettings"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &resourceId, nil +} + +// AutoProvisioningSettingIDInsensitively parses an AutoProvisioningSetting ID into an AutoProvisioningSettingId struct, insensitively +// This should only be used to parse an ID for rewriting, the AutoProvisioningSettingID +// method should be used instead for validation etc. +// +// Whilst this may seem strange, this enables Terraform have consistent casing +// which works around issues in Core, whilst handling broken API responses. +func AutoProvisioningSettingIDInsensitively(input string) (*AutoProvisioningSettingId, error) { + id, err := resourceids.ParseAzureResourceID(input) + if err != nil { + return nil, err + } + + resourceId := AutoProvisioningSettingId{ + SubscriptionId: id.SubscriptionID, + } + + if resourceId.SubscriptionId == "" { + return nil, fmt.Errorf("ID was missing the 'subscriptions' element") + } + + // find the correct casing for the 'autoProvisioningSettings' segment + autoProvisioningSettingsKey := "autoProvisioningSettings" + for key := range id.Path { + if strings.EqualFold(key, autoProvisioningSettingsKey) { + autoProvisioningSettingsKey = key + break + } + } + if resourceId.Name, err = id.PopSegment(autoProvisioningSettingsKey); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &resourceId, nil +} diff --git a/internal/services/securitycenter/parse/auto_provisioning_setting_test.go b/internal/services/securitycenter/parse/auto_provisioning_setting_test.go new file mode 100644 index 000000000000..0f315385f505 --- /dev/null +++ b/internal/services/securitycenter/parse/auto_provisioning_setting_test.go @@ -0,0 +1,194 @@ +package parse + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "testing" + + "github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids" +) + +var _ resourceids.Id = AutoProvisioningSettingId{} + +func TestAutoProvisioningSettingIDFormatter(t *testing.T) { + actual := NewAutoProvisioningSettingID("12345678-1234-9876-4563-123456789012", "default").ID() + expected := "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/default" + if actual != expected { + t.Fatalf("Expected %q but got %q", expected, actual) + } +} + +func TestAutoProvisioningSettingID(t *testing.T) { + testData := []struct { + Input string + Error bool + Expected *AutoProvisioningSettingId + }{ + + { + // empty + Input: "", + Error: true, + }, + + { + // missing SubscriptionId + Input: "/", + Error: true, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Error: true, + }, + + { + // missing Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/", + Error: true, + }, + + { + // missing value for Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/", + Error: true, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/default", + Expected: &AutoProvisioningSettingId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + Name: "default", + }, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/PROVIDERS/MICROSOFT.SECURITY/AUTOPROVISIONINGSETTINGS/DEFAULT", + Error: true, + }, + } + + for _, v := range testData { + t.Logf("[DEBUG] Testing %q", v.Input) + + actual, err := AutoProvisioningSettingID(v.Input) + if err != nil { + if v.Error { + continue + } + + t.Fatalf("Expect a value but got an error: %s", err) + } + if v.Error { + t.Fatal("Expect an error but didn't get one") + } + + if actual.SubscriptionId != v.Expected.SubscriptionId { + t.Fatalf("Expected %q but got %q for SubscriptionId", v.Expected.SubscriptionId, actual.SubscriptionId) + } + if actual.Name != v.Expected.Name { + t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name) + } + } +} + +func TestAutoProvisioningSettingIDInsensitively(t *testing.T) { + testData := []struct { + Input string + Error bool + Expected *AutoProvisioningSettingId + }{ + + { + // empty + Input: "", + Error: true, + }, + + { + // missing SubscriptionId + Input: "/", + Error: true, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Error: true, + }, + + { + // missing Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/", + Error: true, + }, + + { + // missing value for Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/", + Error: true, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/default", + Expected: &AutoProvisioningSettingId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + Name: "default", + }, + }, + + { + // lower-cased segment names + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoprovisioningsettings/default", + Expected: &AutoProvisioningSettingId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + Name: "default", + }, + }, + + { + // upper-cased segment names + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/AUTOPROVISIONINGSETTINGS/default", + Expected: &AutoProvisioningSettingId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + Name: "default", + }, + }, + + { + // mixed-cased segment names + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/AuToPrOvIsIoNiNgSeTtInGs/default", + Expected: &AutoProvisioningSettingId{ + SubscriptionId: "12345678-1234-9876-4563-123456789012", + Name: "default", + }, + }, + } + + for _, v := range testData { + t.Logf("[DEBUG] Testing %q", v.Input) + + actual, err := AutoProvisioningSettingIDInsensitively(v.Input) + if err != nil { + if v.Error { + continue + } + + t.Fatalf("Expect a value but got an error: %s", err) + } + if v.Error { + t.Fatal("Expect an error but didn't get one") + } + + if actual.SubscriptionId != v.Expected.SubscriptionId { + t.Fatalf("Expected %q but got %q for SubscriptionId", v.Expected.SubscriptionId, actual.SubscriptionId) + } + if actual.Name != v.Expected.Name { + t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name) + } + } +} diff --git a/internal/services/securitycenter/resourceids.go b/internal/services/securitycenter/resourceids.go index b0b8000a760f..9ec0d20b75a8 100644 --- a/internal/services/securitycenter/resourceids.go +++ b/internal/services/securitycenter/resourceids.go @@ -7,4 +7,5 @@ package securitycenter //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=Workspace -id=/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/workspaceSettings/workspace1 //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=AssessmentMetadata -id=/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/assessmentMetadata/metadata1 +//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=AutoProvisioningSetting -id=/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/default -rewrite=true //go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=IotSecuritySolution -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.Security/IoTSecuritySolutions/solution1 diff --git a/internal/services/securitycenter/security_center_auto_provisioning_resource.go b/internal/services/securitycenter/security_center_auto_provisioning_resource.go index 90778d0685f7..64e0d3db1c0b 100644 --- a/internal/services/securitycenter/security_center_auto_provisioning_resource.go +++ b/internal/services/securitycenter/security_center_auto_provisioning_resource.go @@ -5,31 +5,32 @@ import ( "log" "time" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/Azure/azure-sdk-for-go/services/preview/security/mgmt/v3.0/security" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/migration" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" "github.com/hashicorp/terraform-provider-azurerm/internal/timeouts" "github.com/hashicorp/terraform-provider-azurerm/utils" ) -// NOTE: 'default' is the only valid name currently supported by the API -// No other names can be created and the 'default' resource can not be destroyed -const securityCenterAutoProvisioningName = "default" - func resourceSecurityCenterAutoProvisioning() *pluginsdk.Resource { return &pluginsdk.Resource{ - Create: resourceSecurityCenterAutoProvisioningUpdate, + Create: resourceSecurityCenterAutoProvisioningCreateUpdate, Read: resourceSecurityCenterAutoProvisioningRead, - Update: resourceSecurityCenterAutoProvisioningUpdate, + Update: resourceSecurityCenterAutoProvisioningCreateUpdate, Delete: resourceSecurityCenterAutoProvisioningDelete, - // TODO: introduce 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.AutoProvisioningSettingID(id) + return err + }), + + StateUpgraders: pluginsdk.StateUpgrades(map[int]pluginsdk.StateUpgrade{ + 0: migration.AutoProvisioningV0ToV1{}, + }), + SchemaVersion: 1, Timeouts: &pluginsdk.ResourceTimeout{ Create: pluginsdk.DefaultTimeout(60 * time.Minute), @@ -52,9 +53,10 @@ func resourceSecurityCenterAutoProvisioning() *pluginsdk.Resource { } } -func resourceSecurityCenterAutoProvisioningUpdate(d *pluginsdk.ResourceData, meta interface{}) error { +func resourceSecurityCenterAutoProvisioningCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).SecurityCenter.AutoProvisioningClient - ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) + subscriptionId := meta.(*clients.Client).Account.SubscriptionId + ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() // No need for import check as there's always single resource called 'default' @@ -67,38 +69,38 @@ func resourceSecurityCenterAutoProvisioningUpdate(d *pluginsdk.ResourceData, met }, } - // There is no update function or operation in the API, only create - if _, err := client.Create(ctx, securityCenterAutoProvisioningName, settings); err != nil { - return fmt.Errorf("creating/updating Security Center auto provisioning: %+v", err) - } + // NOTE: 'default' is the only valid name currently supported by the API + // No other names can be created and the 'default' resource can not be destroyed + id := parse.NewAutoProvisioningSettingID(subscriptionId, "default") - resp, err := client.Get(ctx, securityCenterAutoProvisioningName) - if err != nil { - return fmt.Errorf("reading Security Center auto provisioning: %+v", err) - } - if resp.ID == nil || *resp.ID == "" { - return fmt.Errorf("Security Center auto provisioning ID is nil or empty") + // There is no update function or operation in the API, only create + if _, err := client.Create(ctx, id.Name, settings); err != nil { + return fmt.Errorf("updating auto-provisioning setting for %s: %+v", id, err) } - d.SetId(*resp.ID) - + d.SetId(id.ID()) return resourceSecurityCenterAutoProvisioningRead(d, meta) } func resourceSecurityCenterAutoProvisioningRead(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client).SecurityCenter.AutoProvisioningClient - ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - resp, err := client.Get(ctx, securityCenterAutoProvisioningName) + id, err := parse.AutoProvisioningSettingID(d.Id()) + if err != nil { + return err + } + + resp, err := client.Get(ctx, id.Name) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - log.Printf("[DEBUG] Security Center subscription was not found: %v", err) + log.Printf("[DEBUG] %s was not found - removing from state", err) d.SetId("") return nil } - return fmt.Errorf("reading Security Center auto provisioning: %+v", err) + return fmt.Errorf("retrieving auto-provisioning setting %s: %+v", id, err) } if properties := resp.AutoProvisioningSettingProperties; properties != nil { @@ -113,17 +115,22 @@ func resourceSecurityCenterAutoProvisioningDelete(d *pluginsdk.ResourceData, met // Instead we reset back to 'Off' which is the default client := meta.(*clients.Client).SecurityCenter.AutoProvisioningClient - ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() + id, err := parse.AutoProvisioningSettingID(d.Id()) + if err != nil { + return err + } + settings := security.AutoProvisioningSetting{ AutoProvisioningSettingProperties: &security.AutoProvisioningSettingProperties{ - AutoProvision: "Off", + AutoProvision: security.AutoProvisionOff, }, } // There is no update function or operation in the API, only create - if _, err := client.Create(ctx, securityCenterAutoProvisioningName, settings); err != nil { + if _, err := client.Create(ctx, id.Name, settings); err != nil { return fmt.Errorf("resetting Security Center auto provisioning to 'Off': %+v", err) } diff --git a/internal/services/securitycenter/security_center_auto_provisioning_resource_test.go b/internal/services/securitycenter/security_center_auto_provisioning_resource_test.go index 18d5500f3dc8..d26d6b13441a 100644 --- a/internal/services/securitycenter/security_center_auto_provisioning_resource_test.go +++ b/internal/services/securitycenter/security_center_auto_provisioning_resource_test.go @@ -5,6 +5,8 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/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" @@ -40,11 +42,14 @@ func TestAccSecurityCenterAutoProvision_update(t *testing.T) { } func (SecurityCenterAutoProvisionResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { - securityCenterAutoProvisioningName := "default" + id, err := parse.AutoProvisioningSettingID(state.ID) + if err != nil { + return nil, err + } - resp, err := clients.SecurityCenter.AutoProvisioningClient.Get(ctx, securityCenterAutoProvisioningName) + resp, err := clients.SecurityCenter.AutoProvisioningClient.Get(ctx, id.Name) if err != nil { - return nil, fmt.Errorf("reading Security Center auto provision (%s): %+v", securityCenterAutoProvisioningName, err) + return nil, fmt.Errorf("retrieving auto-provisioning setting for %s: %+v", *id, err) } return utils.Bool(resp.AutoProvisioningSettingProperties != nil), nil diff --git a/internal/services/securitycenter/validate/auto_provisioning_setting_id.go b/internal/services/securitycenter/validate/auto_provisioning_setting_id.go new file mode 100644 index 000000000000..5e052a4117cf --- /dev/null +++ b/internal/services/securitycenter/validate/auto_provisioning_setting_id.go @@ -0,0 +1,23 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import ( + "fmt" + + "github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/parse" +) + +func AutoProvisioningSettingID(input interface{}, key string) (warnings []string, errors []error) { + v, ok := input.(string) + if !ok { + errors = append(errors, fmt.Errorf("expected %q to be a string", key)) + return + } + + if _, err := parse.AutoProvisioningSettingID(v); err != nil { + errors = append(errors, err) + } + + return +} diff --git a/internal/services/securitycenter/validate/auto_provisioning_setting_id_test.go b/internal/services/securitycenter/validate/auto_provisioning_setting_id_test.go new file mode 100644 index 000000000000..884c7bfcef2b --- /dev/null +++ b/internal/services/securitycenter/validate/auto_provisioning_setting_id_test.go @@ -0,0 +1,64 @@ +package validate + +// NOTE: this file is generated via 'go:generate' - manual changes will be overwritten + +import "testing" + +func TestAutoProvisioningSettingID(t *testing.T) { + cases := []struct { + Input string + Valid bool + }{ + + { + // empty + Input: "", + Valid: false, + }, + + { + // missing SubscriptionId + Input: "/", + Valid: false, + }, + + { + // missing value for SubscriptionId + Input: "/subscriptions/", + Valid: false, + }, + + { + // missing Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/", + Valid: false, + }, + + { + // missing value for Name + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/", + Valid: false, + }, + + { + // valid + Input: "/subscriptions/12345678-1234-9876-4563-123456789012/providers/Microsoft.Security/autoProvisioningSettings/default", + Valid: true, + }, + + { + // upper-cased + Input: "/SUBSCRIPTIONS/12345678-1234-9876-4563-123456789012/PROVIDERS/MICROSOFT.SECURITY/AUTOPROVISIONINGSETTINGS/DEFAULT", + Valid: false, + }, + } + for _, tc := range cases { + t.Logf("[DEBUG] Testing Value %s", tc.Input) + _, errors := AutoProvisioningSettingID(tc.Input, "test") + valid := len(errors) == 0 + + if tc.Valid != valid { + t.Fatalf("Expected %t but got %t", tc.Valid, valid) + } + } +} From 702344efad7a4b29eef11db3d3c53876a1c144b3 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 23 Mar 2022 21:22:41 +0100 Subject: [PATCH 3/3] internal/provider: ensuring that all resources support import --- internal/provider/provider_test.go | 8 +++++ internal/provider/services_test.go | 48 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index e3e1940eb474..611f97cdf7e7 100644 --- a/internal/provider/provider_test.go +++ b/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)) +} diff --git a/internal/provider/services_test.go b/internal/provider/services_test.go index 14c058fafeb2..0d04f8da5d78 100644 --- a/internal/provider/services_test.go +++ b/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) + } + } + } +}