Snapshots: Prevent creation of snapshot of nonexistent dashboard (#86806)

* Add validation to check for existence of dashboard before creation

* add service mock for FindDashboard

* update tests

* reorder function

* update logic change to bool

* update naming validate dashboard

* add tests

* update return val

* remove bool return val

* simplify return statement

* update test

* remove extra spacing

* update mock

* remove unncessary space
This commit is contained in:
Lucy Chen 2024-05-23 15:17:55 -04:00 committed by GitHub
parent 42b0f802de
commit e8625ecd4a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 97 additions and 7 deletions

View File

@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"net/http" "net/http"
"time" "time"
@ -15,6 +16,7 @@ import (
"github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/auth/identity"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
) )
@ -26,6 +28,7 @@ type Service interface {
DeleteExpiredSnapshots(context.Context, *DeleteExpiredSnapshotsCommand) error DeleteExpiredSnapshots(context.Context, *DeleteExpiredSnapshotsCommand) error
GetDashboardSnapshot(context.Context, *GetDashboardSnapshotQuery) (*DashboardSnapshot, error) GetDashboardSnapshot(context.Context, *GetDashboardSnapshotQuery) (*DashboardSnapshot, error)
SearchDashboardSnapshots(context.Context, *GetDashboardSnapshotsQuery) (DashboardSnapshotsList, error) SearchDashboardSnapshots(context.Context, *GetDashboardSnapshotsQuery) (DashboardSnapshotsList, error)
ValidateDashboardExists(context.Context, int64, string) error
} }
var client = &http.Client{ var client = &http.Client{
@ -39,6 +42,18 @@ func CreateDashboardSnapshot(c *contextmodel.ReqContext, cfg dashboardsnapshot.S
return return
} }
uid := cmd.DashboardCreateCommand.Dashboard.GetNestedString("uid")
err := svc.ValidateDashboardExists(c.Req.Context(), c.SignedInUser.GetOrgID(), uid)
if err != nil {
if errors.Is(err, dashboards.ErrDashboardNotFound) {
c.JsonApiErr(http.StatusBadRequest, "Dashboard not found", err)
return
}
c.JsonApiErr(http.StatusInternalServerError, "Failed to get dashboard", err)
return
}
if cmd.DashboardCreateCommand.Name == "" { if cmd.DashboardCreateCommand.Name == "" {
cmd.DashboardCreateCommand.Name = "Unnamed snapshot" cmd.DashboardCreateCommand.Name = "Unnamed snapshot"
} }

View File

@ -4,27 +4,35 @@ import (
"context" "context"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/services/dashboardsnapshots"
"github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets"
) )
type ServiceImpl struct { type ServiceImpl struct {
store dashboardsnapshots.Store store dashboardsnapshots.Store
secretsService secrets.Service secretsService secrets.Service
dashboardService dashboards.DashboardService
} }
// ServiceImpl implements the dashboardsnapshots Service interface // ServiceImpl implements the dashboardsnapshots Service interface
var _ dashboardsnapshots.Service = (*ServiceImpl)(nil) var _ dashboardsnapshots.Service = (*ServiceImpl)(nil)
func ProvideService(store dashboardsnapshots.Store, secretsService secrets.Service) *ServiceImpl { func ProvideService(store dashboardsnapshots.Store, secretsService secrets.Service, dashboardService dashboards.DashboardService) *ServiceImpl {
s := &ServiceImpl{ s := &ServiceImpl{
store: store, store: store,
secretsService: secretsService, secretsService: secretsService,
dashboardService: dashboardService,
} }
return s return s
} }
func (s *ServiceImpl) ValidateDashboardExists(ctx context.Context, orgId int64, dashboardUid string) error {
_, err := s.dashboardService.GetDashboard(ctx, &dashboards.GetDashboardQuery{UID: dashboardUid, OrgID: orgId})
return err
}
func (s *ServiceImpl) CreateDashboardSnapshot(ctx context.Context, cmd *dashboardsnapshots.CreateDashboardSnapshotCommand) (*dashboardsnapshots.DashboardSnapshot, error) { func (s *ServiceImpl) CreateDashboardSnapshot(ctx context.Context, cmd *dashboardsnapshots.CreateDashboardSnapshotCommand) (*dashboardsnapshots.DashboardSnapshot, error) {
marshalledData, err := cmd.Dashboard.MarshalJSON() marshalledData, err := cmd.Dashboard.MarshalJSON()
if err != nil { if err != nil {

View File

@ -4,16 +4,26 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"testing" "testing"
"time"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
dashboardsnapshot "github.com/grafana/grafana/pkg/apis/dashboardsnapshot/v0alpha1" dashboardsnapshot "github.com/grafana/grafana/pkg/apis/dashboardsnapshot/v0alpha1"
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/dashboards"
dashdb "github.com/grafana/grafana/pkg/services/dashboards/database"
dashsvc "github.com/grafana/grafana/pkg/services/dashboards/service"
"github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/services/dashboardsnapshots"
dashsnapdb "github.com/grafana/grafana/pkg/services/dashboardsnapshots/database" dashsnapdb "github.com/grafana/grafana/pkg/services/dashboardsnapshots/database"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder/folderimpl"
"github.com/grafana/grafana/pkg/services/folder/foldertest"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/secrets/database" "github.com/grafana/grafana/pkg/services/secrets/database"
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tests/testsuite" "github.com/grafana/grafana/pkg/tests/testsuite"
) )
@ -26,8 +36,9 @@ func TestDashboardSnapshotsService(t *testing.T) {
sqlStore := db.InitTestDB(t) sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg() cfg := setting.NewCfg()
dsStore := dashsnapdb.ProvideStore(sqlStore, cfg) dsStore := dashsnapdb.ProvideStore(sqlStore, cfg)
fakeDashboardService := &dashboards.FakeDashboardService{}
secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore))
s := ProvideService(dsStore, secretsService) s := ProvideService(dsStore, secretsService, fakeDashboardService)
origSecret := cfg.SecretKey origSecret := cfg.SecretKey
cfg.SecretKey = "dashboard_snapshot_service_test" cfg.SecretKey = "dashboard_snapshot_service_test"
@ -79,3 +90,45 @@ func TestDashboardSnapshotsService(t *testing.T) {
require.Equal(t, rawDashboard, decrypted) require.Equal(t, rawDashboard, decrypted)
}) })
} }
func TestValidateDashboardExists(t *testing.T) {
sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg()
dsStore := dashsnapdb.ProvideStore(sqlStore, cfg)
secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore))
dashboardStore, err := dashdb.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotatest.New(false, nil))
require.NoError(t, err)
dashSvc, err := dashsvc.ProvideDashboardServiceImpl(cfg, dashboardStore, folderimpl.ProvideDashboardFolderStore(sqlStore), nil, nil, nil, acmock.New(), foldertest.NewFakeService(), nil)
require.NoError(t, err)
s := ProvideService(dsStore, secretsService, dashSvc)
ctx := context.Background()
t.Run("returns false when dashboard does not exist", func(t *testing.T) {
err := s.ValidateDashboardExists(ctx, 1, "test")
require.Error(t, err)
require.Equal(t, dashboards.ErrDashboardNotFound, err)
})
t.Run("returns true when dashboard exists", func(t *testing.T) {
err := createDashboard(sqlStore)
require.NoError(t, err)
err = s.ValidateDashboardExists(ctx, 1, "test")
require.NoError(t, err)
})
}
func createDashboard(store db.DB) error {
return store.WithDbSession(context.Background(), func(sess *db.Session) error {
dashboard := &dashboards.Dashboard{
ID: 1,
UID: "test",
OrgID: 1,
Created: time.Now(),
Updated: time.Now(),
}
_, err := sess.Insert(dashboard)
return err
})
}

View File

@ -1,4 +1,4 @@
// Code generated by mockery v2.32.0. DO NOT EDIT. // Code generated by mockery v2.34.0. DO NOT EDIT.
package dashboardsnapshots package dashboardsnapshots
@ -119,6 +119,20 @@ func (_m *MockService) SearchDashboardSnapshots(_a0 context.Context, _a1 *GetDas
return r0, r1 return r0, r1
} }
// ValidateDashboardExists provides a mock function with given fields: _a0, _a1, _a2
func (_m *MockService) ValidateDashboardExists(_a0 context.Context, _a1 int64, _a2 string) error {
ret := _m.Called(_a0, _a1, _a2)
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, int64, string) error); ok {
r0 = rf(_a0, _a1, _a2)
} else {
r0 = ret.Error(0)
}
return r0
}
// NewMockService creates a new instance of MockService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // NewMockService creates a new instance of MockService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value. // The first argument is typically a *testing.T value.
func NewMockService(t interface { func NewMockService(t interface {