From dd4ffc991878e7b10e8de878dfd1ae5995c02884 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Wed, 27 Aug 2025 12:08:25 -0400 Subject: [PATCH] SQL Expressions: Add setting to limit length of query (#110165) sql_expression_query_length_limit Set the maximum length of a SQL query that can be used in a SQL expression. Default is 10000 characters. A setting of 0 means no limit. --- .../setup-grafana/configure-grafana/_index.md | 10 ++++++--- pkg/expr/graph.go | 22 +++++++++++++++++++ pkg/expr/graph_test.go | 3 ++- pkg/expr/service_sql_test.go | 11 ++++++++++ pkg/expr/service_test.go | 1 + pkg/expr/sql/errors.go | 20 +++++++++++++++++ pkg/expr/sql_command.go | 4 ++++ .../apis/query/client/instance_provider.go | 11 +++++----- .../apis/query/clientapi/clientapi.go | 11 +++++----- pkg/registry/apis/query/query.go | 9 ++++---- pkg/setting/setting.go | 4 ++++ 11 files changed, 88 insertions(+), 18 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index cedf7b619e6..46dc82dd857 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -2858,15 +2858,19 @@ Set this to `false` to disable expressions and hide them in the Grafana UI. Defa #### `sql_expression_cell_limit` -Set the maximum number of cells that can be passed to a SQL expression. Default is `100000`. +Set the maximum number of cells that can be passed to a SQL expression. Default is `100000`. A setting of `0` means no limit. #### `sql_expression_output_cell_limit` -Set the maximum number of cells that can be returned from a SQL expression. Default is `100000`. +Set the maximum number of cells that can be returned from a SQL expression. Default is `100000`. A setting of `0` means no limit. + +### `sql_expression_query_length_limit` + +Set the maximum length of a SQL query that can be used in a SQL expression. Default is `10000` characters. A setting of `0` means no limit. #### `sql_expression_timeout` -The duration a SQL expression will run before being cancelled. The default is `10s`. +The duration a SQL expression will run before being cancelled. The default is `10s`. A setting of `0s` means no limit. ### `[geomap]` diff --git a/pkg/expr/graph.go b/pkg/expr/graph.go index 8e5fa8abea1..d04ea82fe47 100644 --- a/pkg/expr/graph.go +++ b/pkg/expr/graph.go @@ -9,6 +9,8 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" "golang.org/x/exp/maps" "gonum.org/v1/gonum/graph/simple" "gonum.org/v1/gonum/graph/topo" @@ -202,6 +204,26 @@ func (s *Service) buildPipeline(ctx context.Context, req *Request) (DataPipeline req.Headers = map[string]string{} } + instrumentSQLError := func(err error, span trace.Span) { + var sqlErr *sql.ErrorWithCategory + if errors.As(err, &sqlErr) { + // The SQL expression (and the entire pipeline) will not be executed, so we + // track the attempt to execute here. + s.metrics.SqlCommandCount.WithLabelValues("error", sqlErr.Category()).Inc() + } + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + } + } + + _, span := s.tracer.Start(ctx, "SSE.BuildPipeline") + var err error + defer func() { + instrumentSQLError(err, span) + span.End() + }() + graph, err := s.buildDependencyGraph(ctx, req) if err != nil { return nil, err diff --git a/pkg/expr/graph_test.go b/pkg/expr/graph_test.go index 1d1b11e63bb..6902860f3d6 100644 --- a/pkg/expr/graph_test.go +++ b/pkg/expr/graph_test.go @@ -233,7 +233,8 @@ func TestServicebuildPipeLine(t *testing.T) { }, } s := Service{ - cfg: setting.NewCfg(), + cfg: setting.NewCfg(), + tracer: &testTracer{}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/expr/service_sql_test.go b/pkg/expr/service_sql_test.go index d65702a9d45..55eb8db6524 100644 --- a/pkg/expr/service_sql_test.go +++ b/pkg/expr/service_sql_test.go @@ -199,4 +199,15 @@ func TestSQLServiceErrors(t *testing.T) { _, err := s.BuildPipeline(t.Context(), req) require.Error(t, err, "whole pipeline fails when selecting a dependency that does not exist") }) + + t.Run("pipeline will fail if query is longer than the configured limit", func(t *testing.T) { + s, req := newMockQueryService(resp, + newABSQLQueries(`SELECT This is too long and does not need to be valid SQL`), + ) + s.cfg.SQLExpressionQueryLengthLimit = 5 + s.features = featuremgmt.WithFeatures(featuremgmt.FlagSqlExpressions) + + _, err := s.BuildPipeline(t.Context(), req) + require.ErrorContains(t, err, "exceeded the configured limit of 5 characters") + }) } diff --git a/pkg/expr/service_test.go b/pkg/expr/service_test.go index af159f1cfb5..42656f55a31 100644 --- a/pkg/expr/service_test.go +++ b/pkg/expr/service_test.go @@ -195,6 +195,7 @@ func TestSQLExpressionCellLimitFromConfig(t *testing.T) { converter: &ResultConverter{ Features: features, }, + tracer: &testTracer{}, } req := &Request{Queries: queries, User: &user.SignedInUser{}} diff --git a/pkg/expr/sql/errors.go b/pkg/expr/sql/errors.go index 24953ff8d30..98f09efead7 100644 --- a/pkg/expr/sql/errors.go +++ b/pkg/expr/sql/errors.go @@ -389,3 +389,23 @@ func MakeColumnNotFoundError(refID string, err error) CategorizedError { return &ErrorWithCategory{category: ErrCategoryColumnNotFound, err: ColumnNotFoundError.Build(data)} } + +const ErrCategoryQueryTooLong = "query_too_long" + +var queryTooLongStr = `sql expression [{{.Public.refId}}] was not run because the SQL query exceeded the configured limit of {{ .Public.queryLengthLimit }} characters` + +var QueryTooLongError = errutil.NewBase( + errutil.StatusBadRequest, sseErrBase+ErrCategoryQueryTooLong).MustTemplate( + queryTooLongStr, + errutil.WithPublic(queryTooLongStr)) + +func MakeQueryTooLongError(refID string, queryLengthLimit int64) CategorizedError { + data := errutil.TemplateData{ + Public: map[string]interface{}{ + "refId": refID, + "queryLengthLimit": queryLengthLimit, + }, + } + + return &ErrorWithCategory{category: ErrCategoryQueryTooLong, err: QueryTooLongError.Build(data)} +} diff --git a/pkg/expr/sql_command.go b/pkg/expr/sql_command.go index 8a93400a9f8..1d67cf47cea 100644 --- a/pkg/expr/sql_command.go +++ b/pkg/expr/sql_command.go @@ -86,6 +86,10 @@ func UnmarshalSQLCommand(ctx context.Context, rn *rawNode, cfg *setting.Cfg) (*S return nil, fmt.Errorf("expected sql expression to be type string, but got type %T", expressionRaw) } + if cfg.SQLExpressionQueryLengthLimit > 0 && len(expression) > int(cfg.SQLExpressionQueryLengthLimit) { + return nil, sql.MakeQueryTooLongError(rn.RefID, cfg.SQLExpressionQueryLengthLimit) + } + formatRaw := rn.Query["format"] format, _ := formatRaw.(string) diff --git a/pkg/registry/apis/query/client/instance_provider.go b/pkg/registry/apis/query/client/instance_provider.go index 96769229daf..49f04edc77e 100644 --- a/pkg/registry/apis/query/client/instance_provider.go +++ b/pkg/registry/apis/query/client/instance_provider.go @@ -47,11 +47,12 @@ func (s *singleTenantInstanceProvider) GetInstance(_ context.Context, _ map[stri func (s *singleTenantInstance) GetSettings() clientapi.InstanceConfigurationSettings { return clientapi.InstanceConfigurationSettings{ - FeatureToggles: s.features, - SQLExpressionCellLimit: s.cfg.SQLExpressionCellLimit, - SQLExpressionOutputCellLimit: s.cfg.SQLExpressionOutputCellLimit, - SQLExpressionTimeout: s.cfg.SQLExpressionTimeout, - ExpressionsEnabled: s.cfg.ExpressionsEnabled, + FeatureToggles: s.features, + SQLExpressionCellLimit: s.cfg.SQLExpressionCellLimit, + SQLExpressionOutputCellLimit: s.cfg.SQLExpressionOutputCellLimit, + SQLExpressionQueryLengthLimit: s.cfg.SQLExpressionQueryLengthLimit, + SQLExpressionTimeout: s.cfg.SQLExpressionTimeout, + ExpressionsEnabled: s.cfg.ExpressionsEnabled, } } diff --git a/pkg/registry/apis/query/clientapi/clientapi.go b/pkg/registry/apis/query/clientapi/clientapi.go index c35e28a4b9e..3a3ec7fed7f 100644 --- a/pkg/registry/apis/query/clientapi/clientapi.go +++ b/pkg/registry/apis/query/clientapi/clientapi.go @@ -21,11 +21,12 @@ type QueryDataClient interface { } type InstanceConfigurationSettings struct { - FeatureToggles featuremgmt.FeatureToggles - SQLExpressionCellLimit int64 - SQLExpressionOutputCellLimit int64 - SQLExpressionTimeout time.Duration - ExpressionsEnabled bool + FeatureToggles featuremgmt.FeatureToggles + SQLExpressionCellLimit int64 + SQLExpressionOutputCellLimit int64 + SQLExpressionQueryLengthLimit int64 + SQLExpressionTimeout time.Duration + ExpressionsEnabled bool } type Instance interface { diff --git a/pkg/registry/apis/query/query.go b/pkg/registry/apis/query/query.go index 4539662c7eb..b0de3e95bcf 100644 --- a/pkg/registry/apis/query/query.go +++ b/pkg/registry/apis/query/query.go @@ -286,10 +286,11 @@ func handleQuery(ctx context.Context, raw query.QueryDataRequest, b QueryAPIBuil exprService := expr.ProvideService( &setting.Cfg{ - ExpressionsEnabled: instanceConfig.ExpressionsEnabled, - SQLExpressionCellLimit: instanceConfig.SQLExpressionCellLimit, - SQLExpressionOutputCellLimit: instanceConfig.SQLExpressionOutputCellLimit, - SQLExpressionTimeout: instanceConfig.SQLExpressionTimeout, + ExpressionsEnabled: instanceConfig.ExpressionsEnabled, + SQLExpressionCellLimit: instanceConfig.SQLExpressionCellLimit, + SQLExpressionOutputCellLimit: instanceConfig.SQLExpressionOutputCellLimit, + SQLExpressionTimeout: instanceConfig.SQLExpressionTimeout, + SQLExpressionQueryLengthLimit: instanceConfig.SQLExpressionQueryLengthLimit, }, nil, nil, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index b31584951bf..a475dfd2a14 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -441,6 +441,9 @@ type Cfg struct { // SQLExpressionOutputCellLimit is the maximum number of cells (rows × columns) that can be outputted by a SQL expression. SQLExpressionOutputCellLimit int64 + // SQLExpressionQueryLengthLimit is the maximum length of a SQL query that can be used in a SQL expression. + SQLExpressionQueryLengthLimit int64 + // SQLExpressionTimeoutSeconds is the duration a SQL expression will run before timing out SQLExpressionTimeout time.Duration @@ -830,6 +833,7 @@ func (cfg *Cfg) readExpressionsSettings() { cfg.SQLExpressionCellLimit = expressions.Key("sql_expression_cell_limit").MustInt64(100000) cfg.SQLExpressionOutputCellLimit = expressions.Key("sql_expression_output_cell_limit").MustInt64(100000) cfg.SQLExpressionTimeout = expressions.Key("sql_expression_timeout").MustDuration(time.Second * 10) + cfg.SQLExpressionQueryLengthLimit = expressions.Key("sql_expression_query_length_limit").MustInt64(10000) } type AnnotationCleanupSettings struct {