From bfaf955ee09cddd9014616d92b090551db2e5813 Mon Sep 17 00:00:00 2001 From: henglu Date: Fri, 18 Mar 2022 15:15:07 +0800 Subject: [PATCH 1/5] fix synapse workspace cmk rotation --- .../synapse/synapse_workspace_key_resource.go | 11 +- .../synapse_workspace_key_resource_test.go | 177 +++++++++++++----- .../synapse/synapse_workspace_resource.go | 36 +++- 3 files changed, 170 insertions(+), 54 deletions(-) diff --git a/internal/services/synapse/synapse_workspace_key_resource.go b/internal/services/synapse/synapse_workspace_key_resource.go index 3735432ed276..2981f5e8062f 100644 --- a/internal/services/synapse/synapse_workspace_key_resource.go +++ b/internal/services/synapse/synapse_workspace_key_resource.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/synapse/mgmt/2021-03-01/synapse" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/locks" keyVaultValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/services/synapse/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/synapse/validate" @@ -115,6 +116,8 @@ func resourceSynapseWorkspaceKeysCreateUpdate(d *pluginsdk.ResourceData, meta in actualKeyName = keyNameTypoed } + locks.ByName(workspaceId.Name, "azurerm_synapse_workspace") + defer locks.UnlockByName(workspaceId.Name, "azurerm_synapse_workspace") keyresult, err := client.CreateOrUpdate(ctx, workspaceId.ResourceGroup, workspaceId.Name, actualKeyName, synapseKey) if err != nil { return fmt.Errorf("creating Synapse Workspace Key %q (Workspace %q): %+v", workspaceId.Name, workspaceId.Name, err) @@ -185,13 +188,9 @@ func resourceSynapseWorkspaceKeysDelete(d *pluginsdk.ResourceData, meta interfac // Azure only lets you delete keys that are not active if !*keyresult.KeyProperties.IsActiveCMK { - keyresult, err := client.Delete(ctx, id.ResourceGroup, id.WorkspaceName, id.KeyName) + _, err := client.Delete(ctx, id.ResourceGroup, id.WorkspaceName, id.KeyName) if err != nil { - return fmt.Errorf("Unable to delete key %s in workspace %s: %v", id.KeyName, id.WorkspaceName, err) - } - - if keyresult.ID == nil || *keyresult.ID == "" { - return fmt.Errorf("empty or nil ID returned for Synapse Key %q during delete", id.KeyName) + return fmt.Errorf("unable to delete key %s in workspace %s: %v", id.KeyName, id.WorkspaceName, err) } } diff --git a/internal/services/synapse/synapse_workspace_key_resource_test.go b/internal/services/synapse/synapse_workspace_key_resource_test.go index 1577fbf6ef24..3b9b02af95f8 100644 --- a/internal/services/synapse/synapse_workspace_key_resource_test.go +++ b/internal/services/synapse/synapse_workspace_key_resource_test.go @@ -15,13 +15,37 @@ import ( type SynapseWorkspaceKeysResource struct{} -func TestAccSynapseWorkspaceKeys_customerManagedKey(t *testing.T) { +func TestAccSynapseWorkspaceKeys_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_synapse_workspace_key", "test") r := SynapseWorkspaceKeysResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.customerManagedKey(data), + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + // CMK takes a while to activate, so validation against the plan tends to fail. + data.ImportStep(), + }) +} + +func TestAccSynapseWorkspaceKeys_basicUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_synapse_workspace_key", "test") + r := SynapseWorkspaceKeysResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + // CMK takes a while to activate, so validation against the plan tends to fail. + data.ImportStep(), + { + Config: r.basicUpdate(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -48,64 +72,70 @@ func (r SynapseWorkspaceKeysResource) Exists(ctx context.Context, client *client return utils.Bool(true), nil } -func (r SynapseWorkspaceKeysResource) customerManagedKey(data acceptance.TestData) string { +func (r SynapseWorkspaceKeysResource) basic(data acceptance.TestData) string { template := r.template(data) return fmt.Sprintf(` %s -data "azurerm_client_config" "current" {} - -resource "azurerm_key_vault" "test" { - name = "acckv%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" - purge_protection_enabled = true +resource "azurerm_synapse_workspace" "test" { + name = "acctestsw%[2]d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + storage_data_lake_gen2_filesystem_id = azurerm_storage_data_lake_gen2_filesystem.test.id + sql_administrator_login = "sqladminuser" + sql_administrator_login_password = "H@Sh1CoR3!" + customer_managed_key { + key_versionless_id = azurerm_key_vault_key.test.versionless_id + key_name = "test_key" + } } -resource "azurerm_key_vault_access_policy" "deployer" { +resource "azurerm_key_vault_access_policy" "workspace_policy" { key_vault_id = azurerm_key_vault.test.id - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id + tenant_id = azurerm_synapse_workspace.test.identity[0].tenant_id + object_id = azurerm_synapse_workspace.test.identity[0].principal_id key_permissions = [ - "Create", "Get", "Delete", "Purge" + "Get", "WrapKey", "UnwrapKey" ] } -resource "azurerm_key_vault_key" "test" { - name = "key" - key_vault_id = azurerm_key_vault.test.id - key_type = "RSA" - key_size = 2048 - key_opts = [ - "unwrapKey", - "wrapKey" - ] - depends_on = [ - azurerm_key_vault_access_policy.deployer - ] +resource "azurerm_synapse_workspace_key" "test" { + customer_managed_key_versionless_id = azurerm_key_vault_key.test.versionless_id + synapse_workspace_id = azurerm_synapse_workspace.test.id + active = true + cusomter_managed_key_name = "test_key" + depends_on = [azurerm_key_vault_access_policy.workspace_policy] } +resource "azurerm_synapse_workspace_key" "test2" { + customer_managed_key_versionless_id = azurerm_key_vault_key.test2.versionless_id + synapse_workspace_id = azurerm_synapse_workspace.test.id + active = false + cusomter_managed_key_name = "test_key2" + depends_on = [azurerm_key_vault_access_policy.workspace_policy, azurerm_synapse_workspace_key.test] +} +`, template, data.RandomInteger) +} + +func (r SynapseWorkspaceKeysResource) basicUpdate(data acceptance.TestData) string { + template := r.template(data) + return fmt.Sprintf(` +%s + resource "azurerm_synapse_workspace" "test" { - name = "acctestsw%d" + name = "acctestsw%[2]d" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location storage_data_lake_gen2_filesystem_id = azurerm_storage_data_lake_gen2_filesystem.test.id sql_administrator_login = "sqladminuser" sql_administrator_login_password = "H@Sh1CoR3!" customer_managed_key { - key_versionless_id = azurerm_key_vault_key.test.versionless_id - key_name = "test_key" - + key_versionless_id = azurerm_key_vault_key.test2.versionless_id + key_name = "test_key2" } - } - - - resource "azurerm_key_vault_access_policy" "workspace_policy" { key_vault_id = azurerm_key_vault.test.id tenant_id = azurerm_synapse_workspace.test.identity[0].tenant_id @@ -119,16 +149,19 @@ resource "azurerm_key_vault_access_policy" "workspace_policy" { resource "azurerm_synapse_workspace_key" "test" { customer_managed_key_versionless_id = azurerm_key_vault_key.test.versionless_id synapse_workspace_id = azurerm_synapse_workspace.test.id - active = true + active = false cusomter_managed_key_name = "test_key" depends_on = [azurerm_key_vault_access_policy.workspace_policy] } - - - - -`, template, data.RandomInteger, data.RandomInteger) +resource "azurerm_synapse_workspace_key" "test2" { + customer_managed_key_versionless_id = azurerm_key_vault_key.test2.versionless_id + synapse_workspace_id = azurerm_synapse_workspace.test.id + active = true + cusomter_managed_key_name = "test_key2" + depends_on = [azurerm_key_vault_access_policy.workspace_policy, azurerm_synapse_workspace_key.test] +} +`, template, data.RandomInteger) } func (r SynapseWorkspaceKeysResource) template(data acceptance.TestData) string { @@ -143,12 +176,12 @@ provider "azurerm" { } resource "azurerm_resource_group" "test" { - name = "acctestRG-synapse-%d" - location = "%s" + name = "acctestRG-synapse-%[3]d" + location = "%[1]s" } resource "azurerm_storage_account" "test" { - name = "acctestacc%s" + name = "acctestacc%[2]s" resource_group_name = azurerm_resource_group.test.name location = azurerm_resource_group.test.location account_kind = "BlobStorage" @@ -157,8 +190,58 @@ resource "azurerm_storage_account" "test" { } resource "azurerm_storage_data_lake_gen2_filesystem" "test" { - name = "acctest-%d" + name = "acctest-%[3]d" storage_account_id = azurerm_storage_account.test.id } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) + +data "azurerm_client_config" "current" {} + +resource "azurerm_key_vault" "test" { + name = "acckv%[3]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" + purge_protection_enabled = true +} + +resource "azurerm_key_vault_access_policy" "deployer" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + key_permissions = [ + "Create", "Get", "Delete", "Purge" + ] +} + +resource "azurerm_key_vault_key" "test" { + name = "key" + key_vault_id = azurerm_key_vault.test.id + key_type = "RSA" + key_size = 2048 + key_opts = [ + "unwrapKey", + "wrapKey" + ] + depends_on = [ + azurerm_key_vault_access_policy.deployer + ] +} + +resource "azurerm_key_vault_key" "test2" { + name = "key2" + key_vault_id = azurerm_key_vault.test.id + key_type = "RSA" + key_size = 2048 + key_opts = [ + "unwrapKey", + "wrapKey" + ] + depends_on = [ + azurerm_key_vault_access_policy.deployer + ] +} + +`, data.Locations.Primary, data.RandomString, data.RandomInteger) } diff --git a/internal/services/synapse/synapse_workspace_resource.go b/internal/services/synapse/synapse_workspace_resource.go index 4d614ac93570..1bfbd93033ef 100644 --- a/internal/services/synapse/synapse_workspace_resource.go +++ b/internal/services/synapse/synapse_workspace_resource.go @@ -1,6 +1,7 @@ package synapse import ( + "context" "fmt" "log" "net/url" @@ -564,7 +565,7 @@ func resourceSynapseWorkspaceUpdate(d *pluginsdk.ResourceData, meta interface{}) return err } - if d.HasChanges("tags", "sql_administrator_login_password", "github_repo", "azure_devops_repo", "customer_managed_key_versionless_id") { + if d.HasChanges("tags", "sql_administrator_login_password", "github_repo", "azure_devops_repo", "customer_managed_key") { publicNetworkAccess := synapse.WorkspacePublicNetworkAccessEnabled if !d.Get("public_network_access_enabled").(bool) { publicNetworkAccess = synapse.WorkspacePublicNetworkAccessDisabled @@ -600,6 +601,26 @@ func resourceSynapseWorkspaceUpdate(d *pluginsdk.ResourceData, meta interface{}) if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { return fmt.Errorf("waiting on updating future for Synapse Workspace %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } + + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{ + "Updating", + "ActivatingWorkspace", + }, + Target: []string{ + "Succeeded", + "Consistent", + "AwaitingUserAction", + }, + Refresh: synapseWorkspaceCMKUpdateStateRefreshFunc(ctx, client, id), + MinTimeout: 5 * time.Second, + ContinuousTargetOccurence: 5, + Timeout: d.Timeout(pluginsdk.TimeoutCreate), + } + + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("failed waiting for updating %s: %+v", id, err) + } } if d.HasChange("aad_admin") { @@ -683,6 +704,19 @@ func resourceSynapseWorkspaceDelete(d *pluginsdk.ResourceData, meta interface{}) return nil } +func synapseWorkspaceCMKUpdateStateRefreshFunc(ctx context.Context, client *synapse.WorkspacesClient, id *parse.WorkspaceId) pluginsdk.StateRefreshFunc { + return func() (interface{}, string, error) { + res, err := client.Get(ctx, id.ResourceGroup, id.Name) + if err != nil { + return nil, "", fmt.Errorf("retrieving %s: %+v", id, err) + } + if res.Encryption != nil && res.Encryption.Cmk != nil { + return res, *res.Encryption.Cmk.Status, nil + } + return res, "Succeeded", nil + } +} + func expandArmWorkspaceDataLakeStorageAccountDetails(storageDataLakeGen2FilesystemId string) *synapse.DataLakeStorageAccountDetails { uri, _ := url.Parse(storageDataLakeGen2FilesystemId) return &synapse.DataLakeStorageAccountDetails{ From c6de7cc1916c2ad268e4230003f3a153220ba943 Mon Sep 17 00:00:00 2001 From: henglu Date: Tue, 22 Mar 2022 09:38:08 +0800 Subject: [PATCH 2/5] remove comment --- .../services/synapse/synapse_workspace_key_resource_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/services/synapse/synapse_workspace_key_resource_test.go b/internal/services/synapse/synapse_workspace_key_resource_test.go index 3b9b02af95f8..e1d9050c0558 100644 --- a/internal/services/synapse/synapse_workspace_key_resource_test.go +++ b/internal/services/synapse/synapse_workspace_key_resource_test.go @@ -26,7 +26,6 @@ func TestAccSynapseWorkspaceKeys_basic(t *testing.T) { check.That(data.ResourceName).ExistsInAzure(r), ), }, - // CMK takes a while to activate, so validation against the plan tends to fail. data.ImportStep(), }) } @@ -42,7 +41,6 @@ func TestAccSynapseWorkspaceKeys_basicUpdate(t *testing.T) { check.That(data.ResourceName).ExistsInAzure(r), ), }, - // CMK takes a while to activate, so validation against the plan tends to fail. data.ImportStep(), { Config: r.basicUpdate(data), @@ -50,7 +48,6 @@ func TestAccSynapseWorkspaceKeys_basicUpdate(t *testing.T) { check.That(data.ResourceName).ExistsInAzure(r), ), }, - // CMK takes a while to activate, so validation against the plan tends to fail. data.ImportStep(), }) } From 768024a7a91d9907c55244cb2f7f32b25eba7323 Mon Sep 17 00:00:00 2001 From: henglu Date: Tue, 22 Mar 2022 10:29:07 +0800 Subject: [PATCH 3/5] fix failed test --- .../services/synapse/synapse_workspace_key_resource_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/services/synapse/synapse_workspace_key_resource_test.go b/internal/services/synapse/synapse_workspace_key_resource_test.go index e1d9050c0558..072dd28f80b4 100644 --- a/internal/services/synapse/synapse_workspace_key_resource_test.go +++ b/internal/services/synapse/synapse_workspace_key_resource_test.go @@ -85,6 +85,9 @@ resource "azurerm_synapse_workspace" "test" { key_versionless_id = azurerm_key_vault_key.test.versionless_id key_name = "test_key" } + identity { + type = "SystemAssigned" + } } resource "azurerm_key_vault_access_policy" "workspace_policy" { From d8dfb99d32e2f1c9430268fba358d46465fe72dc Mon Sep 17 00:00:00 2001 From: henglu Date: Tue, 22 Mar 2022 11:08:59 +0800 Subject: [PATCH 4/5] fix failed test --- .../services/synapse/synapse_workspace_key_resource_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/services/synapse/synapse_workspace_key_resource_test.go b/internal/services/synapse/synapse_workspace_key_resource_test.go index 072dd28f80b4..5ad8b56755d8 100644 --- a/internal/services/synapse/synapse_workspace_key_resource_test.go +++ b/internal/services/synapse/synapse_workspace_key_resource_test.go @@ -134,6 +134,9 @@ resource "azurerm_synapse_workspace" "test" { key_versionless_id = azurerm_key_vault_key.test2.versionless_id key_name = "test_key2" } + identity { + type = "SystemAssigned" + } } resource "azurerm_key_vault_access_policy" "workspace_policy" { From bd699e99f27947a9104cbe537b24a3383eaac136 Mon Sep 17 00:00:00 2001 From: henglu Date: Tue, 22 Mar 2022 12:18:37 +0800 Subject: [PATCH 5/5] add wait in create func --- .../synapse/synapse_workspace_resource.go | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/internal/services/synapse/synapse_workspace_resource.go b/internal/services/synapse/synapse_workspace_resource.go index 1bfbd93033ef..121a7182bef0 100644 --- a/internal/services/synapse/synapse_workspace_resource.go +++ b/internal/services/synapse/synapse_workspace_resource.go @@ -416,6 +416,10 @@ func resourceSynapseWorkspaceCreate(d *pluginsdk.ResourceData, meta interface{}) return fmt.Errorf("waiting for creation of %s: %+v", id, err) } + if err := waitSynapseWorkspaceCMKState(ctx, client, &id); err != nil { + return fmt.Errorf("failed waiting for updating %s: %+v", id, err) + } + aadAdmin := expandArmWorkspaceAadAdminInfo(d.Get("aad_admin").([]interface{})) if aadAdmin != nil { future, err := aadAdminClient.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, *aadAdmin) @@ -602,23 +606,7 @@ func resourceSynapseWorkspaceUpdate(d *pluginsdk.ResourceData, meta interface{}) return fmt.Errorf("waiting on updating future for Synapse Workspace %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) } - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{ - "Updating", - "ActivatingWorkspace", - }, - Target: []string{ - "Succeeded", - "Consistent", - "AwaitingUserAction", - }, - Refresh: synapseWorkspaceCMKUpdateStateRefreshFunc(ctx, client, id), - MinTimeout: 5 * time.Second, - ContinuousTargetOccurence: 5, - Timeout: d.Timeout(pluginsdk.TimeoutCreate), - } - - if _, err := stateConf.WaitForStateContext(ctx); err != nil { + if err := waitSynapseWorkspaceCMKState(ctx, client, id); err != nil { return fmt.Errorf("failed waiting for updating %s: %+v", id, err) } } @@ -704,6 +692,33 @@ func resourceSynapseWorkspaceDelete(d *pluginsdk.ResourceData, meta interface{}) return nil } +func waitSynapseWorkspaceCMKState(ctx context.Context, client *synapse.WorkspacesClient, id *parse.WorkspaceId) error { + deadline, ok := ctx.Deadline() + if !ok { + return fmt.Errorf("context had no deadline") + } + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{ + "Updating", + "ActivatingWorkspace", + }, + Target: []string{ + "Succeeded", + "Consistent", + "AwaitingUserAction", + }, + Refresh: synapseWorkspaceCMKUpdateStateRefreshFunc(ctx, client, id), + MinTimeout: 5 * time.Second, + ContinuousTargetOccurence: 5, + Timeout: time.Until(deadline), + } + + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("failed waiting for updating %s: %+v", id, err) + } + return nil +} + func synapseWorkspaceCMKUpdateStateRefreshFunc(ctx context.Context, client *synapse.WorkspacesClient, id *parse.WorkspaceId) pluginsdk.StateRefreshFunc { return func() (interface{}, string, error) { res, err := client.Get(ctx, id.ResourceGroup, id.Name)