PublicDashboards: Add setting to disable the feature (#78894)

* Replace feature toggle with configuration setting

* Fix permission alert

* Update documentation

* Add back feature toggle

* revert unwanted commited changes

* fix tests

* run prettier

* Update SharePublicDashboard.test.tsx

* fix linter and frontend tests

* Update api.go

* Apply docs edit from code review

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

* Update index.md

* Update docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* Update docs/sources/setup-grafana/configure-grafana/_index.md

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* add isPublicDashboardsEnabled + test

* fix test

* update ff description in registry

* move isPublicDashboardsEnabled

* revert getConfig() update

---------

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com>
This commit is contained in:
Agnès Toulet
2023-12-19 11:43:54 +01:00
committed by GitHub
parent ef60c90dfa
commit fdaf6e3f2e
30 changed files with 159 additions and 137 deletions

View File

@@ -17,16 +17,19 @@ import (
"github.com/grafana/grafana/pkg/services/publicdashboards"
. "github.com/grafana/grafana/pkg/services/publicdashboards/models"
"github.com/grafana/grafana/pkg/services/publicdashboards/validation"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
)
type Api struct {
PublicDashboardService publicdashboards.Service
RouteRegister routing.RouteRegister
AccessControl accesscontrol.AccessControl
Features *featuremgmt.FeatureManager
Log log.Logger
Middleware publicdashboards.Middleware
accessControl accesscontrol.AccessControl
cfg *setting.Cfg
features *featuremgmt.FeatureManager
log log.Logger
routeRegister routing.RouteRegister
}
func ProvideApi(
@@ -35,21 +38,27 @@ func ProvideApi(
ac accesscontrol.AccessControl,
features *featuremgmt.FeatureManager,
md publicdashboards.Middleware,
cfg *setting.Cfg,
) *Api {
api := &Api{
PublicDashboardService: pd,
RouteRegister: rr,
AccessControl: ac,
Features: features,
Log: log.New("publicdashboards.api"),
Middleware: md,
accessControl: ac,
cfg: cfg,
features: features,
log: log.New("publicdashboards.api"),
routeRegister: rr,
}
// attach api if PublicDashboards feature flag is enabled
if features.IsEnabledGlobally(featuremgmt.FlagPublicDashboards) {
// register endpoints if the feature is enabled
if features.IsEnabledGlobally(featuremgmt.FlagPublicDashboards) && cfg.PublicDashboardsEnabled {
api.RegisterAPIEndpoints()
}
if !features.IsEnabledGlobally(featuremgmt.FlagPublicDashboards) {
api.log.Warn("[Deprecated] The publicDashboards feature toggle will be removed in Grafana v11. To disable the public dashboards feature, use the public_dashboards.enabled setting.")
}
return api
}
@@ -59,35 +68,35 @@ func (api *Api) RegisterAPIEndpoints() {
// Anonymous access to public dashboard route is configured in pkg/api/api.go
// because it is deeply dependent on the HTTPServer.Index() method and would result in a
// circular dependency
api.RouteRegister.Group("/api/public/dashboards/:accessToken", func(apiRoute routing.RouteRegister) {
api.routeRegister.Group("/api/public/dashboards/:accessToken", func(apiRoute routing.RouteRegister) {
apiRoute.Get("/", routing.Wrap(api.ViewPublicDashboard))
apiRoute.Get("/annotations", routing.Wrap(api.GetPublicAnnotations))
apiRoute.Post("/panels/:panelId/query", routing.Wrap(api.QueryPublicDashboard))
}, api.Middleware.HandleApi)
// Auth endpoints
auth := accesscontrol.Middleware(api.AccessControl)
auth := accesscontrol.Middleware(api.accessControl)
uidScope := dashboards.ScopeDashboardsProvider.GetResourceScopeUID(accesscontrol.Parameter(":dashboardUid"))
// List public dashboards for org
api.RouteRegister.Get("/api/dashboards/public-dashboards", middleware.ReqSignedIn, routing.Wrap(api.ListPublicDashboards))
api.routeRegister.Get("/api/dashboards/public-dashboards", middleware.ReqSignedIn, routing.Wrap(api.ListPublicDashboards))
// Get public dashboard
api.RouteRegister.Get("/api/dashboards/uid/:dashboardUid/public-dashboards",
api.routeRegister.Get("/api/dashboards/uid/:dashboardUid/public-dashboards",
auth(accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, uidScope)),
routing.Wrap(api.GetPublicDashboard))
// Create Public Dashboard
api.RouteRegister.Post("/api/dashboards/uid/:dashboardUid/public-dashboards",
api.routeRegister.Post("/api/dashboards/uid/:dashboardUid/public-dashboards",
auth(accesscontrol.EvalPermission(dashboards.ActionDashboardsPublicWrite, uidScope)),
routing.Wrap(api.CreatePublicDashboard))
// Update Public Dashboard
api.RouteRegister.Patch("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid",
api.routeRegister.Patch("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid",
auth(accesscontrol.EvalPermission(dashboards.ActionDashboardsPublicWrite, uidScope)),
routing.Wrap(api.UpdatePublicDashboard))
// Delete Public dashboard
api.RouteRegister.Delete("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid",
api.routeRegister.Delete("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid",
auth(accesscontrol.EvalPermission(dashboards.ActionDashboardsPublicWrite, uidScope)),
routing.Wrap(api.DeletePublicDashboard))
}

View File

@@ -13,7 +13,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/publicdashboards"
. "github.com/grafana/grafana/pkg/services/publicdashboards/models"
@@ -27,7 +26,7 @@ var userAdmin = &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleAdmin,
var userViewer = &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}}}}
var anonymousUser = &user.SignedInUser{IsAnonymous: true}
func TestAPIFeatureFlag(t *testing.T) {
func TestAPIFeatureDisabled(t *testing.T) {
testCases := []struct {
Name string
Method string
@@ -71,11 +70,18 @@ func TestAPIFeatureFlag(t *testing.T) {
}
for _, test := range testCases {
t.Run(test.Name, func(t *testing.T) {
t.Run(test.Name+" - setting disabled", func(t *testing.T) {
cfg := setting.NewCfg()
cfg.PublicDashboardsEnabled = false
service := publicdashboards.NewFakePublicDashboardService(t)
features := featuremgmt.WithFeatures()
testServer := setupTestServer(t, cfg, features, service, nil, userAdmin)
testServer := setupTestServer(t, cfg, service, userAdmin, true)
response := callAPI(testServer, test.Method, test.Path, nil, t)
assert.Equal(t, http.StatusNotFound, response.Code)
})
t.Run(test.Name+" - feature flag disabled", func(t *testing.T) {
service := publicdashboards.NewFakePublicDashboardService(t)
testServer := setupTestServer(t, nil, service, userAdmin, false)
response := callAPI(testServer, test.Method, test.Path, nil, t)
assert.Equal(t, http.StatusNotFound, response.Code)
})
@@ -130,9 +136,7 @@ func TestAPIListPublicDashboard(t *testing.T) {
service.On("FindAllWithPagination", mock.Anything, mock.Anything, mock.Anything).
Return(test.Response, test.ResponseErr).Maybe()
cfg := setting.NewCfg()
features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards)
testServer := setupTestServer(t, cfg, features, service, nil, test.User)
testServer := setupTestServer(t, nil, service, test.User, true)
response := callAPI(testServer, http.MethodGet, "/api/dashboards/public-dashboards", nil, t)
assert.Equal(t, test.ExpectedHttpResponse, response.Code)
@@ -259,10 +263,7 @@ func TestAPIDeletePublicDashboard(t *testing.T) {
Return(test.ResponseErr)
}
cfg := setting.NewCfg()
features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards)
testServer := setupTestServer(t, cfg, features, service, nil, test.User)
testServer := setupTestServer(t, nil, service, test.User, true)
response := callAPI(testServer, http.MethodDelete, fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid), nil, t)
assert.Equal(t, test.ExpectedHttpResponse, response.Code)
@@ -347,16 +348,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
Return(test.PublicDashboardResult, test.PublicDashboardErr)
}
cfg := setting.NewCfg()
testServer := setupTestServer(
t,
cfg,
featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
service,
nil,
test.User,
)
testServer := setupTestServer(t, nil, service, test.User, true)
response := callAPI(
testServer,
@@ -474,16 +466,7 @@ func TestApiCreatePublicDashboard(t *testing.T) {
Return(&PublicDashboard{IsEnabled: true}, test.SaveDashboardErr)
}
cfg := setting.NewCfg()
testServer := setupTestServer(
t,
cfg,
featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
service,
nil,
test.User,
)
testServer := setupTestServer(t, nil, service, test.User, true)
response := callAPI(
testServer,
@@ -609,9 +592,6 @@ func TestAPIUpdatePublicDashboard(t *testing.T) {
}
for _, test := range testCases {
cfg := setting.NewCfg()
features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards)
t.Run(test.Name, func(t *testing.T) {
service := publicdashboards.NewFakePublicDashboardService(t)
@@ -620,7 +600,7 @@ func TestAPIUpdatePublicDashboard(t *testing.T) {
Return(test.ExpectedResponse, test.ExpectedError)
}
testServer := setupTestServer(t, cfg, features, service, nil, test.User)
testServer := setupTestServer(t, nil, service, test.User, true)
url := fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid)
body := strings.NewReader(test.Body)

View File

@@ -40,11 +40,12 @@ import (
func setupTestServer(
t *testing.T,
cfg *setting.Cfg,
features *featuremgmt.FeatureManager,
service publicdashboards.Service,
db db.DB,
user *user.SignedInUser,
ffEnabled bool,
) *web.Mux {
t.Helper()
// build router to register routes
rr := routing.NewRouteRegister()
@@ -56,9 +57,18 @@ func setupTestServer(
// set initial context
m.Use(contextProvider(&testContext{user}))
// build api, this will mount the routes at the same time if
// featuremgmt.FlagPublicDashboard is enabled
ProvideApi(service, rr, ac, features, &Middleware{})
features := featuremgmt.WithFeatures()
if ffEnabled {
features = featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards)
}
if cfg == nil {
cfg = setting.NewCfg()
cfg.PublicDashboardsEnabled = true
}
// build api, this will mount the routes at the same time if the feature is enabled
ProvideApi(service, rr, ac, features, &Middleware{}, cfg)
// connect routes to mux
rr.Register(m.Router)

View File

@@ -72,7 +72,7 @@ func (api *Api) QueryPublicDashboard(c *contextmodel.ReqContext) response.Respon
return response.Err(err)
}
return toJsonStreamingResponse(c.Req.Context(), api.Features, resp)
return toJsonStreamingResponse(c.Req.Context(), api.features, resp)
}
// swagger:route GET /public/dashboards/{accessToken}/annotations dashboard_public getPublicAnnotations

View File

@@ -101,16 +101,7 @@ func TestAPIViewPublicDashboard(t *testing.T) {
service.On("GetPublicDashboardForView", mock.Anything, mock.AnythingOfType("string")).
Return(test.DashboardResult, test.Err).Maybe()
cfg := setting.NewCfg()
testServer := setupTestServer(
t,
cfg,
featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
service,
nil,
anonymousUser,
)
testServer := setupTestServer(t, nil, service, anonymousUser, true)
response := callAPI(testServer, http.MethodGet,
fmt.Sprintf("/api/public/dashboards/%s", test.AccessToken),
@@ -202,16 +193,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
setup := func(enabled bool) (*web.Mux, *publicdashboards.FakePublicDashboardService) {
service := publicdashboards.NewFakePublicDashboardService(t)
cfg := setting.NewCfg()
testServer := setupTestServer(
t,
cfg,
featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards, enabled),
service,
nil,
anonymousUser,
)
testServer := setupTestServer(t, nil, service, anonymousUser, true)
return testServer, service
}
@@ -338,6 +320,7 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T)
// create public dashboard
store := publicdashboardsStore.ProvideStore(db, db.Cfg, featuremgmt.WithFeatures())
cfg := setting.NewCfg()
cfg.PublicDashboardsEnabled = true
ac := acmock.New()
ws := publicdashboardsService.ProvideServiceWrapper(store)
folderStore := folderimpl.ProvideDashboardFolderStore(db)
@@ -354,13 +337,7 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T)
require.NoError(t, err)
// setup test server
server := setupTestServer(t,
cfg,
featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
pds,
db,
anonymousUser,
)
server := setupTestServer(t, cfg, pds, anonymousUser, true)
resp := callAPI(server, http.MethodPost,
fmt.Sprintf("/api/public/dashboards/%s/panels/1/query", pubdash.AccessToken),
@@ -436,7 +413,6 @@ func TestAPIGetAnnotations(t *testing.T) {
}
for _, test := range testCases {
t.Run(test.Name, func(t *testing.T) {
cfg := setting.NewCfg()
service := publicdashboards.NewFakePublicDashboardService(t)
if test.ExpectedServiceCalled {
@@ -444,7 +420,7 @@ func TestAPIGetAnnotations(t *testing.T) {
Return(test.Annotations, test.ServiceError).Once()
}
testServer := setupTestServer(t, cfg, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil, anonymousUser)
testServer := setupTestServer(t, nil, service, anonymousUser, true)
path := fmt.Sprintf("/api/public/dashboards/%s/annotations?from=%s&to=%s", test.AccessToken, test.From, test.To)
response := callAPI(testServer, http.MethodGet, path, nil, t)

View File

@@ -58,7 +58,7 @@ func TestIntegrationListPublicDashboard(t *testing.T) {
var publicdashboardStore *PublicDashboardStoreImpl
setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagPublicDashboards}})
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{})
quotaService := quotatest.New(false, nil)
dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)
@@ -448,7 +448,7 @@ func TestIntegrationCreatePublicDashboard(t *testing.T) {
var savedDashboard2 *dashboards.Dashboard
setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagPublicDashboards}})
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{})
quotaService := quotatest.New(false, nil)
store, err := dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)
@@ -528,7 +528,7 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) {
var err error
setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagPublicDashboards}})
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{})
quotaService := quotatest.New(false, nil)
dashboardStore, err = dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)
@@ -788,7 +788,7 @@ func TestGetMetrics(t *testing.T) {
var savedDashboard4 *dashboards.Dashboard
setup := func() {
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagPublicDashboards}})
sqlStore, cfg = db.InitTestDBwithCfg(t, db.InitTestDBOpt{})
quotaService := quotatest.New(false, nil)
store, err := dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)