public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Dashboards: Fix duplicate provisioning when errors occur on title-only based provisioning (#102249)

Browse Source 
Dashboards: fix title based provisioning
This commit is contained in:
Stephanie Hingtgen
2025-03-15 21:12:48 -06:00
committed by GitHub
parent 45e2cb78f2
commit 5c24312625
11 changed files with 349 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/dashboards/dashboard.go
+1 -1
View File
@@ -84,7 +84,7 @@ type Store interface {
GetProvisionedDashboardsByName(ctx context.Context, name string) ([]*Dashboard, error)
GetOrphanedProvisionedDashboards(ctx context.Context, notIn []string) ([]*Dashboard, error)
SaveDashboard(ctx context.Context, cmd SaveDashboardCommand) (*Dashboard, error)
SaveProvisionedDashboard(ctx context.Context, dash *Dashboard, provisioning *DashboardProvisioning) error
SaveProvisionedDashboard(ctx context.Context, cmd SaveDashboardCommand, provisioning *DashboardProvisioning) (*Dashboard, error)
UnprovisionDashboard(ctx context.Context, id int64) error
// ValidateDashboardBeforeSave validates a dashboard before save.
ValidateDashboardBeforeSave(ctx context.Context, dashboard *Dashboard, overwrite bool) (bool, error)
pkg/services/dashboards/database/database.go
+13 -7
View File
@@ -178,19 +178,25 @@ func (d *dashboardStore) GetOrphanedProvisionedDashboards(ctx context.Context, n
return dashes, nil
}
func (d *dashboardStore) SaveProvisionedDashboard(ctx context.Context, dash *dashboards.Dashboard, provisioning *dashboards.DashboardProvisioning) error {
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()
err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
if provisioning.Updated == 0 {
provisioning.Updated = dash.Updated.Unix()
var result *dashboards.Dashboard
var err error
err = d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
result, err = saveDashboard(sess, &cmd, d.emitEntityEvent())
if err != nil {
return err
}
return saveProvisionedData(sess, provisioning, dash)
})
if provisioning.Updated == 0 {
provisioning.Updated = result.Updated.Unix()
}
return err
return saveProvisionedData(sess, provisioning, result)
})
return result, err
}
func (d *dashboardStore) SaveDashboard(ctx context.Context, cmd dashboards.SaveDashboardCommand) (*dashboards.Dashboard, error) {
pkg/services/dashboards/database/database_provisioning_test.go
+4 -8
View File
@@ -52,15 +52,13 @@ func TestIntegrationDashboardProvisioningTest(t *testing.T) {
ExternalID: "/var/grafana.json",
Updated: now.Unix(),
}
dash, err := dashboardStore.SaveDashboard(context.Background(), saveDashboardCmd)
require.NoError(t, err)
dash, err := dashboardStore.SaveProvisionedDashboard(context.Background(), saveDashboardCmd, provisioning)
require.Nil(t, err)
require.NotNil(t, dash)
require.NotEqual(t, 0, dash.ID)
dashId := dash.ID
err = dashboardStore.SaveProvisionedDashboard(context.Background(), dash, provisioning)
require.Nil(t, err)
t.Run("Deleting orphaned provisioned dashboards", func(t *testing.T) {
saveCmd := dashboards.SaveDashboardCommand{
OrgID: 1,
@@ -71,8 +69,6 @@ func TestIntegrationDashboardProvisioningTest(t *testing.T) {
"title": "another_dashboard",
}),
}
anotherDash, err := dashboardStore.SaveDashboard(context.Background(), saveCmd)
require.NoError(t, err)
provisioning := &dashboards.DashboardProvisioning{
Name: "another_reader",
@@ -80,7 +76,7 @@ func TestIntegrationDashboardProvisioningTest(t *testing.T) {
Updated: now.Unix(),
}
err = dashboardStore.SaveProvisionedDashboard(context.Background(), anotherDash, provisioning)
anotherDash, err := dashboardStore.SaveProvisionedDashboard(context.Background(), saveCmd, provisioning)
require.Nil(t, err)
query := &dashboards.GetDashboardsQuery{DashboardIDs: []int64{anotherDash.ID}}
pkg/services/dashboards/database/database_test.go
+26 -16
View File
@@ -253,27 +253,37 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
t.Run("Should delete associated provisioning info, even without the dashboard existing in the db", func(t *testing.T) {
setup()
dash1 := insertTestDashboard(t, dashboardStore, "provisioned", 1, 0, "", false, "provisioned")
dash2 := insertTestDashboard(t, dashboardStore, "orphaned", 1, 0, "", false, "orphaned")
provisioningData := &dashboards.DashboardProvisioning{
ID: 1,
DashboardID: dash1.ID,
Name: "test",
CheckSum: "123",
Updated: 54321,
ExternalID: "/path/to/dashboard",
ID: 1,
Name: "test",
CheckSum: "123",
Updated: 54321,
ExternalID: "/path/to/dashboard",
}
err := dashboardStore.SaveProvisionedDashboard(context.Background(), dash1, provisioningData)
dash1, err := dashboardStore.SaveProvisionedDashboard(context.Background(), dashboards.SaveDashboardCommand{
OrgID: 1,
IsFolder: false,
Dashboard: simplejson.NewFromAny(map[string]any{
"id": nil,
"title": "provisioned",
}),
}, provisioningData)
require.NoError(t, err)
provisioningData2 := &dashboards.DashboardProvisioning{
ID: 1,
DashboardID: dash2.ID,
Name: "orphaned",
CheckSum: "123",
Updated: 54321,
ExternalID: "/path/to/dashboard",
ID: 1,
Name: "orphaned",
CheckSum: "123",
Updated: 54321,
ExternalID: "/path/to/dashboard",
}
err = dashboardStore.SaveProvisionedDashboard(context.Background(), dash2, provisioningData2)
_, err = dashboardStore.SaveProvisionedDashboard(context.Background(), dashboards.SaveDashboardCommand{
OrgID: 1,
IsFolder: false,
Dashboard: simplejson.NewFromAny(map[string]any{
"id": nil,
"title": "orphaned",
}),
}, provisioningData2)
require.NoError(t, err)
// get provisioning data
pkg/services/dashboards/service/dashboard_service.go
+4 -11
View File
@@ -677,24 +677,17 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt
if err != nil {
return nil, err
}
if cmd == nil {
return nil, fmt.Errorf("failed to build save dashboard command. cmd is nil")
}
var dash *dashboards.Dashboard
if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) {
// save the dashboard but then do NOT return
// we want to save the provisioning data to the dashboard_provisioning table still
// to ensure we can safely rollback to mode2 if needed
dash, err = dr.saveProvisionedDashboardThroughK8s(ctx, cmd, provisioning, false)
if err != nil {
return nil, err
}
} else {
dash, err = dr.saveDashboard(ctx, cmd)
if err != nil {
return nil, err
}
dash, err = dr.dashboardStore.SaveProvisionedDashboard(ctx, *cmd, provisioning)
}
err = dr.dashboardStore.SaveProvisionedDashboard(ctx, dash, provisioning)
if err != nil {
return nil, err
}
pkg/services/dashboards/service/dashboard_service_test.go
+4 -9
View File
@@ -159,9 +159,7 @@ func TestDashboardService(t *testing.T) {
dto := &dashboards.SaveDashboardDTO{}
t.Run("Should not return validation error if dashboard is provisioned", func(t *testing.T) {
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(nil).Once()
fakeStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand"), mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
dto.Dashboard = dashboards.NewDashboard("Dash")
dto.Dashboard.SetID(3)
dto.User = &user.SignedInUser{UserID: 1}
@@ -170,9 +168,7 @@ func TestDashboardService(t *testing.T) {
})
t.Run("Should override invalid refresh interval if dashboard is provisioned", func(t *testing.T) {
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(nil).Once()
fakeStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand"), mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
oldRefreshInterval := service.cfg.MinRefreshInterval
service.cfg.MinRefreshInterval = "5m"
defer func() { service.cfg.MinRefreshInterval = oldRefreshInterval }()
@@ -1216,8 +1212,7 @@ func TestSaveProvisionedDashboard(t *testing.T) {
t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) {
service.features = featuremgmt.WithFeatures()
fakeStore.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(nil)
fakeStore.On("SaveDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
dashboard, err := service.SaveProvisionedDashboard(context.Background(), query, &dashboards.DashboardProvisioning{})
require.NoError(t, err)
require.NotNil(t, dashboard)
@@ -1237,7 +1232,7 @@ func TestSaveProvisionedDashboard(t *testing.T) {
t.Run("Should use Kubernetes create if feature flags are enabled", func(t *testing.T) {
ctx, k8sCliMock := setupK8sDashboardTests(service)
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(nil)
fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil)
k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
pkg/services/dashboards/store_mock.go
+20 -8
View File
@@ -694,22 +694,34 @@ func (_m *FakeDashboardStore) SaveDashboard(ctx context.Context, cmd SaveDashboa
return r0, r1
}
// SaveProvisionedDashboard provides a mock function with given fields: ctx, dash, provisioning
func (_m *FakeDashboardStore) SaveProvisionedDashboard(ctx context.Context, dash *Dashboard, provisioning *DashboardProvisioning) error {
ret := _m.Called(ctx, dash, provisioning)
// SaveProvisionedDashboard provides a mock function with given fields: ctx, cmd, provisioning
func (_m *FakeDashboardStore) SaveProvisionedDashboard(ctx context.Context, cmd SaveDashboardCommand, provisioning *DashboardProvisioning) (*Dashboard, error) {
ret := _m.Called(ctx, cmd, provisioning)
if len(ret) == 0 {
panic("no return value specified for SaveProvisionedDashboard")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, *Dashboard, *DashboardProvisioning) error); ok {
r0 = rf(ctx, dash, provisioning)
var r0 *Dashboard
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, SaveDashboardCommand, *DashboardProvisioning) (*Dashboard, error)); ok {
return rf(ctx, cmd, provisioning)
}
if rf, ok := ret.Get(0).(func(context.Context, SaveDashboardCommand, *DashboardProvisioning) *Dashboard); ok {
r0 = rf(ctx, cmd, provisioning)
} else {
r0 = ret.Error(0)
if ret.Get(0) != nil {
r0 = ret.Get(0).(*Dashboard)
}
}
return r0
if rf, ok := ret.Get(1).(func(context.Context, SaveDashboardCommand, *DashboardProvisioning) error); ok {
r1 = rf(ctx, cmd, provisioning)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// SoftDeleteDashboard provides a mock function with given fields: ctx, orgID, dashboardUid