databricks / terraform-provider-databricks

Databricks Terraform Provider
https://registry.terraform.io/providers/databricks/databricks/latest
Other
456 stars 393 forks source link

[ISSUE] `databricks_service_principal` resource recreated SPs when changing `display_name` #2360

Open clarkperkins opened 1 year ago

clarkperkins commented 1 year ago

I am using azure databricks. When I attempt to modify the display_name of an existing databricks_service_principal resource on either the account level or the workspace level, the databricks provider wants to delete and recreate the service principal. This should not be required, as azure databricks allows modifying display names of service accounts.

Configuration

resource "databricks_service_principal" "api_account" {
  provider = databricks.account

  display_name   = "engineering - API Access" // was previously "Engineering API Access"
  application_id = azurerm_user_assigned_identity.engineering_api.client_id
}

Expected Behavior

Plan should report an update-in-place change instead of replacing the service account.

Actual Behavior

Plan reports a replacement of the service account:

  # databricks_service_principal.api_account must be replaced
-/+ resource "databricks_service_principal" "api_account" {
      ~ display_name               = "Engineering API Access" -> "engineering - API Access" # forces replacement
      - force                      = false -> null
      ~ home                       = "/Users/<redacted>" -> (known after apply)
      ~ id                         = "<redacted>" -> (known after apply)
      ~ repos                      = "/Repos/<redacted>" -> (known after apply)
        # (6 unchanged attributes hidden)
    }

Steps to Reproduce

Terraform and provider versions

$ terraform version
Terraform v1.3.0
on darwin_arm64
+ provider registry.terraform.io/cyrilgdn/postgresql v1.19.0
+ provider registry.terraform.io/databricks/databricks v1.17.0
+ provider registry.terraform.io/hashicorp/azuread v2.39.0
+ provider registry.terraform.io/hashicorp/azurerm v3.58.0
+ provider registry.terraform.io/hashicorp/random v3.5.1

Your version of Terraform is out of date! The latest version
is 1.4.6. You can update by downloading from https://www.terraform.io/downloads.html

Debug Output

Important Factoids

iandexter commented 1 year ago

Hi, @clarkperkins -- did those other attributes actually change, or just the displayName after you apply?

clarkperkins commented 1 year ago

I didn't actually run the apply because I don't want to drop and re-create the service principals (as that will remove the PATs associate with the SPs). I will try running the apply in a test environment though to see what happens.

clarkperkins commented 1 year ago

So after running the apply, none of the other attributes actually changed. The id remained the same after dropping / recreating the SP. ~But it did nullify/expire the PATs for the SP as I suspected it would.~ EDIT: see comment below, this was incorrect.

iandexter commented 1 year ago

But it did nullify/expire the PATs for the SP as I suspected it would.

Ah, that is unexpected.

Using the PATCH API just updates displayName, too.

nkvuong commented 1 year ago

this is because we force recreating the sp if the display name is changed - https://github.com/databricks/terraform-provider-databricks/blob/master/scim/resource_service_principal.go#L95

If updating displayName is allowed in all cases, then we need to remove force_new here

clarkperkins commented 1 year ago

I don't have a way to test this out on AWS or GCP databricks - but azure databricks definitely allows updating displayName on service principals.

clarkperkins commented 1 year ago

As an update - I was doing some more testing around this issue today and realized the PATs DO actually still work after deleting and recreating an SP. I must have messed up something in my initial tests a couple of weeks ago. Sorry for the confusion there!