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

keyvault: populating the Key Vaults cache using the KeyVault Resource Provider #24019

Merged
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
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