Skip to content

Commit

Permalink
[Fix] Ignore presence or absence of /Workspace prefix for dashboard…
Browse files Browse the repository at this point in the history
… resource (#4061)

## Changes

Users may choose to include the `/Workspace` prefix in the `parent_path`
attribute of a dashboard to unambiguously refer to the workspace file
system. This prefix is not included in reads, triggering a recreate.

This change ignores this type of diff.

## Tests

- [x] `make test` run locally
- [ ] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [ ] using Go SDK
  • Loading branch information
pietern authored Oct 1, 2024
1 parent afdbafc commit 1e89ad4
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
10 changes: 10 additions & 0 deletions common/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,16 @@ func genericDatabricksData[T, P, C any](
}
}

// WorkspacePathPrefixDiffSuppress suppresses diffs for workspace paths where both sides
// may or may not include the `/Workspace` prefix.
//
// This is the case for dashboards where at create time, the user may include the `/Workspace`
// prefix for the `parent_path` field, but the read response will not include the prefix.
func WorkspacePathPrefixDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
const prefix = "/Workspace"
return strings.TrimPrefix(old, prefix) == strings.TrimPrefix(new, prefix)
}

func EqualFoldDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
if strings.EqualFold(old, new) {
log.Printf("[INFO] Suppressing diff on %s", k)
Expand Down
8 changes: 8 additions & 0 deletions common/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ func TestCustomizeDiffRobustness(t *testing.T) {
assert.EqualError(t, err, "cannot customize diff for sample: panic: oops")
}

func TestWorkspacePathPrefixDiffSuppress(t *testing.T) {
assert.True(t, WorkspacePathPrefixDiffSuppress("k", "/Workspace/foo/bar", "/Workspace/foo/bar", nil))
assert.True(t, WorkspacePathPrefixDiffSuppress("k", "/Workspace/foo/bar", "/foo/bar", nil))
assert.True(t, WorkspacePathPrefixDiffSuppress("k", "/foo/bar", "/Workspace/foo/bar", nil))
assert.True(t, WorkspacePathPrefixDiffSuppress("k", "/foo/bar", "/foo/bar", nil))
assert.False(t, WorkspacePathPrefixDiffSuppress("k", "/Workspace/1", "/Workspace/2", nil))
}

func TestEqualFoldDiffSuppress(t *testing.T) {
assert.True(t, EqualFoldDiffSuppress("k", "A", "a", nil))
assert.False(t, EqualFoldDiffSuppress("k", "A", "A2", nil))
Expand Down
2 changes: 1 addition & 1 deletion dashboards/resource_dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (Dashboard) CustomizeSchema(s *common.CustomizableSchema) *common.Customiza
s.SchemaPath("md5").SetComputed()

// ForceNew fields
s.SchemaPath("parent_path").SetForceNew()
s.SchemaPath("parent_path").SetCustomSuppressDiff(common.WorkspacePathPrefixDiffSuppress).SetForceNew()

// ConflictsWith fields
s.SchemaPath("serialized_dashboard").SetConflictsWith([]string{"file_path"})
Expand Down
18 changes: 17 additions & 1 deletion internal/acceptance/dashboard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ resource "databricks_permissions" "dashboard_usage" {
return templateString
}

// Altough EmbedCredentials is an optional field, please specify its value if you want to modify it.
// Although EmbedCredentials is an optional field, please specify its value if you want to modify it.
func (t *templateStruct) SetAttributes(mapper map[string]string) templateStruct {
// Switch case for each attribute. If it is set in the mapper, set it in the struct
if val, ok := mapper["display_name"]; ok {
Expand Down Expand Up @@ -491,3 +491,19 @@ func TestAccDashboardTestAll(t *testing.T) {
}),
})
}

func TestAccDashboardWithWorkspacePrefix(t *testing.T) {
var template templateStruct

// Test that the dashboard can use a /Workspace prefix on the parent path and not trigger recreation.
// If this does NOT work, the test fails with an error that the non-refresh plan is non-empty.

WorkspaceLevel(t, Step{
Template: makeTemplate(template.SetAttributes(map[string]string{
"display_name": fmt.Sprintf("Test Dashboard - %s", qa.RandomName()),
"warehouse_id": "{env.TEST_DEFAULT_WAREHOUSE_ID}",
"parent_path": "/Workspace/Shared/provider-test",
"serialized_dashboard": `{\"pages\":[{\"name\":\"new_name\",\"displayName\":\"New Page\"}]}`,
})),
})
}

0 comments on commit 1e89ad4

Please sign in to comment.