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.

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

fix perfomance issue of keyvault #6866

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
88 changes: 20 additions & 68 deletions azurerm/helpers/azure/key_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,89 +3,41 @@ package azure
import (
"context"
"fmt"
"regexp"

"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault"
"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
"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
func GetKeyVaultNameFromBaseUrl(keyVaultUrl string) (string, error) {
groups := regexp.MustCompile(`^https://(.+)\.vault\.azure\.net/?$`).FindStringSubmatch(keyVaultUrl)
if len(groups) != 2 {
return "", fmt.Errorf("parsing keyVaultUrl: %q, expected group: 2", keyVaultUrl)
}
resourceGroup := id.ResourceGroup

vaultName, ok := id.Path["vaults"]
if !ok {
return "", fmt.Errorf("resource id does not contain `vaults`: %q", keyVaultId)
}
return groups[1], nil
}

resp, err := client.Get(ctx, resourceGroup, vaultName)
func GetKeyVaultIDFromBaseUrl(ctx context.Context, client *resources.Client, keyVaultUrl string) (*string, error) {
name, err := GetKeyVaultNameFromBaseUrl(keyVaultUrl)
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 nil, err
}

return *resp.Properties.VaultURI, nil
}

func GetKeyVaultIDFromBaseUrl(ctx context.Context, client *keyvault.VaultsClient, keyVaultUrl string) (*string, error) {
list, err := client.ListComplete(ctx, utils.Int32(1000))
filter := fmt.Sprintf("name eq '%s'", name)
keyVaults, err := client.List(ctx, filter, "", nil)
if err != nil {
return nil, fmt.Errorf("Error GetKeyVaultId unable to list Key Vaults %v", err)
return nil, fmt.Errorf("listing key vault with name %q: %+v", name, err)
}

for list.NotDone() {
v := list.Value()

if v.ID == nil {
return nil, fmt.Errorf("v.ID was nil")
}

vid, err := ParseAzureResourceID(*v.ID)
if err != nil {
return nil, fmt.Errorf("Error parsing ID for Key Vault URI %q: %s", *v.ID, err)
}
resourceGroup := vid.ResourceGroup
name := vid.Path["vaults"]

// resp does not appear to contain the vault properties, so lets fetch them
get, err := client.Get(ctx, resourceGroup, name)
if err != nil {
if utils.ResponseWasNotFound(get.Response) {
if e := list.NextWithContext(ctx); e != nil {
return nil, fmt.Errorf("Error getting next vault on KeyVault url %q : %+v", keyVaultUrl, err)
}
continue
}
return nil, fmt.Errorf("Error making Read request on KeyVault %q (Resource Group %q): %+v", name, resourceGroup, err)
}

if get.ID == nil || get.Properties == nil || get.Properties.VaultURI == nil {
return nil, fmt.Errorf("KeyVault %q (Resource Group %q) has nil ID, properties or vault URI", name, resourceGroup)
}

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

if e := list.NextWithContext(ctx); e != nil {
return nil, fmt.Errorf("Error getting next vault on KeyVault url %q : %+v", keyVaultUrl, err)
}
values := keyVaults.Values()
if len(values) == 0 {
return nil, nil
} else if len(values) > 1 {
return nil, fmt.Errorf("more than one key Vault found with Url: %q", keyVaultUrl)
}

// we haven't found it, but Data Sources and Resources need to handle this error separately
return nil, nil
return values[0].ID, nil
}

func KeyVaultExists(ctx context.Context, client *keyvault.VaultsClient, keyVaultId string) (bool, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault"
"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
"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"
Expand Down Expand Up @@ -94,6 +95,7 @@ func resourceArmDiskEncryptionSet() *schema.Resource {
func resourceArmDiskEncryptionSetCreateUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient
vaultClient := meta.(*clients.Client).KeyVault.VaultsClient
resourcesClient := meta.(*clients.Client).Resource.ResourcesClient
ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

Expand All @@ -113,7 +115,7 @@ func resourceArmDiskEncryptionSetCreateUpdate(d *schema.ResourceData, meta inter
}

keyVaultKeyId := d.Get("key_vault_key_id").(string)
keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, vaultClient, keyVaultKeyId)
keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, vaultClient, resourcesClient, keyVaultKeyId)
if err != nil {
return fmt.Errorf("Error validating Key Vault Key %q for Disk Encryption Set: %+v", keyVaultKeyId, err)
}
Expand Down Expand Up @@ -264,12 +266,12 @@ type diskEncryptionSetKeyVault struct {
softDeleteEnabled bool
}

func diskEncryptionSetRetrieveKeyVault(ctx context.Context, client *keyvault.VaultsClient, id string) (*diskEncryptionSetKeyVault, error) {
func diskEncryptionSetRetrieveKeyVault(ctx context.Context, client *keyvault.VaultsClient, resourcesClient *resources.Client, id string) (*diskEncryptionSetKeyVault, error) {
keyVaultKeyId, err := azure.ParseKeyVaultChildID(id)
if err != nil {
return nil, err
}
keyVaultID, err := azure.GetKeyVaultIDFromBaseUrl(ctx, client, keyVaultKeyId.KeyVaultBaseUrl)
keyVaultID, err := azure.GetKeyVaultIDFromBaseUrl(ctx, resourcesClient, keyVaultKeyId.KeyVaultBaseUrl)
if err != nil {
return nil, fmt.Errorf("Error retrieving the Resource ID the Key Vault at URL %q: %s", keyVaultKeyId.KeyVaultBaseUrl, err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@ import (
"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/services/keyvault/parse"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/keyvault/validate"
"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"
)

// todo refactor and find a home for this wayward func
func resourceArmKeyVaultChildResourceImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
client := meta.(*clients.Client).KeyVault.VaultsClient
client := meta.(*clients.Client).Resource.ResourcesClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

Expand Down Expand Up @@ -73,7 +75,7 @@ func resourceArmKeyVaultCertificate() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: azure.ValidateResourceID,
ValidateFunc: validate.KeyVaultID,
},

"certificate": {
Expand Down Expand Up @@ -336,24 +338,19 @@ func resourceArmKeyVaultCertificate() *schema.Resource {
}

func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface{}) error {
vaultClient := meta.(*clients.Client).KeyVault.VaultsClient
client := meta.(*clients.Client).KeyVault.ManagementClient
ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
keyVaultId := d.Get("key_vault_id").(string)

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

if features.ShouldResourcesBeImported() {
existing, err := client.GetCertificate(ctx, keyVaultBaseUrl, name, "")
existing, err := client.GetCertificate(ctx, kvID.BaseUrl(), name, "")
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("Error checking for presence of existing Certificate %q (Key Vault %q): %s", name, keyVaultBaseUrl, err)
return fmt.Errorf("Error checking for presence of existing Certificate %q (Key Vault %q): %s", name, kvID.BaseUrl(), err)
}
}

Expand All @@ -374,7 +371,7 @@ func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface
CertificatePolicy: &policy,
Tags: tags.Expand(t),
}
if _, err := client.ImportCertificate(ctx, keyVaultBaseUrl, name, importParameters); err != nil {
if _, err := client.ImportCertificate(ctx, kvID.BaseUrl(), name, importParameters); err != nil {
return err
}
} else {
Expand All @@ -383,9 +380,9 @@ func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface
CertificatePolicy: &policy,
Tags: tags.Expand(t),
}
if resp, err := client.CreateCertificate(ctx, keyVaultBaseUrl, name, parameters); err != nil {
if resp, err := client.CreateCertificate(ctx, kvID.BaseUrl(), name, parameters); err != nil {
if meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults && utils.ResponseWasConflict(resp.Response) {
recoveredCertificate, err := client.RecoverDeletedCertificate(ctx, keyVaultBaseUrl, name)
recoveredCertificate, err := client.RecoverDeletedCertificate(ctx, kvID.BaseUrl(), name)
if err != nil {
return err
}
Expand All @@ -411,21 +408,21 @@ func resourceArmKeyVaultCertificateCreate(d *schema.ResourceData, meta interface
}
}

log.Printf("[DEBUG] Waiting for Key Vault Certificate %q in Vault %q to be provisioned", name, keyVaultBaseUrl)
log.Printf("[DEBUG] Waiting for Key Vault Certificate %q in Vault %q to be provisioned", name, kvID.BaseUrl())
stateConf := &resource.StateChangeConf{
Pending: []string{"Provisioning"},
Target: []string{"Ready"},
Refresh: keyVaultCertificateCreationRefreshFunc(ctx, client, keyVaultBaseUrl, name),
Refresh: keyVaultCertificateCreationRefreshFunc(ctx, client, kvID.BaseUrl(), name),
MinTimeout: 15 * time.Second,
Timeout: d.Timeout(schema.TimeoutCreate),
}

if _, err := stateConf.WaitForState(); err != nil {
return fmt.Errorf("Error waiting for Certificate %q in Vault %q to become available: %s", name, keyVaultBaseUrl, err)
return fmt.Errorf("Error waiting for Certificate %q in Vault %q to become available: %s", name, kvID.BaseUrl(), err)
}
}

resp, err := client.GetCertificate(ctx, keyVaultBaseUrl, name, "")
resp, err := client.GetCertificate(ctx, kvID.BaseUrl(), name, "")
if err != nil {
return err
}
Expand All @@ -452,6 +449,7 @@ func keyVaultCertificateCreationRefreshFunc(ctx context.Context, client *keyvaul

func resourceArmKeyVaultCertificateRead(d *schema.ResourceData, meta interface{}) error {
keyVaultClient := meta.(*clients.Client).KeyVault.VaultsClient
resourcesClient := meta.(*clients.Client).Resource.ResourcesClient
client := meta.(*clients.Client).KeyVault.ManagementClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand All @@ -461,7 +459,7 @@ func resourceArmKeyVaultCertificateRead(d *schema.ResourceData, meta interface{}
return err
}

keyVaultId, err := azure.GetKeyVaultIDFromBaseUrl(ctx, keyVaultClient, id.KeyVaultBaseUrl)
keyVaultId, err := azure.GetKeyVaultIDFromBaseUrl(ctx, resourcesClient, id.KeyVaultBaseUrl)
if err != nil {
return fmt.Errorf("Error retrieving the Resource ID the Key Vault at URL %q: %s", id.KeyVaultBaseUrl, err)
}
Expand Down Expand Up @@ -520,6 +518,7 @@ func resourceArmKeyVaultCertificateRead(d *schema.ResourceData, meta interface{}

func resourceArmKeyVaultCertificateDelete(d *schema.ResourceData, meta interface{}) error {
keyVaultClient := meta.(*clients.Client).KeyVault.VaultsClient
resourcesClient := meta.(*clients.Client).Resource.ResourcesClient
client := meta.(*clients.Client).KeyVault.ManagementClient
ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand All @@ -529,7 +528,7 @@ func resourceArmKeyVaultCertificateDelete(d *schema.ResourceData, meta interface
return err
}

keyVaultId, err := azure.GetKeyVaultIDFromBaseUrl(ctx, keyVaultClient, id.KeyVaultBaseUrl)
keyVaultId, err := azure.GetKeyVaultIDFromBaseUrl(ctx, resourcesClient, id.KeyVaultBaseUrl)
if err != nil {
return fmt.Errorf("Error retrieving the Resource ID the Key Vault at URL %q: %s", id.KeyVaultBaseUrl, err)
}
Expand Down
15 changes: 6 additions & 9 deletions azurerm/internal/services/keyvault/key_vault_key_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/keyvault/parse"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/keyvault/validate"
"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"
Expand All @@ -30,7 +32,7 @@ func dataSourceArmKeyVaultKey() *schema.Resource {
"key_vault_id": {
Type: schema.TypeString,
Required: true,
ValidateFunc: azure.ValidateResourceID,
ValidateFunc: validate.KeyVaultID,
},

"key_type": {
Expand Down Expand Up @@ -72,23 +74,18 @@ func dataSourceArmKeyVaultKey() *schema.Resource {
}

func dataSourceArmKeyVaultKeyRead(d *schema.ResourceData, meta interface{}) error {
vaultClient := meta.(*clients.Client).KeyVault.VaultsClient
client := meta.(*clients.Client).KeyVault.ManagementClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
keyVaultId := d.Get("key_vault_id").(string)
kvID, _ := parse.KeyVaultID(keyVaultId)

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

resp, err := client.GetKey(ctx, keyVaultBaseUri, name, "")
resp, err := client.GetKey(ctx, kvID.BaseUrl(), name, "")
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
return fmt.Errorf("Key %q was not found in Key Vault at URI %q", name, keyVaultBaseUri)
return fmt.Errorf("Key %q was not found in Key Vault at URI %q", name, kvID.BaseUrl())
}

return err
Expand Down
Loading