azurerm_databricks_workspace: Add support for CMK for disk encryption, multiple other updates

[favoretti]

Introduce 2 new arguments:

managed_disk_cmk_key_vault_key_id
managed_disk_cmk_rotation_to_latest_version_enabled

Introduce 1 new attribute:
disk_encryption_set_id

public_network_access_enabled, network_security_group_rules_required, managed_services_cmk_key_vault_key_id can be updated now.

$ TF_ACC=1 go test -v ./internal/services/databricks -timeout=1000m -run='managedDiskCMK'
=== RUN   TestAccDatabricksWorkspace_managedDiskCMK
=== PAUSE TestAccDatabricksWorkspace_managedDiskCMK
=== CONT  TestAccDatabricksWorkspace_managedDiskCMK
--- PASS: TestAccDatabricksWorkspace_managedDiskCMK (599.13s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/databricks	600.301s

$ TF_ACC=1 go test -v ./internal/services/databricks -timeout=1000m -run='TestAccDatabricksWorkspace_update'
=== RUN   TestAccDatabricksWorkspace_update
=== PAUSE TestAccDatabricksWorkspace_update
=== RUN   TestAccDatabricksWorkspace_updateSKU
=== PAUSE TestAccDatabricksWorkspace_updateSKU
=== CONT  TestAccDatabricksWorkspace_update
=== CONT  TestAccDatabricksWorkspace_updateSKU
--- PASS: TestAccDatabricksWorkspace_updateSKU (838.39s)
--- PASS: TestAccDatabricksWorkspace_update (853.92s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/databricks	855.058s

[favoretti]

@alexott here :)

[stephybun]

Thanks for this PR @favoretti, I left a few suggestions in-line but overall this is looking good!

[alexott]

looks good for me.
Will DBFS encryption change come as a separate PR?

[favoretti]

looks good for me. Will DBFS encryption change come as a separate PR?

As discussed offline, made it updatable as well.

[stephybun]

Could we add a test for updating the properties that are no longer ForceNew? Otherwise this should be good to go 👍

[alexott]

@favoretti new functionality is also supporting update of no_public_ip from false to true, but right now it's marked as ForceNew so it will recreate the workspace. I just tested with removed ForceNew and was able to update the existing workspace without recreation. The only issue is that such update is allowed only from false to true and only for workspaces with VNet injection, not for with managed VNets. This somehow should be added into the validation logic.

[alexott]

managed_disk_cmk_key_vault_key_id is marked as ForceNew so it recreates things on update

Also, I don't see that managedDiskIdentity.principalId and managedDiskIdentity.tenantId are exposed - we need them to set KV access policy

[favoretti]

$ TF_ACC=1 go test -v ./internal/services/databricks -timeout=1000m -run='managedDiskCMK'
=== RUN   TestAccDatabricksWorkspace_managedDiskCMK
=== PAUSE TestAccDatabricksWorkspace_managedDiskCMK
=== CONT  TestAccDatabricksWorkspace_managedDiskCMK
--- PASS: TestAccDatabricksWorkspace_managedDiskCMK (649.28s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/databricks	650.953s

[stephybun]

There's a new test failure regarding the update of public network access

------- Stdout: -------
=== RUN   TestAccDatabricksWorkspace_extendedUpdate
=== PAUSE TestAccDatabricksWorkspace_extendedUpdate
=== CONT  TestAccDatabricksWorkspace_extendedUpdate
testcase.go:110: Step 3/4 error: Error running apply: exit status 1
Error: creating/updating Workspace (Subscription: "*******"
Resource Group Name: "acctestRG-databricks-230119204137101457"
Workspace Name: "acctestDBW-230119204137101457"): performing CreateOrUpdate: workspaces.WorkspacesClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="InvalidPublicNetworkAccess" Message="Updating public network access is currently not allowed"
with azurerm_databricks_workspace.test,
on terraform_plugin_test.tf line 87, in resource "azurerm_databricks_workspace" "test":
87: resource "azurerm_databricks_workspace" "test" {
--- FAIL: TestAccDatabricksWorkspace_extendedUpdate (534.84s)
FAIL

[alexott]

managed_disk_cmk_key_vault_key_id is still marked with ForceNew: true that causes workspace recreation. I made a custom build without it, and was able to perform upgrade. Documentation also needs to be updated to reflect this.

no_public_ip is also marked as ForceNew: true. We need to remove it as we support in-place update for a limited set of scenarios (only false -> true and only for VNet-injected workspaces). Documentation also needs to be updated

[alexott]

We need to remove Changing this forces a new resource to be created.

[favoretti]

Ok, so for the managed_disk_cmk... I've removed both ForceNew and docs. But as to no_public_ip - I'd leave it on ForceNew, since the use-case where it can be updated is somewhat all too narrow.

@stephybun What do you think?
@alexott Also, can you please let @stephybun know what needs to be done to the subscription by MSFT to allow for public_network_access updates? Thanks!

[alexott]

ForceNew on no_public_ip unfortunately may block some of the old customers from upgrade... I don't see the problem of failing if the people are trying to update it, instead of forcing recreation

[favoretti]

I've updated the no_public_ip parameter to not force re-creation and added a note in the docs describing which use-case allows for an update.

[stephybun]

I think we should add a ForceNewIfChange to recreate the workspace if a user tries to update from true to false.

[alexott]

@stephybun we need to check with dev team if it's really required. I expect that it should be possible to update it in-place

[alexott]

lgtm

[stephybun]

One more comment about the update test, but otherwise this looks like it's good to go!

[stephybun]

Thanks for this @favoretti! I hope you don't mind I pushed a commit to change the value for no_public_ip in the complete test config to still test the update of this property for a valid scenario. The tests are passing so this LGTM 🎉

[github-actions]

This functionality has been released in v3.41.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.