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.

Sign up for GitHub

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

Jump to bottom

azurerm_synapse_workspace_key - can not be correctly rotated #15897

Merged
merged 6 commits into from Mar 22, 2022
Merged

azurerm_synapse_workspace_key - can not be correctly rotated #15897

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bfaf955
fix synapse workspace cmk rotation
ms-henglu Mar 18, 2022
c6de7cc
remove comment
ms-henglu Mar 22, 2022
768024a
fix failed test
ms-henglu Mar 22, 2022
d8dfb99
fix failed test
ms-henglu Mar 22, 2022
bd699e9
add wait in create func
ms-henglu Mar 22, 2022
95f6940
Merge branch 'main' into ticket-13872626-fix-workspace-cmk-rotation
katbyte Mar 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 deletions internal/services/synapse/synapse_workspace_key_resource.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}

Expand Down
177 changes: 130 additions & 47 deletions internal/services/synapse/synapse_workspace_key_resource_test.go
View file
Open in desktop
Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be waiting on activation then? with a state wait until it is done so this never fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @katbyte ,

Yes, I've added the waiting part in the first commit. And I've removed this comments in the second commit. Please take another look, thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only the the update function? what about create? shouldn't it be there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, I've added another commit to add it in both create and update.

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),
),
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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"
Expand All @@ -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)
}
36 changes: 35 additions & 1 deletion internal/services/synapse/synapse_workspace_resource.go
View file
Open in desktop
@@ -1,6 +1,7 @@
package synapse

import (
"context"
"fmt"
"log"
"net/url"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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{
Expand Down