From 8f0a9729f0d79b811014be369970ade1245b50c8 Mon Sep 17 00:00:00 2001 From: Ezequiel Victorero Date: Thu, 16 Mar 2023 15:39:17 -0300 Subject: [PATCH] PublicDashboards: Allow hidden queries execution (#64858) --- pkg/api/http_server.go | 4 +- pkg/server/wire.go | 1 + pkg/services/live/live.go | 4 +- .../publicdashboards/api/common_test.go | 2 +- .../publicdashboards/service/query.go | 20 +++-- .../publicdashboards/service/query_test.go | 76 ++++--------------- .../publicdashboards/service/service.go | 4 +- pkg/services/query/query.go | 33 +++++--- pkg/services/query/query_service_mock.go | 72 ++++++++++++++++++ pkg/services/query/query_test.go | 2 +- 10 files changed, 126 insertions(+), 92 deletions(-) create mode 100644 pkg/services/query/query_service_mock.go diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 5503cce9e60..ebbf0522fc4 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -170,7 +170,7 @@ type HTTPServer struct { pluginsUpdateChecker *updatechecker.PluginsService searchUsersService searchusers.Service teamGuardian teamguardian.TeamGuardian - queryDataService *query.Service + queryDataService query.Service serviceAccountsService serviceaccounts.Service authInfoService login.AuthInfoService authenticator loginpkg.Authenticator @@ -233,7 +233,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi quotaService quota.Service, socialService social.Service, tracer tracing.Tracer, encryptionService encryption.Internal, grafanaUpdateChecker *updatechecker.GrafanaService, pluginsUpdateChecker *updatechecker.PluginsService, searchUsersService searchusers.Service, - dataSourcesService datasources.DataSourceService, queryDataService *query.Service, + dataSourcesService datasources.DataSourceService, queryDataService query.Service, teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, authInfoService login.AuthInfoService, storageService store.StorageService, httpEntityStore httpentitystore.HTTPEntityStore, notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService, diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 9668f60cd50..a0bbacb7440 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -172,6 +172,7 @@ var wireBasicSet = wire.NewSet( New, api.ProvideHTTPServer, query.ProvideService, + wire.Bind(new(query.Service), new(*query.ServiceImpl)), bus.ProvideBus, wire.Bind(new(bus.Bus), new(*bus.InProcBus)), thumbs.ProvideService, diff --git a/pkg/services/live/live.go b/pkg/services/live/live.go index 9650e6cf6b8..bf127195f31 100644 --- a/pkg/services/live/live.go +++ b/pkg/services/live/live.go @@ -75,7 +75,7 @@ type CoreGrafanaScope struct { func ProvideService(plugCtxProvider *plugincontext.Provider, cfg *setting.Cfg, routeRegister routing.RouteRegister, pluginStore plugins.Store, pluginClient plugins.Client, cacheService *localcache.CacheService, dataSourceCache datasources.CacheService, sqlStore db.DB, secretsService secrets.Service, - usageStatsService usagestats.Service, queryDataService *query.Service, toggles featuremgmt.FeatureToggles, + usageStatsService usagestats.Service, queryDataService query.Service, toggles featuremgmt.FeatureToggles, accessControl accesscontrol.AccessControl, dashboardService dashboards.DashboardService, annotationsRepo annotations.Repository, orgService org.Service) (*GrafanaLive, error) { g := &GrafanaLive{ @@ -408,7 +408,7 @@ type GrafanaLive struct { SecretsService secrets.Service pluginStore plugins.Store pluginClient plugins.Client - queryDataService *query.Service + queryDataService query.Service orgService org.Service node *centrifuge.Node diff --git a/pkg/services/publicdashboards/api/common_test.go b/pkg/services/publicdashboards/api/common_test.go index 47a96d39363..511342475c5 100644 --- a/pkg/services/publicdashboards/api/common_test.go +++ b/pkg/services/publicdashboards/api/common_test.go @@ -104,7 +104,7 @@ func callAPI(server *web.Mux, method, path string, body io.Reader, t *testing.T) // helper to query.Service // allows us to stub the cache and plugin clients -func buildQueryDataService(t *testing.T, cs datasources.CacheService, fpc *fakePluginClient, store db.DB) *query.Service { +func buildQueryDataService(t *testing.T, cs datasources.CacheService, fpc *fakePluginClient, store db.DB) *query.ServiceImpl { // build database if we need one if store == nil { store = db.InitTestDB(t) diff --git a/pkg/services/publicdashboards/service/query.go b/pkg/services/publicdashboards/service/query.go index 3445061cd39..5bf29ae6a17 100644 --- a/pkg/services/publicdashboards/service/query.go +++ b/pkg/services/publicdashboards/service/query.go @@ -260,18 +260,16 @@ func extractQueriesFromPanels(panels []interface{}, result map[int64][]*simplejs for _, queryObj := range panel.Get("targets").MustArray() { query := simplejson.NewFromAny(queryObj) - if hideAttr, exists := query.CheckGet("hide"); !exists || !hideAttr.MustBool() { - // We dont support exemplars for public dashboards currently - query.Del("exemplar") - - // if query target has no datasource, set it to have the datasource on the panel - if _, ok := query.CheckGet("datasource"); !ok { - uid := getDataSourceUidFromJson(panel) - datasource := map[string]interface{}{"type": "public-ds", "uid": uid} - query.Set("datasource", datasource) - } - panelQueries = append(panelQueries, query) + // We dont support exemplars for public dashboards currently + query.Del("exemplar") + + // if query target has no datasource, set it to have the datasource on the panel + if _, ok := query.CheckGet("datasource"); !ok { + uid := getDataSourceUidFromJson(panel) + datasource := map[string]interface{}{"type": "public-ds", "uid": uid} + query.Set("datasource", datasource) } + panelQueries = append(panelQueries, query) } result[panel.Get("id").MustInt64()] = panelQueries diff --git a/pkg/services/publicdashboards/service/query_test.go b/pkg/services/publicdashboards/service/query_test.go index d0d68a520c7..bf26a3c80cb 100644 --- a/pkg/services/publicdashboards/service/query_test.go +++ b/pkg/services/publicdashboards/service/query_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/publicdashboards/database" "github.com/grafana/grafana/pkg/services/publicdashboards/internal" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" + "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/tag/tagimpl" @@ -439,10 +440,14 @@ func TestGetQueryDataResponse(t *testing.T) { require.NoError(t, err) publicdashboardStore := database.ProvideStore(sqlStore) + fakeQueryService := &query.FakeQueryService{} + fakeQueryService.On("QueryData", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&backend.QueryDataResponse{}, nil) + service := &PublicDashboardServiceImpl{ log: log.New("test.logger"), store: publicdashboardStore, intervalCalculator: intervalv2.NewCalculator(), + QueryDataService: fakeQueryService, } publicDashboardQueryDTO := PublicDashboardQueryDTO{ @@ -450,7 +455,7 @@ func TestGetQueryDataResponse(t *testing.T) { MaxDataPoints: int64(1), } - t.Run("Returns nil when query is hidden", func(t *testing.T) { + t.Run("Returns query data even when the query is hidden", func(t *testing.T) { hiddenQuery := map[string]interface{}{ "datasource": map[string]interface{}{ "type": "mysql", @@ -484,7 +489,7 @@ func TestGetQueryDataResponse(t *testing.T) { require.NoError(t, err) resp, _ := service.GetQueryDataResponse(context.Background(), true, publicDashboardQueryDTO, 1, pubdashDto.AccessToken) - require.Nil(t, resp) + require.NotNil(t, resp) }) } @@ -1003,7 +1008,7 @@ func TestBuildMetricRequest(t *testing.T) { require.ErrorContains(t, err, ErrPanelNotFound.Error()) }) - t.Run("metric request built without hidden query", func(t *testing.T) { + t.Run("metric request built with hidden query", func(t *testing.T) { hiddenQuery := map[string]interface{}{ "datasource": map[string]interface{}{ "type": "mysql", @@ -1048,9 +1053,9 @@ func TestBuildMetricRequest(t *testing.T) { require.Equal(t, publicDashboardQueryDTO.MaxDataPoints, reqDTO.Queries[i].Get("maxDataPoints").MustInt64()) } - require.Len(t, reqDTO.Queries, 1) + require.Len(t, reqDTO.Queries, 2) - require.NotEqual( + require.Equal( t, simplejson.NewFromAny(hiddenQuery), reqDTO.Queries[0], @@ -1059,49 +1064,8 @@ func TestBuildMetricRequest(t *testing.T) { require.Equal( t, simplejson.NewFromAny(nonHiddenQuery), - reqDTO.Queries[0], - ) - }) - - t.Run("metric request built with 0 queries len when all queries are hidden", func(t *testing.T) { - customPanels := []interface{}{ - map[string]interface{}{ - "id": 1, - "datasource": map[string]interface{}{ - "uid": "ds1", - }, - "targets": []interface{}{map[string]interface{}{ - "datasource": map[string]interface{}{ - "type": "mysql", - "uid": "ds1", - }, - "hide": true, - "refId": "A", - }, map[string]interface{}{ - "datasource": map[string]interface{}{ - "type": "prometheus", - "uid": "ds2", - }, - "hide": true, - "refId": "B", - }}, - }} - - publicDashboard := insertTestDashboard(t, dashboardStore, "testDashWithAllQueriesHidden", 1, 0, true, []map[string]interface{}{}, customPanels) - - reqDTO, err := service.buildMetricRequest( - context.Background(), - publicDashboard, - publicDashboardPD, - 1, - publicDashboardQueryDTO, + reqDTO.Queries[1], ) - require.NoError(t, err) - - require.Equal(t, from, reqDTO.From) - require.Equal(t, to, reqDTO.To) - - require.Len(t, reqDTO.Queries, 0) }) } @@ -1110,11 +1074,6 @@ func TestBuildAnonymousUser(t *testing.T) { dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, sqlStore.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotatest.New(false, nil)) require.NoError(t, err) dashboard := insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true, []map[string]interface{}{}, nil) - // publicdashboardStore := database.ProvideStore(sqlStore) - // service := &PublicDashboardServiceImpl{ - // log: log.New("test.logger"), - // store: publicdashboardStore, - // } t.Run("will add datasource read and query permissions to user for each datasource in dashboard", func(t *testing.T) { user := buildAnonymousUser(context.Background(), dashboard) @@ -1239,25 +1198,20 @@ func TestGroupQueriesByPanelId(t *testing.T) { }`, string(query)) }) - t.Run("hidden query filtered", func(t *testing.T) { + t.Run("hidden query not filtered", func(t *testing.T) { json, err := simplejson.NewJson([]byte(dashboardWithOneHiddenQuery)) require.NoError(t, err) queries := groupQueriesByPanelId(json)[2] - require.Len(t, queries, 1) - for _, query := range queries { - if hideAttr, exists := query.CheckGet("hide"); exists && hideAttr.MustBool() { - require.Fail(t, "hidden queries should have been filtered") - } - } + require.Len(t, queries, 2) }) - t.Run("hidden query filtered, so empty queries returned", func(t *testing.T) { + t.Run("hidden queries not filtered, so queries returned", func(t *testing.T) { json, err := simplejson.NewJson([]byte(dashboardWithAllHiddenQueries)) require.NoError(t, err) queries := groupQueriesByPanelId(json)[2] - require.Len(t, queries, 0) + require.Len(t, queries, 2) }) t.Run("queries inside panels inside rows are returned", func(t *testing.T) { diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 601d33f9efc..ee624c73a26 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -30,7 +30,7 @@ type PublicDashboardServiceImpl struct { cfg *setting.Cfg store publicdashboards.Store intervalCalculator intervalv2.Calculator - QueryDataService *query.Service + QueryDataService query.Service AnnotationsRepo annotations.Repository ac accesscontrol.AccessControl serviceWrapper publicdashboards.ServiceWrapper @@ -47,7 +47,7 @@ var _ publicdashboards.Service = (*PublicDashboardServiceImpl)(nil) func ProvideService( cfg *setting.Cfg, store publicdashboards.Store, - qds *query.Service, + qds query.Service, anno annotations.Repository, ac accesscontrol.AccessControl, serviceWrapper publicdashboards.ServiceWrapper, diff --git a/pkg/services/query/query.go b/pkg/services/query/query.go index 426843d3ca2..4ea54261f4a 100644 --- a/pkg/services/query/query.go +++ b/pkg/services/query/query.go @@ -37,8 +37,8 @@ func ProvideService( pluginRequestValidator validations.PluginRequestValidator, dataSourceService datasources.DataSourceService, pluginClient plugins.Client, -) *Service { - g := &Service{ +) *ServiceImpl { + g := &ServiceImpl{ cfg: cfg, dataSourceCache: dataSourceCache, expressionService: expressionService, @@ -51,7 +51,16 @@ func ProvideService( return g } -type Service struct { +//go:generate mockery --name Service --structname FakeQueryService --inpackage --filename query_service_mock.go +type Service interface { + Run(ctx context.Context) error + QueryData(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*backend.QueryDataResponse, error) +} + +// Gives us compile time error if the service does not adhere to the contract of the interface +var _ Service = (*ServiceImpl)(nil) + +type ServiceImpl struct { cfg *setting.Cfg dataSourceCache datasources.CacheService expressionService *expr.Service @@ -61,14 +70,14 @@ type Service struct { log log.Logger } -// Run Service. -func (s *Service) Run(ctx context.Context) error { +// Run ServiceImpl. +func (s *ServiceImpl) Run(ctx context.Context) error { <-ctx.Done() return ctx.Err() } // QueryData processes queries and returns query responses. It handles queries to single or mixed datasources, as well as expressions. -func (s *Service) QueryData(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*backend.QueryDataResponse, error) { +func (s *ServiceImpl) QueryData(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*backend.QueryDataResponse, error) { // Parse the request into parsed queries grouped by datasource uid parsedReq, err := s.parseMetricRequest(ctx, user, skipCache, reqDTO) if err != nil { @@ -88,7 +97,7 @@ func (s *Service) QueryData(ctx context.Context, user *user.SignedInUser, skipCa } // executeConcurrentQueries executes queries to multiple datasources concurrently and returns the aggregate result. -func (s *Service) executeConcurrentQueries(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest, queriesbyDs map[string][]parsedQuery) (*backend.QueryDataResponse, error) { +func (s *ServiceImpl) executeConcurrentQueries(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest, queriesbyDs map[string][]parsedQuery) (*backend.QueryDataResponse, error) { g, ctx := errgroup.WithContext(ctx) g.SetLimit(8) // arbitrary limit to prevent too many concurrent requests rchan := make(chan backend.Responses, len(queriesbyDs)) @@ -158,7 +167,7 @@ func buildErrorResponses(err error, queries []*simplejson.Json) backend.Response } // handleExpressions handles POST /api/ds/query when there is an expression. -func (s *Service) handleExpressions(ctx context.Context, user *user.SignedInUser, parsedReq *parsedRequest) (*backend.QueryDataResponse, error) { +func (s *ServiceImpl) handleExpressions(ctx context.Context, user *user.SignedInUser, parsedReq *parsedRequest) (*backend.QueryDataResponse, error) { exprReq := expr.Request{ Queries: []expr.Query{}, } @@ -199,7 +208,7 @@ func (s *Service) handleExpressions(ctx context.Context, user *user.SignedInUser } // handleQuerySingleDatasource handles one or more queries to a single datasource -func (s *Service) handleQuerySingleDatasource(ctx context.Context, user *user.SignedInUser, parsedReq *parsedRequest) (*backend.QueryDataResponse, error) { +func (s *ServiceImpl) handleQuerySingleDatasource(ctx context.Context, user *user.SignedInUser, parsedReq *parsedRequest) (*backend.QueryDataResponse, error) { queries := parsedReq.getFlattenedQueries() ds := queries[0].datasource if err := s.pluginRequestValidator.Validate(ds.URL, nil); err != nil { @@ -237,7 +246,7 @@ func (s *Service) handleQuerySingleDatasource(ctx context.Context, user *user.Si } // parseRequest parses a request into parsed queries grouped by datasource uid -func (s *Service) parseMetricRequest(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*parsedRequest, error) { +func (s *ServiceImpl) parseMetricRequest(ctx context.Context, user *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*parsedRequest, error) { if len(reqDTO.Queries) == 0 { return nil, ErrNoQueriesFound } @@ -298,7 +307,7 @@ func (s *Service) parseMetricRequest(ctx context.Context, user *user.SignedInUse return req, req.validateRequest(ctx) } -func (s *Service) getDataSourceFromQuery(ctx context.Context, user *user.SignedInUser, skipCache bool, query *simplejson.Json, history map[string]*datasources.DataSource) (*datasources.DataSource, error) { +func (s *ServiceImpl) getDataSourceFromQuery(ctx context.Context, user *user.SignedInUser, skipCache bool, query *simplejson.Json, history map[string]*datasources.DataSource) (*datasources.DataSource, error) { var err error uid := query.Get("datasource").Get("uid").MustString() @@ -342,7 +351,7 @@ func (s *Service) getDataSourceFromQuery(ctx context.Context, user *user.SignedI return nil, ErrInvalidDatasourceID } -func (s *Service) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { +func (s *ServiceImpl) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { return func(ds *datasources.DataSource) (map[string]string, error) { return s.dataSourceService.DecryptedValues(ctx, ds) } diff --git a/pkg/services/query/query_service_mock.go b/pkg/services/query/query_service_mock.go new file mode 100644 index 00000000000..49f4ff03a4a --- /dev/null +++ b/pkg/services/query/query_service_mock.go @@ -0,0 +1,72 @@ +// Code generated by mockery v2.14.0. DO NOT EDIT. + +package query + +import ( + context "context" + + backend "github.com/grafana/grafana-plugin-sdk-go/backend" + + dtos "github.com/grafana/grafana/pkg/api/dtos" + + mock "github.com/stretchr/testify/mock" + + user "github.com/grafana/grafana/pkg/services/user" +) + +// FakeQueryService is an autogenerated mock type for the Service type +type FakeQueryService struct { + mock.Mock +} + +// QueryData provides a mock function with given fields: ctx, _a1, skipCache, reqDTO +func (_m *FakeQueryService) QueryData(ctx context.Context, _a1 *user.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*backend.QueryDataResponse, error) { + ret := _m.Called(ctx, _a1, skipCache, reqDTO) + + var r0 *backend.QueryDataResponse + if rf, ok := ret.Get(0).(func(context.Context, *user.SignedInUser, bool, dtos.MetricRequest) *backend.QueryDataResponse); ok { + r0 = rf(ctx, _a1, skipCache, reqDTO) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*backend.QueryDataResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *user.SignedInUser, bool, dtos.MetricRequest) error); ok { + r1 = rf(ctx, _a1, skipCache, reqDTO) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Run provides a mock function with given fields: ctx +func (_m *FakeQueryService) Run(ctx context.Context) error { + ret := _m.Called(ctx) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context) error); ok { + r0 = rf(ctx) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type mockConstructorTestingTNewFakeQueryService interface { + mock.TestingT + Cleanup(func()) +} + +// NewFakeQueryService creates a new instance of FakeQueryService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewFakeQueryService(t mockConstructorTestingTNewFakeQueryService) *FakeQueryService { + mock := &FakeQueryService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/services/query/query_test.go b/pkg/services/query/query_test.go index c00d3e024fb..524342443e1 100644 --- a/pkg/services/query/query_test.go +++ b/pkg/services/query/query_test.go @@ -463,7 +463,7 @@ type testContext struct { secretStore secretskvs.SecretsKVStore dataSourceCache *fakeDataSourceCache pluginRequestValidator *fakePluginRequestValidator - queryService *Service // implementation belonging to this package + queryService *ServiceImpl // implementation belonging to this package signedInUser *user.SignedInUser }