From 5c243126256f011116800fc439cfad5b87ea67f1 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Sat, 15 Mar 2025 21:12:48 -0600 Subject: [PATCH] Dashboards: Fix duplicate provisioning when errors occur on title-only based provisioning (#102249) Dashboards: fix title based provisioning --- .../apis/dashboard/legacy/sql_dashboards.go | 38 ++++-- .../dashboard/legacy/sql_dashboards_test.go | 68 ++++++++- pkg/registry/apis/dashboard/legacy/storage.go | 65 ++++++++- .../apis/dashboard/legacy/storage_test.go | 129 ++++++++++++++++++ pkg/services/dashboards/dashboard.go | 2 +- pkg/services/dashboards/database/database.go | 20 ++- .../database/database_provisioning_test.go | 12 +- .../dashboards/database/database_test.go | 42 +++--- .../dashboards/service/dashboard_service.go | 15 +- .../service/dashboard_service_test.go | 13 +- pkg/services/dashboards/store_mock.go | 28 ++-- 11 files changed, 349 insertions(+), 83 deletions(-) create mode 100644 pkg/registry/apis/dashboard/legacy/storage_test.go diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 6563300c354..85c2e732cb9 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -5,7 +5,6 @@ import ( "database/sql" "encoding/json" "fmt" - "path/filepath" "strconv" "strings" "sync" @@ -320,15 +319,8 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRo // if the reader cannot be found, it may be an orphaned provisioned dashboard resolvedPath := a.provisioning.GetDashboardProvisionerResolvedPath(origin_name.String) if resolvedPath != "" { - originPath, err := filepath.Rel( - resolvedPath, - origin_path.String, - ) - if err != nil { - return nil, err - } meta.SetSourceProperties(utils.SourceProperties{ - Path: originPath, // relative path within source + Path: origin_path.String, Checksum: origin_hash.String, TimestampMillis: origin_ts.Int64, }) @@ -392,8 +384,7 @@ func (a *dashboardSqlAccess) DeleteDashboard(ctx context.Context, orgId int64, u return dash, true, nil } -// SaveDashboard implements DashboardAccess. -func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, dash *dashboard.Dashboard) (*dashboard.Dashboard, bool, error) { +func (a *dashboardSqlAccess) buildSaveDashboardCommand(ctx context.Context, orgId int64, dash *dashboard.Dashboard) (*dashboards.SaveDashboardCommand, bool, error) { created := false user, ok := claims.AuthInfoFrom(ctx) if !ok || user == nil { @@ -424,16 +415,17 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das var err error userID, err = identity.UserIdentifier(user.GetSubject()) if err != nil { - return nil, false, err + return nil, created, err } } apiVersion := strings.TrimPrefix(dash.APIVersion, dashboard.GROUP+"/") meta, err := utils.MetaAccessor(dash) if err != nil { - return nil, false, err + return nil, created, err } - out, err := a.dashStore.SaveDashboard(ctx, dashboards.SaveDashboardCommand{ + + return &dashboards.SaveDashboardCommand{ OrgID: orgId, Message: meta.GetMessage(), PluginID: dashboardOG.GetPluginIDFromMeta(meta), @@ -442,7 +434,21 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das Overwrite: true, // already passed the revisionVersion checks! UserID: userID, APIVersion: apiVersion, - }) + }, created, nil +} + +func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, dash *dashboard.Dashboard) (*dashboard.Dashboard, bool, error) { + user, ok := claims.AuthInfoFrom(ctx) + if !ok || user == nil { + return nil, false, fmt.Errorf("no user found in context") + } + + cmd, created, err := a.buildSaveDashboardCommand(ctx, orgId, dash) + if err != nil { + return nil, created, err + } + + out, err := a.dashStore.SaveDashboard(ctx, *cmd) if err != nil { return nil, false, err } @@ -452,6 +458,8 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das dash, _, err = a.GetDashboard(ctx, orgId, out.UID, 0) if err != nil { return nil, false, err + } else if dash == nil { + return nil, false, fmt.Errorf("unable to retrieve dashboard after save") } // stash the raw value in context (if requested) diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go index 692376ebb4b..2e8d5485e9e 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go @@ -6,12 +6,17 @@ import ( "time" "github.com/DATA-DOG/go-sqlmock" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/provisioning" + "github.com/grafana/grafana/pkg/services/user" ) func TestScanRow(t *testing.T) { @@ -32,7 +37,7 @@ func TestScanRow(t *testing.T) { title := "Test Dashboard" folderUID := "folder123" timestamp := time.Now() - k8sTimestamp := v1.Time{Time: timestamp} + k8sTimestamp := metav1.Time{Time: timestamp} version := int64(2) message := "updated message" createdUser := "creator" @@ -91,7 +96,7 @@ func TestScanRow(t *testing.T) { require.Equal(t, utils.ManagerKindClassicFP, m.Kind) // nolint:staticcheck require.Equal(t, "provisioner", m.Identity) - require.Equal(t, "../"+pathToFile, s.Path) // relative to provisioner + require.Equal(t, pathToFile, s.Path) require.Equal(t, "hashing", s.Checksum) require.NoError(t, err) require.Equal(t, int64(100000), s.TimestampMillis) @@ -119,3 +124,60 @@ func TestScanRow(t *testing.T) { require.Equal(t, "", meta.GetAnnotations()[utils.AnnoKeySourceChecksum]) // hash is not used on plugins }) } + +func TestBuildSaveDashboardCommand(t *testing.T) { + mockStore := &dashboards.FakeDashboardStore{} + access := &dashboardSqlAccess{ + dashStore: mockStore, + } + dash := &dashboard.Dashboard{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "dashboard.grafana.app/v0alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dash", + }, + Spec: common.Unstructured{ + Object: map[string]interface{}{ + "title": "Test Dashboard", + "id": 123, + }, + }, + } + + // fail if no user in context + _, _, err := access.buildSaveDashboardCommand(context.Background(), 1, dash) + require.Error(t, err) + + ctx := identity.WithRequester(context.Background(), &user.SignedInUser{ + OrgID: 1, + OrgRole: "Admin", + }) + // create new dashboard + mockStore.On("GetDashboard", mock.Anything, mock.Anything).Return(nil, nil).Once() + cmd, created, err := access.buildSaveDashboardCommand(ctx, 1, dash) + require.NoError(t, err) + require.Equal(t, true, created) + require.NotNil(t, cmd) + require.Equal(t, "test-dash", cmd.Dashboard.Get("uid").MustString()) + _, exists := cmd.Dashboard.CheckGet("id") + require.False(t, exists) // id should be removed + require.Equal(t, cmd.OrgID, int64(1)) + require.True(t, cmd.Overwrite) + + // now update existing dashboard + mockStore.On("GetDashboard", mock.Anything, mock.Anything).Return( + &dashboards.Dashboard{ + ID: 1234, + APIVersion: "dashboard.grafana.app/v0alpha1", + }, nil).Once() + cmd, created, err = access.buildSaveDashboardCommand(ctx, 1, dash) + require.NoError(t, err) + require.Equal(t, false, created) + require.NotNil(t, cmd) + require.Equal(t, "test-dash", cmd.Dashboard.Get("uid").MustString()) + require.Equal(t, cmd.Dashboard.Get("id").MustInt64(), int64(1234)) // should set to existing ID + require.Equal(t, cmd.APIVersion, "v0alpha1") // should trim prefix + require.Equal(t, cmd.OrgID, int64(1)) + require.True(t, cmd.Overwrite) +} diff --git a/pkg/registry/apis/dashboard/legacy/storage.go b/pkg/registry/apis/dashboard/legacy/storage.go index e24007813ed..4e00cd6d803 100644 --- a/pkg/registry/apis/dashboard/legacy/storage.go +++ b/pkg/registry/apis/dashboard/legacy/storage.go @@ -11,6 +11,7 @@ import ( dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/storage/unified/resource" ) @@ -27,6 +28,36 @@ func getDashboardFromEvent(event resource.WriteEvent) (*dashboard.Dashboard, err return dash, err } +func getProvisioningDataFromEvent(event resource.WriteEvent) (*dashboards.DashboardProvisioning, error) { + obj, ok := event.Object.GetRuntimeObject() + if !ok { + return nil, fmt.Errorf("object is not a runtime object") + } + meta, err := utils.MetaAccessor(obj) + if err != nil { + return nil, err + } + + provisioningData, ok := meta.GetManagerProperties() + if !ok || (provisioningData.Kind != utils.ManagerKindClassicFP) { //nolint:staticcheck + return nil, nil + } + source, ok := meta.GetSourceProperties() + if !ok { + return nil, nil + } + provisioning := &dashboards.DashboardProvisioning{ + Name: provisioningData.Identity, + ExternalID: source.Path, + CheckSum: source.Checksum, + } + if source.TimestampMillis > 0 { + provisioning.Updated = time.UnixMilli(source.TimestampMillis).Unix() + } + + return provisioning, nil +} + func isDashboardKey(key *resource.ResourceKey, requireName bool) error { gr := dashboard.DashboardResourceInfo.GroupResource() if key.Group != gr.Group { @@ -63,20 +94,44 @@ func (a *dashboardSqlAccess) WriteEvent(ctx context.Context, event resource.Writ if err != nil { return 0, err } - - after, _, err := a.SaveDashboard(ctx, info.OrgID, dash) + // In unistore, provisioning data is stored as annotations on the dashboard object. In legacy, it is stored in a separate + // database table. For the legacy fallback, we need to save the provisioning data in the same transaction - so we need to handle these separately. + // Without this, we can end up having dashboards created in legacy, unistore timing out, and then never saving the provisioning data, which + // results in duplicated dashboards on next startup. + provisioning, err := getProvisioningDataFromEvent(event) if err != nil { return 0, err } - if after != nil { - meta, err := utils.MetaAccessor(after) + if provisioning != nil { + cmd, _, err := a.buildSaveDashboardCommand(ctx, info.OrgID, dash) if err != nil { return 0, err } - rv, err = meta.GetResourceVersionInt64() + + after, err := a.dashStore.SaveProvisionedDashboard(ctx, *cmd, provisioning) if err != nil { return 0, err } + + // dashboard version is the RV in legacy storage + if after != nil { + rv = int64(after.Version) + } + } else { + after, _, err := a.SaveDashboard(ctx, info.OrgID, dash) + if err != nil { + return 0, err + } + if after != nil { + meta, err := utils.MetaAccessor(after) + if err != nil { + return 0, err + } + rv, err = meta.GetResourceVersionInt64() + if err != nil { + return 0, err + } + } } } default: diff --git a/pkg/registry/apis/dashboard/legacy/storage_test.go b/pkg/registry/apis/dashboard/legacy/storage_test.go new file mode 100644 index 00000000000..cbe204c4015 --- /dev/null +++ b/pkg/registry/apis/dashboard/legacy/storage_test.go @@ -0,0 +1,129 @@ +package legacy + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/storage/unified/resource" +) + +func TestGetProvisioningDataFromEvent(t *testing.T) { + tests := []struct { + name string + manager utils.ManagerProperties + source utils.SourceProperties + want *dashboards.DashboardProvisioning + }{ + { + name: "valid provisioning data", + manager: utils.ManagerProperties{ + Kind: utils.ManagerKindClassicFP, //nolint:staticcheck + Identity: "test-name", + }, + source: utils.SourceProperties{ + Path: "test-path", + Checksum: "test-checksum", + TimestampMillis: 1000, + }, + want: &dashboards.DashboardProvisioning{ + Name: "test-name", + ExternalID: "test-path", + CheckSum: "test-checksum", + Updated: 1, + }, + }, + { + name: "non-provisioned dashboard", + manager: utils.ManagerProperties{ + Kind: "different-kind", + }, + source: utils.SourceProperties{}, + want: nil, + }, + { + name: "missing runtime object", + manager: utils.ManagerProperties{}, + source: utils.SourceProperties{}, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := &unstructured.Unstructured{ + Object: map[string]any{}, + } + meta, err := utils.MetaAccessor(res) + require.NoError(t, err) + meta.SetManagerProperties(tt.manager) + meta.SetSourceProperties(tt.source) + got, err := getProvisioningDataFromEvent(resource.WriteEvent{ + Object: meta, + }) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} + +// test that we use the save provisioning function if the file based provisioning is set +func TestWriteProvisioningEvent(t *testing.T) { + dashData := &dashboards.Dashboard{ + Title: "Test Dashboard", + Version: 2, + } + dashBytes, err := json.Marshal(dashData) + require.NoError(t, err) + + key := &resource.ResourceKey{ + Group: dashboard.DashboardResourceInfo.GroupResource().Group, + Resource: dashboard.DashboardResourceInfo.GroupResource().Resource, + Name: "test-dashboard", + Namespace: "stacks-1", + } + + res := &unstructured.Unstructured{ + Object: map[string]any{}, + } + meta, err := utils.MetaAccessor(res) + require.NoError(t, err) + meta.SetManagerProperties(utils.ManagerProperties{ + Kind: utils.ManagerKindClassicFP, //nolint:staticcheck + Identity: "test-name", + }) + meta.SetSourceProperties(utils.SourceProperties{ + Path: "test-path", + Checksum: "test-checksum", + TimestampMillis: 1000, + }) + + event := resource.WriteEvent{ + Type: resource.WatchEvent_ADDED, + Key: key, + Object: meta, + Value: dashBytes, + } + + mockStore := dashboards.NewFakeDashboardStore(t) + mockStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(dashData, nil) + + access := &dashboardSqlAccess{ + dashStore: mockStore, + } + + ctx := identity.WithRequester(context.Background(), &user.SignedInUser{}) + rv, err := access.WriteEvent(ctx, event) + require.NoError(t, err) + require.Equal(t, int64(2), rv) + mockStore.AssertExpectations(t) +} diff --git a/pkg/services/dashboards/dashboard.go b/pkg/services/dashboards/dashboard.go index 2561b74e987..c53a1769714 100644 --- a/pkg/services/dashboards/dashboard.go +++ b/pkg/services/dashboards/dashboard.go @@ -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) diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 14cc779405f..078eabe019e 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -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) { diff --git a/pkg/services/dashboards/database/database_provisioning_test.go b/pkg/services/dashboards/database/database_provisioning_test.go index 1cf53ce5fb2..ec82181c4d3 100644 --- a/pkg/services/dashboards/database/database_provisioning_test.go +++ b/pkg/services/dashboards/database/database_provisioning_test.go @@ -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}} diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 9849cc80691..962814f146c 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -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 diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index be4e3a5e2d3..924a435cd2d 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -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 } diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 2dc45e6494e..005d8409886 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -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) diff --git a/pkg/services/dashboards/store_mock.go b/pkg/services/dashboards/store_mock.go index 10a41e691c1..303ef03130a 100644 --- a/pkg/services/dashboards/store_mock.go +++ b/pkg/services/dashboards/store_mock.go @@ -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