From 0cfd63cb3ebf7d9f665608c2e995cbc0636301c1 Mon Sep 17 00:00:00 2001
From: James Nugent <james@jen20.com>
Date: Wed, 20 Feb 2019 09:59:53 -0800
Subject: [PATCH 1/2] Add acctest for Key Vault Policy with bad Vault ID

This commit adds an acceptance test reproducing the scenario of attempting to
create an `azurerm_key_vault_access_policy` resource when specifying the ID of a
Key Vault which does not exist, but is in the correct URI-path-style format. It
generates these by appending the string "NOPE" to the ID of a valid Key Vault
(which the tests creates), but could equally truncate the name (which is how
this was encountered in the first place).

Applying this particular configuration, I expect an error - probably of the form
"Error retrieving Key Vault" (but the exact text is arbitrary). Instead, we see
that no error is produced.

If this scenario is run via the CLI, the CLI marks the resource as successfully
created, but no state is created for it.
---
 ...source_arm_key_vault_access_policy_test.go | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/azurerm/resource_arm_key_vault_access_policy_test.go b/azurerm/resource_arm_key_vault_access_policy_test.go
index c3f67e4aec73..65b5364fb1d5 100644
--- a/azurerm/resource_arm_key_vault_access_policy_test.go
+++ b/azurerm/resource_arm_key_vault_access_policy_test.go
@@ -2,6 +2,7 @@ package azurerm
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -175,6 +176,24 @@ func TestAccAzureRMKeyVaultAccessPolicy_update(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMKeyVaultAccessPolicy_nonExistentVault(t *testing.T) {
+	rs := acctest.RandString(6)
+	config := testAccAzureRMKeyVaultAccessPolicy_nonExistentVault(rs, testLocation())
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMKeyVaultDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:             config,
+				ExpectNonEmptyPlan: true,
+				ExpectError:        regexp.MustCompile(`Error retrieving Key Vault`),
+			},
+		},
+	})
+}
+
 func testCheckAzureRMKeyVaultAccessPolicyExists(resourceName string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		client := testAccProvider.Meta().(*ArmClient).keyVaultClient
@@ -386,3 +405,45 @@ resource "azurerm_key_vault" "test" {
 }
 `, rString, location, rString)
 }
+
+func testAccAzureRMKeyVaultAccessPolicy_nonExistentVault(rString string, location string) string {
+	return fmt.Sprintf(`
+data "azurerm_client_config" "current" {}
+
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-%s"
+  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}"
+
+  sku {
+    name = "standard"
+  }
+
+  tags {
+    environment = "Production"
+  }
+}
+
+resource "azurerm_key_vault_access_policy" "test" {
+  # Must appear to be URL, but not actually exist - appending a string works
+  key_vault_id = "${azurerm_key_vault.test.id}NOPE"
+
+  tenant_id = "${data.azurerm_client_config.current.tenant_id}"
+  object_id = "${data.azurerm_client_config.current.service_principal_object_id}"
+
+  key_permissions = [
+    "get",
+  ]
+
+  secret_permissions = [
+    "get",
+  ]
+}
+`, rString, location, rString)
+}

From e99279ba26b46fd7339f313cf7fccc566cf06955 Mon Sep 17 00:00:00 2001
From: James Nugent <james@jen20.com>
Date: Thu, 21 Feb 2019 08:17:58 -0800
Subject: [PATCH 2/2] Remove key vault policy from state iff it existed

This fixes the test added in the previous commit, while retaining the ability to
reflect access policy deletion by deleting the key vault itself.
---
 azurerm/resource_arm_key_vault_access_policy.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/azurerm/resource_arm_key_vault_access_policy.go b/azurerm/resource_arm_key_vault_access_policy.go
index 1118d33fa202..b4e31181f6f2 100644
--- a/azurerm/resource_arm_key_vault_access_policy.go
+++ b/azurerm/resource_arm_key_vault_access_policy.go
@@ -145,7 +145,11 @@ func resourceArmKeyVaultAccessPolicyCreateOrDelete(d *schema.ResourceData, meta
 
 	keyVault, err := client.Get(ctx, resourceGroup, vaultName)
 	if err != nil {
-		if utils.ResponseWasNotFound(keyVault.Response) {
+		// 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)
 			d.SetId("")
 			return nil