Switch Key Vault sub resources to use Key Vault ID instead of URL #2820

katbyte · 2019-02-01T09:18:46Z

This will allow us to easily check if the key vault exists before doing a update/read/delete on it causing a NXDOMAIN error (as the domain used for the API call vanishes when the key vault is deleted)

fixes #2396

katbyte · 2019-02-01T09:24:56Z

Tests:

[01:09:20] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$ testazure TestAccAzureRMKeyVaultSecret
==> Fixing source code with gofmt...
# This logic should match the search logic in scripts/gofmtcheck.sh
gofmt -s -w `find . -name '*.go' | grep -v vendor`
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccAzureRMKeyVaultSecret -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKeyVaultSecret_basic
=== PAUSE TestAccAzureRMKeyVaultSecret_basic
=== RUN   TestAccAzureRMKeyVaultSecret_requiresImport
=== PAUSE TestAccAzureRMKeyVaultSecret_requiresImport
=== RUN   TestAccAzureRMKeyVaultSecret_disappears
=== PAUSE TestAccAzureRMKeyVaultSecret_disappears
=== RUN   TestAccAzureRMKeyVaultSecret_disappearsWhenParentKeyVaultDeleted
=== PAUSE TestAccAzureRMKeyVaultSecret_disappearsWhenParentKeyVaultDeleted
=== RUN   TestAccAzureRMKeyVaultSecret_complete
=== PAUSE TestAccAzureRMKeyVaultSecret_complete
=== RUN   TestAccAzureRMKeyVaultSecret_update
=== PAUSE TestAccAzureRMKeyVaultSecret_update
=== CONT  TestAccAzureRMKeyVaultSecret_basic
=== CONT  TestAccAzureRMKeyVaultSecret_complete
=== CONT  TestAccAzureRMKeyVaultSecret_disappears
=== CONT  TestAccAzureRMKeyVaultSecret_disappearsWhenParentKeyVaultDeleted
=== CONT  TestAccAzureRMKeyVaultSecret_update
=== CONT  TestAccAzureRMKeyVaultSecret_requiresImport
--- PASS: TestAccAzureRMKeyVaultSecret_disappearsWhenParentKeyVaultDeleted (207.41s)
--- PASS: TestAccAzureRMKeyVaultSecret_disappears (209.66s)
--- PASS: TestAccAzureRMKeyVaultSecret_complete (216.53s)
--- PASS: TestAccAzureRMKeyVaultSecret_basic (219.29s)
--- PASS: TestAccAzureRMKeyVaultSecret_requiresImport (222.59s)
--- PASS: TestAccAzureRMKeyVaultSecret_update (238.51s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	238.587s
[01:19:20] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$

[01:19:20] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$ testazure TestAccAzureRMKeyVaultKey
==> Fixing source code with gofmt...
# This logic should match the search logic in scripts/gofmtcheck.sh
gofmt -s -w `find . -name '*.go' | grep -v vendor`
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccAzureRMKeyVaultKey -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKeyVaultKey_basicEC
=== PAUSE TestAccAzureRMKeyVaultKey_basicEC
=== RUN   TestAccAzureRMKeyVaultKey_requiresImport
=== PAUSE TestAccAzureRMKeyVaultKey_requiresImport
=== RUN   TestAccAzureRMKeyVaultKey_basicRSA
=== PAUSE TestAccAzureRMKeyVaultKey_basicRSA
=== RUN   TestAccAzureRMKeyVaultKey_basicRSAHSM
=== PAUSE TestAccAzureRMKeyVaultKey_basicRSAHSM
=== RUN   TestAccAzureRMKeyVaultKey_complete
=== PAUSE TestAccAzureRMKeyVaultKey_complete
=== RUN   TestAccAzureRMKeyVaultKey_update
=== PAUSE TestAccAzureRMKeyVaultKey_update
=== RUN   TestAccAzureRMKeyVaultKey_disappears
=== PAUSE TestAccAzureRMKeyVaultKey_disappears
=== RUN   TestAccAzureRMKeyVaultKey_disappearsWhenParentKeyVaultDeleted
=== PAUSE TestAccAzureRMKeyVaultKey_disappearsWhenParentKeyVaultDeleted
=== CONT  TestAccAzureRMKeyVaultKey_basicEC
=== CONT  TestAccAzureRMKeyVaultKey_update
=== CONT  TestAccAzureRMKeyVaultKey_basicRSAHSM
=== CONT  TestAccAzureRMKeyVaultKey_complete
=== CONT  TestAccAzureRMKeyVaultKey_disappears
=== CONT  TestAccAzureRMKeyVaultKey_disappearsWhenParentKeyVaultDeleted
=== CONT  TestAccAzureRMKeyVaultKey_basicRSA
=== CONT  TestAccAzureRMKeyVaultKey_requiresImport
--- PASS: TestAccAzureRMKeyVaultKey_disappearsWhenParentKeyVaultDeleted (207.71s)
--- PASS: TestAccAzureRMKeyVaultKey_disappears (209.58s)
--- PASS: TestAccAzureRMKeyVaultKey_basicRSA (217.37s)
--- PASS: TestAccAzureRMKeyVaultKey_basicEC (217.47s)
--- PASS: TestAccAzureRMKeyVaultKey_basicRSAHSM (217.85s)
--- PASS: TestAccAzureRMKeyVaultKey_complete (219.02s)
--- PASS: TestAccAzureRMKeyVaultKey_requiresImport (222.09s)
--- PASS: TestAccAzureRMKeyVaultKey_update (236.26s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	236.298s
[01:23:48] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$
[01:09:30] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$ make fmt; make testacc TEST=./azurerm TESTARGS=-test.run=TestAccAzureRMKeyVaultCertificate
==> Fixing source code with gofmt...
# This logic should match the search logic in scripts/gofmtcheck.sh
gofmt -s -w `find . -name '*.go' | grep -v vendor`
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccAzureRMKeyVaultCertificate -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKeyVaultCertificate_basicImportPFX
=== PAUSE TestAccAzureRMKeyVaultCertificate_basicImportPFX
=== RUN   TestAccAzureRMKeyVaultCertificate_requiresImport
=== PAUSE TestAccAzureRMKeyVaultCertificate_requiresImport
=== RUN   TestAccAzureRMKeyVaultCertificate_disappears
=== PAUSE TestAccAzureRMKeyVaultCertificate_disappears
=== RUN   TestAccAzureRMKeyVaultCertificate_disappearsWhenParentKeyVaultDeleted
=== PAUSE TestAccAzureRMKeyVaultCertificate_disappearsWhenParentKeyVaultDeleted
=== RUN   TestAccAzureRMKeyVaultCertificate_basicGenerate
=== PAUSE TestAccAzureRMKeyVaultCertificate_basicGenerate
=== RUN   TestAccAzureRMKeyVaultCertificate_basicGenerateSans
=== PAUSE TestAccAzureRMKeyVaultCertificate_basicGenerateSans
=== RUN   TestAccAzureRMKeyVaultCertificate_basicGenerateTags
=== PAUSE TestAccAzureRMKeyVaultCertificate_basicGenerateTags
=== RUN   TestAccAzureRMKeyVaultCertificate_basicExtendedKeyUsage
=== PAUSE TestAccAzureRMKeyVaultCertificate_basicExtendedKeyUsage
=== CONT  TestAccAzureRMKeyVaultCertificate_basicImportPFX
=== CONT  TestAccAzureRMKeyVaultCertificate_basicGenerateSans
=== CONT  TestAccAzureRMKeyVaultCertificate_requiresImport
=== CONT  TestAccAzureRMKeyVaultCertificate_basicExtendedKeyUsage
=== CONT  TestAccAzureRMKeyVaultCertificate_disappearsWhenParentKeyVaultDeleted
=== CONT  TestAccAzureRMKeyVaultCertificate_basicGenerate
=== CONT  TestAccAzureRMKeyVaultCertificate_disappears
=== CONT  TestAccAzureRMKeyVaultCertificate_basicGenerateTags
--- PASS: TestAccAzureRMKeyVaultCertificate_basicGenerateSans (214.40s)
--- PASS: TestAccAzureRMKeyVaultCertificate_basicImportPFX (219.39s)
--- PASS: TestAccAzureRMKeyVaultCertificate_requiresImport (225.42s)
--- PASS: TestAccAzureRMKeyVaultCertificate_disappearsWhenParentKeyVaultDeleted (226.00s)
--- PASS: TestAccAzureRMKeyVaultCertificate_disappears (228.36s)
--- PASS: TestAccAzureRMKeyVaultCertificate_basicGenerate (230.92s)
--- PASS: TestAccAzureRMKeyVaultCertificate_basicExtendedKeyUsage (239.43s)

tombuildsstuff

I've left a few comments inline but this is looking good - if we can apply this fix to the other Key Vault resource/data sources this should be otherwise good to merge 👍

azurerm/helpers/azure/key_vault.go

azurerm/resource_arm_key_vault_certificate.go

tombuildsstuff · 2019-02-05T21:49:58Z

azurerm/resource_arm_key_vault_secret.go

@@ -30,10 +30,21 @@ func resourceArmKeyVaultSecret() *schema.Resource {
 				ValidateFunc: azure.ValidateKeyVaultChildName,
 			},

+			"vault_id": {


(as above) would this make more sense as key_vault_id?

azurerm/helpers/azure/key_vault.go

website/docs/r/key_vault_secret.html.markdown

website/docs/r/key_vault_key.html.markdown

website/docs/r/key_vault_certificate.html.markdown

Co-Authored-By: katbyte <[email protected]>

katbyte · 2019-02-07T05:35:15Z

ds tests:

[21:22:13] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$ testazure TestAccDataSourceAzureRMKeyVault
==> Fixing source code with gofmt...
# This logic should match the search logic in scripts/gofmtcheck.sh
gofmt -s -w `find . -name '*.go' | grep -v vendor`
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccDataSourceAzureRMKeyVault -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_key
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_key
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_secret
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_secret
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_certificate
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_certificate
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecret
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecret
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_keyCertificate
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_keyCertificate
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_secretCertificate
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_secretCertificate
=== RUN   TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecretCertificate
=== PAUSE TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecretCertificate
=== RUN   TestAccDataSourceAzureRMKeyVaultKey_complete
=== PAUSE TestAccDataSourceAzureRMKeyVaultKey_complete
=== RUN   TestAccDataSourceAzureRMKeyVaultSecret_basic
=== PAUSE TestAccDataSourceAzureRMKeyVaultSecret_basic
=== RUN   TestAccDataSourceAzureRMKeyVaultSecret_complete
=== PAUSE TestAccDataSourceAzureRMKeyVaultSecret_complete
=== RUN   TestAccDataSourceAzureRMKeyVault_basic
=== PAUSE TestAccDataSourceAzureRMKeyVault_basic
=== RUN   TestAccDataSourceAzureRMKeyVault_complete
=== PAUSE TestAccDataSourceAzureRMKeyVault_complete
=== RUN   TestAccDataSourceAzureRMKeyVault_networkAcls
=== PAUSE TestAccDataSourceAzureRMKeyVault_networkAcls
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_key
=== CONT  TestAccDataSourceAzureRMKeyVaultKey_complete
=== CONT  TestAccDataSourceAzureRMKeyVault_complete
=== CONT  TestAccDataSourceAzureRMKeyVaultSecret_complete
=== CONT  TestAccDataSourceAzureRMKeyVault_basic
=== CONT  TestAccDataSourceAzureRMKeyVaultSecret_basic
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_keyCertificate
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecretCertificate
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_secretCertificate
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecretCertificate (22.98s)
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_key (23.09s)
=== CONT  TestAccDataSourceAzureRMKeyVault_networkAcls
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_keyCertificate (23.21s)
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_certificate
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_certificate (20.36s)
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecret
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_secretCertificate (20.69s)
=== CONT  TestAccDataSourceAzureRMKeyVaultAccessPolicy_secret
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_secret (20.68s)
--- PASS: TestAccDataSourceAzureRMKeyVaultAccessPolicy_keySecret (20.80s)
--- PASS: TestAccDataSourceAzureRMKeyVaultSecret_basic (217.07s)
--- PASS: TestAccDataSourceAzureRMKeyVault_complete (218.80s)
--- PASS: TestAccDataSourceAzureRMKeyVault_basic (220.04s)
--- PASS: TestAccDataSourceAzureRMKeyVaultSecret_complete (231.18s)
--- PASS: TestAccDataSourceAzureRMKeyVaultKey_complete (231.94s)
--- PASS: TestAccDataSourceAzureRMKeyVault_networkAcls (273.55s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	296.690s
[21:30:09] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸keyvault/refactor_children$

mbfrahry

LGTM minus the follow up comment

mbfrahry · 2019-02-07T16:10:59Z

azurerm/resource_arm_key_vault_certificate.go

+		return []*schema.ResourceData{d}, fmt.Errorf("Error Unable to list Key Vaults for Key Vault Child import: %v", err)
+	}
+
+	for list.NotDone() {


Just following up on this thread. Do we need to do anything with this?

hbuckle · 2019-02-11T13:39:21Z

It seems that vault_uri no longer works at all now (I know it is deprecated, but it should still work) as the logic to handle vault_uri or key_vault_id is only present in resourceArmKeyVaultKeyCreate - the other functions are only using key_vault_id

error:

azurerm_key_vault_key.key: Error checking if key vault "" for Key "key" in Vault at url "https://somevault.vault.azure.net/" exists: keyVaultId is empty

katbyte · 2019-02-12T08:08:53Z

Thanks for the heads up @hbuckle. This is definitely a regression and being tracked in #2865, with a fix in #2874

ivanignatiev · 2019-07-29T15:32:59Z

Verification helper functions are searching for KeyVault in the current context when KeyVault can be placed completely in other subscription which allowed to access if subscriptions are under the same tenant.

Deprecation of vault_uri parameter is blocking deployments in the cases of multi-subscription environments and raises "Resource Group not found" error.

If we precise vault_uri it will not find Key Vault because it searching Key Vault in the current subscription when it is placed in another one.

ghost · 2019-07-30T14:50:30Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

katbyte added 2 commits January 31, 2019 00:49

ket vault children: switch from url to id

daa6ac3

fix vault based tests & update docs

b3d4d13

ghost added size/XL documentation labels Feb 1, 2019

katbyte added this to the 1.22.0 milestone Feb 1, 2019

katbyte requested a review from tombuildsstuff February 1, 2019 09:18

katbyte added bug service/key-vault Key Vault labels Feb 1, 2019

katbyte requested a review from a team February 5, 2019 17:49

tombuildsstuff suggested changes Feb 5, 2019

View reviewed changes

katbyte and others added 4 commits February 5, 2019 14:35

Apply suggestions from code review

156001d

Co-Authored-By: katbyte <[email protected]>

Update azurerm/resource_arm_key_vault_certificate.go

4426543

Update website/docs/r/key_vault_certificate.html.markdown

46fb0b5

Co-Authored-By: katbyte <[email protected]>

renamed value_id to key_vault_id and did data sources

dba2e8d

mbfrahry approved these changes Feb 7, 2019

View reviewed changes

dimplified key vault child importer

f849240

katbyte force-pushed the keyvault/refactor_children branch from cfeb4ac to f849240 Compare February 7, 2019 18:51

katbyte merged commit 2b0fc5e into master Feb 7, 2019

katbyte deleted the keyvault/refactor_children branch February 7, 2019 20:37

katbyte added a commit that referenced this pull request Feb 7, 2019

Update CHNAGELOG.md to include #2820

2736d39

lukehoban mentioned this pull request Feb 13, 2019

Error with keyvault after upgrade to 0.16.8. pulumi/pulumi-azure#180

Closed

javisst mentioned this pull request Jun 26, 2019

Fix documentation for azurerm_key_vault_access_policy #3733

Merged

ivanignatiev mentioned this pull request Jul 29, 2019

Key Vault Key Not Found for the multi-subscription case #3959

Closed

ghost locked and limited conversation to collaborators Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch Key Vault sub resources to use Key Vault ID instead of URL #2820

Switch Key Vault sub resources to use Key Vault ID instead of URL #2820

katbyte commented Feb 1, 2019

katbyte commented Feb 1, 2019

tombuildsstuff left a comment

tombuildsstuff Feb 5, 2019

katbyte commented Feb 7, 2019

mbfrahry left a comment

mbfrahry Feb 7, 2019

hbuckle commented Feb 11, 2019

katbyte commented Feb 12, 2019

ivanignatiev commented Jul 29, 2019 •

edited

Loading

ghost commented Jul 30, 2019

Switch Key Vault sub resources to use Key Vault ID instead of URL #2820

Switch Key Vault sub resources to use Key Vault ID instead of URL #2820

Conversation

katbyte commented Feb 1, 2019

katbyte commented Feb 1, 2019

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Feb 5, 2019

Choose a reason for hiding this comment

katbyte commented Feb 7, 2019

mbfrahry left a comment

Choose a reason for hiding this comment

mbfrahry Feb 7, 2019

Choose a reason for hiding this comment

hbuckle commented Feb 11, 2019

katbyte commented Feb 12, 2019

ivanignatiev commented Jul 29, 2019 • edited Loading

ghost commented Jul 30, 2019

ivanignatiev commented Jul 29, 2019 •

edited

Loading