From 01b63ac1017359f0aab9436001933748a1af921a Mon Sep 17 00:00:00 2001 From: Alex Ott Date: Tue, 7 Jan 2025 17:10:09 +0100 Subject: [PATCH] [Fix] Send only what is required in Update of `databricks_credential` (#4349) ## Changes This is a workaround for a problem in the API spec - right now, the Update operation tries to send the full struct, although the API accepts only one attribute of that struct. For GCP we should omit the struct completely... The problem doesn't exist for `storage_credential` on AWS as it uses the correct structure for the Update request, but problem exists for GCP. Resolves #4335 ## Tests - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [x] covered with integration tests in `internal/acceptance` - [x] using Go SDK - [ ] using TF Plugin Framework --- catalog/resource_credential.go | 11 +++++ catalog/resource_storage_credential.go | 6 +++ internal/acceptance/credential_test.go | 45 ++++++++++++------- .../acceptance/storage_credential_test.go | 22 ++++++--- 4 files changed, 61 insertions(+), 23 deletions(-) diff --git a/catalog/resource_credential.go b/catalog/resource_credential.go index 0f5921c81c..5d9d6edf3b 100644 --- a/catalog/resource_credential.go +++ b/catalog/resource_credential.go @@ -139,6 +139,17 @@ func ResourceCredential() common.Resource { } updateCredRequest.Owner = "" + // Workaround until backend team fixes API issue + if updateCredRequest.AwsIamRole != nil { // Update API accepts only RoleArn, not the rest of attributes + updateCredRequest.AwsIamRole = &catalog.AwsIamRole{RoleArn: updateCredRequest.AwsIamRole.RoleArn} + } + if updateCredRequest.AzureManagedIdentity != nil { + updateCredRequest.AzureManagedIdentity.CredentialId = "" // this is Computed attribute + } + if updateCredRequest.DatabricksGcpServiceAccount != nil { // we can't update it at all + updateCredRequest.DatabricksGcpServiceAccount = nil + } + // End of workaround _, err = w.Credentials.UpdateCredential(ctx, updateCredRequest) if err != nil { if d.HasChange("owner") { diff --git a/catalog/resource_storage_credential.go b/catalog/resource_storage_credential.go index 25a5a9ffd8..40f6fd4721 100644 --- a/catalog/resource_storage_credential.go +++ b/catalog/resource_storage_credential.go @@ -185,6 +185,9 @@ func ResourceStorageCredential() common.Resource { if d.HasChange("read_only") { update.ForceSendFields = append(update.ForceSendFields, "ReadOnly") } + if update.DatabricksGcpServiceAccount != nil { // we can't update it at all + update.DatabricksGcpServiceAccount = nil + } update.Owner = "" _, err := acc.StorageCredentials.Update(ctx, catalog.AccountsUpdateStorageCredential{ CredentialInfo: &update, @@ -232,6 +235,9 @@ func ResourceStorageCredential() common.Resource { if d.HasChange("read_only") { update.ForceSendFields = append(update.ForceSendFields, "ReadOnly") } + if update.DatabricksGcpServiceAccount != nil { // we can't update it at all + update.DatabricksGcpServiceAccount = nil + } update.Owner = "" _, err = w.StorageCredentials.Update(ctx, update) if err != nil { diff --git a/internal/acceptance/credential_test.go b/internal/acceptance/credential_test.go index 80e3cb5b8f..24c88d62ab 100644 --- a/internal/acceptance/credential_test.go +++ b/internal/acceptance/credential_test.go @@ -1,35 +1,48 @@ package acceptance import ( + "fmt" "testing" ) -func TestUcAccCredential(t *testing.T) { - LoadUcwsEnv(t) - if IsAws(t) { - UnityWorkspaceLevel(t, Step{ - Template: ` +func awsCredentialWithComment(comment string) string { + return fmt.Sprintf(` resource "databricks_credential" "external" { - name = "service-cred-{var.RANDOM}" + name = "service-cred-{var.STICKY_RANDOM}" aws_iam_role { role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}" } purpose = "SERVICE" skip_validation = true - comment = "Managed by TF" - }`, - }) - } else if IsGcp(t) { - UnityWorkspaceLevel(t, Step{ - // TODO: update purpose to SERVICE when it's released - Template: ` + comment = "%s" + }`, comment) +} + +func gcpCredentialWithComment(comment string) string { + // TODO: update purpose to SERVICE when it's released + return fmt.Sprintf(` resource "databricks_credential" "external" { - name = "storage-cred-{var.RANDOM}" + name = "storage-cred-{var.STICKY_RANDOM}" databricks_gcp_service_account {} purpose = "STORAGE" skip_validation = true - comment = "Managed by TF" - }`, + comment = "%s" + }`, comment) +} + +func TestUcAccCredential(t *testing.T) { + LoadUcwsEnv(t) + if IsAws(t) { + UnityWorkspaceLevel(t, Step{ + Template: awsCredentialWithComment("Managed by TF"), + }, Step{ + Template: awsCredentialWithComment("Managed by TF updated"), + }) + } else if IsGcp(t) { + UnityWorkspaceLevel(t, Step{ + Template: gcpCredentialWithComment("Managed by TF"), + }, Step{ + Template: gcpCredentialWithComment("Managed by TF updated"), }) } } diff --git a/internal/acceptance/storage_credential_test.go b/internal/acceptance/storage_credential_test.go index bdc10e8324..26f1b03275 100644 --- a/internal/acceptance/storage_credential_test.go +++ b/internal/acceptance/storage_credential_test.go @@ -1,9 +1,21 @@ package acceptance import ( + "fmt" "testing" ) +func gcpStorageCredentialWithComment(comment string) string { + // TODO: update purpose to SERVICE when it's released + return fmt.Sprintf(` + resource "databricks_storage_credential" "external" { + name = "cred-{var.RANDOM}" + databricks_gcp_service_account {} + skip_validation = true + comment = "%s" + }`, comment) +} + func TestUcAccStorageCredential(t *testing.T) { LoadUcwsEnv(t) if IsAws(t) { @@ -30,13 +42,9 @@ func TestUcAccStorageCredential(t *testing.T) { }) } else if IsGcp(t) { UnityWorkspaceLevel(t, Step{ - Template: ` - resource "databricks_storage_credential" "external" { - name = "cred-{var.RANDOM}" - databricks_gcp_service_account {} - skip_validation = true - comment = "Managed by TF" - }`, + Template: gcpStorageCredentialWithComment("Managed by TF"), + }, Step{ + Template: gcpStorageCredentialWithComment("Managed by TF updated"), }) } }