Skip to content

Commit

Permalink
r/key_vault: conditionally polling the Data Plane endpoint when `publ…
Browse files Browse the repository at this point in the history 
…ic_network_access_enabled` is set to `false` (#23823)
  • Loading branch information
@tombuildsstuff
tombuildsstuff authored Feb 16, 2024
1 parent 293d575 commit ac1b256
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 30 deletions.
102 changes: 72 additions & 30 deletions internal/services/keyvault/key_vault_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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()),
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
73 changes: 73 additions & 0 deletions internal/services/keyvault/key_vault_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit ac1b256

Please sign in to comment.