diff --git a/.betterer.results b/.betterer.results index 7ae336bc244..7c4b2921cd5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4145,8 +4145,7 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"], [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Do not use any type assertions.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"] + [0, 0, 0, "Unexpected any. Specify a different type.", "4"] ], "public/app/features/dashboard/services/TimeSrv.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/pkg/services/publicdashboards/api/api.go b/pkg/services/publicdashboards/api/api.go index 240fa205137..d183c4fec95 100644 --- a/pkg/services/publicdashboards/api/api.go +++ b/pkg/services/publicdashboards/api/api.go @@ -11,14 +11,11 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/publicdashboards" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" - "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -27,7 +24,6 @@ type Api struct { PublicDashboardService publicdashboards.Service RouteRegister routing.RouteRegister AccessControl accesscontrol.AccessControl - QueryDataService *query.Service Features *featuremgmt.FeatureManager } @@ -35,14 +31,12 @@ func ProvideApi( pd publicdashboards.Service, rr routing.RouteRegister, ac accesscontrol.AccessControl, - qds *query.Service, features *featuremgmt.FeatureManager, ) *Api { api := &Api{ PublicDashboardService: pd, RouteRegister: rr, AccessControl: ac, - QueryDataService: qds, Features: features, } @@ -157,35 +151,16 @@ func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "invalid panel ID", err) } - // Get the dashboard - pubdash, dashboard, err := api.PublicDashboardService.GetPublicDashboard(c.Req.Context(), web.Params(c.Req)[":accessToken"]) - if err != nil { - return response.Error(http.StatusInternalServerError, "could not fetch dashboard", err) + reqDTO := &PublicDashboardQueryDTO{} + if err = web.Bind(c.Req, reqDTO); err != nil { + return response.Error(http.StatusBadRequest, "bad request data", err) } - // Build the request data objecct - reqDTO, err := api.PublicDashboardService.BuildPublicDashboardMetricRequest( - c.Req.Context(), - dashboard, - pubdash, - panelId, - ) + resp, err := api.PublicDashboardService.GetQueryDataResponse(c.Req.Context(), c.SkipCache, reqDTO, panelId, web.Params(c.Req)[":accessToken"]) if err != nil { - return handleDashboardErr(http.StatusInternalServerError, "Failed to get queries for public dashboard", err) + return handlePublicDashboardErr(err) } - // Build anonymous user for the request - anonymousUser, err := api.PublicDashboardService.BuildAnonymousUser(c.Req.Context(), dashboard) - if err != nil { - return response.Error(http.StatusInternalServerError, "could not create anonymous user", err) - } - - // Make the request - resp, err := api.QueryDataService.QueryDataMultipleSources(c.Req.Context(), anonymousUser, c.SkipCache, reqDTO, true) - - if err != nil { - return handleQueryMetricsError(err) - } return toJsonStreamingResponse(api.Features, resp) } @@ -209,24 +184,8 @@ func handleDashboardErr(defaultCode int, defaultMsg string, err error) response. return response.Error(defaultCode, defaultMsg, err) } -// Copied from pkg/api/metrics.go -func handleQueryMetricsError(err error) *response.NormalResponse { - if errors.Is(err, datasources.ErrDataSourceAccessDenied) { - return response.Error(http.StatusForbidden, "Access denied to data source", err) - } - if errors.Is(err, datasources.ErrDataSourceNotFound) { - return response.Error(http.StatusNotFound, "Data source not found", err) - } - var badQuery *query.ErrBadQuery - if errors.As(err, &badQuery) { - return response.Error(http.StatusBadRequest, util.Capitalize(badQuery.Message), err) - } - - if errors.Is(err, backendplugin.ErrPluginNotRegistered) { - return response.Error(http.StatusNotFound, "Plugin not found", err) - } - - return response.Error(http.StatusInternalServerError, "Query data error", err) +func handlePublicDashboardErr(err error) response.Response { + return handleDashboardErr(http.StatusInternalServerError, "Unexpected Error", err) } // Copied from pkg/api/metrics.go diff --git a/pkg/services/publicdashboards/api/api_test.go b/pkg/services/publicdashboards/api/api_test.go index 580b77ce926..5b20db2f188 100644 --- a/pkg/services/publicdashboards/api/api_test.go +++ b/pkg/services/publicdashboards/api/api_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/aws/aws-sdk-go/aws" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/api/dtos" @@ -23,7 +24,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" dashboardStore "github.com/grafana/grafana/pkg/services/dashboards/database" "github.com/grafana/grafana/pkg/services/datasources" - fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes" + "github.com/grafana/grafana/pkg/services/datasources/service" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/publicdashboards" @@ -40,11 +41,13 @@ func TestAPIGetPublicDashboard(t *testing.T) { t.Run("It should 404 if featureflag is not enabled", func(t *testing.T) { cfg := setting.NewCfg() cfg.RBACEnabled = false - qs := buildQueryDataService(t, nil, nil, nil) service := publicdashboards.NewFakePublicDashboardService(t) service.On("GetPublicDashboard", mock.Anything, mock.AnythingOfType("string")). Return(&PublicDashboard{}, &models.Dashboard{}, nil).Maybe() - testServer := setupTestServer(t, cfg, qs, featuremgmt.WithFeatures(), service, nil) + service.On("GetPublicDashboardConfig", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")). + Return(&PublicDashboard{}, nil).Maybe() + + testServer := setupTestServer(t, cfg, featuremgmt.WithFeatures(), service, nil) response := callAPI(testServer, http.MethodGet, "/api/public/dashboards", nil, t) assert.Equal(t, http.StatusNotFound, response.Code) @@ -53,7 +56,7 @@ func TestAPIGetPublicDashboard(t *testing.T) { assert.Equal(t, http.StatusNotFound, response.Code) // control set. make sure routes are mounted - testServer = setupTestServer(t, cfg, qs, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil) + testServer = setupTestServer(t, cfg, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil) response = callAPI(testServer, http.MethodGet, "/api/public/dashboards/asdf", nil, t) assert.NotEqual(t, http.StatusNotFound, response.Code) }) @@ -102,7 +105,6 @@ func TestAPIGetPublicDashboard(t *testing.T) { testServer := setupTestServer( t, cfg, - buildQueryDataService(t, nil, nil, nil), featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil, @@ -182,7 +184,6 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) { testServer := setupTestServer( t, cfg, - buildQueryDataService(t, nil, nil, nil), featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil, @@ -249,7 +250,6 @@ func TestApiSavePublicDashboardConfig(t *testing.T) { testServer := setupTestServer( t, cfg, - buildQueryDataService(t, nil, nil, nil), featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil, @@ -277,40 +277,56 @@ func TestApiSavePublicDashboardConfig(t *testing.T) { // `/public/dashboards/:uid/query`` endpoint test func TestAPIQueryPublicDashboard(t *testing.T) { - cacheService := &fakeDatasources.FakeCacheService{ - DataSources: []*datasources.DataSource{ - {Uid: "mysqlds"}, - {Uid: "promds"}, - {Uid: "promds2"}, - }, - } - - // used to determine whether fakePluginClient returns an error - queryReturnsError := false - - fakePluginClient := &fakePluginClient{ - QueryDataHandlerFunc: func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { - if queryReturnsError { - return nil, errors.New("error") - } - - resp := backend.Responses{} - - for _, query := range req.Queries { - resp[query.RefID] = backend.DataResponse{ - Frames: []*data.Frame{ - { - RefID: query.RefID, - Name: "query-" + query.RefID, + mockedResponse := &backend.QueryDataResponse{ + Responses: map[string]backend.DataResponse{ + "test": { + Frames: data.Frames{ + &data.Frame{ + Name: "anyDataFrame", + Fields: []*data.Field{ + data.NewField("anyGroupName", nil, []*string{ + aws.String("group_a"), aws.String("group_b"), aws.String("group_c"), + }), }, }, - } - } - return &backend.QueryDataResponse{Responses: resp}, nil + }, + Error: nil, + }, }, } - qds := buildQueryDataService(t, cacheService, fakePluginClient, nil) + expectedResponse := `{ + "results": { + "test": { + "frames": [ + { + "schema": { + "name": "anyDataFrame", + "fields": [ + { + "name": "anyGroupName", + "type": "string", + "typeInfo": { + "frame": "string", + "nullable": true + } + } + ] + }, + "data": { + "values": [ + [ + "group_a", + "group_b", + "group_c" + ] + ] + } + } + ] + } + } +}` setup := func(enabled bool) (*web.Mux, *publicdashboards.FakePublicDashboardService) { service := publicdashboards.NewFakePublicDashboardService(t) @@ -320,7 +336,6 @@ func TestAPIQueryPublicDashboard(t *testing.T) { testServer := setupTestServer( t, cfg, - qds, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards, enabled), service, nil, @@ -341,51 +356,29 @@ func TestAPIQueryPublicDashboard(t *testing.T) { require.Equal(t, http.StatusBadRequest, resp.Code) }) + t.Run("Status code is 400 when the intervalMS is lesser than 0", func(t *testing.T) { + server, fakeDashboardService := setup(true) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), "abc123").Return(&backend.QueryDataResponse{}, ErrPublicDashboardBadRequest) + resp := callAPI(server, http.MethodPost, "/api/public/dashboards/abc123/panels/2/query", strings.NewReader(`{"intervalMs":-100,"maxDataPoints":1000}`), t) + require.Equal(t, http.StatusBadRequest, resp.Code) + }) + + t.Run("Status code is 400 when the maxDataPoints is lesser than 0", func(t *testing.T) { + server, fakeDashboardService := setup(true) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), "abc123").Return(&backend.QueryDataResponse{}, ErrPublicDashboardBadRequest) + resp := callAPI(server, http.MethodPost, "/api/public/dashboards/abc123/panels/2/query", strings.NewReader(`{"intervalMs":100,"maxDataPoints":-1000}`), t) + require.Equal(t, http.StatusBadRequest, resp.Code) + }) + t.Run("Returns query data when feature toggle is enabled", func(t *testing.T) { server, fakeDashboardService := setup(true) - - fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil) - fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil) - fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{ - Queries: []*simplejson.Json{ - simplejson.MustJson([]byte(` - { - "datasource": { - "type": "prometheus", - "uid": "promds" - }, - "exemplar": true, - "expr": "query_2_A", - "interval": "", - "legendFormat": "", - "refId": "A" - } - `)), - }, - }, nil) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), "abc123").Return(mockedResponse, nil) resp := callAPI(server, http.MethodPost, "/api/public/dashboards/abc123/panels/2/query", strings.NewReader("{}"), t) require.JSONEq( t, - `{ - "results": { - "A": { - "frames": [ - { - "data": { - "values": [] - }, - "schema": { - "fields": [], - "refId": "A", - "name": "query-A" - } - } - ] - } - } - }`, + expectedResponse, resp.Body.String(), ) require.Equal(t, http.StatusOK, resp.Code) @@ -393,107 +386,10 @@ func TestAPIQueryPublicDashboard(t *testing.T) { t.Run("Status code is 500 when the query fails", func(t *testing.T) { server, fakeDashboardService := setup(true) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), "abc123").Return(&backend.QueryDataResponse{}, fmt.Errorf("error")) - fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil) - fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil) - fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{ - Queries: []*simplejson.Json{ - simplejson.MustJson([]byte(` - { - "datasource": { - "type": "prometheus", - "uid": "promds" - }, - "exemplar": true, - "expr": "query_2_A", - "interval": "", - "legendFormat": "", - "refId": "A" - } - `)), - }, - }, nil) - - queryReturnsError = true resp := callAPI(server, http.MethodPost, "/api/public/dashboards/abc123/panels/2/query", strings.NewReader("{}"), t) require.Equal(t, http.StatusInternalServerError, resp.Code) - queryReturnsError = false - }) - - t.Run("Status code is 200 when a panel has queries from multiple datasources", func(t *testing.T) { - server, fakeDashboardService := setup(true) - - fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil) - fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil) - fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{ - Queries: []*simplejson.Json{ - simplejson.MustJson([]byte(` -{ - "datasource": { - "type": "prometheus", - "uid": "promds" - }, - "exemplar": true, - "expr": "query_2_A", - "interval": "", - "legendFormat": "", - "refId": "A" - } - `)), - simplejson.MustJson([]byte(` -{ - "datasource": { - "type": "prometheus", - "uid": "promds2" - }, - "exemplar": true, - "expr": "query_2_B", - "interval": "", - "legendFormat": "", - "refId": "B" - } - `)), - }, - }, nil) - - resp := callAPI(server, http.MethodPost, "/api/public/dashboards/abc123/panels/2/query", strings.NewReader("{}"), t) - require.JSONEq( - t, - `{ - "results": { - "A": { - "frames": [ - { - "data": { - "values": [] - }, - "schema": { - "fields": [], - "refId": "A", - "name": "query-A" - } - } - ] - }, - "B": { - "frames": [ - { - "data": { - "values": [] - }, - "schema": { - "fields": [], - "refId": "B", - "name": "query-B" - } - } - ] - } - } - }`, - resp.Body.String(), - ) - require.Equal(t, http.StatusOK, resp.Code) }) } @@ -557,14 +453,13 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T) store := publicdashboardsStore.ProvideStore(db) cfg := setting.NewCfg() cfg.RBACEnabled = false - service := publicdashboardsService.ProvideService(cfg, store) + service := publicdashboardsService.ProvideService(cfg, store, qds) pubdash, err := service.SavePublicDashboardConfig(context.Background(), &user.SignedInUser{}, savePubDashboardCmd) require.NoError(t, err) // setup test server server := setupTestServer(t, cfg, - qds, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, db, diff --git a/pkg/services/publicdashboards/api/common_test.go b/pkg/services/publicdashboards/api/common_test.go index e636e9fa12f..7a585f98ea0 100644 --- a/pkg/services/publicdashboards/api/common_test.go +++ b/pkg/services/publicdashboards/api/common_test.go @@ -43,7 +43,6 @@ type Server struct { func setupTestServer( t *testing.T, cfg *setting.Cfg, - qs *query.Service, features *featuremgmt.FeatureManager, service publicdashboards.Service, db *sqlstore.SQLStore, @@ -85,7 +84,7 @@ func setupTestServer( // build api, this will mount the routes at the same time if // featuremgmt.FlagPublicDashboard is enabled - ProvideApi(service, rr, ac, qs, features) + ProvideApi(service, rr, ac, features) // connect routes to mux rr.Register(m.Router) diff --git a/pkg/services/publicdashboards/api/middleware_test.go b/pkg/services/publicdashboards/api/middleware_test.go index 68332754e67..d21ca3a798e 100644 --- a/pkg/services/publicdashboards/api/middleware_test.go +++ b/pkg/services/publicdashboards/api/middleware_test.go @@ -1,15 +1,19 @@ package api import ( + "context" "fmt" "net/http" "net/http/httptest" "testing" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" + fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes" "github.com/grafana/grafana/pkg/services/publicdashboards" publicdashboardsService "github.com/grafana/grafana/pkg/services/publicdashboards/service" + "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" "github.com/stretchr/testify/mock" @@ -57,7 +61,27 @@ func TestRequiresValidAccessToken(t *testing.T) { func mockAccessTokenExistsResponse(returnArguments ...interface{}) *publicdashboardsService.PublicDashboardServiceImpl { fakeStore := &publicdashboards.FakePublicDashboardStore{} fakeStore.On("AccessTokenExists", mock.Anything, mock.Anything).Return(returnArguments[0], returnArguments[1]) - return publicdashboardsService.ProvideService(setting.NewCfg(), fakeStore) + + qds := query.ProvideService( + nil, + nil, + nil, + &fakePluginRequestValidator{}, + &fakeDatasources.FakeDataSourceService{}, + &fakePluginClient{ + QueryDataHandlerFunc: func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { + resp := backend.Responses{ + "A": backend.DataResponse{ + Error: fmt.Errorf("query failed"), + }, + } + return &backend.QueryDataResponse{Responses: resp}, nil + }, + }, + &fakeOAuthTokenService{}, + ) + + return publicdashboardsService.ProvideService(setting.NewCfg(), fakeStore, qds) } func runMiddleware(request *http.Request, pubdashService *publicdashboardsService.PublicDashboardServiceImpl) *httptest.ResponseRecorder { diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index 0feb241552b..a9b31d5e0a8 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -49,6 +49,10 @@ var ( Reason: "Public dashboard has template variables", StatusCode: 422, } + ErrPublicDashboardBadRequest = PublicDashboardErr{ + Reason: "Bad Request", + StatusCode: 400, + } ) type PublicDashboard struct { @@ -108,6 +112,11 @@ type SavePublicDashboardConfigDTO struct { PublicDashboard *PublicDashboard } +type PublicDashboardQueryDTO struct { + IntervalMs int64 + MaxDataPoints int64 +} + // // COMMANDS // diff --git a/pkg/services/publicdashboards/public_dashboard_service_mock.go b/pkg/services/publicdashboards/public_dashboard_service_mock.go index 018cebd4a81..1ba4f77b013 100644 --- a/pkg/services/publicdashboards/public_dashboard_service_mock.go +++ b/pkg/services/publicdashboards/public_dashboard_service_mock.go @@ -4,16 +4,13 @@ package publicdashboards import ( context "context" - - dtos "github.com/grafana/grafana/pkg/api/dtos" - mock "github.com/stretchr/testify/mock" - - models "github.com/grafana/grafana/pkg/models" - - publicdashboardsmodels "github.com/grafana/grafana/pkg/services/publicdashboards/models" - testing "testing" + mock "github.com/stretchr/testify/mock" + models "github.com/grafana/grafana/pkg/models" + publicdashboardsmodels "github.com/grafana/grafana/pkg/services/publicdashboards/models" + "github.com/grafana/grafana-plugin-sdk-go/backend" + user "github.com/grafana/grafana/pkg/services/user" ) @@ -66,20 +63,20 @@ func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, da return r0, r1 } -// BuildPublicDashboardMetricRequest provides a mock function with given fields: ctx, dashboard, publicDashboard, panelId -func (_m *FakePublicDashboardService) BuildPublicDashboardMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *publicdashboardsmodels.PublicDashboard, panelId int64) (dtos.MetricRequest, error) { - ret := _m.Called(ctx, dashboard, publicDashboard, panelId) +// GetQueryDataResponse provides a mock function with given fields: ctx, skipCache, reqDTO, panelId, accessToken +func (_m *FakePublicDashboardService) GetQueryDataResponse(ctx context.Context, skipCache bool, reqDTO *publicdashboardsmodels.PublicDashboardQueryDTO, panelId int64, accessToken string) (*backend.QueryDataResponse, error) { + ret := _m.Called(ctx, skipCache, reqDTO, panelId, accessToken) - var r0 dtos.MetricRequest - if rf, ok := ret.Get(0).(func(context.Context, *models.Dashboard, *publicdashboardsmodels.PublicDashboard, int64) dtos.MetricRequest); ok { - r0 = rf(ctx, dashboard, publicDashboard, panelId) + var r0 *backend.QueryDataResponse + if rf, ok := ret.Get(0).(func(context.Context, bool, *publicdashboardsmodels.PublicDashboardQueryDTO, int64, string) *backend.QueryDataResponse); ok { + r0 = rf(ctx, skipCache, reqDTO, panelId, accessToken) } else { - r0 = ret.Get(0).(dtos.MetricRequest) + r0 = ret.Get(0).(*backend.QueryDataResponse) } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *models.Dashboard, *publicdashboardsmodels.PublicDashboard, int64) error); ok { - r1 = rf(ctx, dashboard, publicDashboard, panelId) + if rf, ok := ret.Get(1).(func(context.Context, bool, *publicdashboardsmodels.PublicDashboardQueryDTO, int64, string) error); ok { + r1 = rf(ctx, skipCache, reqDTO, panelId, accessToken) } else { r1 = ret.Error(1) } diff --git a/pkg/services/publicdashboards/publicdashboard.go b/pkg/services/publicdashboards/publicdashboard.go index 683afb45d15..82d583fc7bf 100644 --- a/pkg/services/publicdashboards/publicdashboard.go +++ b/pkg/services/publicdashboards/publicdashboard.go @@ -3,7 +3,7 @@ package publicdashboards import ( "context" - "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/models" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" "github.com/grafana/grafana/pkg/services/user" @@ -18,7 +18,7 @@ type Service interface { GetDashboard(ctx context.Context, dashboardUid string) (*models.Dashboard, error) GetPublicDashboardConfig(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error) SavePublicDashboardConfig(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardConfigDTO) (*PublicDashboard, error) - BuildPublicDashboardMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64) (dtos.MetricRequest, error) + GetQueryDataResponse(ctx context.Context, skipCache bool, reqDTO *PublicDashboardQueryDTO, panelId int64, accessToken string) (*backend.QueryDataResponse, error) PublicDashboardEnabled(ctx context.Context, dashboardUid string) (bool, error) AccessTokenExists(ctx context.Context, accessToken string) (bool, error) } diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 914e685aecb..d878b970bae 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -6,6 +6,7 @@ import ( "time" "github.com/google/uuid" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" @@ -14,16 +15,21 @@ 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/services/query" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/tsdb/intervalv2" + "github.com/grafana/grafana/pkg/tsdb/legacydata" ) // Define the Service Implementation. We're generating mock implementation // automatically type PublicDashboardServiceImpl struct { - log log.Logger - cfg *setting.Cfg - store publicdashboards.Store + log log.Logger + cfg *setting.Cfg + store publicdashboards.Store + intervalCalculator intervalv2.Calculator + QueryDataService *query.Service } var LogPrefix = "publicdashboards.service" @@ -37,11 +43,14 @@ var _ publicdashboards.Service = (*PublicDashboardServiceImpl)(nil) func ProvideService( cfg *setting.Cfg, store publicdashboards.Store, + qds *query.Service, ) *PublicDashboardServiceImpl { return &PublicDashboardServiceImpl{ - log: log.New(LogPrefix), - cfg: cfg, - store: store, + log: log.New(LogPrefix), + cfg: cfg, + store: store, + intervalCalculator: intervalv2.NewCalculator(), + QueryDataService: qds, } } @@ -180,25 +189,58 @@ func (pd *PublicDashboardServiceImpl) updatePublicDashboardConfig(ctx context.Co return dto.PublicDashboard.Uid, pd.store.UpdatePublicDashboardConfig(ctx, cmd) } -// BuildPublicDashboardMetricRequest merges public dashboard parameters with -// dashboard and returns a metrics request to be sent to query backend -func (pd *PublicDashboardServiceImpl) BuildPublicDashboardMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64) (dtos.MetricRequest, error) { - if !publicDashboard.IsEnabled { - return dtos.MetricRequest{}, ErrPublicDashboardNotFound +func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context, skipCache bool, reqDTO *PublicDashboardQueryDTO, panelId int64, accessToken string) (*backend.QueryDataResponse, error) { + if err := validation.ValidateQueryPublicDashboardRequest(reqDTO); err != nil { + return nil, ErrPublicDashboardBadRequest } - queriesByPanel := models.GroupQueriesByPanelId(dashboard.Data) + publicDashboard, dashboard, err := pd.GetPublicDashboard(ctx, accessToken) + if err != nil { + return nil, err + } - if _, ok := queriesByPanel[panelId]; !ok { + metricReqDTO, err := pd.buildPublicDashboardMetricRequest( + ctx, + dashboard, + publicDashboard, + panelId, + reqDTO, + ) + if err != nil { + return nil, err + } + + anonymousUser, err := pd.BuildAnonymousUser(ctx, dashboard) + if err != nil { + return nil, err + } + + return pd.QueryDataService.QueryDataMultipleSources(ctx, anonymousUser, skipCache, metricReqDTO, true) +} + +// BuildPublicDashboardMetricRequest merges public dashboard parameters with +// dashboard and returns a metrics request to be sent to query backend +func (pd *PublicDashboardServiceImpl) buildPublicDashboardMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64, reqDTO *PublicDashboardQueryDTO) (dtos.MetricRequest, error) { + // group queries by panel + queriesByPanel := models.GroupQueriesByPanelId(dashboard.Data) + queries, ok := queriesByPanel[panelId] + if !ok { return dtos.MetricRequest{}, ErrPublicDashboardPanelNotFound } ts := publicDashboard.BuildTimeSettings(dashboard) + // determine safe resolution to query data at + safeInterval, safeResolution := pd.getSafeIntervalAndMaxDataPoints(reqDTO, ts) + for i := range queries { + queries[i].Set("intervalMs", safeInterval) + queries[i].Set("maxDataPoints", safeResolution) + } + return dtos.MetricRequest{ From: ts.From, To: ts.To, - Queries: queriesByPanel[panelId], + Queries: queries, }, nil } @@ -240,6 +282,35 @@ func GenerateAccessToken() (string, error) { return fmt.Sprintf("%x", token[:]), nil } +// intervalMS and maxQueryData values are being calculated on the frontend for regular dashboards +// we are doing the same for public dashboards but because this access would be public, we need a way to keep this +// values inside reasonable bounds to avoid an attack that could hit data sources with a small interval and a big +// time range and perform big calculations +// this is an additional validation, all data sources implements QueryData interface and should have proper validations +// of these limits +// for the maxDataPoints we took a hard limit from prometheus which is 11000 +func (pd *PublicDashboardServiceImpl) getSafeIntervalAndMaxDataPoints(reqDTO *PublicDashboardQueryDTO, ts *TimeSettings) (int64, int64) { + // arbitrary max value for all data sources, it is actually a hard limit defined in prometheus + safeResolution := int64(11000) + + // interval calculated on the frontend + interval := time.Duration(reqDTO.IntervalMs) * time.Millisecond + + // calculate a safe interval with time range from dashboard and safeResolution + dataTimeRange := legacydata.NewDataTimeRange(ts.From, ts.To) + tr := backend.TimeRange{ + From: dataTimeRange.GetFromAsTimeUTC(), + To: dataTimeRange.GetToAsTimeUTC(), + } + safeInterval := pd.intervalCalculator.CalculateSafeInterval(tr, safeResolution) + + if interval > safeInterval.Value { + return reqDTO.IntervalMs, reqDTO.MaxDataPoints + } + + return safeInterval.Value.Milliseconds(), safeResolution +} + // Log when PublicDashboard.IsEnabled changed func (pd *PublicDashboardServiceImpl) logIsEnabledChanged(existingPubdash *PublicDashboard, newPubdash *PublicDashboard, u *user.SignedInUser) { if publicDashboardIsEnabledChanged(existingPubdash, newPubdash) { diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 75404678a13..83c6fca3762 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -21,11 +21,12 @@ import ( database "github.com/grafana/grafana/pkg/services/publicdashboards/database" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/tsdb/intervalv2" ) -var timeSettings, _ = simplejson.NewJson([]byte(`{"from": "now-12", "to": "now"}`)) +var timeSettings, _ = simplejson.NewJson([]byte(`{"from": "now-12h", "to": "now"}`)) var defaultPubdashTimeSettings, _ = simplejson.NewJson([]byte(`{}`)) -var dashboardData = simplejson.NewFromAny(map[string]interface{}{"time": map[string]interface{}{"from": "now-8", "to": "now"}}) +var dashboardData = simplejson.NewFromAny(map[string]interface{}{"time": map[string]interface{}{"from": "now-8h", "to": "now"}}) var SignedInUser = &user.SignedInUser{UserID: 1234, Login: "user@login.com"} func TestLogPrefix(t *testing.T) { @@ -360,8 +361,14 @@ func TestBuildPublicDashboardMetricRequest(t *testing.T) { nonPublicDashboard := insertTestDashboard(t, dashboardStore, "testNonPublicDashie", 1, 0, true, []map[string]interface{}{}) service := &PublicDashboardServiceImpl{ - log: log.New("test.logger"), - store: publicdashboardStore, + log: log.New("test.logger"), + store: publicdashboardStore, + intervalCalculator: intervalv2.NewCalculator(), + } + + publicDashboardQueryDTO := &PublicDashboardQueryDTO{ + IntervalMs: int64(10000000), + MaxDataPoints: int64(200), } dto := &SavePublicDashboardConfigDTO{ @@ -389,65 +396,69 @@ func TestBuildPublicDashboardMetricRequest(t *testing.T) { }, } - nonPublicDashboardPD, err := service.SavePublicDashboardConfig(context.Background(), SignedInUser, nonPublicDto) + _, err = service.SavePublicDashboardConfig(context.Background(), SignedInUser, nonPublicDto) require.NoError(t, err) t.Run("extracts queries from provided dashboard", func(t *testing.T) { - reqDTO, err := service.BuildPublicDashboardMetricRequest( + reqDTO, err := service.buildPublicDashboardMetricRequest( context.Background(), publicDashboard, publicDashboardPD, 1, + publicDashboardQueryDTO, ) require.NoError(t, err) require.Equal(t, timeSettings.Get("from").MustString(), reqDTO.From) require.Equal(t, timeSettings.Get("to").MustString(), reqDTO.To) + + for i := range reqDTO.Queries { + require.Equal(t, publicDashboardQueryDTO.IntervalMs, reqDTO.Queries[i].Get("intervalMs").MustInt64()) + require.Equal(t, publicDashboardQueryDTO.MaxDataPoints, reqDTO.Queries[i].Get("maxDataPoints").MustInt64()) + } + require.Len(t, reqDTO.Queries, 2) + require.Equal( t, - simplejson.MustJson([]byte(`{ - "datasource": { + simplejson.NewFromAny(map[string]interface{}{ + "datasource": map[string]interface{}{ "type": "mysql", - "uid": "ds1" + "uid": "ds1", }, - "refId": "A" - }`)), + "intervalMs": int64(10000000), + "maxDataPoints": int64(200), + "refId": "A", + }), reqDTO.Queries[0], ) + require.Equal( t, - simplejson.MustJson([]byte(`{ - "datasource": { + simplejson.NewFromAny(map[string]interface{}{ + "datasource": map[string]interface{}{ "type": "prometheus", - "uid": "ds2" + "uid": "ds2", }, - "refId": "B" - }`)), + "intervalMs": int64(10000000), + "maxDataPoints": int64(200), + "refId": "B", + }), reqDTO.Queries[1], ) }) t.Run("returns an error when panel missing", func(t *testing.T) { - _, err := service.BuildPublicDashboardMetricRequest( + _, err := service.buildPublicDashboardMetricRequest( context.Background(), publicDashboard, publicDashboardPD, 49, + publicDashboardQueryDTO, ) require.ErrorContains(t, err, "Panel not found") }) - - t.Run("returns an error when dashboard not public", func(t *testing.T) { - _, err := service.BuildPublicDashboardMetricRequest( - context.Background(), - nonPublicDashboard, - nonPublicDashboardPD, - 2, - ) - require.ErrorContains(t, err, "Public dashboard not found") - }) } func insertTestDashboard(t *testing.T, dashboardStore *dashboardsDB.DashboardStore, title string, orgId int64, @@ -513,6 +524,87 @@ func insertTestDashboard(t *testing.T, dashboardStore *dashboardsDB.DashboardSto return dash } +func TestPublicDashboardServiceImpl_getSafeIntervalAndMaxDataPoints(t *testing.T) { + type args struct { + reqDTO *PublicDashboardQueryDTO + ts *TimeSettings + } + tests := []struct { + name string + args args + wantSafeInterval int64 + wantSafeMaxDataPoints int64 + }{ + { + name: "return original interval", + args: args{ + reqDTO: &PublicDashboardQueryDTO{ + IntervalMs: 10000, + MaxDataPoints: 300, + }, + ts: &TimeSettings{ + From: "now-3h", + To: "now", + }, + }, + wantSafeInterval: 10000, + wantSafeMaxDataPoints: 300, + }, + { + name: "return safe interval because of a small interval", + args: args{ + reqDTO: &PublicDashboardQueryDTO{ + IntervalMs: 1000, + MaxDataPoints: 300, + }, + ts: &TimeSettings{ + From: "now-6h", + To: "now", + }, + }, + wantSafeInterval: 2000, + wantSafeMaxDataPoints: 11000, + }, + { + name: "return safe interval for long time range", + args: args{ + reqDTO: &PublicDashboardQueryDTO{ + IntervalMs: 100, + MaxDataPoints: 300, + }, + ts: &TimeSettings{ + From: "now-90d", + To: "now", + }, + }, + wantSafeInterval: 600000, + wantSafeMaxDataPoints: 11000, + }, + { + name: "return safe interval when reqDTO is empty", + args: args{ + reqDTO: &PublicDashboardQueryDTO{}, + ts: &TimeSettings{ + From: "now-90d", + To: "now", + }, + }, + wantSafeInterval: 600000, + wantSafeMaxDataPoints: 11000, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pd := &PublicDashboardServiceImpl{ + intervalCalculator: intervalv2.NewCalculator(), + } + got, got1 := pd.getSafeIntervalAndMaxDataPoints(tt.args.reqDTO, tt.args.ts) + assert.Equalf(t, tt.wantSafeInterval, got, "getSafeIntervalAndMaxDataPoints(%v, %v)", tt.args.reqDTO, tt.args.ts) + assert.Equalf(t, tt.wantSafeMaxDataPoints, got1, "getSafeIntervalAndMaxDataPoints(%v, %v)", tt.args.reqDTO, tt.args.ts) + }) + } +} + func TestDashboardEnabledChanged(t *testing.T) { t.Run("created isEnabled: false", func(t *testing.T) { assert.False(t, publicDashboardIsEnabledChanged(nil, &PublicDashboard{IsEnabled: false})) diff --git a/pkg/services/publicdashboards/validation/validator.go b/pkg/services/publicdashboards/validation/validator.go index 4f9773c365f..4d23c22c7d8 100644 --- a/pkg/services/publicdashboards/validation/validator.go +++ b/pkg/services/publicdashboards/validation/validator.go @@ -1,6 +1,8 @@ package validation import ( + "fmt" + "github.com/grafana/grafana/pkg/models" publicDashboardModels "github.com/grafana/grafana/pkg/services/publicdashboards/models" ) @@ -18,3 +20,15 @@ func hasTemplateVariables(dashboard *models.Dashboard) bool { return len(templateVariables) > 0 } + +func ValidateQueryPublicDashboardRequest(req *publicDashboardModels.PublicDashboardQueryDTO) error { + if req.IntervalMs < 0 { + return fmt.Errorf("intervalMS should be greater than 0") + } + + if req.MaxDataPoints < 0 { + return fmt.Errorf("maxDataPoints should be greater than 0") + } + + return nil +} diff --git a/pkg/tsdb/intervalv2/intervalv2.go b/pkg/tsdb/intervalv2/intervalv2.go index 77a66f591d8..43f8b103f49 100644 --- a/pkg/tsdb/intervalv2/intervalv2.go +++ b/pkg/tsdb/intervalv2/intervalv2.go @@ -213,7 +213,7 @@ func roundInterval(interval time.Duration) time.Duration { // 12.5m case interval <= 750000*time.Millisecond: return time.Millisecond * 600000 // 10m - // 12.5m + // 17.5m case interval <= 1050000*time.Millisecond: return time.Millisecond * 900000 // 15m // 25m diff --git a/public/app/features/dashboard/services/PublicDashboardDataSource.ts b/public/app/features/dashboard/services/PublicDashboardDataSource.ts index 1066a11023b..f116a76ff21 100644 --- a/public/app/features/dashboard/services/PublicDashboardDataSource.ts +++ b/public/app/features/dashboard/services/PublicDashboardDataSource.ts @@ -58,30 +58,15 @@ export class PublicDashboardDataSource extends DataSourceApi { * Ideally final -- any other implementation may not work as expected */ query(request: DataQueryRequest): Observable { - const { intervalMs, maxDataPoints, range, requestId, publicDashboardAccessToken, panelId } = request; - let targets = request.targets; - - const queries = targets.map((q) => { - return { - ...q, - publicDashboardAccessToken, - intervalMs, - maxDataPoints, - }; - }); + const { intervalMs, maxDataPoints, requestId, publicDashboardAccessToken, panelId } = request; + let queries: DataQuery[]; // Return early if no queries exist - if (!queries.length) { + if (!request.targets.length) { return of({ data: [] }); } - const body: any = { queries, publicDashboardAccessToken, panelId }; - - if (range) { - body.range = range; - body.from = range.from.valueOf().toString(); - body.to = range.to.valueOf().toString(); - } + const body: any = { intervalMs, maxDataPoints }; return getBackendSrv() .fetch({ @@ -92,7 +77,7 @@ export class PublicDashboardDataSource extends DataSourceApi { }) .pipe( switchMap((raw) => { - return of(toDataQueryResponse(raw, queries as DataQuery[])); + return of(toDataQueryResponse(raw, queries)); }), catchError((err) => { return of(toDataQueryResponse(err));