Skip to content

Commit

Permalink
Merge pull request #24019 from hashicorp/f/keyvault-populating-cache-…
Browse files Browse the repository at this point in the history
…from-the-keyvault-listbysubscription-endpoint

`keyvault`: populating the Key Vaults cache using the KeyVault Resource Provider
  • Loading branch information
tombuildsstuff authored Nov 28, 2023
2 parents 8ee43b3 + e50a3fe commit 4e7c2c4
Show file tree
Hide file tree
Showing 34 changed files with 218 additions and 183 deletions.
3 changes: 2 additions & 1 deletion internal/services/cdn/cdn_endpoint_custom_domain_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,8 @@ func expandArmCdnEndpointCustomDomainUserManagedHttpsSettings(ctx context.Contex
return nil, err
}

keyVaultIdRaw, err := clients.KeyVault.KeyVaultIDFromBaseUrl(ctx, clients.Resource, keyVaultSecretId.KeyVaultBaseUrl)
subscriptionId := commonids.NewSubscriptionID(clients.Account.SubscriptionId)
keyVaultIdRaw, err := clients.KeyVault.KeyVaultIDFromBaseUrl(ctx, subscriptionId, keyVaultSecretId.KeyVaultBaseUrl)
if err != nil {
return nil, fmt.Errorf("retrieving the Resource ID the Key Vault at URL %q: %s", keyVaultSecretId.KeyVaultBaseUrl, err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func ExpandCdnFrontDoorCustomerCertificateParameters(ctx context.Context, input
useLatest = true
}

keyVaultBaseId, err := clients.KeyVault.KeyVaultIDFromBaseUrl(ctx, clients.Resource, certificateId.KeyVaultBaseUrl)
subscriptionId := commonids.NewSubscriptionID(clients.Account.SubscriptionId)
keyVaultBaseId, err := clients.KeyVault.KeyVaultIDFromBaseUrl(ctx, subscriptionId, certificateId.KeyVaultBaseUrl)
if err != nil {
return nil, fmt.Errorf("retrieving the Key Vault Resource ID from the Key Vault Base URL %q: %s", certificateId.KeyVaultBaseUrl, err)
}
Expand Down
12 changes: 5 additions & 7 deletions internal/services/compute/disk_encryption_set_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/client"
keyVaultParse "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse"
keyVaultValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/validate"
resourcesClient "github.com/hashicorp/terraform-provider-azurerm/internal/services/resource/client"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
"github.com/hashicorp/terraform-provider-azurerm/internal/timeouts"
Expand Down Expand Up @@ -116,7 +115,6 @@ func resourceDiskEncryptionSetCreate(d *pluginsdk.ResourceData, meta interface{}
client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient
keyVaultsClient := meta.(*clients.Client).KeyVault
keyVaultKeyClient := meta.(*clients.Client).KeyVault.ManagementClient
resourcesClient := meta.(*clients.Client).Resource
subscriptionId := meta.(*clients.Client).Account.SubscriptionId
ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand Down Expand Up @@ -149,7 +147,7 @@ func resourceDiskEncryptionSetCreate(d *pluginsdk.ResourceData, meta interface{}
}
}

keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, keyVaultsClient, resourcesClient, *keyVaultKey)
keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, keyVaultsClient, subscriptionId, *keyVaultKey)
if err != nil {
return fmt.Errorf("validating Key Vault Key %q for Disk Encryption Set: %+v", keyVaultKey.ID(), err)
}
Expand Down Expand Up @@ -313,7 +311,6 @@ func resourceDiskEncryptionSetUpdate(d *pluginsdk.ResourceData, meta interface{}
client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient
keyVaultsClient := meta.(*clients.Client).KeyVault
keyVaultKeyClient := meta.(*clients.Client).KeyVault.ManagementClient
resourcesClient := meta.(*clients.Client).Resource
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

Expand Down Expand Up @@ -344,7 +341,7 @@ func resourceDiskEncryptionSetUpdate(d *pluginsdk.ResourceData, meta interface{}
}

if d.HasChange("key_vault_key_id") {
keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, keyVaultsClient, resourcesClient, *keyVaultKey)
keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, keyVaultsClient, id.SubscriptionId, *keyVaultKey)
if err != nil {
return fmt.Errorf("validating Key Vault Key %q for Disk Encryption Set: %+v", keyVaultKey.ID(), err)
}
Expand Down Expand Up @@ -464,8 +461,9 @@ type diskEncryptionSetKeyVault struct {
softDeleteEnabled bool
}

func diskEncryptionSetRetrieveKeyVault(ctx context.Context, keyVaultsClient *client.Client, resourcesClient *resourcesClient.Client, keyVaultKeyId keyVaultParse.NestedItemId) (*diskEncryptionSetKeyVault, error) {
keyVaultID, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, resourcesClient, keyVaultKeyId.KeyVaultBaseUrl)
func diskEncryptionSetRetrieveKeyVault(ctx context.Context, keyVaultsClient *client.Client, subscriptionId string, keyVaultKeyId keyVaultParse.NestedItemId) (*diskEncryptionSetKeyVault, error) {
subscriptionResourceId := commonids.NewSubscriptionID(subscriptionId)
keyVaultID, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionResourceId, keyVaultKeyId.KeyVaultBaseUrl)
if err != nil {
return nil, fmt.Errorf("retrieving the Resource ID the Key Vault at URL %q: %s", keyVaultKeyId.KeyVaultBaseUrl, err)
}
Expand Down
12 changes: 5 additions & 7 deletions internal/services/containers/kubernetes_cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
keyVaultClient "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/client"
keyVaultParse "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/parse"
keyVaultValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/validate"
resourcesClient "github.com/hashicorp/terraform-provider-azurerm/internal/services/resource/client"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
Expand Down Expand Up @@ -1573,7 +1572,6 @@ func resourceKubernetesClusterCreate(d *pluginsdk.ResourceData, meta interface{}
tenantId := meta.(*clients.Client).Account.TenantId
client := meta.(*clients.Client).Containers.KubernetesClustersClient
keyVaultsClient := meta.(*clients.Client).KeyVault
resourcesClient := meta.(*clients.Client).Resource
env := meta.(*clients.Client).Containers.Environment
ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand Down Expand Up @@ -1706,7 +1704,7 @@ func resourceKubernetesClusterCreate(d *pluginsdk.ResourceData, meta interface{}
}

azureKeyVaultKmsRaw := d.Get("key_management_service").([]interface{})
securityProfile.AzureKeyVaultKms, err = expandKubernetesClusterAzureKeyVaultKms(ctx, keyVaultsClient, resourcesClient, d, azureKeyVaultKmsRaw)
securityProfile.AzureKeyVaultKms, err = expandKubernetesClusterAzureKeyVaultKms(ctx, keyVaultsClient, id.SubscriptionId, d, azureKeyVaultKmsRaw)
if err != nil {
return err
}
Expand Down Expand Up @@ -1876,7 +1874,6 @@ func resourceKubernetesClusterUpdate(d *pluginsdk.ResourceData, meta interface{}
nodePoolsClient := containersClient.AgentPoolsClient
clusterClient := containersClient.KubernetesClustersClient
keyVaultsClient := meta.(*clients.Client).KeyVault
resourcesClient := meta.(*clients.Client).Resource
env := containersClient.Environment
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand Down Expand Up @@ -2245,7 +2242,7 @@ func resourceKubernetesClusterUpdate(d *pluginsdk.ResourceData, meta interface{}
if d.HasChanges("key_management_service") {
updateCluster = true
azureKeyVaultKmsRaw := d.Get("key_management_service").([]interface{})
azureKeyVaultKms, _ := expandKubernetesClusterAzureKeyVaultKms(ctx, keyVaultsClient, resourcesClient, d, azureKeyVaultKmsRaw)
azureKeyVaultKms, _ := expandKubernetesClusterAzureKeyVaultKms(ctx, keyVaultsClient, id.SubscriptionId, d, azureKeyVaultKmsRaw)
if existing.Model.Properties.SecurityProfile == nil {
existing.Model.Properties.SecurityProfile = &managedclusters.ManagedClusterSecurityProfile{}
}
Expand Down Expand Up @@ -3913,7 +3910,7 @@ func expandKubernetesClusterAutoScalerProfile(input []interface{}) *managedclust
}
}

func expandKubernetesClusterAzureKeyVaultKms(ctx context.Context, keyVaultsClient *keyVaultClient.Client, resourcesClient *resourcesClient.Client, d *pluginsdk.ResourceData, input []interface{}) (*managedclusters.AzureKeyVaultKms, error) {
func expandKubernetesClusterAzureKeyVaultKms(ctx context.Context, keyVaultsClient *keyVaultClient.Client, subscriptionId string, d *pluginsdk.ResourceData, input []interface{}) (*managedclusters.AzureKeyVaultKms, error) {
if ((input == nil) || len(input) == 0) && d.HasChanges("key_management_service") {
return &managedclusters.AzureKeyVaultKms{
Enabled: utils.Bool(false),
Expand All @@ -3933,11 +3930,12 @@ func expandKubernetesClusterAzureKeyVaultKms(ctx context.Context, keyVaultsClien

// Set Key vault Resource ID in case public access is disabled
if kvAccess == managedclusters.KeyVaultNetworkAccessTypesPrivate {
subscriptionResourceId := commonids.NewSubscriptionID(subscriptionId)
keyVaultKeyId, err := keyVaultParse.ParseNestedItemID(*azureKeyVaultKms.KeyId)
if err != nil {
return nil, err
}
keyVaultID, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, resourcesClient, keyVaultKeyId.KeyVaultBaseUrl)
keyVaultID, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionResourceId, keyVaultKeyId.KeyVaultBaseUrl)
if err != nil {
return nil, fmt.Errorf("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 @@ -12,6 +12,7 @@ import (

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
"github.com/hashicorp/go-azure-sdk/resource-manager/databricks/2023-02-01/workspaces"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -129,7 +130,8 @@ func databricksWorkspaceCustomerManagedKeyCreateUpdate(d *pluginsdk.ResourceData
}

// make sure the key vault exists
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, meta.(*clients.Client).Resource, key.KeyVaultBaseUrl)
subscriptionId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionId, key.KeyVaultBaseUrl)
if err != nil || keyVaultIdRaw == nil {
return fmt.Errorf("retrieving the Resource ID for the Key Vault at URL %q: %+v", key.KeyVaultBaseUrl, err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
"github.com/hashicorp/go-azure-sdk/resource-manager/databricks/2023-02-01/workspaces"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -122,7 +123,8 @@ func databricksWorkspaceRootDbfsCustomerManagedKeyCreate(d *pluginsdk.ResourceDa
}

// make sure the key vault exists
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, meta.(*clients.Client).Resource, key.KeyVaultBaseUrl)
subscriptionId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionId, key.KeyVaultBaseUrl)
if err != nil || keyVaultIdRaw == nil {
return fmt.Errorf("retrieving the Resource ID for the Key Vault at URL %q: %+v", key.KeyVaultBaseUrl, err)
}
Expand Down Expand Up @@ -269,7 +271,8 @@ func databricksWorkspaceRootDbfsCustomerManagedKeyUpdate(d *pluginsdk.ResourceDa
}

// make sure the key vault exists
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, meta.(*clients.Client).Resource, key.KeyVaultBaseUrl)
subscriptionId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionId, key.KeyVaultBaseUrl)
if err != nil || keyVaultIdRaw == nil {
return fmt.Errorf("retrieving the Resource ID for the Key Vault at URL %q: %+v", key.KeyVaultBaseUrl, err)
}
Expand Down
6 changes: 4 additions & 2 deletions internal/services/databricks/databricks_workspace_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,8 @@ func resourceDatabricksWorkspaceCreateUpdate(d *pluginsdk.ResourceData, meta int
}

// make sure the key vault exists
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, meta.(*clients.Client).Resource, key.KeyVaultBaseUrl)
subscriptionResourceId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionResourceId, key.KeyVaultBaseUrl)
if err != nil || keyVaultIdRaw == nil {
return fmt.Errorf("retrieving the Resource ID for the customer-managed keys for managed services Key Vault at URL %q: %+v", key.KeyVaultBaseUrl, err)
}
Expand All @@ -488,7 +489,8 @@ func resourceDatabricksWorkspaceCreateUpdate(d *pluginsdk.ResourceData, meta int
}

// make sure the key vault exists
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, meta.(*clients.Client).Resource, key.KeyVaultBaseUrl)
subscriptionResourceId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultIdRaw, err := keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionResourceId, key.KeyVaultBaseUrl)
if err != nil || keyVaultIdRaw == nil {
return fmt.Errorf("retrieving the Resource ID for the customer-managed keys for managed disk Key Vault at URL %q: %+v", key.KeyVaultBaseUrl, err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func resourceDataFactoryLinkedServiceKeyVaultCreateUpdate(d *pluginsdk.ResourceD
func resourceDataFactoryLinkedServiceKeyVaultRead(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).DataFactory.LinkedServiceClient
keyVaultsClient := meta.(*clients.Client).KeyVault
resourcesClient := meta.(*clients.Client).Resource
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

Expand Down Expand Up @@ -239,7 +238,8 @@ func resourceDataFactoryLinkedServiceKeyVaultRead(d *pluginsdk.ResourceData, met

var keyVaultId *string
if baseUrl != "" {
keyVaultId, err = keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, resourcesClient, baseUrl)
subscriptionId := commonids.NewSubscriptionID(id.SubscriptionId)
keyVaultId, err = keyVaultsClient.KeyVaultIDFromBaseUrl(ctx, subscriptionId, baseUrl)
if err != nil {
return err
}
Expand Down
88 changes: 48 additions & 40 deletions internal/services/keyvault/client/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import (

"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonids"
resourcesClient "github.com/hashicorp/terraform-provider-azurerm/internal/services/resource/client"
"github.com/hashicorp/terraform-provider-azurerm/utils"
"github.com/hashicorp/go-azure-sdk/resource-manager/keyvault/2023-02-01/vaults"
)

var (
Expand Down Expand Up @@ -111,7 +110,7 @@ func (c *Client) Exists(ctx context.Context, keyVaultId commonids.KeyVaultId) (b
return true, nil
}

func (c *Client) KeyVaultIDFromBaseUrl(ctx context.Context, resourcesClient *resourcesClient.Client, keyVaultBaseUrl string) (*string, error) {
func (c *Client) KeyVaultIDFromBaseUrl(ctx context.Context, subscriptionId commonids.SubscriptionId, keyVaultBaseUrl string) (*string, error) {
keyVaultName, err := c.parseNameFromBaseUrl(keyVaultBaseUrl)
if err != nil {
return nil, err
Expand All @@ -126,53 +125,62 @@ func (c *Client) KeyVaultIDFromBaseUrl(ctx context.Context, resourcesClient *res
lock[cacheKey].Lock()
defer lock[cacheKey].Unlock()

// Check the cache to determine if we have an entry for this key vault
if v, ok := keyVaultsCache[cacheKey]; ok {
return &v.keyVaultId, nil
}

filter := fmt.Sprintf("resourceType eq 'Microsoft.KeyVault/vaults' and name eq '%s'", *keyVaultName)
result, err := resourcesClient.ResourcesClient.List(ctx, filter, "", utils.Int32(5))
// Pull out the list of Key Vaults available within the Subscription to re-populate the cache
//
// Whilst we've historically used the Resources API to query the single Key Vault in question
// this endpoint has caching related issues - and whilst the ResourceGraph API has been suggested
// as an alternative that fixes this, we've seen similar caching issues there.
// Therefore, we're falling back on querying all the Key Vaults within the specified Subscription, which
// comes from the `KeyVault` Resource Provider rather than the `Resources` Resource Provider - which
// is an approach we've used previously, but now with better caching.
//
// Whilst querying ALL Key Vaults within a Subscription IS excessive where only a single Key Vault
// is used - having the cache populated (one-time, per Provider launch) should alleviate problems
// in Terraform Configurations defining a large number of Key Vault items.
//
// @tombuildsstuff: I vaguely recall the `ListBySubscription` API having a low rate limit (5x/second?)
// however the rate-limits defined here seem to apply only to Managed HSMs and not Key Vaults?
// https://learn.microsoft.com/en-us/azure/key-vault/general/service-limits
//
// Finally, it's worth noting that we intentionally List ALL the Key Vaults within a Subscription
// to be able to cache ALL of them - prior to looking up the specific Key Vault we're interested
// in from the freshly populated cache.
// This fixes an issue in the previous implementation where the Cache was being repeatedly semi-populated
// until the specified Key Vault was found, at which point we skipped populating the cache, which
// affected both the `Resources` API implementation:
// https://github.com/hashicorp/terraform-provider-azurerm/blob/3e88e5e74e12577d785f10298281b1b3c172254f/internal/services/keyvault/client/helpers.go#L133-L173
// and the `ListBySubscription` endpoint:
// https://github.com/hashicorp/terraform-provider-azurerm/blob/a5e728dc62e832e74d7bb0f40a79af0ae5a79e1e/azurerm/helpers/azure/key_vault.go#L42-L89
opts := vaults.DefaultListBySubscriptionOperationOptions()
results, err := c.VaultsClient.ListBySubscriptionComplete(ctx, subscriptionId, opts)
if err != nil {
return nil, fmt.Errorf("listing resources matching %q: %+v", filter, err)
}

for result.NotDone() {
for _, v := range result.Values() {
if v.ID == nil {
continue
}

id, err := commonids.ParseKeyVaultID(*v.ID)
if err != nil {
return nil, fmt.Errorf("parsing %q: %+v", *v.ID, err)
}
if !strings.EqualFold(id.VaultName, *keyVaultName) {
continue
}

resp, err := c.VaultsClient.Get(ctx, *id)
if err != nil {
return nil, fmt.Errorf("retrieving %s: %+v", *id, err)
}
vaultUri := ""
if model := resp.Model; model != nil {
if model.Properties.VaultUri != nil {
vaultUri = *model.Properties.VaultUri
}
}
if vaultUri == "" {
return nil, fmt.Errorf("retrieving %s: `properties.VaultUri` was nil", id)
}
c.AddToCache(*id, vaultUri)
return utils.String(id.ID()), nil
return nil, fmt.Errorf("listing the Key Vaults within %s: %+v", subscriptionId, err)
}
for _, item := range results.Items {
if item.Id == nil || item.Properties.VaultUri == nil {
continue
}

if err := result.NextWithContext(ctx); err != nil {
return nil, fmt.Errorf("iterating over results: %+v", err)
// Populate the key vault into the cache
keyVaultId, err := commonids.ParseKeyVaultIDInsensitively(*item.Id)
if err != nil {
return nil, fmt.Errorf("parsing %q as a Key Vault ID: %+v", *item.Id, err)
}
vaultUri := *item.Properties.VaultUri
c.AddToCache(*keyVaultId, vaultUri)
}

// Now that the cache has been repopulated, check if we have the key vault or not
if v, ok := keyVaultsCache[cacheKey]; ok {
return &v.keyVaultId, nil
}

// we haven't found it, but Data Sources and Resources need to handle this error separately
// We haven't found it, but Data Sources and Resources need to handle this error separately
return nil, nil
}

Expand Down
Loading

0 comments on commit 4e7c2c4

Please sign in to comment.