From 0fa2d182d6323db54c4e749903140b1bc89fb46a Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Mon, 3 Feb 2025 21:38:57 -0700 Subject: [PATCH] K8s: Fix dashboard deletion in folders (#100023) --- .../folderimpl/folder_unifiedstorage.go | 34 +++- .../folderimpl/folder_unifiedstorage_test.go | 148 ++++++++++++++++++ 2 files changed, 180 insertions(+), 2 deletions(-) diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index a1473e7558d..6d4ae13da06 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -36,11 +36,13 @@ import ( "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/search" "github.com/grafana/grafana/pkg/util" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // interface to allow for testing type folderK8sHandler interface { getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) + getDashboardClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) getNamespace(orgID int64) string getSearcher(ctx context.Context) resource.ResourceClient } @@ -688,7 +690,7 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol // if dashboard restore is on we don't delete public dashboards, the hard delete will take care of it later if !s.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) { - // We need a list of dashboard uids inside the folder to delete related public dashboards + // We need a list of dashboard uids inside the folder to delete related dashboards & public dashboards var dashboardUIDs []string // we cannot use the dashboard service directly due to circular dependencies, // so either use the search client if the feature is enabled or use the dashboard store @@ -722,10 +724,17 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol if err != nil { return folder.ErrInternal.Errorf("failed to fetch dashboards: %w", err) } - dashboardUIDs = make([]string, len(hits.Hits)) + k8sDeleteClient, created := s.k8sclient.getDashboardClient(ctx, cmd.OrgID) + if !created { + return folder.ErrInternal.Errorf("failed to create client to get dashboards") + } for i, dashboard := range hits.Hits { dashboardUIDs[i] = dashboard.Name + err = k8sDeleteClient.Delete(ctx, dashboard.Name, metav1.DeleteOptions{}) + if err != nil { + return folder.ErrInternal.Errorf("failed to delete child dashboard: %w", err) + } } } else { dashes, err := s.dashboardStore.FindDashboards(ctx, &dashboards.FindPersistedDashboardsQuery{SignedInUser: cmd.SignedInUser, FolderUIDs: folders, OrgId: cmd.OrgID}) @@ -735,6 +744,13 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol dashboardUIDs = make([]string, len(dashes)) for i, dashboard := range dashes { dashboardUIDs[i] = dashboard.UID + err = s.dashboardStore.DeleteDashboard(ctx, &dashboards.DeleteDashboardCommand{ + UID: dashboard.UID, + OrgID: cmd.OrgID, + }) + if err != nil { + return folder.ErrInternal.Errorf("failed to delete child dashboard: %w", err) + } } } // Delete all public dashboards in the folders @@ -986,6 +1002,20 @@ func (fk8s *foldk8sHandler) getClient(ctx context.Context, orgID int64) (dynamic return dyn.Resource(fk8s.gvr).Namespace(fk8s.getNamespace(orgID)), true } +func (fk8s *foldk8sHandler) getDashboardClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) { + cfg := fk8s.restConfigProvider(ctx) + if cfg == nil { + return nil, false + } + + dyn, err := dynamic.NewForConfig(cfg) + if err != nil { + return nil, false + } + + return dyn.Resource(dashboardv0.DashboardResourceInfo.GroupVersionResource()).Namespace(fk8s.getNamespace(orgID)), true +} + func (fk8s *foldk8sHandler) getNamespace(orgID int64) string { return fk8s.namespacer(orgID) } diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index e2d50566662..b97b2ea51e9 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" clientrest "k8s.io/client-go/rest" @@ -682,6 +683,46 @@ func (r resourceClientMock) Search(ctx context.Context, in *resource.ResourceSea }, nil } + if len(in.Options.Fields) > 0 && + in.Options.Fields[0].Key == resource.SEARCH_FIELD_FOLDER && + in.Options.Fields[0].Operator == "in" && + len(in.Options.Fields[0].Values) > 0 { + rows := []*resource.ResourceTableRow{} + for i, row := range in.Options.Fields[0].Values { + rows = append(rows, &resource.ResourceTableRow{ + Key: &resource.ResourceKey{ + Name: row, + Resource: "folders", + }, + Cells: [][]byte{ + []byte(fmt.Sprintf("%d", i)), // set legacy id as the row id + []byte(fmt.Sprintf("folder%d", i)), // set title as folder + row id + []byte(""), + }, + }) + } + return &resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "_id", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: rows, + }, + TotalHits: int64(len(rows)), + }, nil + } + // not found return &resource.ResourceSearchResponse{ Results: &resource.ResourceTable{}, @@ -716,6 +757,11 @@ func (m *mockFoldersK8sCli) getClient(ctx context.Context, orgID int64) (dynamic return args.Get(0).(dynamic.ResourceInterface), args.Bool(1) } +func (m *mockFoldersK8sCli) getDashboardClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) { + args := m.Called(ctx, orgID) + return args.Get(0).(dynamic.ResourceInterface), args.Bool(1) +} + func (m *mockFoldersK8sCli) getNamespace(orgID int64) string { if orgID == 1 { return "default" @@ -834,3 +880,105 @@ func TestSearchFoldersFromApiServer(t *testing.T) { require.Equal(t, expectedResult, result) }) } + +type mockDashboardCli struct { + mock.Mock + dynamic.ResourceInterface +} + +func (c *mockDashboardCli) Delete(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + args := c.Called(ctx, name, options) + return args.Error(0) +} + +func TestDeleteFoldersFromApiServer(t *testing.T) { + fakeK8sClient := new(mockFoldersK8sCli) + fakeFolderStore := folder.NewFakeStore() + dashboardStore := dashboards.NewFakeDashboardStore(t) + publicDashboardFakeService := publicdashboards.NewFakePublicDashboardServiceWrapper(t) + service := Service{ + k8sclient: fakeK8sClient, + unifiedStore: fakeFolderStore, + dashboardStore: dashboardStore, + publicDashboardService: publicDashboardFakeService, + registry: make(map[string]folder.RegistryService), + features: featuremgmt.WithFeatures(featuremgmt.FlagKubernetesFoldersServiceV2), + } + fakeK8sClient.On("getSearcher", mock.Anything).Return(fakeK8sClient) + user := &user.SignedInUser{OrgID: 1} + ctx := identity.WithRequester(context.Background(), user) + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ + CanSaveValue: true, + CanViewValue: true, + }) + db, cfg := sqlstore.InitTestDB(t) + + alertingStore := ngstore.DBstore{ + SQLStore: db, + Cfg: cfg.UnifiedAlerting, + Logger: log.New("test-alerting-store"), + AccessControl: actest.FakeAccessControl{ExpectedEvaluate: true}, + } + require.NoError(t, service.RegisterService(alertingStore)) + + t.Run("Should delete folder", func(t *testing.T) { + dashboardStore.On("FindDashboards", mock.Anything, mock.Anything).Return([]dashboards.DashboardSearchProjection{}, nil).Once() + publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + err := service.deleteFromApiServer(ctx, &folder.DeleteFolderCommand{ + UID: "uid", + OrgID: 1, + SignedInUser: user, + }) + require.NoError(t, err) + }) + + t.Run("Should delete dashboards and public dashboards within the folder", func(t *testing.T) { + dashboardStore.On("FindDashboards", mock.Anything, mock.Anything).Return([]dashboards.DashboardSearchProjection{ + { + UID: "test", + OrgID: 1, + }, + { + UID: "test2", + OrgID: 1, + }, + }, nil).Once() + dashboardStore.On("DeleteDashboard", mock.Anything, &dashboards.DeleteDashboardCommand{ + UID: "test", + OrgID: 1, + }).Return(nil).Once() + dashboardStore.On("DeleteDashboard", mock.Anything, &dashboards.DeleteDashboardCommand{ + UID: "test2", + OrgID: 1, + }).Return(nil).Once() + publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, int64(1), []string{"test", "test2"}).Return(nil).Once() + err := service.deleteFromApiServer(ctx, &folder.DeleteFolderCommand{ + UID: "uid", + OrgID: 1, + SignedInUser: user, + }) + require.NoError(t, err) + dashboardStore.AssertExpectations(t) + publicDashboardFakeService.AssertExpectations(t) + }) + + // enable k8s ff for dashboards, retest + service.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesFoldersServiceV2, featuremgmt.FlagKubernetesCliDashboards) + + t.Run("Should delete dashboards and public dashboards within the folder through k8s if the ff is enabled", func(t *testing.T) { + dashboardK8sCli := mockDashboardCli{} + dashboardK8sCli.On("Delete", mock.Anything, "uid1", mock.Anything, mock.Anything).Return(nil).Once() + fakeK8sClient.On("getDashboardClient", mock.Anything, mock.Anything).Return(&dashboardK8sCli, true) + fakeK8sClient.On("getSearcher", mock.Anything).Return(fakeK8sClient) + publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, int64(1), []string{"uid1"}).Return(nil).Once() + err := service.deleteFromApiServer(ctx, &folder.DeleteFolderCommand{ + UID: "uid1", + OrgID: 1, + SignedInUser: user, + }) + require.NoError(t, err) + dashboardStore.AssertExpectations(t) + publicDashboardFakeService.AssertExpectations(t) + dashboardK8sCli.AssertExpectations(t) + }) +}