From 445e88cb932ca7be91d31d97d5d7b919a1285df3 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Sun, 2 Nov 2025 21:39:15 +0100 Subject: [PATCH] Dashboard Provisioning: Add duplicate cleanup for modes 0-2 (#113336) --- .../dashboard/legacysearcher/search_client.go | 4 +- .../legacysearcher/search_client_test.go | 4 +- pkg/services/dashboards/dashboard.go | 1 + pkg/services/dashboards/database/database.go | 56 ++++++++++++++++++- pkg/services/dashboards/models.go | 14 +++-- .../dashboards/service/dashboard_service.go | 34 +++++++++++ .../service/dashboard_service_test.go | 7 +++ pkg/services/dashboards/store_mock.go | 30 ++++++++++ 8 files changed, 139 insertions(+), 11 deletions(-) diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client.go b/pkg/registry/apis/dashboard/legacysearcher/search_client.go index 446e7e07373..24c49cc5624 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client.go @@ -312,7 +312,7 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso for _, dashboard := range provisioningData { list.Results.Rows = append(list.Results.Rows, &resourcepb.ResourceTableRow{ Key: getResourceKey(&dashboards.DashboardSearchProjection{ - UID: dashboard.Dashboard.UID, + UID: dashboard.UID, }, req.Options.Key.Namespace), Cells: c.createDetailedProvisioningCells(dashboard, query), }) @@ -512,7 +512,7 @@ func (c *DashboardSearchClient) createProvisioningCells(dashboard *dashboards.Da } func (c *DashboardSearchClient) createDetailedProvisioningCells(dashboard *dashboards.DashboardProvisioningSearchResults, query *dashboards.FindPersistedDashboardsQuery) [][]byte { - cells := c.createCommonCells(dashboard.Dashboard.Title, dashboard.Dashboard.FolderUID, dashboard.Dashboard.ID, []byte("[]")) + cells := c.createCommonCells(dashboard.Title, dashboard.FolderUID, dashboard.ID, []byte("[]")) return append(cells, []byte(query.ManagedBy), []byte(dashboard.Provisioner), diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go index 94fd1accff3..489331db2b7 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go @@ -454,7 +454,9 @@ func TestDashboardSearchClient_Search(t *testing.T) { t.Run("Should retrieve dashboards by provisioner name through a different function", func(t *testing.T) { mockStore.On("GetProvisionedDashboardsByName", mock.Anything, "test", mock.Anything).Return([]*dashboards.DashboardProvisioningSearchResults{ { - Dashboard: dashboards.Dashboard{UID: "uid", Title: "Test Dashboard", FolderUID: "folder1"}, + UID: "uid", + Title: "Test Dashboard", + FolderUID: "folder1", ExternalID: "test", Provisioner: string(utils.ManagerKindClassicFP), // nolint:staticcheck }, diff --git a/pkg/services/dashboards/dashboard.go b/pkg/services/dashboards/dashboard.go index 4e077e08d9d..0334cfc8990 100644 --- a/pkg/services/dashboards/dashboard.go +++ b/pkg/services/dashboards/dashboard.go @@ -87,6 +87,7 @@ type Store interface { GetProvisionedDataByDashboardUID(ctx context.Context, orgID int64, dashboardUID string) (*DashboardProvisioningSearchResults, error) GetProvisionedDashboardsByName(ctx context.Context, name string, orgID int64) ([]*DashboardProvisioningSearchResults, error) GetOrphanedProvisionedDashboards(ctx context.Context, notIn []string, orgID int64) ([]*Dashboard, error) + GetDuplicateProvisionedDashboards(ctx context.Context) ([]*DashboardProvisioningSearchResults, error) SaveDashboard(ctx context.Context, cmd SaveDashboardCommand) (*Dashboard, error) SaveProvisionedDashboard(ctx context.Context, cmd SaveDashboardCommand, provisioning *DashboardProvisioning) (*Dashboard, error) UnprovisionDashboard(ctx context.Context, id int64) error diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 490bc51f475..65ca0dd7ea0 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -209,7 +209,7 @@ func (d *dashboardStore) GetProvisionedDataByDashboardID(ctx context.Context, da return sess.Table(`dashboard`). Join(`INNER`, `dashboard_provisioning`, `dashboard.id = dashboard_provisioning.dashboard_id`). Where(`dashboard_provisioning.dashboard_id = ?`, dashboardID). - Select("dashboard.*, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). + Select("dashboard.id, dashboard.uid, dashboard.title, dashboard.folder_uid, dashboard.org_id, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). Find(&data) }) if err != nil { @@ -241,7 +241,7 @@ func (d *dashboardStore) GetProvisionedDataByDashboardUID(ctx context.Context, o return sess.Table(`dashboard`). Join(`INNER`, `dashboard_provisioning`, `dashboard.id = dashboard_provisioning.dashboard_id`). Where(`dashboard_provisioning.dashboard_id = ?`, dashboard.ID). - Select("dashboard.*, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). + Select("dashboard.id, dashboard.uid, dashboard.title, dashboard.folder_uid, dashboard.org_id, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). Find(&provisionedDashboard) }) if err != nil { @@ -275,7 +275,7 @@ func (d *dashboardStore) GetProvisionedDashboardsByName(ctx context.Context, nam return sess.Table(`dashboard`). Join(`INNER`, `dashboard_provisioning`, `dashboard.id = dashboard_provisioning.dashboard_id`). Where(`dashboard_provisioning.name = ? AND dashboard.org_id = ?`, name, orgID). - Select("dashboard.*, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). + Select("dashboard.id, dashboard.uid, dashboard.title, dashboard.folder_uid, dashboard.org_id, dashboard_provisioning.name, dashboard_provisioning.external_id, dashboard_provisioning.updated as provisioning_updated, dashboard_provisioning.check_sum"). Find(&dashes) }) if err != nil { @@ -301,6 +301,56 @@ func (d *dashboardStore) GetOrphanedProvisionedDashboards(ctx context.Context, n return dashes, nil } +func (d *dashboardStore) GetDuplicateProvisionedDashboards(ctx context.Context) ([]*dashboards.DashboardProvisioningSearchResults, error) { + ctx, span := tracer.Start(ctx, "dashboards.database.GetDuplicateProvisionedDashboards") + defer span.End() + + dashes := []*dashboards.DashboardProvisioningSearchResults{} + err := d.store.WithDbSession(ctx, func(sess *db.Session) error { + type duplicateGroup struct { + Name string `xorm:"name"` + ExternalID string `xorm:"external_id"` + CheckSum string `xorm:"check_sum"` + } + + duplicateGroups := []duplicateGroup{} + err := sess.SQL(` + SELECT dp.name, dp.external_id + FROM dashboard_provisioning dp + INNER JOIN dashboard d ON d.id = dp.dashboard_id + GROUP BY dp.name, dp.external_id, dp.check_sum + HAVING COUNT(*) > 1 + `).Find(&duplicateGroups) + + if err != nil { + return err + } + + if len(duplicateGroups) == 0 { + return nil + } + + for _, group := range duplicateGroups { + var groupDashes []*dashboards.DashboardProvisioningSearchResults + err := sess.Table(`dashboard`). + Join(`INNER`, `dashboard_provisioning`, `dashboard.id = dashboard_provisioning.dashboard_id`). + Where(`dashboard_provisioning.name = ? AND dashboard_provisioning.external_id = ?`, group.Name, group.ExternalID). + Find(&groupDashes) + if err != nil { + return err + } + dashes = append(dashes, groupDashes...) + } + + return nil + }) + + if err != nil { + return nil, err + } + return dashes, nil +} + func (d *dashboardStore) SaveProvisionedDashboard(ctx context.Context, cmd dashboards.SaveDashboardCommand, provisioning *dashboards.DashboardProvisioning) (*dashboards.Dashboard, error) { ctx, span := tracer.Start(ctx, "dashboards.database.SaveProvisionedDashboard") defer span.End() diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index 24b414118f3..270e449ac07 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -241,11 +241,15 @@ type DeleteOrphanedProvisionedDashboardsCommand struct { } type DashboardProvisioningSearchResults struct { - Dashboard Dashboard `xorm:"extends"` - Provisioner string `xorm:"name"` - ExternalID string `xorm:"external_id"` - CheckSum string `xorm:"check_sum"` - ProvisionUpdate int64 `xorm:"provisioning_updated"` + ID int64 `xorm:"id"` + UID string `xorm:"uid"` + Title string `xorm:"title"` + FolderUID string `xorm:"folder_uid"` + OrgID int64 `xorm:"org_id"` + Provisioner string `xorm:"name"` + ExternalID string `xorm:"external_id"` + CheckSum string `xorm:"check_sum"` + ProvisionUpdate int64 `xorm:"provisioning_updated"` } // diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 78e66c0eab9..7e4dd89dc7d 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -876,6 +876,12 @@ func (dr *DashboardServiceImpl) waitForSearchQuery(ctx context.Context, query *d } func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context.Context, cmd *dashboards.DeleteOrphanedProvisionedDashboardsCommand) error { + // cleanup duplicate provisioned dashboards first (this will have the same name and external_id) + // note: only works in modes 1-3 + if err := dr.DeleteDuplicateProvisionedDashboards(ctx); err != nil { + dr.log.Error("Failed to delete duplicate provisioned dashboards", "error", err) + } + // check each org for orphaned provisioned dashboards orgs, err := dr.orgService.Search(ctx, &org.SearchOrgsQuery{}) if err != nil { @@ -914,6 +920,34 @@ func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context. return nil } +func (dr *DashboardServiceImpl) DeleteDuplicateProvisionedDashboards(ctx context.Context) error { + duplicates, err := dr.dashboardStore.GetDuplicateProvisionedDashboards(ctx) + if err != nil { + return err + } + + type provisioningKey struct { + name string + externalID string + } + + groups := make(map[provisioningKey][]*dashboards.DashboardProvisioningSearchResults) + for _, dash := range duplicates { + key := provisioningKey{ + name: dash.Provisioner, + externalID: dash.ExternalID, + } + if _, exists := groups[key]; exists { + if err = dr.deleteDashboard(ctx, dash.ID, dash.UID, dash.OrgID, false); err != nil { + dr.log.Error("Failed to delete duplicate provisioned dashboard", "error", err, "dashboardUID", dash.UID, "dashboardID", dash.ID) + } + } + groups[key] = append(groups[key], dash) + } + + return nil +} + func (dr *DashboardServiceImpl) ValidateDashboardRefreshInterval(minRefreshInterval string, targetRefreshInterval string) error { if minRefreshInterval == "" { return nil diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index e9ee3ea675b..7aa0e07762c 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -629,16 +629,19 @@ func TestGetProvisionedDashboardDataByDashboardUID(t *testing.T) { func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { fakePublicDashboardService := publicdashboards.NewFakePublicDashboardServiceWrapper(t) + fakeDashboardStore := &dashboards.FakeDashboardStore{} service := &DashboardServiceImpl{ cfg: setting.NewCfg(), orgService: &orgtest.FakeOrgService{ ExpectedOrgs: []*org.OrgDTO{{ID: 1}, {ID: 2}}, }, publicDashboardService: fakePublicDashboardService, + dashboardStore: fakeDashboardStore, log: log.NewNopLogger(), } t.Run("Should delete across all orgs, but only delete file based provisioned dashboards", func(t *testing.T) { + fakeDashboardStore.On("GetDuplicateProvisionedDashboards", mock.Anything).Return([]*dashboards.DashboardProvisioningSearchResults{}, nil) _, k8sCliMock := setupK8sDashboardTests(service) k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") k8sCliMock.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -783,6 +786,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { }) t.Run("Should retry until deleted dashboard not found in search", func(t *testing.T) { + fakeDashboardStore.On("GetDuplicateProvisionedDashboards", mock.Anything).Return([]*dashboards.DashboardProvisioningSearchResults{}, nil) repo := "test" singleOrgService := &DashboardServiceImpl{ cfg: setting.NewCfg(), @@ -790,6 +794,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { ExpectedOrgs: []*org.OrgDTO{{ID: 1}}, }, publicDashboardService: fakePublicDashboardService, + dashboardStore: fakeDashboardStore, log: log.NewNopLogger(), } ctx, k8sCliMock := setupK8sDashboardTests(singleOrgService) @@ -876,6 +881,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { }) t.Run("Will not wait for indexer when no dashboards were deleted", func(t *testing.T) { + fakeDashboardStore.On("GetDuplicateProvisionedDashboards", mock.Anything).Return([]*dashboards.DashboardProvisioningSearchResults{}, nil) repo := "test" singleOrgService := &DashboardServiceImpl{ cfg: setting.NewCfg(), @@ -883,6 +889,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { ExpectedOrgs: []*org.OrgDTO{{ID: 1}}, }, publicDashboardService: fakePublicDashboardService, + dashboardStore: fakeDashboardStore, log: log.NewNopLogger(), } ctx, k8sCliMock := setupK8sDashboardTests(singleOrgService) diff --git a/pkg/services/dashboards/store_mock.go b/pkg/services/dashboards/store_mock.go index 4a3e0d15d90..6a329e10301 100644 --- a/pkg/services/dashboards/store_mock.go +++ b/pkg/services/dashboards/store_mock.go @@ -305,6 +305,36 @@ func (_m *FakeDashboardStore) GetOrphanedProvisionedDashboards(ctx context.Conte return r0, r1 } +// GetDuplicateProvisionedDashboards provides a mock function with given fields: ctx +func (_m *FakeDashboardStore) GetDuplicateProvisionedDashboards(ctx context.Context) ([]*DashboardProvisioningSearchResults, error) { + ret := _m.Called(ctx) + + if len(ret) == 0 { + panic("no return value specified for GetDuplicateProvisionedDashboards") + } + + var r0 []*DashboardProvisioningSearchResults + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) ([]*DashboardProvisioningSearchResults, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) []*DashboardProvisioningSearchResults); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*DashboardProvisioningSearchResults) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetProvisionedDashboardData provides a mock function with given fields: ctx, name func (_m *FakeDashboardStore) GetProvisionedDashboardData(ctx context.Context, name string) ([]*DashboardProvisioning, error) { ret := _m.Called(ctx, name)