Skip to content

Commit

Permalink
Fix deletion of dashboard if it was trashed out of band
Browse files Browse the repository at this point in the history
  • Loading branch information
pietern committed Nov 15, 2024
1 parent 5396981 commit 16503fc
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 4 deletions.
27 changes: 26 additions & 1 deletion dashboards/resource_dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package dashboards

import (
"context"
"errors"
"log"
"strings"

"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/dashboards"
"github.com/databricks/terraform-provider-databricks/common"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -175,9 +177,32 @@ func ResourceDashboard() common.Resource {
if err != nil {
return err
}
return w.Lakeview.Trash(ctx, dashboards.TrashDashboardRequest{

// Attempt to trash the dashboard.
err = w.Lakeview.Trash(ctx, dashboards.TrashDashboardRequest{
DashboardId: d.Id(),
})

// If the dashboard was already trashed, we'll get a 403 (Permission Denied) error.
// There may be other cases where we get a 403, so we first confirm that the
// dashboard state is actually trashed, and if so, return success.
if errors.Is(err, apierr.ErrPermissionDenied) {
dashboard, nerr := w.Lakeview.Get(ctx, dashboards.GetDashboardRequest{
DashboardId: d.Id(),
})

// Return original error if we can't get the dashboard state.
if nerr != nil {
return err
}

// If the dashboard is trashed, return success.
if dashboard.LifecycleState == dashboards.LifecycleStateTrashed {
return nil
}
}

return err
},
}
}
Expand Down
79 changes: 76 additions & 3 deletions dashboards/resource_dashboard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"fmt"
"testing"

"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/experimental/mocks"
"github.com/databricks/databricks-sdk-go/service/dashboards"
"github.com/databricks/terraform-provider-databricks/qa"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

Expand Down Expand Up @@ -222,9 +224,12 @@ func TestDashboardUpdate(t *testing.T) {
}

func TestDashboardDelete(t *testing.T) {
qa.ResourceFixture{
_, err := qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
w.GetMockLakeviewAPI().EXPECT().Trash(mock.Anything, dashboards.TrashDashboardRequest{
e := w.GetMockLakeviewAPI().EXPECT()

// Expect the dashboard to be trashed.
e.Trash(mock.Anything, dashboards.TrashDashboardRequest{
DashboardId: "xyz",
}).Return(nil)
},
Expand All @@ -237,5 +242,73 @@ func TestDashboardDelete(t *testing.T) {
parent_path = "/path"
serialized_dashboard = "serialized_json"
`,
}.ApplyNoError(t)
}.Apply(t)

// Expect this to succeed.
assert.NoError(t, err)
}

func TestDashboardDeletePermissionDenied(t *testing.T) {
_, err := qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
e := w.GetMockLakeviewAPI().EXPECT()

// First, expect the dashboard to be trashed.
e.Trash(mock.Anything, dashboards.TrashDashboardRequest{
DashboardId: "xyz",
}).Return(apierr.ErrPermissionDenied)

// Then, expect to get the dashboard to confirm if it was already trashed.
// We confirm below that the below error is ignored and the original error is returned.
e.Get(mock.Anything, dashboards.GetDashboardRequest{
DashboardId: "xyz",
}).Return(nil, fmt.Errorf("some other error"))
},
Resource: ResourceDashboard(),
Delete: true,
ID: "xyz",
HCL: `
display_name = "Dashboard name"
warehouse_id = "abc"
parent_path = "/path"
serialized_dashboard = "serialized_json"
`,
}.Apply(t)

// We expect the original error to be returned.
assert.Equal(t, err, apierr.ErrPermissionDenied)
}

func TestDashboardDeleteAlreadyTrashed(t *testing.T) {
_, err := qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
e := w.GetMockLakeviewAPI().EXPECT()

// First, expect the dashboard to be trashed.
e.Trash(mock.Anything, dashboards.TrashDashboardRequest{
DashboardId: "xyz",
}).Return(apierr.ErrPermissionDenied)

// Then, expect to get the dashboard to confirm if it was already trashed.
e.Get(mock.Anything, dashboards.GetDashboardRequest{
DashboardId: "xyz",
}).Return(&dashboards.Dashboard{
DashboardId: "xyz",
ParentPath: "/path",
LifecycleState: dashboards.LifecycleStateTrashed,
}, nil)
},
Resource: ResourceDashboard(),
Delete: true,
ID: "xyz",
HCL: `
display_name = "Dashboard name"
warehouse_id = "abc"
parent_path = "/path"
serialized_dashboard = "serialized_json"
`,
}.Apply(t)

// Expect this to succeed.
assert.NoError(t, err)
}

0 comments on commit 16503fc

Please sign in to comment.