From 1e89ad4d0a71d2867d6fdddab1a5726b5287ee1d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 15:19:16 +0200 Subject: [PATCH] [Fix] Ignore presence or absence of `/Workspace` prefix for dashboard 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 --- common/resource.go | 10 ++++++++++ common/resource_test.go | 8 ++++++++ dashboards/resource_dashboard.go | 2 +- internal/acceptance/dashboard_test.go | 18 +++++++++++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/common/resource.go b/common/resource.go index 77ba894853..9e639eb962 100644 --- a/common/resource.go +++ b/common/resource.go @@ -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) diff --git a/common/resource_test.go b/common/resource_test.go index 360c10b476..e93885a02c 100644 --- a/common/resource_test.go +++ b/common/resource_test.go @@ -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)) diff --git a/dashboards/resource_dashboard.go b/dashboards/resource_dashboard.go index 531a6e5de4..d872b33f49 100644 --- a/dashboards/resource_dashboard.go +++ b/dashboards/resource_dashboard.go @@ -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"}) diff --git a/internal/acceptance/dashboard_test.go b/internal/acceptance/dashboard_test.go index 5fbf28b03a..49118c9455 100644 --- a/internal/acceptance/dashboard_test.go +++ b/internal/acceptance/dashboard_test.go @@ -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 { @@ -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\"}]}`, + })), + }) +}