remove validatedQueries feature toggle (#48381)

* remove validatedQueries feature toggle
This commit is contained in:
Jeff Levin 2022-05-16 14:17:05 -07:00 committed by GitHub
parent 952cb4fc0b
commit 2691872c7a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 5 additions and 578 deletions

View File

@ -38,7 +38,6 @@ export interface FeatureToggles {
newNavigation?: boolean;
showFeatureFlagsInUI?: boolean;
disable_http_request_histogram?: boolean;
validatedQueries?: boolean;
publicDashboards?: boolean;
lokiLive?: boolean;
swaggerUi?: boolean;

View File

@ -434,9 +434,6 @@ func (hs *HTTPServer) registerRoutes() {
// DataSource w/ expressions
apiRoute.Post("/ds/query", authorize(reqSignedIn, ac.EvalPermission(datasources.ActionQuery)), routing.Wrap(hs.QueryMetricsV2))
// Validated query
apiRoute.Post("/dashboards/org/:orgId/uid/:dashboardUid/panels/:panelId/query", authorize(reqSignedIn, ac.EvalPermission(datasources.ActionQuery)), routing.Wrap(hs.QueryMetricsFromDashboard))
apiRoute.Group("/alerts", func(alertsRoute routing.RouteRegister) {
alertsRoute.Post("/test", routing.Wrap(hs.AlertTest))
alertsRoute.Post("/:alertId/pause", reqEditorRole, routing.Wrap(hs.PauseAlert))

View File

@ -38,7 +38,6 @@ import (
"github.com/grafana/grafana/pkg/services/searchusers"
"github.com/grafana/grafana/pkg/services/searchusers/filters"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana/pkg/web/webtest"
@ -330,15 +329,6 @@ func setupHTTPServer(t *testing.T, useFakeAccessControl bool, enableAccessContro
return setupHTTPServerWithCfg(t, useFakeAccessControl, enableAccessControl, setting.NewCfg())
}
func setupHTTPServerWithMockDb(t *testing.T, useFakeAccessControl bool, enableAccessControl bool) accessControlScenarioContext {
// Use a new conf
cfg := setting.NewCfg()
db := sqlstore.InitTestDB(t)
db.Cfg = setting.NewCfg()
return setupHTTPServerWithCfgDb(t, useFakeAccessControl, enableAccessControl, cfg, db, mockstore.NewSQLStoreMock())
}
func setupHTTPServerWithCfg(t *testing.T, useFakeAccessControl, enableAccessControl bool, cfg *setting.Cfg) accessControlScenarioContext {
db := sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{})
return setupHTTPServerWithCfgDb(t, useFakeAccessControl, enableAccessControl, cfg, db, db)

View File

@ -1,10 +1,8 @@
package api
import (
"context"
"errors"
"net/http"
"strconv"
"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana/pkg/api/dtos"
@ -13,27 +11,11 @@ import (
"github.com/grafana/grafana/pkg/plugins/backendplugin"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/query"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/tsdb/legacydata"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
// QueryMetricsV2 returns query metrics.
// POST /api/ds/query DataSource query w/ expressions
func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext) response.Response {
reqDTO := dtos.MetricRequest{}
if err := web.Bind(c.Req, &reqDTO); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
resp, err := hs.queryDataService.QueryData(c.Req.Context(), c.SignedInUser, c.SkipCache, reqDTO, true)
if err != nil {
return hs.handleQueryMetricsError(err)
}
return hs.toJsonStreamingResponse(resp)
}
func (hs *HTTPServer) handleQueryMetricsError(err error) *response.NormalResponse {
if errors.Is(err, models.ErrDataSourceAccessDenied) {
return response.Error(http.StatusForbidden, "Access denied to data source", err)
@ -53,96 +35,14 @@ func (hs *HTTPServer) handleQueryMetricsError(err error) *response.NormalRespons
return response.Error(http.StatusInternalServerError, "Query data error", err)
}
func parseDashboardQueryParams(params map[string]string) (models.GetDashboardQuery, int64, error) {
query := models.GetDashboardQuery{}
if params[":orgId"] == "" || params[":dashboardUid"] == "" || params[":panelId"] == "" {
return query, 0, models.ErrDashboardOrPanelIdentifierNotSet
}
orgId, err := strconv.ParseInt(params[":orgId"], 10, 64)
if err != nil {
return query, 0, models.ErrDashboardPanelIdentifierInvalid
}
panelId, err := strconv.ParseInt(params[":panelId"], 10, 64)
if err != nil {
return query, 0, models.ErrDashboardPanelIdentifierInvalid
}
query.Uid = params[":dashboardUid"]
query.OrgId = orgId
return query, panelId, nil
}
func checkDashboardAndPanel(ctx context.Context, ss sqlstore.Store, query models.GetDashboardQuery, panelId int64) error {
// Query the dashboard
if err := ss.GetDashboard(ctx, &query); err != nil {
return err
}
if query.Result == nil {
return models.ErrDashboardCorrupt
}
// dashboard saved but no panels
dashboard := query.Result
if dashboard.Data == nil {
return models.ErrDashboardCorrupt
}
// FIXME: parse the dashboard JSON in a more performant/structured way.
panels := dashboard.Data.Get("panels")
for i := 0; ; i++ {
panel, ok := panels.CheckGetIndex(i)
if !ok {
break
}
if panel.Get("id").MustInt64(-1) == panelId {
return nil
}
}
// no panel with that ID
return models.ErrDashboardPanelNotFound
}
// QueryMetricsFromDashboard returns query metrics.
// POST /dashboards/org/:orgId/uid/:dashboardUid/panels/:panelId/query DataSource query w/ expressions
func (hs *HTTPServer) QueryMetricsFromDashboard(c *models.ReqContext) response.Response {
// check feature flag
if !hs.Features.IsEnabled(featuremgmt.FlagValidatedQueries) {
return response.Respond(http.StatusNotFound, "404 page not found\n")
}
// build query
// QueryMetricsV2 returns query metrics.
// POST /api/ds/query DataSource query w/ expressions
func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext) response.Response {
reqDTO := dtos.MetricRequest{}
if err := web.Bind(c.Req, &reqDTO); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
params := web.Params(c.Req)
getDashboardQuery, panelId, err := parseDashboardQueryParams(params)
// check dashboard: inside the statement is the happy path. we should maybe
// refactor this as it's not super obvious
if err == nil {
err = checkDashboardAndPanel(c.Req.Context(), hs.SQLStore, getDashboardQuery, panelId)
}
// 404 if dashboard or panel not found
if err != nil {
c.Logger.Warn("Failed to find dashboard or panel for validated query", "err", err)
var dashboardErr models.DashboardErr
if ok := errors.As(err, &dashboardErr); ok {
return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), err)
}
return response.Error(http.StatusNotFound, "Dashboard or panel not found", err)
}
// return panel data
resp, err := hs.queryDataService.QueryData(c.Req.Context(), c.SignedInUser, c.SkipCache, reqDTO, true)
if err != nil {
return hs.handleQueryMetricsError(err)

View File

@ -2,37 +2,25 @@ package api
import (
"context"
"encoding/json"
"fmt"
"net/http"
"strconv"
"strings"
"testing"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/web/webtest"
"golang.org/x/oauth2"
"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes"
datasources "github.com/grafana/grafana/pkg/services/datasources/service"
"github.com/grafana/grafana/pkg/services/query"
"github.com/grafana/grafana/pkg/services/secrets/fakes"
"github.com/grafana/grafana/pkg/services/secrets/kvstore"
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
var (
queryDatasourceInput = `{
"from": "",
var queryDatasourceInput = `{
"from": "",
"to": "",
"queries": [
{
@ -46,123 +34,6 @@ var (
]
}`
getDashboardByIdOutput = `{
"annotations": {
"list": [
{
"builtIn": 1,
"datasource": "-- Grafana --",
"enable": true,
"hide": true,
"iconColor": "rgba(0, 211, 255, 1)",
"name": "Annotations & Alerts",
"target": {
"limit": 100,
"matchAny": false,
"tags": [],
"type": "dashboard"
},
"type": "dashboard"
}
]
},
"editable": true,
"fiscalYearStartMonth": 0,
"graphTooltip": 0,
"links": [],
"liveNow": false,
"panels": [
{
"fieldConfig": {
"defaults": {
"color": {
"mode": "palette-classic"
},
"custom": {
"axisLabel": "",
"axisPlacement": "auto",
"barAlignment": 0,
"drawStyle": "line",
"fillOpacity": 0,
"gradientMode": "none",
"hideFrom": {
"legend": false,
"tooltip": false,
"viz": false
},
"lineInterpolation": "linear",
"lineWidth": 1,
"pointSize": 5,
"scaleDistribution": {
"type": "linear"
},
"showPoints": "auto",
"spanNulls": false,
"stacking": {
"group": "A",
"mode": "none"
},
"thresholdsStyle": {
"mode": "off"
}
},
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green",
"value": null
},
{
"color": "red",
"value": 80
}
]
}
},
"overrides": []
},
"gridPos": {
"h": 9,
"w": 12,
"x": 0,
"y": 0
},
"id": 2,
"options": {
"legend": {
"calcs": [],
"displayMode": "list",
"placement": "bottom"
},
"tooltip": {
"mode": "single",
"sort": "none"
}
},
"title": "Panel Title",
"type": "timeseries"
}
],
"schemaVersion": 35,
"style": "dark",
"tags": [],
"templating": {
"list": []
},
"time": {
"from": "now-6h",
"to": "now"
},
"timepicker": {},
"timezone": "",
"title": "New dashboard",
"version": 0,
"weekStart": ""
}`
)
type fakePluginRequestValidator struct {
err error
}
@ -184,326 +55,6 @@ func (ts *fakeOAuthTokenService) IsOAuthPassThruEnabled(*models.DataSource) bool
return ts.passThruEnabled
}
func (c *dashboardFakePluginClient) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
c.req = req
resp := backend.Responses{}
return &backend.QueryDataResponse{Responses: resp}, nil
}
type dashboardFakePluginClient struct {
plugins.Client
req *backend.QueryDataRequest
}
// `/dashboards/org/:orgId/uid/:dashboardUid/panels/:panelId/query` endpoints test
func TestAPIEndpoint_Metrics_QueryMetricsFromDashboard(t *testing.T) {
sc := setupHTTPServerWithMockDb(t, false, false)
secretsStore := kvstore.SetupTestService(t)
secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
ds := datasources.ProvideService(nil, secretsService, secretsStore, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService())
setInitCtxSignedInViewer(sc.initCtx)
sc.hs.queryDataService = query.ProvideService(
nil,
nil,
nil,
&fakePluginRequestValidator{},
ds,
&dashboardFakePluginClient{},
&fakeOAuthTokenService{},
)
sc.hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagValidatedQueries, true)
dashboardJson, err := simplejson.NewFromReader(strings.NewReader(getDashboardByIdOutput))
if err != nil {
t.Fatalf("Failed to unmarshal dashboard json: %v", err)
}
mockDb := sc.hs.SQLStore.(*mockstore.SQLStoreMock)
t.Run("Can query a valid dashboard", func(t *testing.T) {
mockDb.ExpectedDashboard = &models.Dashboard{
Uid: "1",
OrgId: testOrgID,
Data: dashboardJson,
}
mockDb.ExpectedError = nil
response := callAPI(
sc.server,
http.MethodPost,
fmt.Sprintf("/api/dashboards/org/%d/uid/%s/panels/%s/query", testOrgID, "1", "2"),
strings.NewReader(queryDatasourceInput),
t,
)
assert.Equal(t, http.StatusOK, response.Code)
})
t.Run("Cannot query without a valid orgid or dashboard or panel ID", func(t *testing.T) {
mockDb.ExpectedDashboard = nil
mockDb.ExpectedError = models.ErrDashboardOrPanelIdentifierNotSet
response := callAPI(
sc.server,
http.MethodPost,
"/api/dashboards/org//uid//panels//query",
strings.NewReader(queryDatasourceInput),
t,
)
assert.Equal(t, http.StatusBadRequest, response.Code)
var res map[string]interface{}
err := json.Unmarshal(response.Body.Bytes(), &res)
assert.NoError(t, err)
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["error"])
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["message"])
})
t.Run("Cannot query without a valid orgid", func(t *testing.T) {
response := callAPI(
sc.server,
http.MethodPost,
fmt.Sprintf("/api/dashboards/org//uid/%s/panels/%s/query", "1", "2"),
strings.NewReader(queryDatasourceInput),
t,
)
assert.Equal(t, http.StatusBadRequest, response.Code)
var res map[string]interface{}
assert.NoError(t, json.Unmarshal(response.Body.Bytes(), &res))
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["error"])
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["message"])
})
t.Run("Cannot query without a valid dashboard or panel ID", func(t *testing.T) {
response := callAPI(
sc.server,
http.MethodPost,
fmt.Sprintf("/api/dashboards/org//uid/%s/panels/%s/query", "1", "2"),
strings.NewReader(queryDatasourceInput),
t,
)
assert.Equal(t, http.StatusBadRequest, response.Code)
var res map[string]interface{}
assert.NoError(t, json.Unmarshal(response.Body.Bytes(), &res))
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["error"])
assert.Equal(t, models.ErrDashboardOrPanelIdentifierNotSet.Error(), res["message"])
})
t.Run("Cannot query when ValidatedQueries is disabled", func(t *testing.T) {
sc.hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagValidatedQueries, false)
response := callAPI(
sc.server,
http.MethodPost,
"/api/dashboards/uid/1/panels/1/query",
strings.NewReader(queryDatasourceInput),
t,
)
assert.Equal(t, http.StatusNotFound, response.Code)
assert.Equal(
t,
"404 page not found\n",
response.Body.String(),
)
})
}
func TestAPIEndpoint_Metrics_checkDashboardAndPanel(t *testing.T) {
dashboardJson, err := simplejson.NewFromReader(strings.NewReader(getDashboardByIdOutput))
if err != nil {
t.Fatalf("Failed to unmarshal dashboard json: %v", err)
}
tests := []struct {
name string
orgId int64
dashboardUid string
panelId int64
dashboardQueryResult *models.Dashboard
expectedError error
}{
{
name: "Work when correct dashboardId and panelId given",
orgId: testOrgID,
dashboardUid: "1",
panelId: 2,
dashboardQueryResult: &models.Dashboard{
Uid: "1",
OrgId: testOrgID,
Data: dashboardJson,
},
expectedError: nil,
},
{
name: "404 on invalid orgId",
orgId: 7,
dashboardUid: "1",
panelId: 2,
dashboardQueryResult: nil,
expectedError: models.ErrDashboardNotFound,
},
{
name: "404 on invalid dashboardId",
orgId: testOrgID,
dashboardUid: "",
panelId: 2,
dashboardQueryResult: nil,
expectedError: models.ErrDashboardNotFound,
},
{
name: "404 on invalid panelId",
orgId: testOrgID,
dashboardUid: "1",
panelId: 0,
dashboardQueryResult: nil,
expectedError: models.ErrDashboardNotFound,
},
{
name: "Fails when the dashboard does not exist",
orgId: testOrgID,
dashboardUid: "1",
panelId: 2,
dashboardQueryResult: nil,
expectedError: models.ErrDashboardNotFound,
},
{
name: "Fails when the panel does not exist",
orgId: testOrgID,
dashboardUid: "1",
panelId: 3,
dashboardQueryResult: &models.Dashboard{
Id: 1,
OrgId: testOrgID,
Data: dashboardJson,
},
expectedError: models.ErrDashboardPanelNotFound,
},
{
name: "Fails when the dashboard contents are nil",
orgId: testOrgID,
dashboardUid: "1",
panelId: 3,
dashboardQueryResult: &models.Dashboard{
Uid: "1",
OrgId: testOrgID,
Data: nil,
},
expectedError: models.ErrDashboardCorrupt,
},
}
ss := mockstore.NewSQLStoreMock()
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ss.ExpectedDashboard = test.dashboardQueryResult
ss.ExpectedError = test.expectedError
query := models.GetDashboardQuery{
OrgId: test.orgId,
Uid: test.dashboardUid,
}
assert.Equal(t, test.expectedError, checkDashboardAndPanel(context.Background(), ss, query, test.panelId))
})
}
}
func TestAPIEndpoint_Metrics_ParseDashboardQueryParams(t *testing.T) {
tests := []struct {
name string
params map[string]string
expectedDashboardQuery models.GetDashboardQuery
expectedPanelId int64
expectedError error
}{
{
name: "Work when correct orgId, dashboardId and panelId given",
params: map[string]string{
":orgId": strconv.FormatInt(testOrgID, 10),
":dashboardUid": "1",
":panelId": "2",
},
expectedDashboardQuery: models.GetDashboardQuery{
Uid: "1",
OrgId: 1,
},
expectedPanelId: 2,
expectedError: nil,
},
{
name: "Get error when dashboardUid not given",
params: map[string]string{
":orgId": strconv.FormatInt(testOrgID, 10),
":dashboardUid": "",
":panelId": "1",
},
expectedDashboardQuery: models.GetDashboardQuery{},
expectedPanelId: 0,
expectedError: models.ErrDashboardOrPanelIdentifierNotSet,
},
{
name: "Get error when panelId not given",
params: map[string]string{
":orgId": strconv.FormatInt(testOrgID, 10),
":dashboardUid": "1",
":panelId": "",
},
expectedDashboardQuery: models.GetDashboardQuery{},
expectedPanelId: 0,
expectedError: models.ErrDashboardOrPanelIdentifierNotSet,
},
{
name: "Get error when orgId not given",
params: map[string]string{
":orgId": "",
":dashboardUid": "1",
":panelId": "2",
},
expectedDashboardQuery: models.GetDashboardQuery{},
expectedPanelId: 0,
expectedError: models.ErrDashboardOrPanelIdentifierNotSet,
},
{
name: "Get error when panelId not is invalid",
params: map[string]string{
":orgId": strconv.FormatInt(testOrgID, 10),
":dashboardUid": "1",
":panelId": "aaa",
},
expectedDashboardQuery: models.GetDashboardQuery{},
expectedPanelId: 0,
expectedError: models.ErrDashboardPanelIdentifierInvalid,
},
{
name: "Get error when orgId not is invalid",
params: map[string]string{
":orgId": "aaa",
":dashboardUid": "1",
":panelId": "2",
},
expectedDashboardQuery: models.GetDashboardQuery{},
expectedPanelId: 0,
expectedError: models.ErrDashboardPanelIdentifierInvalid,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// other validations?
dashboardQuery, panelId, err := parseDashboardQueryParams(test.params)
assert.Equal(t, test.expectedDashboardQuery, dashboardQuery)
assert.Equal(t, test.expectedPanelId, panelId)
assert.Equal(t, test.expectedError, err)
})
}
}
// `/ds/query` endpoint test
func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) {
qds := query.ProvideService(

View File

@ -127,12 +127,6 @@ var (
Description: "Do not create histograms for http requests",
State: FeatureStateAlpha,
},
{
Name: "validatedQueries",
Description: "only execute the query saved in a panel",
State: FeatureStateAlpha,
RequiresDevMode: true,
},
{
Name: "publicDashboards",
Description: "enables public access to dashboards",

View File

@ -95,10 +95,6 @@ const (
// Do not create histograms for http requests
FlagDisableHttpRequestHistogram = "disable_http_request_histogram"
// FlagValidatedQueries
// only execute the query saved in a panel
FlagValidatedQueries = "validatedQueries"
// FlagPublicDashboards
// enables public access to dashboards
FlagPublicDashboards = "publicDashboards"