From 399a8a632ddaa2b7645584b2c1657b208ee461fb Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 17 Apr 2025 13:51:26 +0200 Subject: [PATCH 1/8] Read dashboard definition from file_path via filer interface --- .../deploy/dashboard/databricks.yml.tmpl | 11 +++ acceptance/bundle/deploy/dashboard/output.txt | 23 ++++++ .../dashboard/sample-dashboard.lvdash.json | 1 + acceptance/bundle/deploy/dashboard/script | 17 +++++ acceptance/bundle/deploy/dashboard/test.toml | 38 ++++++++++ ...nfigure_dashboards_serialized_dashboard.go | 75 +++++++++++++++++++ .../terraform/tfdyn/convert_dashboard.go | 28 ------- .../terraform/tfdyn/convert_dashboard_test.go | 48 ------------ bundle/phases/initialize.go | 10 +-- 9 files changed, 170 insertions(+), 81 deletions(-) create mode 100644 acceptance/bundle/deploy/dashboard/databricks.yml.tmpl create mode 100644 acceptance/bundle/deploy/dashboard/output.txt create mode 100644 acceptance/bundle/deploy/dashboard/sample-dashboard.lvdash.json create mode 100644 acceptance/bundle/deploy/dashboard/script create mode 100644 acceptance/bundle/deploy/dashboard/test.toml create mode 100644 bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go diff --git a/acceptance/bundle/deploy/dashboard/databricks.yml.tmpl b/acceptance/bundle/deploy/dashboard/databricks.yml.tmpl new file mode 100644 index 0000000000..27b425c98a --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/databricks.yml.tmpl @@ -0,0 +1,11 @@ +bundle: + name: deploy-dashboard-test-$UNIQUE_NAME + +resources: + dashboards: + dashboard1: + display_name: $DASHBOARD_DISPLAY_NAME + warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID + embed_credentials: true + file_path: "sample-dashboard.lvdash.json" + parent_path: /Users/$CURRENT_USER_NAME diff --git a/acceptance/bundle/deploy/dashboard/output.txt b/acceptance/bundle/deploy/dashboard/output.txt new file mode 100644 index 0000000000..42d9a779a1 --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/output.txt @@ -0,0 +1,23 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-test-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] lakeview get [DASHBOARD_ID] +{ + "lifecycle_state": "ACTIVE", + "parent_path": "/Users/[USERNAME]", + "path": "/Users/[USERNAME]/test bundle-deploy-dashboard [UUID].lvdash.json", + "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}" +} + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete dashboard dashboard1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-test-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/deploy/dashboard/sample-dashboard.lvdash.json b/acceptance/bundle/deploy/dashboard/sample-dashboard.lvdash.json new file mode 100644 index 0000000000..425cb33fb3 --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/sample-dashboard.lvdash.json @@ -0,0 +1 @@ +{"pages":[{"name":"02724bf2","displayName":"Dashboard test bundle-deploy-dashboard"}]} diff --git a/acceptance/bundle/deploy/dashboard/script b/acceptance/bundle/deploy/dashboard/script new file mode 100644 index 0000000000..35aa749f11 --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/script @@ -0,0 +1,17 @@ +DASHBOARD_DISPLAY_NAME="test bundle-deploy-dashboard $(uuid)" +if [ -z "$CLOUD_ENV" ]; then + DASHBOARD_DISPLAY_NAME="test bundle/deploy/ 6260d50f-e8ff-4905-8f28-812345678903" # use hard-coded uuid when running locally + export TEST_DEFAULT_WAREHOUSE_ID="warehouse-1234" +fi + +export DASHBOARD_DISPLAY_NAME +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +trace $CLI bundle deploy +DASHBOARD_ID=$($CLI bundle summary --output json | jq -r '.resources.dashboards.dashboard1.id') +trace $CLI lakeview get $DASHBOARD_ID | jq '{lifecycle_state, parent_path, path, serialized_dashboard}' diff --git a/acceptance/bundle/deploy/dashboard/test.toml b/acceptance/bundle/deploy/dashboard/test.toml new file mode 100644 index 0000000000..15d4860470 --- /dev/null +++ b/acceptance/bundle/deploy/dashboard/test.toml @@ -0,0 +1,38 @@ +Local = true +Cloud = true +RequiresWarehouse = true + +Ignore = [ + "databricks.yml", +] + +[[Repls]] +Old = "[0-9a-f]{32}" +New = "[DASHBOARD_ID]" + +[[Server]] +Pattern = "POST /api/2.0/lakeview/dashboards" +Response.Body = ''' +{ + "dashboard_id":"1234567890abcdef1234567890abcdef" +} +''' + +[[Server]] +Pattern = "POST /api/2.0/lakeview/dashboards/{dashboard_id}/published" + +[[Server]] +Pattern = "GET /api/2.0/lakeview/dashboards/{dashboard_id}" +Response.Body = ''' +{ + "dashboard_id":"1234567890abcdef1234567890abcdef", + "display_name": "test dashboard 6260d50f-e8ff-4905-8f28-812345678903", + "lifecycle_state": "ACTIVE", + "path": "/Users/[USERNAME]/test bundle-deploy-dashboard [UUID].lvdash.json", + "parent_path": "/Users/tester@databricks.com", + "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}" +} +''' + +[[Server]] +Pattern = "DELETE /api/2.0/lakeview/dashboards/{dashboard_id}" diff --git a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go new file mode 100644 index 0000000000..5d99cabbd0 --- /dev/null +++ b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go @@ -0,0 +1,75 @@ +package resourcemutator + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +const ( + filePathFieldName = "file_path" + serializedDashboardFieldName = "serialized_dashboard" +) + +type configureDashboardSerializedDashboard struct{} + +func ConfigureDashboardSerializedDashboard() bundle.Mutator { + return &configureDashboardSerializedDashboard{} +} + +func (c configureDashboardSerializedDashboard) Name() string { + return "ConfigureDashboardSerializedDashboard" +} + +func (c configureDashboardSerializedDashboard) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + ) + + // Configure serialized_dashboard field for all dashboards. + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + // Include "serialized_dashboard" field if "file_path" is set. + // Note: the Terraform resource supports "file_path" natively, but we read the contents of the dashboard here + // to cover the use case of deployments from the workspace + if path, ok := v.Get(filePathFieldName).AsString(); ok { + + contents, err := b.SyncRoot.ReadFile(path) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err) + } + + v, err := dyn.Set(v, serializedDashboardFieldName, dyn.V(string(contents))) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) + } + + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". + return dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + switch len(p) { + case 0: + return v, nil + case 1: + if p[0] == dyn.Key(filePathFieldName) { + return v, dyn.ErrDrop + } + } + + // Skip everything else. + return v, dyn.ErrSkip + }) + } + return v, nil + }) + }) + + diags = diags.Extend(diag.FromErr(err)) + return diags +} diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go index 3ec8b489f5..8734565999 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -12,7 +12,6 @@ import ( ) const ( - filePathFieldName = "file_path" serializedDashboardFieldName = "serialized_dashboard" ) @@ -48,33 +47,6 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er log.Debugf(ctx, "dashboard normalization diagnostic: %s", diag.Summary) } - // Include "serialized_dashboard" field if "file_path" is set. - // Note: the Terraform resource supports "file_path" natively, but its - // change detection mechanism doesn't work as expected at the time of writing (Sep 30). - if path, ok := vout.Get(filePathFieldName).AsString(); ok { - vout, err = dyn.Set(vout, serializedDashboardFieldName, dyn.V(fmt.Sprintf("${file(%q)}", path))) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) - } - // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". - vout, err = dyn.Walk(vout, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - switch len(p) { - case 0: - return v, nil - case 1: - if p[0] == dyn.Key(filePathFieldName) { - return v, dyn.ErrDrop - } - } - - // Skip everything else. - return v, dyn.ErrSkip - }) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to drop file_path: %w", err) - } - } - // Marshal "serialized_dashboard" as JSON if it is set in the input but not in the output. vout, err = marshalSerializedDashboard(vin, vout) if err != nil { diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index 669cf66f16..0f95b60937 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -59,54 +59,6 @@ func TestConvertDashboard(t *testing.T) { }, out.Permissions["dashboard_my_dashboard"]) } -func TestConvertDashboardFilePath(t *testing.T) { - src := resources.Dashboard{ - FilePath: "some/path", - } - - vin, err := convert.FromTyped(src, dyn.NilValue) - require.NoError(t, err) - - ctx := context.Background() - out := schema.NewResources() - err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) - require.NoError(t, err) - - // Assert that the "serialized_dashboard" is included. - assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ - "serialized_dashboard": "${file(\"some/path\")}", - }) - - // Assert that the "file_path" doesn't carry over. - assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ - "file_path": "some/path", - }) -} - -func TestConvertDashboardFilePathQuoted(t *testing.T) { - src := resources.Dashboard{ - FilePath: `C:\foo\bar\baz\dashboard.lvdash.json`, - } - - vin, err := convert.FromTyped(src, dyn.NilValue) - require.NoError(t, err) - - ctx := context.Background() - out := schema.NewResources() - err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) - require.NoError(t, err) - - // Assert that the "serialized_dashboard" is included. - assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ - "serialized_dashboard": `${file("C:\\foo\\bar\\baz\\dashboard.lvdash.json")}`, - }) - - // Assert that the "file_path" doesn't carry over. - assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ - "file_path": `C:\foo\bar\baz\dashboard.lvdash.json`, - }) -} - func TestConvertDashboardSerializedDashboardString(t *testing.T) { src := resources.Dashboard{ SerializedDashboard: `{ "json": true }`, diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index bc66f3da9d..67756f430b 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -119,6 +119,11 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // ApplyPresets through ResourceProcessor. resourcemutator.ApplyTargetMode(), + // Reads (typed): b.SyncRoot (checks if bundle root is in /Workspace/) + // Updates (typed): b.SyncRoot (replaces with extension-aware path when running on Databricks Runtime) + // Configure use of WSFS for reads if the CLI is running on Databricks. + mutator.ConfigureWSFS(), + // Static resources (e.g. YAML) are already loaded, we initialize and normalize them before Python resourcemutator.ProcessStaticResources(), @@ -136,11 +141,6 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Provides diagnostic recommendations if the current deployment identity isn't explicitly granted CAN_MANAGE permissions permissions.PermissionDiagnostics(), - // Reads (typed): b.SyncRoot (checks if bundle root is in /Workspace/) - // Updates (typed): b.SyncRoot (replaces with extension-aware path when running on Databricks Runtime) - // Configure use of WSFS for reads if the CLI is running on Databricks. - mutator.ConfigureWSFS(), - mutator.TranslatePaths(), // Reads (typed): b.Config.Experimental.PythonWheelWrapper, b.Config.Presets.SourceLinkedDeployment (checks Python wheel wrapper and deployment mode settings) From c28677409ca460ede54869ec4789275a87fe0faa Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:14:03 +0200 Subject: [PATCH 2/8] fix acceptance test --- acceptance/bundle/debug/out.stderr.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/bundle/debug/out.stderr.txt b/acceptance/bundle/debug/out.stderr.txt index 490d984a7f..cc0bae03c4 100644 --- a/acceptance/bundle/debug/out.stderr.txt +++ b/acceptance/bundle/debug/out.stderr.txt @@ -39,6 +39,7 @@ 10:07:59 Debug: Apply pid=12345 mutator=ResolveResourceReferences 10:07:59 Debug: Apply pid=12345 mutator=ResolveVariableReferences 10:07:59 Debug: Apply pid=12345 mutator=ApplyTargetMode +10:07:59 Debug: Apply pid=12345 mutator=ConfigureWSFS 10:07:59 Debug: Apply pid=12345 mutator=ProcessStaticResources 10:07:59 Debug: Apply pid=12345 mutator=ProcessStaticResources mutator=ResolveVariableReferences(resources) 10:07:59 Debug: Apply pid=12345 mutator=ProcessStaticResources mutator=NormalizePaths @@ -47,7 +48,6 @@ 10:07:59 Debug: Apply pid=12345 mutator=PythonMutator(load_resources) 10:07:59 Debug: Apply pid=12345 mutator=PythonMutator(apply_mutators) 10:07:59 Debug: Apply pid=12345 mutator=CheckPermissions -10:07:59 Debug: Apply pid=12345 mutator=ConfigureWSFS 10:07:59 Debug: Apply pid=12345 mutator=TranslatePaths 10:07:59 Debug: Apply pid=12345 mutator=PythonWrapperWarning 10:07:59 Debug: Apply pid=12345 mutator=artifacts.Prepare From efd5faa4b7d3e5ab5063cdd60c5c805d53ae17a0 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Tue, 22 Apr 2025 14:57:42 +0200 Subject: [PATCH 3/8] style fix: avoid indentation by returning early when ok is false --- ...nfigure_dashboards_serialized_dashboard.go | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go index 5d99cabbd0..1ea31416c6 100644 --- a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go +++ b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go @@ -24,7 +24,7 @@ func (c configureDashboardSerializedDashboard) Name() string { return "ConfigureDashboardSerializedDashboard" } -func (c configureDashboardSerializedDashboard) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics pattern := dyn.NewPattern( @@ -39,34 +39,35 @@ func (c configureDashboardSerializedDashboard) Apply(ctx context.Context, b *bun // Include "serialized_dashboard" field if "file_path" is set. // Note: the Terraform resource supports "file_path" natively, but we read the contents of the dashboard here // to cover the use case of deployments from the workspace - if path, ok := v.Get(filePathFieldName).AsString(); ok { + path, ok := v.Get(filePathFieldName).AsString() + if !ok { + return v, nil + } - contents, err := b.SyncRoot.ReadFile(path) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err) - } + contents, err := b.SyncRoot.ReadFile(path) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err) + } - v, err := dyn.Set(v, serializedDashboardFieldName, dyn.V(string(contents))) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) - } + v, err = dyn.Set(v, serializedDashboardFieldName, dyn.V(string(contents))) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) + } - // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". - return dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - switch len(p) { - case 0: - return v, nil - case 1: - if p[0] == dyn.Key(filePathFieldName) { - return v, dyn.ErrDrop - } + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". + return dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + switch len(p) { + case 0: + return v, nil + case 1: + if p[0] == dyn.Key(filePathFieldName) { + return v, dyn.ErrDrop } + } - // Skip everything else. - return v, dyn.ErrSkip - }) - } - return v, nil + // Skip everything else. + return v, dyn.ErrSkip + }) }) }) From 2e760eea8481298c7c84f69881367f9b7aad97c6 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Tue, 22 Apr 2025 17:34:49 +0200 Subject: [PATCH 4/8] refactor: move ConfigureDashboardSerializedDashboard call down the transformation pipeline, closer to terraform write --- .../mutator/paths/dashboard_paths_visitor.go | 2 +- ...nfigure_dashboards_serialized_dashboard.go | 47 +++++++------------ .../translate_paths_dashboards_test.go | 2 +- bundle/phases/bind.go | 4 ++ bundle/phases/deploy.go | 3 ++ bundle/phases/destroy.go | 3 ++ 6 files changed, 29 insertions(+), 32 deletions(-) diff --git a/bundle/config/mutator/paths/dashboard_paths_visitor.go b/bundle/config/mutator/paths/dashboard_paths_visitor.go index 7a40f91b8a..b0a25dfe2f 100644 --- a/bundle/config/mutator/paths/dashboard_paths_visitor.go +++ b/bundle/config/mutator/paths/dashboard_paths_visitor.go @@ -13,6 +13,6 @@ func VisitDashboardPaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) { ) return dyn.MapByPattern(value, pattern, func(path dyn.Path, value dyn.Value) (dyn.Value, error) { - return fn(path, TranslateModeLocalAbsoluteFile, value) + return fn(path, TranslateModeLocalRelative, value) }) } diff --git a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go index 1ea31416c6..8d66d33940 100644 --- a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go +++ b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go @@ -4,14 +4,10 @@ import ( "context" "fmt" - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" -) -const ( - filePathFieldName = "file_path" - serializedDashboardFieldName = "serialized_dashboard" + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type configureDashboardSerializedDashboard struct{} @@ -25,42 +21,32 @@ func (c configureDashboardSerializedDashboard) Name() string { } func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - + for _, dashboard := range b.Config.Resources.Dashboards { + path := dashboard.FilePath + if path == "" { + continue + } + contents, err := b.SyncRoot.ReadFile(path) + if err != nil { + return diag.FromErr(fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err)) + } + dashboard.SerializedDashboard = string(contents) + } + + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". pattern := dyn.NewPattern( dyn.Key("resources"), dyn.Key("dashboards"), dyn.AnyKey(), ) - - // Configure serialized_dashboard field for all dashboards. err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - // Include "serialized_dashboard" field if "file_path" is set. - // Note: the Terraform resource supports "file_path" natively, but we read the contents of the dashboard here - // to cover the use case of deployments from the workspace - path, ok := v.Get(filePathFieldName).AsString() - if !ok { - return v, nil - } - - contents, err := b.SyncRoot.ReadFile(path) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err) - } - - v, err = dyn.Set(v, serializedDashboardFieldName, dyn.V(string(contents))) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) - } - - // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". return dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { switch len(p) { case 0: return v, nil case 1: - if p[0] == dyn.Key(filePathFieldName) { + if p[0] == dyn.Key("file_path") { return v, dyn.ErrDrop } } @@ -71,6 +57,7 @@ func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundl }) }) + var diags diag.Diagnostics diags = diags.Extend(diag.FromErr(err)) return diags } diff --git a/bundle/config/mutator/translate_paths_dashboards_test.go b/bundle/config/mutator/translate_paths_dashboards_test.go index ef84934371..75f794b974 100644 --- a/bundle/config/mutator/translate_paths_dashboards_test.go +++ b/bundle/config/mutator/translate_paths_dashboards_test.go @@ -49,7 +49,7 @@ func TestTranslatePathsDashboards_FilePathRelativeSubDirectory(t *testing.T) { // Assert that the file path for the dashboard has been converted to its local absolute path. assert.Equal( t, - filepath.ToSlash(filepath.Join(dir, "src", "my_dashboard.lvdash.json")), + filepath.ToSlash(filepath.Join("src", "my_dashboard.lvdash.json")), b.Config.Resources.Dashboards["dashboard"].FilePath, ) } diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index ae54e86570..fc36d6698b 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -3,6 +3,8 @@ package phases import ( "context" + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" @@ -25,6 +27,7 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions) (d diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), + resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Import(opts), terraform.StatePush(), @@ -48,6 +51,7 @@ func Unbind(ctx context.Context, b *bundle.Bundle, resourceType, resourceKey str diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), + resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Unbind(resourceType, resourceKey), terraform.StatePush(), diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index f69fe3f439..bb4b7dfabc 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -5,6 +5,8 @@ import ( "errors" "slices" + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/artifacts" @@ -209,6 +211,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand deploy.StatePush(), permissions.ApplyWorkspaceRootPermissions(), terraform.Interpolate(), + resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Plan(terraform.PlanGoal("deploy")), ) diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index b2c01343b5..072df83eac 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -5,6 +5,8 @@ import ( "errors" "net/http" + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" @@ -118,6 +120,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle) (diags diag.Diagnostics) { diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), + resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Plan(terraform.PlanGoal("destroy")), )) From f6a27a7301eb5bbbeacc72e51135168bdc6914e2 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Wed, 23 Apr 2025 13:12:29 +0200 Subject: [PATCH 5/8] revert the ConfigureDashboardSerializedDashboard mutator to dynamic style --- ...nfigure_dashboards_serialized_dashboard.go | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go index 8d66d33940..1ea31416c6 100644 --- a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go +++ b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go @@ -4,10 +4,14 @@ import ( "context" "fmt" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +const ( + filePathFieldName = "file_path" + serializedDashboardFieldName = "serialized_dashboard" ) type configureDashboardSerializedDashboard struct{} @@ -21,32 +25,42 @@ func (c configureDashboardSerializedDashboard) Name() string { } func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { - for _, dashboard := range b.Config.Resources.Dashboards { - path := dashboard.FilePath - if path == "" { - continue - } - contents, err := b.SyncRoot.ReadFile(path) - if err != nil { - return diag.FromErr(fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err)) - } - dashboard.SerializedDashboard = string(contents) - } - - // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". + var diags diag.Diagnostics + pattern := dyn.NewPattern( dyn.Key("resources"), dyn.Key("dashboards"), dyn.AnyKey(), ) + + // Configure serialized_dashboard field for all dashboards. err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + // Include "serialized_dashboard" field if "file_path" is set. + // Note: the Terraform resource supports "file_path" natively, but we read the contents of the dashboard here + // to cover the use case of deployments from the workspace + path, ok := v.Get(filePathFieldName).AsString() + if !ok { + return v, nil + } + + contents, err := b.SyncRoot.ReadFile(path) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to read serialized dashboard from file_path %s: %w", path, err) + } + + v, err = dyn.Set(v, serializedDashboardFieldName, dyn.V(string(contents))) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) + } + + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". return dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { switch len(p) { case 0: return v, nil case 1: - if p[0] == dyn.Key("file_path") { + if p[0] == dyn.Key(filePathFieldName) { return v, dyn.ErrDrop } } @@ -57,7 +71,6 @@ func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundl }) }) - var diags diag.Diagnostics diags = diags.Extend(diag.FromErr(err)) return diags } From 8a8c0843d37afb1e40870541c9e5c47af8eef0ad Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Wed, 23 Apr 2025 13:55:21 +0200 Subject: [PATCH 6/8] small fix: fix test fixture and add a clarifying comment to the mutator --- acceptance/bundle/deploy/dashboard/test.toml | 2 +- .../configure_dashboards_serialized_dashboard.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/deploy/dashboard/test.toml b/acceptance/bundle/deploy/dashboard/test.toml index 15d4860470..36afb30921 100644 --- a/acceptance/bundle/deploy/dashboard/test.toml +++ b/acceptance/bundle/deploy/dashboard/test.toml @@ -28,7 +28,7 @@ Response.Body = ''' "dashboard_id":"1234567890abcdef1234567890abcdef", "display_name": "test dashboard 6260d50f-e8ff-4905-8f28-812345678903", "lifecycle_state": "ACTIVE", - "path": "/Users/[USERNAME]/test bundle-deploy-dashboard [UUID].lvdash.json", + "path": "/Users/[USERNAME]/test bundle-deploy-dashboard 6260d50f-e8ff-4905-8f28-812345678903.lvdash.json", "parent_path": "/Users/tester@databricks.com", "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}" } diff --git a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go index 1ea31416c6..893a299786 100644 --- a/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go +++ b/bundle/config/mutator/resourcemutator/configure_dashboards_serialized_dashboard.go @@ -38,7 +38,7 @@ func (c configureDashboardSerializedDashboard) Apply(_ context.Context, b *bundl return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { // Include "serialized_dashboard" field if "file_path" is set. // Note: the Terraform resource supports "file_path" natively, but we read the contents of the dashboard here - // to cover the use case of deployments from the workspace + // to be able to read file contents in Databricks Workspace (reading a dashboard file via file system fails there) path, ok := v.Get(filePathFieldName).AsString() if !ok { return v, nil From e7e0b45e3d503cf568aab5ce61d2deab2a42b049 Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 24 Apr 2025 10:37:04 +0200 Subject: [PATCH 7/8] remove unused paths translation mode TranslateModeLocalAbsoluteFile --- bundle/config/mutator/paths/translation_mode.go | 4 ---- bundle/config/mutator/translate_paths.go | 16 ---------------- 2 files changed, 20 deletions(-) diff --git a/bundle/config/mutator/paths/translation_mode.go b/bundle/config/mutator/paths/translation_mode.go index a15e627b7d..8f3f76dbcd 100644 --- a/bundle/config/mutator/paths/translation_mode.go +++ b/bundle/config/mutator/paths/translation_mode.go @@ -13,10 +13,6 @@ const ( // TranslateModeDirectory translates a path to a remote directory. TranslateModeDirectory - // TranslateModeLocalAbsoluteFile translates a path to the local absolute file path. - // It returns an error if the path does not exist or is a directory. - TranslateModeLocalAbsoluteFile - // TranslateModeLocalAbsoluteDirectory translates a path to the local absolute directory path. // It returns an error if the path does not exist or is not a directory. TranslateModeLocalAbsoluteDirectory diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 7e1820fd02..1f772910c2 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -141,8 +141,6 @@ func (t *translateContext) rewritePath( interp, err = t.translateFilePath(ctx, input, localPath, localRelPath) case paths.TranslateModeDirectory: interp, err = t.translateDirectoryPath(ctx, input, localPath, localRelPath) - case paths.TranslateModeLocalAbsoluteFile: - interp, err = t.translateLocalAbsoluteFilePath(ctx, input, localPath, localRelPath) case paths.TranslateModeLocalAbsoluteDirectory: interp, err = t.translateLocalAbsoluteDirectoryPath(ctx, input, localPath, localRelPath) case paths.TranslateModeLocalRelative: @@ -232,20 +230,6 @@ func (t *translateContext) translateDirectoryPath(ctx context.Context, literal, return path.Join(t.remoteRoot, localRelPath), nil } -func (t *translateContext) translateLocalAbsoluteFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { - info, err := t.b.SyncRoot.Stat(localRelPath) - if errors.Is(err, fs.ErrNotExist) { - return "", fmt.Errorf("file %s not found", literal) - } - if err != nil { - return "", fmt.Errorf("unable to determine if %s is a file: %w", filepath.FromSlash(localFullPath), err) - } - if info.IsDir() { - return "", fmt.Errorf("expected %s to be a file but found a directory", literal) - } - return localFullPath, nil -} - func (t *translateContext) translateLocalAbsoluteDirectoryPath(ctx context.Context, literal, localFullPath, _ string) (string, error) { info, err := os.Stat(filepath.FromSlash(localFullPath)) if errors.Is(err, fs.ErrNotExist) { From c3f2bda4ecc8ce5f11512330ac2c1bd04b773e7b Mon Sep 17 00:00:00 2001 From: Anton Nekipelov <226657+anton-107@users.noreply.github.com> Date: Thu, 24 Apr 2025 14:40:03 +0200 Subject: [PATCH 8/8] move ConfigureDashboardSerializedDashboard to be called from applyNormalizeMutators --- bundle/config/mutator/resourcemutator/resource_mutator.go | 5 +++++ bundle/phases/bind.go | 4 ---- bundle/phases/deploy.go | 3 --- bundle/phases/destroy.go | 3 --- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/resource_mutator.go b/bundle/config/mutator/resourcemutator/resource_mutator.go index 1d85b425ca..7bc18ca0d8 100644 --- a/bundle/config/mutator/resourcemutator/resource_mutator.go +++ b/bundle/config/mutator/resourcemutator/resource_mutator.go @@ -125,6 +125,11 @@ func applyNormalizeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnost // Updates (typed): resources.pipelines.*.{schema,target}, resources.volumes.*.schema_name (converts implicit schema references to explicit ${resources.schemas..name} syntax) // Translates implicit schema references in DLT pipelines or UC Volumes to explicit syntax to capture dependencies CaptureSchemaDependency(), + + // Reads (dynamic): resources.dashboards.*.file_path + // Updates (dynamic): resources.dashboards.*.serialized_dashboard + // Drops (dynamic): resources.dashboards.*.file_path + ConfigureDashboardSerializedDashboard(), ) } diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index fc36d6698b..ae54e86570 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -3,8 +3,6 @@ package phases import ( "context" - "github.com/databricks/cli/bundle/config/mutator/resourcemutator" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" @@ -27,7 +25,6 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions) (d diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), - resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Import(opts), terraform.StatePush(), @@ -51,7 +48,6 @@ func Unbind(ctx context.Context, b *bundle.Bundle, resourceType, resourceKey str diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), - resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Unbind(resourceType, resourceKey), terraform.StatePush(), diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index bb4b7dfabc..f69fe3f439 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -5,8 +5,6 @@ import ( "errors" "slices" - "github.com/databricks/cli/bundle/config/mutator/resourcemutator" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/artifacts" @@ -211,7 +209,6 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand deploy.StatePush(), permissions.ApplyWorkspaceRootPermissions(), terraform.Interpolate(), - resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Plan(terraform.PlanGoal("deploy")), ) diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 072df83eac..b2c01343b5 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -5,8 +5,6 @@ import ( "errors" "net/http" - "github.com/databricks/cli/bundle/config/mutator/resourcemutator" - "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" @@ -120,7 +118,6 @@ func Destroy(ctx context.Context, b *bundle.Bundle) (diags diag.Diagnostics) { diags = diags.Extend(bundle.ApplySeq(ctx, b, terraform.StatePull(), terraform.Interpolate(), - resourcemutator.ConfigureDashboardSerializedDashboard(), terraform.Write(), terraform.Plan(terraform.PlanGoal("destroy")), ))