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

api: Validate dashboards on save via coremodels, behind feature toggle (#48252)

Browse Source 
* Add coremodelValidation feature flag

* coremodels: use stubs when feature flag is off

* api: validate dashboards on save

* Need pointer receiver for FeatureManager

* Update dashboard Go model

* Align doc comments

* Include CoremodelRegistry in test

* Wedge coremodel in on all test cases, ugh

* Ugh fix comment again

* Update pkg/framework/coremodel/staticregistry/provide.go

Co-authored-by: Artur Wierzbicki <wierzbicki.artur.94@gmail.com>

* Update Thema (and its deps) for better errs

* omg whitespace

Co-authored-by: Artur Wierzbicki <wierzbicki.artur.94@gmail.com>
This commit is contained in:
sam boyer
2022-05-22 02:44:12 +02:00
committed by GitHub
co-authored by Artur Wierzbicki
parent 03fe1435a0
commit a3402641d6
11 changed files with 111 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/dashboard.go
+25
View File
@@ -16,11 +16,14 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/components/dashdiffs"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/coremodel/dashboard"
"github.com/grafana/grafana/pkg/cuectx"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/guardian"
pref "github.com/grafana/grafana/pkg/services/preference"
"github.com/grafana/grafana/pkg/services/star"
@@ -305,6 +308,28 @@ func (hs *HTTPServer) PostDashboard(c *models.ReqContext) response.Response {
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
if hs.Features.IsEnabled(featuremgmt.FlagValidateDashboardsOnSave) {
// Ideally, coremodel validation calls would be integrated into the web
// framework. But this does the job for now.
if cm, has := hs.CoremodelRegistry.Get("dashboard"); has {
schv, err := cmd.Dashboard.Get("schemaVersion").Int()
// Only try to validate if the schemaVersion is at least the handoff version
// (the minimum schemaVersion against which the dashboard schema is known to
// work), or if schemaVersion is absent (which will happen once the Thema
// schema becomes canonical).
if err != nil || schv >= dashboard.HandoffSchemaVersion {
// Can't fail, web.Bind() already ensured it's valid JSON
b, _ := cmd.Dashboard.Bytes()
v, _ := cuectx.JSONtoCUE("dashboard.json", b)
if _, err := cm.CurrentSchema().Validate(v); err != nil {
return response.Error(http.StatusBadRequest, "invalid dashboard json", err)
}
}
}
}
return hs.postDashboard(c, cmd)
}
pkg/api/dashboard_test.go
+28 -7
View File
@@ -9,6 +9,9 @@ import (
"net/http"
"testing"
"github.com/grafana/grafana/pkg/coremodel/dashboard"
"github.com/grafana/grafana/pkg/cuectx"
"github.com/grafana/grafana/pkg/framework/coremodel"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@@ -23,7 +26,7 @@ import (
"github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/database"
service "github.com/grafana/grafana/pkg/services/dashboards/service"
"github.com/grafana/grafana/pkg/services/dashboards/service"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/libraryelements"
@@ -51,6 +54,7 @@ func TestGetHomeDashboard(t *testing.T) {
Cfg: cfg,
pluginStore: &fakePluginStore{},
SQLStore: mockstore.NewSQLStoreMock(),
CoremodelRegistry: setupDashboardCoremodel(t),
preferenceService: prefService,
}
@@ -127,12 +131,13 @@ func TestDashboardAPIEndpoint(t *testing.T) {
mockSQLStore := mockstore.NewSQLStoreMock()
hs := &HTTPServer{
Cfg: setting.NewCfg(),
pluginStore: &fakePluginStore{},
SQLStore: mockSQLStore,
AccessControl: accesscontrolmock.New(),
Features: featuremgmt.WithFeatures(),
dashboardService: dashboardService,
Cfg: setting.NewCfg(),
pluginStore: &fakePluginStore{},
SQLStore: mockSQLStore,
CoremodelRegistry: setupDashboardCoremodel(t),
AccessControl: accesscontrolmock.New(),
Features: featuremgmt.WithFeatures(),
dashboardService: dashboardService,
}
setUp := func() {
@@ -235,6 +240,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
Live: newTestLive(t, sql),
LibraryPanelService: &mockLibraryPanelService{},
LibraryElementService: &mockLibraryElementService{},
CoremodelRegistry: setupDashboardCoremodel(t),
SQLStore: mockSQLStore,
AccessControl: accesscontrolmock.New(),
dashboardService: dashboardService,
@@ -914,6 +920,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
LibraryPanelService: &mockLibraryPanelService{},
LibraryElementService: &mockLibraryElementService{},
dashboardProvisioningService: mockDashboardProvisioningService{},
CoremodelRegistry: setupDashboardCoremodel(t),
SQLStore: mockSQLStore,
AccessControl: accesscontrolmock.New(),
dashboardService: dashboardService,
@@ -958,6 +965,7 @@ func getDashboardShouldReturn200WithConfig(t *testing.T, sc *scenarioContext, pr
LibraryElementService: &libraryElementsService,
SQLStore: sc.sqlStore,
ProvisioningService: provisioningService,
CoremodelRegistry: setupDashboardCoremodel(t),
AccessControl: accesscontrolmock.New(),
dashboardProvisioningService: service.ProvideDashboardService(
cfg, dashboardStore, nil, features,
@@ -1026,6 +1034,7 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s
pluginStore: &fakePluginStore{},
LibraryPanelService: &mockLibraryPanelService{},
LibraryElementService: &mockLibraryElementService{},
CoremodelRegistry: setupDashboardCoremodel(t),
dashboardService: dashboardService,
folderService: folderService,
Features: featuremgmt.WithFeatures(),
@@ -1057,6 +1066,7 @@ func postDiffScenario(t *testing.T, desc string, url string, routePattern string
QuotaService: &quota.QuotaService{Cfg: cfg},
LibraryPanelService: &mockLibraryPanelService{},
LibraryElementService: &mockLibraryElementService{},
CoremodelRegistry: setupDashboardCoremodel(t),
SQLStore: sqlmock,
}
@@ -1090,6 +1100,7 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout
QuotaService: &quota.QuotaService{Cfg: cfg},
LibraryPanelService: &mockLibraryPanelService{},
LibraryElementService: &mockLibraryElementService{},
CoremodelRegistry: setupDashboardCoremodel(t),
dashboardService: mock,
SQLStore: sqlStore,
Features: featuremgmt.WithFeatures(),
@@ -1116,6 +1127,16 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout
})
}
func setupDashboardCoremodel(t *testing.T) *coremodel.Registry {
// TODO abstract and generalize this further for wider reuse
t.Helper()
dcm, err := dashboard.ProvideCoremodel(cuectx.ProvideThemaLibrary())
require.NoError(t, err)
reg, err := coremodel.NewRegistry(dcm)
require.NoError(t, err)
return reg
}
func (sc *scenarioContext) ToJSON() *simplejson.Json {
result := simplejson.New()
err := json.NewDecoder(sc.resp.Body).Decode(result)
pkg/api/http_server.go
+4 -1
View File
@@ -17,6 +17,7 @@ import (
"github.com/grafana/grafana/pkg/api/routing"
httpstatic "github.com/grafana/grafana/pkg/api/static"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/framework/coremodel"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/remotecache"
@@ -154,6 +155,7 @@ type HTTPServer struct {
folderPermissionsService accesscontrol.FolderPermissionsService
dashboardPermissionsService accesscontrol.DashboardPermissionsService
starService star.Service
CoremodelRegistry *coremodel.Registry
}
type ServerOptions struct {
@@ -187,7 +189,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
dashboardsnapshotsService *dashboardsnapshots.Service, commentsService *comments.Service, pluginSettings *pluginSettings.Service,
avatarCacheServer *avatar.AvatarCacheServer, preferenceService pref.Service, entityEventsService store.EntityEventsService,
teamsPermissionsService accesscontrol.TeamPermissionsService, folderPermissionsService accesscontrol.FolderPermissionsService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService, starService star.Service,
dashboardPermissionsService accesscontrol.DashboardPermissionsService, starService star.Service, coremodelRegistry *coremodel.Registry,
) (*HTTPServer, error) {
web.Env = cfg.Env
m := web.New()
@@ -265,6 +267,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
folderPermissionsService: folderPermissionsService,
dashboardPermissionsService: dashboardPermissionsService,
starService: starService,
CoremodelRegistry: coremodelRegistry,
}
if hs.Listener != nil {
hs.log.Debug("Using provided listener")