From 79b79e333aa41666412491246025ba34068087bb Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 5 Dec 2023 12:00:16 +0100 Subject: [PATCH] r/key_vault: conditionally polling the Data Plane endpoint when `public_network_access_enabled` is set to `false` --- .../services/keyvault/key_vault_resource.go | 102 ++++++++++++------ .../keyvault/key_vault_resource_test.go | 73 +++++++++++++ 2 files changed, 145 insertions(+), 30 deletions(-) diff --git a/internal/services/keyvault/key_vault_resource.go b/internal/services/keyvault/key_vault_resource.go index ac9ef514fd44..531e0bde41a3 100644 --- a/internal/services/keyvault/key_vault_resource.go +++ b/internal/services/keyvault/key_vault_resource.go @@ -272,7 +272,6 @@ func resourceKeyVaultCreate(d *pluginsdk.ResourceData, meta interface{}) error { } // if so, does the user want us to recover it? - recoverSoftDeletedKeyVault := false if !response.WasNotFound(softDeletedKeyVault.HttpResponse) && !response.WasStatusCode(softDeletedKeyVault.HttpResponse, http.StatusForbidden) { if !meta.(*clients.Client).Features.KeyVault.RecoverSoftDeletedKeyVaults { @@ -283,6 +282,12 @@ func resourceKeyVaultCreate(d *pluginsdk.ResourceData, meta interface{}) error { recoverSoftDeletedKeyVault = true } + isPublic := d.Get("public_network_access_enabled").(bool) + contactRaw := d.Get("contact").(*pluginsdk.Set).List() + if !isPublic && len(contactRaw) > 0 { + return fmt.Errorf("`contact` cannot be specified when `public_network_access_enabled` is set to `false`") + } + tenantUUID := d.Get("tenant_id").(string) enabledForDeployment := d.Get("enabled_for_deployment").(bool) enabledForDiskEncryption := d.Get("enabled_for_disk_encryption").(bool) @@ -321,7 +326,7 @@ func resourceKeyVaultCreate(d *pluginsdk.ResourceData, meta interface{}) error { Tags: tags.Expand(t), } - if d.Get("public_network_access_enabled").(bool) { + if isPublic { parameters.Properties.PublicNetworkAccess = utils.String("Enabled") } else { parameters.Properties.PublicNetworkAccess = utils.String("Disabled") @@ -375,27 +380,46 @@ func resourceKeyVaultCreate(d *pluginsdk.ResourceData, meta interface{}) error { d.SetId(id.ID()) meta.(*clients.Client).KeyVault.AddToCache(id, vaultUri) - log.Printf("[DEBUG] Waiting for %s to become available", id) - deadline, ok := ctx.Deadline() - if !ok { - return fmt.Errorf("internal-error: context had no deadline") - } - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"pending"}, - Target: []string{"available"}, - Refresh: keyVaultRefreshFunc(vaultUri), - Delay: 30 * time.Second, - PollInterval: 10 * time.Second, - ContinuousTargetOccurence: 10, - Timeout: time.Until(deadline), - } - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become available: %s", id, err) + // When Public Network Access is Enabled (i.e. it's Public) we can hit the Data Plane API until + // we get a valid response repeatedly - ensuring that the API is fully online before proceeding. + // + // This works around an issue where the provisioning of dependent resources fails, due to the + // Key Vault not being fully online - which is a particular issue when recreating the Key Vault. + // + // When Public Network Access is Disabled (i.e. it's Private) we don't poll - meaning that users + // are more likely to encounter issues in downstream resources (particularly when using Private + // Link due to DNS replication delays) - however there isn't a great deal we can do about that + // given the Data Plane API isn't going to be publicly available. + // + // As such we poll to check the Key Vault is available, if it's public, to ensure that downstream + // operations can succeed. + if isPublic { + log.Printf("[DEBUG] Waiting for %s to become available", id) + deadline, ok := ctx.Deadline() + if !ok { + return fmt.Errorf("internal-error: context had no deadline") + } + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"pending"}, + Target: []string{"available"}, + Refresh: keyVaultRefreshFunc(vaultUri), + Delay: 30 * time.Second, + PollInterval: 10 * time.Second, + ContinuousTargetOccurence: 10, + Timeout: time.Until(deadline), + } + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for %s to become available: %s", id, err) + } } - if v, ok := d.GetOk("contact"); ok { + if len(contactRaw) > 0 { + if !isPublic { + return fmt.Errorf("`contact` cannot be specified when `public_network_access_enabled` is set to `false`") + } + contacts := dataplane.Contacts{ - ContactList: expandKeyVaultCertificateContactList(v.(*pluginsdk.Set).List()), + ContactList: expandKeyVaultCertificateContactList(contactRaw), } if _, err := dataPlaneClient.SetCertificateContacts(ctx, vaultUri, contacts); err != nil { return fmt.Errorf("failed to set Contacts for %s: %+v", id, err) @@ -432,6 +456,11 @@ func resourceKeyVaultUpdate(d *pluginsdk.ResourceData, meta interface{}) error { return fmt.Errorf("retrieving %s: `properties` was nil", *id) } + isPublic := true + if model := existing.Model; model != nil && model.Properties.PublicNetworkAccess != nil { + isPublic = strings.EqualFold(*model.Properties.PublicNetworkAccess, "Enabled") + } + update := vaults.VaultPatchParameters{} if d.HasChange("access_policy") { @@ -537,7 +566,8 @@ func resourceKeyVaultUpdate(d *pluginsdk.ResourceData, meta interface{}) error { update.Properties = &vaults.VaultPatchProperties{} } - if d.Get("public_network_access_enabled").(bool) { + isPublic = d.Get("public_network_access_enabled").(bool) + if isPublic { update.Properties.PublicNetworkAccess = utils.String("Enabled") } else { update.Properties.PublicNetworkAccess = utils.String("Disabled") @@ -594,6 +624,9 @@ func resourceKeyVaultUpdate(d *pluginsdk.ResourceData, meta interface{}) error { } if d.HasChange("contact") { + if !isPublic { + return fmt.Errorf("`contact` cannot be specified when `public_network_access_enabled` is set to `false`") + } contacts := dataplane.Contacts{ ContactList: expandKeyVaultCertificateContactList(d.Get("contact").(*pluginsdk.Set).List()), } @@ -644,18 +677,28 @@ func resourceKeyVaultRead(d *pluginsdk.ResourceData, meta interface{}) error { } vaultUri := "" - if resp.Model != nil && resp.Model.Properties.VaultUri != nil { - vaultUri = *resp.Model.Properties.VaultUri + isPublic := true + if model := resp.Model; model != nil { + if model.Properties.VaultUri != nil { + vaultUri = *model.Properties.VaultUri + } + if model.Properties.PublicNetworkAccess != nil { + isPublic = strings.EqualFold(*model.Properties.PublicNetworkAccess, "Enabled") + } } if vaultUri != "" { meta.(*clients.Client).KeyVault.AddToCache(*id, vaultUri) } - contactsResp, err := dataplaneClient.GetCertificateContacts(ctx, vaultUri) - if err != nil { - if !utils.ResponseWasForbidden(contactsResp.Response) && !utils.ResponseWasNotFound(contactsResp.Response) { - return fmt.Errorf("retrieving `contact` for KeyVault: %+v", err) + var contactsResp *dataplane.Contacts + if isPublic { + contacts, err := dataplaneClient.GetCertificateContacts(ctx, vaultUri) + if err != nil { + if !utils.ResponseWasForbidden(contacts.Response) && !utils.ResponseWasNotFound(contacts.Response) { + return fmt.Errorf("retrieving `contact` for KeyVault: %+v", err) + } } + contactsResp = &contacts } d.Set("name", id.VaultName) @@ -713,7 +756,6 @@ func resourceKeyVaultRead(d *pluginsdk.ResourceData, meta interface{}) error { if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) } - } return nil @@ -932,9 +974,9 @@ func flattenKeyVaultNetworkAcls(input *vaults.NetworkRuleSet) []interface{} { } } -func flattenKeyVaultCertificateContactList(input dataplane.Contacts) []interface{} { +func flattenKeyVaultCertificateContactList(input *dataplane.Contacts) []interface{} { results := make([]interface{}, 0) - if input.ContactList == nil { + if input == nil || input.ContactList == nil { return results } diff --git a/internal/services/keyvault/key_vault_resource_test.go b/internal/services/keyvault/key_vault_resource_test.go index 16c0e4886f68..792ae7568070 100644 --- a/internal/services/keyvault/key_vault_resource_test.go +++ b/internal/services/keyvault/key_vault_resource_test.go @@ -231,6 +231,36 @@ func TestAccKeyVault_updateContacts(t *testing.T) { }) } +func TestAccKeyVault_publicNetworkAccessDisabled(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_key_vault", "test") + r := KeyVaultResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.publicNetworkAccessDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + // then enable it + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.publicNetworkAccessDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func TestAccKeyVault_justCert(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_key_vault", "test") r := KeyVaultResource{} @@ -515,6 +545,49 @@ resource "azurerm_key_vault" "import" { `, r.basic(data)) } +func (KeyVaultResource) publicNetworkAccessDisabled(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +data "azurerm_client_config" "current" { +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_key_vault" "test" { + name = "vault%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + sku_name = "standard" + soft_delete_retention_days = 7 + public_network_access_enabled = false + + access_policy { + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + certificate_permissions = [ + "ManageContacts", + ] + + key_permissions = [ + "Create", + ] + + secret_permissions = [ + "Set", + ] + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + func (KeyVaultResource) networkAclsTemplate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" {