From 96c6f52e237c232a8fceaed9ce2d0da32060a74b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 21 Oct 2024 12:26:14 +0200 Subject: [PATCH] Check for remote modification of dashboards --- .../terraform/check_modified_dashboards.go | 127 ++++++++++++ .../check_modified_dashboards_test.go | 189 ++++++++++++++++++ bundle/deploy/terraform/util.go | 5 +- bundle/phases/deploy.go | 1 + 4 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 bundle/deploy/terraform/check_modified_dashboards.go create mode 100644 bundle/deploy/terraform/check_modified_dashboards_test.go diff --git a/bundle/deploy/terraform/check_modified_dashboards.go b/bundle/deploy/terraform/check_modified_dashboards.go new file mode 100644 index 000000000..1a0bbcafc --- /dev/null +++ b/bundle/deploy/terraform/check_modified_dashboards.go @@ -0,0 +1,127 @@ +package terraform + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + tfjson "github.com/hashicorp/terraform-json" +) + +type dashboardState struct { + Name string + ID string + ETag string +} + +func collectDashboards(ctx context.Context, b *bundle.Bundle) ([]dashboardState, error) { + state, err := ParseResourcesState(ctx, b) + if err != nil && state == nil { + return nil, err + } + + var dashboards []dashboardState + for _, resource := range state.Resources { + if resource.Mode != tfjson.ManagedResourceMode { + continue + } + for _, instance := range resource.Instances { + id := instance.Attributes.ID + if id == "" { + continue + } + + switch resource.Type { + case "databricks_dashboard": + etag := instance.Attributes.ETag + if etag == "" { + continue + } + + dashboards = append(dashboards, dashboardState{ + Name: resource.Name, + ID: id, + ETag: etag, + }) + } + } + } + + return dashboards, nil +} + +type checkModifiedDashboards struct { +} + +func (l *checkModifiedDashboards) Name() string { + return "CheckModifiedDashboards" +} + +func (l *checkModifiedDashboards) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // This mutator is relevant only if the bundle includes dashboards. + if len(b.Config.Resources.Dashboards) == 0 { + return nil + } + + // If the user has forced the deployment, skip this check. + if b.Config.Bundle.Force { + return nil + } + + dashboards, err := collectDashboards(ctx, b) + if err != nil { + return diag.FromErr(err) + } + + var diags diag.Diagnostics + for _, dashboard := range dashboards { + // Skip dashboards that are not defined in the bundle. + // These will be destroyed upon deployment. + if _, ok := b.Config.Resources.Dashboards[dashboard.Name]; !ok { + continue + } + + path := dyn.MustPathFromString(fmt.Sprintf("resources.dashboards.%s", dashboard.Name)) + loc := b.Config.GetLocation(path.String()) + actual, err := b.WorkspaceClient().Lakeview.GetByDashboardId(ctx, dashboard.ID) + if err != nil { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("failed to get dashboard %q", dashboard.Name), + Detail: err.Error(), + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + continue + } + + // If the ETag is the same, the dashboard has not been modified. + if actual.Etag == dashboard.ETag { + continue + } + + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("dashboard %q has been modified remotely", dashboard.Name), + Detail: "" + + "This dashboard has been modified remotely since the last bundle deployment.\n" + + "These modifications are untracked and will be overwritten on deploy.\n" + + "\n" + + "Make sure that the local dashboard definition matches what you intend to deploy\n" + + "before proceeding with the deployment.\n" + + "\n" + + "Run `databricks bundle deploy --force` to bypass this error." + + "", + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + } + + return diags +} + +func CheckModifiedDashboards() *checkModifiedDashboards { + return &checkModifiedDashboards{} +} diff --git a/bundle/deploy/terraform/check_modified_dashboards_test.go b/bundle/deploy/terraform/check_modified_dashboards_test.go new file mode 100644 index 000000000..ff6f19b83 --- /dev/null +++ b/bundle/deploy/terraform/check_modified_dashboards_test.go @@ -0,0 +1,189 @@ +package terraform + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func mockDashboardBundle(t *testing.T) *bundle.Bundle { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "dash1": { + DisplayName: "My Special Dashboard", + }, + }, + }, + }, + } + return b +} + +func TestCheckModifiedDashboards_NoDashboards(t *testing.T) { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{}, + }, + } + + diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_FirstDeployment(t *testing.T) { + b := mockDashboardBundle(t) + diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_ExistingStateNoChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1000", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // No changes, so no diags. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_ExistingStateChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1234", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // The dashboard has changed, so expect an error. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary) + } +} + +func TestCheckModifiedDashboards_ExistingStateFailureToGet(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(nil, fmt.Errorf("failure")). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // Unable to get the dashboard, so expect an error. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary) + } +} + +func writeFakeDashboardState(t *testing.T, ctx context.Context, b *bundle.Bundle) { + tfDir, err := Dir(ctx, b) + require.NoError(t, err) + + // Write fake state file. + testutil.WriteFile(t, ` + { + "version": 4, + "terraform_version": "1.5.5", + "resources": [ + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash1", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1000", + "id": "id1" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_job", + "name": "job", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "1234" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash2", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1001", + "id": "id2" + } + } + ] + } + ] + } + `, filepath.Join(tfDir, TerraformStateFileName)) +} diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 64d667b5f..4da015c23 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -13,7 +13,7 @@ import ( // Partial representation of the Terraform state file format. // We are only interested global version and serial numbers, -// plus resource types, names, modes, and ids. +// plus resource types, names, modes, IDs, and ETags (for dashboards). type resourcesState struct { Version int `json:"version"` Resources []stateResource `json:"resources"` @@ -33,7 +33,8 @@ type stateResourceInstance struct { } type stateInstanceAttributes struct { - ID string `json:"id"` + ID string `json:"id"` + ETag string `json:"etag,omitempty"` } func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (*resourcesState, error) { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index cb0ecf75d..514a02ee7 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -152,6 +152,7 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Defer( bundle.Seq( terraform.StatePull(), + terraform.CheckModifiedDashboards(), deploy.StatePull(), mutator.ValidateGitDetails(), artifacts.CleanUp(),