Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Feb 7, 2019
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
daa6ac3
ket vault children: switch from url to id
katbyte Jan 31, 2019
b3d4d13
fix vault based tests & update docs
katbyte Feb 1, 2019
156001d
Apply suggestions from code review
katbyte Feb 5, 2019
4426543
Update azurerm/resource_arm_key_vault_certificate.go
katbyte Feb 6, 2019
46fb0b5
Update website/docs/r/key_vault_certificate.html.markdown
tombuildsstuff Feb 6, 2019
dba2e8d
renamed value_id to key_vault_id and did data sources
katbyte Feb 7, 2019
f849240
dimplified key vault child importer
katbyte Feb 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions azurerm/helpers/azure/key_vault.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package azure
tombuildsstuff marked this conversation as resolved.
Show resolved Hide resolved

import (
"context"
"fmt"
"log"

"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

func GetKeyVaultBaseUrlFromID(ctx context.Context, client keyvault.VaultsClient, keyVaultId string) (string, error) {

if keyVaultId == "" {
return "", fmt.Errorf("keyVaultId is empty")
}

id, err := ParseAzureResourceID(keyVaultId)
if err != nil {
return "", err
}
resourceGroup := id.ResourceGroup

vaultName, ok := id.Path["vaults"]
if !ok {
return "", fmt.Errorf("resource id does not contain `valuts`: %q", keyVaultId)
katbyte marked this conversation as resolved.
Show resolved Hide resolved
}

resp, err := client.Get(ctx, resourceGroup, vaultName)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
return "", fmt.Errorf("Error unable to find KeyVault %q (Resource Group %q): %+v", vaultName, resourceGroup, err)
}
return "", fmt.Errorf("Error making Read request on KeyVault %q (Resource Group %q): %+v", vaultName, resourceGroup, err)
}

if resp.Properties == nil || resp.Properties.VaultURI == nil {
return "", fmt.Errorf("vault (%s) response properties or VaultURI is nil", keyVaultId)
}

return *resp.Properties.VaultURI, nil
}

func GetKeyVaultIDFromBaseUrl(ctx context.Context, client keyvault.VaultsClient, keyVaultUrl string) (string, error) {

list, err := client.ListComplete(ctx, utils.Int32(1))
katbyte marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return "", fmt.Errorf("Error GetKeyVaultId unable to list Key Vaults %v", err)
}

for list.NotDone() {
v := list.Value()
if v.ID == nil {
log.Printf("[DEBUG]GetKeyVaultId: v.ID was nil, continuing")
continue
}

vid, err := ParseAzureResourceID(*v.ID)
if err != nil {
log.Printf("[DEBUG] GetKeyVaultId: unable to parse v.ID (%s): %v", *v.ID, err)
continue
}
resourceGroup := vid.ResourceGroup
name := vid.Path["vaults"]

//resp does not appear to contain the vault properties, so lets fech them
get, err := client.Get(ctx, resourceGroup, name)
if err != nil {
log.Printf("[DEBUG] GetKeyVaultId: Error making Read request on KeyVault %q (Resource Group %q): %+v", name, resourceGroup, err)
continue
}

if get.ID == nil || get.Properties == nil || get.Properties.VaultURI == nil {
log.Printf("[DEBUG] GetKeyVaultId: KeyVault %q (Resource Group %q) has nil ID, properties or vault URI", name, resourceGroup)
continue
}

if keyVaultUrl == *get.Properties.VaultURI {
return *get.ID, nil
}

e := list.NextWithContext(ctx)
if e != nil {
return "", fmt.Errorf("Error GetKeyVaultId: Error getting next value on KeyVault %q (Resource Group %q): %+v", name, resourceGroup, err)
}
}

return "", fmt.Errorf("Error GetKeyVaultId unable to find Key Vault with url %q", keyVaultUrl)
}

func KeyVaultExists(ctx context.Context, client keyvault.VaultsClient, keyVaultId string) (bool, error) {

if keyVaultId == "" {
return false, fmt.Errorf("keyVaultId is empty")
}

id, err := ParseAzureResourceID(keyVaultId)
if err != nil {
return false, err
}
resourceGroup := id.ResourceGroup

vaultName, ok := id.Path["vaults"]
if !ok {
return false, fmt.Errorf("resource id does not contain `valuts`: %q", keyVaultId)
katbyte marked this conversation as resolved.
Show resolved Hide resolved
}

resp, err := client.Get(ctx, resourceGroup, vaultName)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
return false, nil
}
return false, fmt.Errorf("Error making Read request on KeyVault %q (Resource Group %q): %+v", vaultName, resourceGroup, err)
}

if resp.Properties == nil || resp.Properties.VaultURI == nil {
return false, fmt.Errorf("vault (%s) response properties or VaultURI is nil", keyVaultId)
}

return true, nil
}
117 changes: 113 additions & 4 deletions azurerm/resource_arm_key_vault_certificate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,70 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

//todo refactor and find a home for this wayward func
func resourceArmKeyVaultChildResourceImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
client := meta.(*ArmClient).keyVaultClient
ctx := meta.(*ArmClient).StopContext

id, err := azure.ParseKeyVaultChildID(d.Id())
if err != nil {
return []*schema.ResourceData{d}, fmt.Errorf("Error Unable to parse ID (%s) for Key Vault Child import: %v", d.Id(), err)
}

list, err := client.ListComplete(ctx, utils.Int32(1))
katbyte marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return []*schema.ResourceData{d}, fmt.Errorf("Error Unable to list Key Vaults for Key Vault Child import: %v", err)
}

for list.NotDone() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we not reuse the helper function here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reuse which helper function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be able to call GetKeyVaultBaseUrlFromID?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally forgot, thanks for the reminder!

v := list.Value()
if v.ID == nil {
log.Printf("[DEBUG] Key Vault Child import: v.ID was nil, continuing")
continue
}

vid, err := parseAzureResourceID(*v.ID)
if err != nil {
log.Printf("[DEBUG] Key Vault Child import: unable to parse v.ID (%s): %v", *v.ID, err)
continue
}
resourceGroup := vid.ResourceGroup
name := vid.Path["vaults"]

//resp does not appear to contain the vault properties, so lets fech them
get, err := client.Get(ctx, resourceGroup, name)
if err != nil {
log.Printf("[DEBUG] Key Vault Child import: Error making Read request on KeyVault %q (Resource Group %q): %+v", name, resourceGroup, err)
continue
}

if get.ID == nil || get.Properties == nil || get.Properties.VaultURI == nil {
log.Printf("[DEBUG] Key Vault Child import: KeyVault %q (Resource Group %q) has nil ID, properties or vault URI", name, resourceGroup)
continue
}

if id.KeyVaultBaseUrl == *get.Properties.VaultURI {
d.Set("vault_id", *get.ID)
katbyte marked this conversation as resolved.
Show resolved Hide resolved
break
}

e := list.NextWithContext(ctx)
if e != nil {
return []*schema.ResourceData{d}, e
}
}

return []*schema.ResourceData{d}, nil
}

func resourceArmKeyVaultCertificate() *schema.Resource {
return &schema.Resource{
// TODO: support Updating once we have more information about what can be updated
Create: resourceArmKeyVaultCertificateCreate,
Read: resourceArmKeyVaultCertificateRead,
Delete: resourceArmKeyVaultCertificateDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
State: resourceArmKeyVaultChildResourceImporter,
},

Schema: map[string]*schema.Schema{
Expand All @@ -37,10 +93,21 @@ func resourceArmKeyVaultCertificate() *schema.Resource {
ValidateFunc: azure.ValidateKeyVaultChildName,
},

"vault_id": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to better match the other resources I feel like this could make more sense as key_vault_id?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change key_vault.vault_uri key_vault.key_vault_uri` as well then?

Copy link
Contributor

@tombuildsstuff tombuildsstuff Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could do - but it's called VaultUri in the API (whereas the other is a Terraform specific field) - so I feel like it'd be best not too - WDYT?

Copy link
Collaborator Author

@katbyte katbyte Feb 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems inconsistent to have one be key_vault_id and one vault_uri when they are essential the same?

Type: schema.TypeString,
Optional: true, //todo required in 2.0
Computed: true, //todo required in 2.0
ForceNew: true,
ValidateFunc: azure.ValidateResourceID,
},

//todo remove in 2.0
"vault_uri": {
Type: schema.TypeString,
Required: true,
Optional: true,
ForceNew: true,
Computed: true,
Deprecated: "This property has been deprecated in favour of the vault_id property. This will prevent a class of bugs as described in https://github.com/terraform-providers/terraform-provider-azurerm/issues/2396 and will be removed in version 2.0 of the provider",
ValidateFunc: validate.URLIsHTTPS,
},

Expand Down Expand Up @@ -298,12 +365,33 @@ func resourceArmKeyVaultCertificate() *schema.Resource {
}

func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface{}) error {
vaultClient := meta.(*ArmClient).keyVaultClient
client := meta.(*ArmClient).keyVaultManagementClient
ctx := meta.(*ArmClient).StopContext

name := d.Get("name").(string)
keyVaultId := d.Get("vault_id").(string)
keyVaultBaseUrl := d.Get("vault_uri").(string)

if keyVaultBaseUrl == "" {
if keyVaultId == "" {
return fmt.Errorf("one of `vault_id` or `vault_uri` must be set")
}

pKeyVaultBaseUrl, err := azure.GetKeyVaultBaseUrlFromID(ctx, vaultClient, keyVaultId)
if err != nil {
return fmt.Errorf("Error looking up Certificate %q vault url form id %q: %+v", name, keyVaultId, err)
}

keyVaultBaseUrl = pKeyVaultBaseUrl
} else {
id, err := azure.GetKeyVaultIDFromBaseUrl(ctx, vaultClient, keyVaultBaseUrl)
if err != nil {
return fmt.Errorf("Error unable to find key vault ID from URL %q for certificate %q: %+v", keyVaultBaseUrl, name, err)
}
d.Set("vault_id", id)
}

if requireResourcesToBeImported {
existing, err := client.GetCertificate(ctx, keyVaultBaseUrl, name, "")
if err != nil {
Expand Down Expand Up @@ -384,21 +472,31 @@ func resourceArmKeyVaultCertificateRead(d *schema.ResourceData, meta interface{}
client := meta.(*ArmClient).keyVaultManagementClient
ctx := meta.(*ArmClient).StopContext

keyVaultId := d.Get("vault_id").(string)
id, err := azure.ParseKeyVaultChildID(d.Id())
if err != nil {
return err
}

cert, err := client.GetCertificate(ctx, id.KeyVaultBaseUrl, id.Name, "")
ok, err := azure.KeyVaultExists(ctx, meta.(*ArmClient).keyVaultClient, keyVaultId)
if err != nil {
return fmt.Errorf("Error checking if key vault %q for Certificate %q in Vault at url %q exists: %v", keyVaultId, id.Name, id.KeyVaultBaseUrl, err)
}
if !ok {
log.Printf("[DEBUG] Certificate %q Key Vault %q was not found in Key Vault at URI %q - removing from state", id.Name, keyVaultId, id.KeyVaultBaseUrl)
d.SetId("")
return nil
}

cert, err := client.GetCertificate(ctx, id.KeyVaultBaseUrl, id.Name, "")
if err != nil {
if utils.ResponseWasNotFound(cert.Response) {
log.Printf("[DEBUG] Certificate %q was not found in Key Vault at URI %q - removing from state", id.Name, id.KeyVaultBaseUrl)
d.SetId("")
return nil
}

return err
return fmt.Errorf("Error reading Key Vault Certificate Policy: %+v", err)
katbyte marked this conversation as resolved.
Show resolved Hide resolved
}

d.Set("name", id.Name)
Expand Down Expand Up @@ -434,11 +532,22 @@ func resourceArmKeyVaultCertificateDelete(d *schema.ResourceData, meta interface
client := meta.(*ArmClient).keyVaultManagementClient
ctx := meta.(*ArmClient).StopContext

keyVaultId := d.Get("vault_id").(string)
id, err := azure.ParseKeyVaultChildID(d.Id())
if err != nil {
return err
}

ok, err := azure.KeyVaultExists(ctx, meta.(*ArmClient).keyVaultClient, keyVaultId)
if err != nil {
return fmt.Errorf("Error checking if key vault %q for Certificate %q in Vault at url %q exists: %v", keyVaultId, id.Name, id.KeyVaultBaseUrl, err)
}
if !ok {
log.Printf("[DEBUG] Certificate %q Key Vault %q was not found in Key Vault at URI %q - removing from state", id.Name, keyVaultId, id.KeyVaultBaseUrl)
d.SetId("")
return nil
}

resp, err := client.DeleteCertificate(ctx, id.KeyVaultBaseUrl, id.Name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
Expand Down
Loading