Skip to content

Commit

Permalink
[Fix] Send only what is required in Update of databricks_credential (
Browse files Browse the repository at this point in the history
…#4349)

## Changes
<!-- Summary of your changes that are easy to understand -->

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
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant 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
  • Loading branch information
alexott authored Jan 7, 2025
1 parent 3a60749 commit 01b63ac
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 23 deletions.
11 changes: 11 additions & 0 deletions catalog/resource_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
6 changes: 6 additions & 0 deletions catalog/resource_storage_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
45 changes: 29 additions & 16 deletions internal/acceptance/credential_test.go
Original file line number Diff line number Diff line change
@@ -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"),
})
}
}
Expand Down
22 changes: 15 additions & 7 deletions internal/acceptance/storage_credential_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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"),
})
}
}
Expand Down

0 comments on commit 01b63ac

Please sign in to comment.