From e99e6b4571ef1f1ef8df89986864d3d659896c4f Mon Sep 17 00:00:00 2001 From: jackofallops Date: Thu, 30 Apr 2020 14:02:07 +0100 Subject: [PATCH 1/6] added recovery capabilty for secrets --- .../keyvault/resource_arm_key_vault_secret.go | 99 +++++++++++++++---- .../resource_arm_key_vault_secret_test.go | 88 +++++++++++++++++ azurerm/utils/response.go | 4 + website/docs/index.html.markdown | 2 + 4 files changed, 172 insertions(+), 21 deletions(-) diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go index 76342089bc89..cc949cd21582 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go @@ -3,16 +3,17 @@ package keyvault import ( "fmt" "log" + "net/http" "time" "github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault" "github.com/Azure/go-autorest/autorest/date" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -99,17 +100,15 @@ func resourceArmKeyVaultSecretCreate(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("Error looking up Secret %q vault url from id %q: %+v", name, keyVaultId, err) } - if features.ShouldResourcesBeImported() { - existing, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") - if err != nil { - if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("Error checking for presence of existing Secret %q (Key Vault %q): %s", name, keyVaultBaseUrl, err) - } + existing, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") + if err != nil { + if !utils.ResponseWasNotFound(existing.Response) { + return fmt.Errorf("Error checking for presence of existing Secret %q (Key Vault %q): %s", name, keyVaultBaseUrl, err) } + } - if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_key_vault_secret", *existing.ID) - } + if existing.ID != nil && *existing.ID != "" { + return tf.ImportAsExistsError("azurerm_key_vault_secret", *existing.ID) } value := d.Get("value").(string) @@ -135,20 +134,55 @@ func resourceArmKeyVaultSecretCreate(d *schema.ResourceData, meta interface{}) e parameters.SecretAttributes.Expires = &expirationUnixTime } - if _, err := client.SetSecret(ctx, keyVaultBaseUrl, name, parameters); err != nil { - return err - } + recovered := false - // "" indicates the latest version - read, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") - if err != nil { - return err - } - if read.ID == nil { - return fmt.Errorf("Cannot read KeyVault Secret '%s' (in key vault '%s')", name, keyVaultBaseUrl) + if resp, err := client.SetSecret(ctx, keyVaultBaseUrl, name, parameters); err != nil { + // In the case that the Secret already exists in a Soft Deleted / Recoverable state we check if `recover_soft_deleted_key_vaults` is set + // and attempt recovery where appropriate + if meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults && utils.ResponseWasConflict(resp.Response) { + recoveredSecret, err := client.RecoverDeletedSecret(ctx, keyVaultBaseUrl, name) + if err != nil { + return err + } + log.Printf("[DEBUG] Recovering Secret %q with ID: %q", name, *recoveredSecret.ID) + // We need to wait for consistency, recovered Key Vault Child items are not as readily available as newly created + if secret := recoveredSecret.ID; secret != nil { + stateConf := &resource.StateChangeConf{ + Pending: []string{"pending"}, + Target: []string{"available"}, + Refresh: keyVaultSecretRefreshFunc(*secret), + Delay: 30 * time.Second, + PollInterval: 10 * time.Second, + ContinuousTargetOccurence: 10, + Timeout: d.Timeout(schema.TimeoutCreate), + } + + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf("Error waiting for Key Vault Secret %q to become available: %s", name, err) + } + log.Printf("[DEBUG] Secret %q recovered with ID: %q", name, *recoveredSecret.ID) + } + recovered = true + d.SetId(*recoveredSecret.ID) + } else { + // If the error response was anything else, or `recover_soft_deleted_key_vaults` is `false` just return the error + return err + } } - d.SetId(*read.ID) + if !recovered { + // "" indicates the latest version + read, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") + if err != nil { + return err + } + + if read.ID == nil { + return fmt.Errorf("Cannot read KeyVault Secret '%s' (in key vault '%s')", name, keyVaultBaseUrl) + } + + d.SetId(*read.ID) + } return resourceArmKeyVaultSecretRead(d, meta) } @@ -339,3 +373,26 @@ func resourceArmKeyVaultSecretDelete(d *schema.ResourceData, meta interface{}) e _, err = client.DeleteSecret(ctx, id.KeyVaultBaseUrl, id.Name) return err } + +func keyVaultSecretRefreshFunc(secretUri string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + log.Printf("[DEBUG] Checking to see if KeyVault Secret %q is available..", secretUri) + + var PTransport = &http.Transport{Proxy: http.ProxyFromEnvironment} + + client := &http.Client{ + Transport: PTransport, + } + + conn, err := client.Get(secretUri) + if err != nil { + log.Printf("[DEBUG] Didn't find KeyVault secret at %q", secretUri) + return nil, "pending", fmt.Errorf("Error checking secret at %q: %s", secretUri, err) + } + + defer conn.Body.Close() + + log.Printf("[DEBUG] Found KeyVault Secret %q", secretUri) + return "available", "available", nil + } +} diff --git a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_secret_test.go b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_secret_test.go index fc1da116d514..d04e4179256d 100644 --- a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_secret_test.go +++ b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_secret_test.go @@ -146,6 +146,37 @@ func TestAccAzureRMKeyVaultSecret_update(t *testing.T) { }) } +func TestAccAzureRMKeyVaultSecret_recovery(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_key_vault_secret", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMKeyVaultSecretDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMKeyVaultSecret_softDeleteRecovery(data, false), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultSecretExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "value", "rick-and-morty"), + ), + }, + { + Config: testAccAzureRMKeyVaultSecret_softDeleteRecovery(data, false), + Destroy: true, + }, + { + // purge true here to make sure when we end the test there's no soft-deleted items left behind + Config: testAccAzureRMKeyVaultSecret_softDeleteRecovery(data, true), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultSecretExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "value", "rick-and-morty"), + ), + }, + }, + }) +} + func testCheckAzureRMKeyVaultSecretDestroy(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).KeyVault.ManagementClient vaultClient := acceptance.AzureProvider.Meta().(*clients.Client).KeyVault.VaultsClient @@ -437,3 +468,60 @@ resource "azurerm_key_vault_secret" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } + +func testAccAzureRMKeyVaultSecret_softDeleteRecovery(data acceptance.TestData, purge bool) string { + return fmt.Sprintf(` +provider "azurerm" { + features { + key_vault { + purge_soft_delete_on_destroy = "%t" + recover_soft_deleted_key_vaults = true + } + } +} + +data "azurerm_client_config" "current" { +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-kvs-%d" + location = "%s" +} + +resource "azurerm_key_vault" "test" { + name = "acctestkv-%s" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + soft_delete_enabled = true + + sku_name = "standard" + + access_policy { + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + key_permissions = [ + "get", + ] + + secret_permissions = [ + "get", + "recover", + "delete", + "set", + ] + } + + tags = { + environment = "Production" + } +} + +resource "azurerm_key_vault_secret" "test" { + name = "secret-%s" + value = "rick-and-morty" + key_vault_id = azurerm_key_vault.test.id +} +`, purge, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +} diff --git a/azurerm/utils/response.go b/azurerm/utils/response.go index 3141e98a2478..48f1107cb8ab 100644 --- a/azurerm/utils/response.go +++ b/azurerm/utils/response.go @@ -15,6 +15,10 @@ func ResponseWasForbidden(resp autorest.Response) bool { return ResponseWasStatusCode(resp, http.StatusForbidden) } +func ResponseWasConflict(resp autorest.Response) bool { + return ResponseWasStatusCode(resp, http.StatusConflict) +} + func ResponseErrorIsRetryable(err error) bool { if arerr, ok := err.(autorest.DetailedError); ok { err = arerr.Original diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown index c8c7a54f6b64..be96a2fac9a4 100644 --- a/website/docs/index.html.markdown +++ b/website/docs/index.html.markdown @@ -169,6 +169,8 @@ The `key_vault` block supports the following: ~> **Note:** When purge protection is enabled, a key vault or an object in the deleted state cannot be purged until the retention period(90 days) has passed. +~> **Note:** When recovering soft deleted Key Vault items (Keys, Certificates, and Secrets) the Principal used by Terraform needs the `"recover"` permission. + --- The `virtual_machine` block supports the following: From 366b1848282dc546b01fa65ed964da5aac3c8676 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Thu, 30 Apr 2020 15:28:48 +0100 Subject: [PATCH 2/6] added key soft-delete recovery support --- .../keyvault/resource_arm_key_vault_key.go | 48 ++++++-- .../keyvault/resource_arm_key_vault_secret.go | 4 +- .../tests/resource_arm_key_vault_key_test.go | 112 ++++++++++++++++++ 3 files changed, 154 insertions(+), 10 deletions(-) diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go index 339960f390a3..072e554c737f 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault" "github.com/Azure/go-autorest/autorest/date" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" @@ -221,17 +222,48 @@ func resourceArmKeyVaultKeyCreate(d *schema.ResourceData, meta interface{}) erro parameters.KeyAttributes.Expires = &expirationUnixTime } - if _, err := client.CreateKey(ctx, keyVaultBaseUri, name, parameters); err != nil { - return fmt.Errorf("Error Creating Key: %+v", err) - } + recovered := false - // "" indicates the latest version - read, err := client.GetKey(ctx, keyVaultBaseUri, name, "") - if err != nil { - return err + if resp, err := client.CreateKey(ctx, keyVaultBaseUri, name, parameters); err != nil { + if meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults && utils.ResponseWasConflict(resp.Response) { + recoveredKey, err := client.RecoverDeletedKey(ctx, keyVaultBaseUri, name) + if err != nil { + return err + } + log.Printf("[DEBUG] Recovering Key %q with ID: %q", name, *recoveredKey.Key.Kid) + if kid := recoveredKey.Key.Kid; kid != nil { + stateConf := &resource.StateChangeConf{ + Pending: []string{"pending"}, + Target: []string{"available"}, + Refresh: keyVaultChildItemRefreshFunc(*kid), + Delay: 30 * time.Second, + PollInterval: 10 * time.Second, + ContinuousTargetOccurence: 10, + Timeout: d.Timeout(schema.TimeoutCreate), + } + + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf("Error waiting for Key Vault Secret %q to become available: %s", name, err) + } + log.Printf("[DEBUG] Key %q recovered with ID: %q", name, *kid) + } + + recovered = true + d.SetId(*recoveredKey.Key.Kid) + } else { + return fmt.Errorf("Error Creating Key: %+v", err) + } } - d.SetId(*read.Key.Kid) + if !recovered { + // "" indicates the latest version + read, err := client.GetKey(ctx, keyVaultBaseUri, name, "") + if err != nil { + return err + } + + d.SetId(*read.Key.Kid) + } return resourceArmKeyVaultKeyRead(d, meta) } diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go index cc949cd21582..f21e9225ada9 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go @@ -150,7 +150,7 @@ func resourceArmKeyVaultSecretCreate(d *schema.ResourceData, meta interface{}) e stateConf := &resource.StateChangeConf{ Pending: []string{"pending"}, Target: []string{"available"}, - Refresh: keyVaultSecretRefreshFunc(*secret), + Refresh: keyVaultChildItemRefreshFunc(*secret), Delay: 30 * time.Second, PollInterval: 10 * time.Second, ContinuousTargetOccurence: 10, @@ -374,7 +374,7 @@ func resourceArmKeyVaultSecretDelete(d *schema.ResourceData, meta interface{}) e return err } -func keyVaultSecretRefreshFunc(secretUri string) resource.StateRefreshFunc { +func keyVaultChildItemRefreshFunc(secretUri string) resource.StateRefreshFunc { return func() (interface{}, string, error) { log.Printf("[DEBUG] Checking to see if KeyVault Secret %q is available..", secretUri) diff --git a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_key_test.go b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_key_test.go index db84383728a2..be94fcb0ec16 100644 --- a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_key_test.go +++ b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_key_test.go @@ -153,6 +153,43 @@ func TestAccAzureRMKeyVaultKey_complete(t *testing.T) { }) } +func TestAccAzureRMKeyVaultKey_softDeleteRecovery(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_key_vault_key", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMKeyVaultKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMKeyVaultKey_softDeleteRecovery(data, false), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultKeyExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "not_before_date", "2020-01-01T01:02:03Z"), + resource.TestCheckResourceAttr(data.ResourceName, "expiration_date", "2021-01-01T01:02:03Z"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.hello", "world"), + ), + }, + data.ImportStep("key_size"), + { + Config: testAccAzureRMKeyVaultKey_softDeleteRecovery(data, false), + Destroy: true, + }, + { + Config: testAccAzureRMKeyVaultKey_softDeleteRecovery(data, true), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultKeyExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "not_before_date", "2020-01-01T01:02:03Z"), + resource.TestCheckResourceAttr(data.ResourceName, "expiration_date", "2021-01-01T01:02:03Z"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.hello", "world"), + ), + }, + }, + }) +} + func TestAccAzureRMKeyVaultKey_update(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_key_vault_key", "test") @@ -791,3 +828,78 @@ resource "azurerm_key_vault_key" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } + +func testAccAzureRMKeyVaultKey_softDeleteRecovery(data acceptance.TestData, purge bool) string { + return fmt.Sprintf(` +provider "azurerm" { + features { + key_vault { + purge_soft_delete_on_destroy = "%t" + recover_soft_deleted_key_vaults = true + } + } +} + +data "azurerm_client_config" "current" { +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-kvk-%d" + location = "%s" +} + +resource "azurerm_key_vault" "test" { + name = "acctestkv-%s" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + soft_delete_enabled = true + + sku_name = "premium" + + access_policy { + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + key_permissions = [ + "create", + "recover", + "delete", + "get", + ] + + secret_permissions = [ + "get", + "delete", + "set", + ] + } + + tags = { + environment = "Production" + } +} + +resource "azurerm_key_vault_key" "test" { + name = "key-%s" + key_vault_id = azurerm_key_vault.test.id + key_type = "RSA" + key_size = 2048 + not_before_date = "2020-01-01T01:02:03Z" + expiration_date = "2021-01-01T01:02:03Z" + + key_opts = [ + "decrypt", + "encrypt", + "sign", + "unwrapKey", + "verify", + "wrapKey", + ] + + tags = { + "hello" = "world" + } +} +`, purge, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +} From d63759018d3d7ace0fd59218a03a1459f2c8066c Mon Sep 17 00:00:00 2001 From: jackofallops Date: Thu, 30 Apr 2020 16:26:33 +0100 Subject: [PATCH 3/6] added certificate soft-delete recovery support --- .../resource_arm_key_vault_certificate.go | 28 +++- ...resource_arm_key_vault_certificate_test.go | 134 ++++++++++++++++++ 2 files changed, 160 insertions(+), 2 deletions(-) diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_certificate.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_certificate.go index 83068b44f346..bcfab8c27d04 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_certificate.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_certificate.go @@ -383,8 +383,32 @@ func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface CertificatePolicy: &policy, Tags: tags.Expand(t), } - if _, err := client.CreateCertificate(ctx, keyVaultBaseUrl, name, parameters); err != nil { - return err + if resp, err := client.CreateCertificate(ctx, keyVaultBaseUrl, name, parameters); err != nil { + if meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults && utils.ResponseWasConflict(resp.Response) { + recoveredCertificate, err := client.RecoverDeletedCertificate(ctx, keyVaultBaseUrl, name) + if err != nil { + return err + } + log.Printf("[DEBUG] Recovering Secret %q with ID: %q", name, *recoveredCertificate.ID) + if certificate := recoveredCertificate.ID; certificate != nil { + stateConf := &resource.StateChangeConf{ + Pending: []string{"pending"}, + Target: []string{"available"}, + Refresh: keyVaultChildItemRefreshFunc(*certificate), + Delay: 30 * time.Second, + PollInterval: 10 * time.Second, + ContinuousTargetOccurence: 10, + Timeout: d.Timeout(schema.TimeoutCreate), + } + + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf("Error waiting for Key Vault Secret %q to become available: %s", name, err) + } + log.Printf("[DEBUG] Secret %q recovered with ID: %q", name, *recoveredCertificate.ID) + } + } else { + return err + } } log.Printf("[DEBUG] Waiting for Key Vault Certificate %q in Vault %q to be provisioned", name, keyVaultBaseUrl) diff --git a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_certificate_test.go b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_certificate_test.go index fa27aefa2f00..eb16c58b8d25 100644 --- a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_certificate_test.go +++ b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_certificate_test.go @@ -118,6 +118,38 @@ func TestAccAzureRMKeyVaultCertificate_basicGenerate(t *testing.T) { }) } +func TestAccAzureRMKeyVaultCertificate_softDeleteRecovery(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_key_vault_certificate", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMKeyVaultCertificateDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMKeyVaultCertificate_softDeleteRecovery(data, false), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultCertificateExists(data.ResourceName), + resource.TestCheckResourceAttrSet(data.ResourceName, "secret_id"), + resource.TestCheckResourceAttrSet(data.ResourceName, "certificate_data"), + ), + }, + { + Config: testAccAzureRMKeyVaultCertificate_softDeleteRecovery(data, false), + Destroy: true, + }, + { + Config: testAccAzureRMKeyVaultCertificate_softDeleteRecovery(data, true), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKeyVaultCertificateExists(data.ResourceName), + resource.TestCheckResourceAttrSet(data.ResourceName, "secret_id"), + resource.TestCheckResourceAttrSet(data.ResourceName, "certificate_data"), + ), + }, + }, + }) +} + func TestAccAzureRMKeyVaultCertificate_basicGenerateSans(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_key_vault_certificate", "test") @@ -922,3 +954,105 @@ resource "azurerm_key_vault_certificate" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } + +func testAccAzureRMKeyVaultCertificate_softDeleteRecovery(data acceptance.TestData, purge bool) string { + return fmt.Sprintf(` +provider "azurerm" { + features { + key_vault { + purge_soft_delete_on_destroy = "%t" + recover_soft_deleted_key_vaults = true + } + } +} + +data "azurerm_client_config" "current" { +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-kvc-%d" + location = "%s" +} + +resource "azurerm_key_vault" "test" { + name = "acctestkeyvault%s" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + soft_delete_enabled = true + + sku_name = "standard" + + access_policy { + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + certificate_permissions = [ + "create", + "delete", + "get", + "recover", + "update", + ] + + key_permissions = [ + "create", + ] + + secret_permissions = [ + "set", + ] + + storage_permissions = [ + "set", + ] + } +} + +resource "azurerm_key_vault_certificate" "test" { + name = "acctestcert%s" + key_vault_id = azurerm_key_vault.test.id + + certificate_policy { + issuer_parameters { + name = "Self" + } + + key_properties { + exportable = true + key_size = 2048 + key_type = "RSA" + reuse_key = true + } + + lifetime_action { + action { + action_type = "AutoRenew" + } + + trigger { + days_before_expiry = 30 + } + } + + secret_properties { + content_type = "application/x-pkcs12" + } + + x509_certificate_properties { + key_usage = [ + "cRLSign", + "dataEncipherment", + "digitalSignature", + "keyAgreement", + "keyCertSign", + "keyEncipherment", + ] + + subject = "CN=hello-world" + validity_in_months = 12 + } + } +} +`, purge, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +} From 2b9f82a5270a091cef4cc66b02c347922c990416 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Thu, 30 Apr 2020 16:38:26 +0100 Subject: [PATCH 4/6] simplified recovery logic --- .../keyvault/resource_arm_key_vault_key.go | 19 +++++---------- .../keyvault/resource_arm_key_vault_secret.go | 24 +++++++------------ 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go index 072e554c737f..af620ae3f66d 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_key.go @@ -222,8 +222,6 @@ func resourceArmKeyVaultKeyCreate(d *schema.ResourceData, meta interface{}) erro parameters.KeyAttributes.Expires = &expirationUnixTime } - recovered := false - if resp, err := client.CreateKey(ctx, keyVaultBaseUri, name, parameters); err != nil { if meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults && utils.ResponseWasConflict(resp.Response) { recoveredKey, err := client.RecoverDeletedKey(ctx, keyVaultBaseUri, name) @@ -247,24 +245,19 @@ func resourceArmKeyVaultKeyCreate(d *schema.ResourceData, meta interface{}) erro } log.Printf("[DEBUG] Key %q recovered with ID: %q", name, *kid) } - - recovered = true - d.SetId(*recoveredKey.Key.Kid) } else { return fmt.Errorf("Error Creating Key: %+v", err) } } - if !recovered { - // "" indicates the latest version - read, err := client.GetKey(ctx, keyVaultBaseUri, name, "") - if err != nil { - return err - } - - d.SetId(*read.Key.Kid) + // "" indicates the latest version + read, err := client.GetKey(ctx, keyVaultBaseUri, name, "") + if err != nil { + return err } + d.SetId(*read.Key.Kid) + return resourceArmKeyVaultKeyRead(d, meta) } diff --git a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go index f21e9225ada9..328b6985b64a 100644 --- a/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go +++ b/azurerm/internal/services/keyvault/resource_arm_key_vault_secret.go @@ -134,8 +134,6 @@ func resourceArmKeyVaultSecretCreate(d *schema.ResourceData, meta interface{}) e parameters.SecretAttributes.Expires = &expirationUnixTime } - recovered := false - if resp, err := client.SetSecret(ctx, keyVaultBaseUrl, name, parameters); err != nil { // In the case that the Secret already exists in a Soft Deleted / Recoverable state we check if `recover_soft_deleted_key_vaults` is set // and attempt recovery where appropriate @@ -162,28 +160,24 @@ func resourceArmKeyVaultSecretCreate(d *schema.ResourceData, meta interface{}) e } log.Printf("[DEBUG] Secret %q recovered with ID: %q", name, *recoveredSecret.ID) } - recovered = true - d.SetId(*recoveredSecret.ID) } else { // If the error response was anything else, or `recover_soft_deleted_key_vaults` is `false` just return the error return err } } - if !recovered { - // "" indicates the latest version - read, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") - if err != nil { - return err - } - - if read.ID == nil { - return fmt.Errorf("Cannot read KeyVault Secret '%s' (in key vault '%s')", name, keyVaultBaseUrl) - } + // "" indicates the latest version + read, err := client.GetSecret(ctx, keyVaultBaseUrl, name, "") + if err != nil { + return err + } - d.SetId(*read.ID) + if read.ID == nil { + return fmt.Errorf("Cannot read KeyVault Secret '%s' (in key vault '%s')", name, keyVaultBaseUrl) } + d.SetId(*read.ID) + return resourceArmKeyVaultSecretRead(d, meta) } From 24e224651906fc1b256425c4c62697af2bf495d8 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Fri, 1 May 2020 11:19:01 +0100 Subject: [PATCH 5/6] fix purge_protection keyvault tests --- .../keyvault/tests/resource_arm_key_vault_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go index cd6507acea42..ba8bc87d63c3 100644 --- a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go +++ b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go @@ -385,7 +385,7 @@ func TestAccAzureRMKeyVault_purgeProtectionEnabled(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMKeyVaultExists(data.ResourceName), resource.TestCheckResourceAttr(data.ResourceName, "purge_protection_enabled", "true"), - resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "false"), + resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "true"), // API rejects false if purge protection is enabled ), }, data.ImportStep(), @@ -436,7 +436,7 @@ func TestAccAzureRMKeyVault_purgeProtectionViaUpdate(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMKeyVaultExists(data.ResourceName), resource.TestCheckResourceAttr(data.ResourceName, "purge_protection_enabled", "true"), - resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "false"), + resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "true"), // API rejects false if purge protection is enabled ), }, data.ImportStep(), @@ -1084,9 +1084,10 @@ resource "azurerm_key_vault" "test" { resource_group_name = azurerm_resource_group.test.name tenant_id = data.azurerm_client_config.current.tenant_id sku_name = "premium" - purge_protection_enabled = %t + soft_delete_enabled = "%t" + purge_protection_enabled = "%t" } -`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, enabled) +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, enabled, enabled) } func testAccAzureRMKeyVault_softDelete(data acceptance.TestData, enabled bool) string { From 54df90c15d4d039ac1d044f5be80afefef86d889 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Fri, 1 May 2020 11:49:10 +0100 Subject: [PATCH 6/6] missed a fix --- .../services/keyvault/tests/resource_arm_key_vault_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go index ba8bc87d63c3..673e5d7b95ba 100644 --- a/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go +++ b/azurerm/internal/services/keyvault/tests/resource_arm_key_vault_test.go @@ -457,7 +457,7 @@ func TestAccAzureRMKeyVault_purgeProtectionAttemptToDisable(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMKeyVaultExists(data.ResourceName), resource.TestCheckResourceAttr(data.ResourceName, "purge_protection_enabled", "true"), - resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "false"), + resource.TestCheckResourceAttr(data.ResourceName, "soft_delete_enabled", "true"), ), }, data.ImportStep(),