Config: Add configuration option to define custom user-facing general error message for certain error types (#70023)

---------

Co-authored-by: Summer Wollin <summer.wollin@grafana.com>
Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com>
Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>
This commit is contained in:
Michael Mandrus 2023-06-16 11:46:47 -04:00 committed by GitHub
parent 6be0ca396f
commit 66d2214c3b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 69 additions and 33 deletions

View File

@ -882,6 +882,9 @@ level = info
# optional settings to set different levels for specific loggers. Ex filters = sqlstore:debug
filters =
# Set the default error message shown to users. This message is displayed instead of sensitive backend errors which should be obfuscated.
user_facing_default_error = "please inspect Grafana server log for details"
# For "console" mode only
[log.console]
level =

View File

@ -842,6 +842,9 @@
# optional settings to set different levels for specific loggers. Ex filters = sqlstore:debug
;filters =
# Set the default error message shown to users. This message is displayed instead of sensitive backend errors which should be obfuscated. Default is the same as the sample value.
;user_facing_default_error = "please inspect Grafana server log for details"
# For "console" mode only
[log.console]
;level =

View File

@ -1249,6 +1249,10 @@ Options are "debug", "info", "warn", "error", and "critical". Default is `info`.
Optional settings to set different levels for specific loggers.
For example: `filters = sqlstore:debug`
### user_facing_default_error
Use this configuration option to set the default error message shown to users. This message is displayed instead of sensitive backend errors, which should be obfuscated. The default message is `Please inspect the Grafana server log for details.`.
<hr>
## [log.console]

View File

@ -32,7 +32,7 @@ func (hs *HTTPServer) AdminReEncryptSecrets(c *contextmodel.ReqContext) response
}
if !success {
return response.Error(http.StatusPartialContent, "Something unexpected happened, refer to the server logs for more details", err)
return response.Error(http.StatusPartialContent, fmt.Sprintf("Something unexpected happened - %s", hs.Cfg.UserFacingDefaultError), err)
}
return response.Respond(http.StatusOK, "Secrets re-encrypted successfully")
@ -45,7 +45,7 @@ func (hs *HTTPServer) AdminRollbackSecrets(c *contextmodel.ReqContext) response.
}
if !success {
return response.Error(http.StatusPartialContent, "Something unexpected happened, refer to the server logs for more details", err)
return response.Error(http.StatusPartialContent, fmt.Sprintf("Something unexpected happened - %s", hs.Cfg.UserFacingDefaultError), err)
}
return response.Respond(http.StatusOK, "Secrets rolled back successfully")

View File

@ -153,7 +153,7 @@ func Recovery(cfg *setting.Cfg) web.Middleware {
if ctx != nil && ctx.IsApiRequest() {
resp := make(map[string]interface{})
resp["message"] = "Internal Server Error - Check the Grafana server logs for the detailed error message."
resp["message"] = fmt.Sprintf("Internal Server Error - %s", cfg.UserFacingDefaultError)
if data.ErrorMsg != "" {
resp["error"] = fmt.Sprintf("%v - %v", data.Title, data.ErrorMsg)

View File

@ -24,7 +24,7 @@ func TestRecoveryMiddleware(t *testing.T) {
sc.req.Header.Set("content-type", "application/json")
assert.Equal(t, 500, sc.resp.Code)
assert.Equal(t, "Internal Server Error - Check the Grafana server logs for the detailed error message.", sc.respJson["message"])
assert.Equal(t, "Internal Server Error - test error", sc.respJson["message"])
assert.True(t, strings.HasPrefix(sc.respJson["error"].(string), "Server Error"))
})
})
@ -50,6 +50,7 @@ func recoveryScenario(t *testing.T, desc string, url string, fn scenarioFunc) {
t.Run(desc, func(t *testing.T) {
cfg := setting.NewCfg()
cfg.ErrTemplateName = "error-template"
cfg.UserFacingDefaultError = "test error"
sc := &scenarioContext{
t: t,
url: url,

View File

@ -117,7 +117,7 @@ func (s *ServiceImpl) executeConcurrentQueries(ctx context.Context, user *user.S
} else if theErrString, ok := r.(string); ok {
err = fmt.Errorf(theErrString)
} else {
err = fmt.Errorf("unexpected error, see the server log for details")
err = fmt.Errorf("unexpected error - %s", s.cfg.UserFacingDefaultError)
}
// Due to the panic, there is no valid response for any query for this datasource. Append an error for each one.
rchan <- buildErrorResponses(err, queries)

View File

@ -537,6 +537,9 @@ type Cfg struct {
CustomResponseHeaders map[string]string
// This is used to override the general error message shown to users when we want to obfuscate a sensitive backend error
UserFacingDefaultError string
// DatabaseInstrumentQueries is used to decide if database queries
// should be instrumented with metrics, logs and traces.
// This needs to be on the global object since its used in the
@ -1224,6 +1227,9 @@ func (cfg *Cfg) Load(args CommandLineArgs) error {
databaseSection := iniFile.Section("database")
cfg.DatabaseInstrumentQueries = databaseSection.Key("instrument_queries").MustBool(false)
logSection := iniFile.Section("log")
cfg.UserFacingDefaultError = logSection.Key("user_facing_default_error").MustString("please inspect Grafana server log for details")
return nil
}

View File

@ -109,9 +109,11 @@ func newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFactoryFunc {
RowLimit: cfg.DataProxyRowLimit,
}
queryResultTransformer := mssqlQueryResultTransformer{}
queryResultTransformer := mssqlQueryResultTransformer{
userError: cfg.UserFacingDefaultError,
}
return sqleng.NewQueryDataHandler(config, &queryResultTransformer, newMssqlMacroEngine(), logger)
return sqleng.NewQueryDataHandler(cfg, config, &queryResultTransformer, newMssqlMacroEngine(), logger)
}
}
@ -197,14 +199,16 @@ func generateConnectionString(dsInfo sqleng.DataSourceInfo) (string, error) {
return connStr, nil
}
type mssqlQueryResultTransformer struct{}
type mssqlQueryResultTransformer struct {
userError string
}
func (t *mssqlQueryResultTransformer) TransformQueryError(logger log.Logger, err error) error {
// go-mssql overrides source error, so we currently match on string
// ref https://github.com/denisenkom/go-mssqldb/blob/045585d74f9069afe2e115b6235eb043c8047043/tds.go#L904
if strings.HasPrefix(strings.ToLower(err.Error()), "unable to open tcp connection with host") {
logger.Error("Query error", "error", err)
return sqleng.ErrConnectionFailed
return sqleng.ErrConnectionFailed.Errorf("failed to connect to server - %s", t.userError)
}
return err

View File

@ -16,6 +16,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb/sqleng"
)
@ -57,7 +58,7 @@ func TestMSSQL(t *testing.T) {
MetricColumnTypes: []string{"VARCHAR", "CHAR", "NVARCHAR", "NCHAR"},
RowLimit: 1000000,
}
endpoint, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newMssqlMacroEngine(), logger)
endpoint, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newMssqlMacroEngine(), logger)
require.NoError(t, err)
sess := x.NewSession()
@ -793,7 +794,7 @@ func TestMSSQL(t *testing.T) {
MetricColumnTypes: []string{"VARCHAR", "CHAR", "NVARCHAR", "NCHAR"},
RowLimit: 1000000,
}
endpoint, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newMssqlMacroEngine(), logger)
endpoint, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newMssqlMacroEngine(), logger)
require.NoError(t, err)
query := &backend.QueryDataRequest{
Queries: []backend.DataQuery{
@ -1199,7 +1200,7 @@ func TestMSSQL(t *testing.T) {
RowLimit: 1,
}
handler, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newMssqlMacroEngine(), logger)
handler, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newMssqlMacroEngine(), logger)
require.NoError(t, err)
t.Run("When doing a table query that returns 2 rows should limit the result to 1 row", func(t *testing.T) {

View File

@ -1,7 +1,6 @@
package mysql
import (
"errors"
"fmt"
"regexp"
"strings"
@ -9,6 +8,7 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana-plugin-sdk-go/backend/gtime"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb/sqleng"
)
@ -19,18 +19,23 @@ var restrictedRegExp = regexp.MustCompile(`(?im)([\s]*show[\s]+grants|[\s,]sessi
type mySQLMacroEngine struct {
*sqleng.SQLMacroEngineBase
logger log.Logger
logger log.Logger
userError string
}
func newMysqlMacroEngine(logger log.Logger) sqleng.SQLMacroEngine {
return &mySQLMacroEngine{SQLMacroEngineBase: sqleng.NewSQLMacroEngineBase(), logger: logger}
func newMysqlMacroEngine(logger log.Logger, cfg *setting.Cfg) sqleng.SQLMacroEngine {
return &mySQLMacroEngine{
SQLMacroEngineBase: sqleng.NewSQLMacroEngineBase(),
logger: logger,
userError: cfg.UserFacingDefaultError,
}
}
func (m *mySQLMacroEngine) Interpolate(query *backend.DataQuery, timeRange backend.TimeRange, sql string) (string, error) {
matches := restrictedRegExp.FindAllStringSubmatch(sql, 1)
if len(matches) > 0 {
m.logger.Error("show grants, session_user(), current_user(), system_user() or user() not allowed in query")
return "", errors.New("invalid query - inspect Grafana server log for details")
return "", fmt.Errorf("invalid query - %s", m.userError)
}
// TODO: Handle error

View File

@ -8,13 +8,15 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
)
func TestMacroEngine(t *testing.T) {
engine := &mySQLMacroEngine{
logger: log.New("test"),
logger: log.New("test"),
userError: "inspect Grafana server log for details",
}
query := &backend.DataQuery{}
@ -193,7 +195,7 @@ func TestMacroEngine(t *testing.T) {
}
func TestMacroEngineConcurrency(t *testing.T) {
engine := newMysqlMacroEngine(log.New("test"))
engine := newMysqlMacroEngine(log.New("test"), setting.NewCfg())
query1 := backend.DataQuery{
JSON: []byte{},
}

View File

@ -141,9 +141,11 @@ func newInstanceSettings(cfg *setting.Cfg, httpClientProvider httpclient.Provide
RowLimit: cfg.DataProxyRowLimit,
}
rowTransformer := mysqlQueryResultTransformer{}
rowTransformer := mysqlQueryResultTransformer{
userError: cfg.UserFacingDefaultError,
}
return sqleng.NewQueryDataHandler(config, &rowTransformer, newMysqlMacroEngine(logger), logger)
return sqleng.NewQueryDataHandler(cfg, config, &rowTransformer, newMysqlMacroEngine(logger, cfg), logger)
}
}
@ -184,6 +186,7 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest)
}
type mysqlQueryResultTransformer struct {
userError string
}
func (t *mysqlQueryResultTransformer) TransformQueryError(logger log.Logger, err error) error {
@ -192,15 +195,13 @@ func (t *mysqlQueryResultTransformer) TransformQueryError(logger log.Logger, err
if driverErr.Number != mysqlerr.ER_PARSE_ERROR && driverErr.Number != mysqlerr.ER_BAD_FIELD_ERROR &&
driverErr.Number != mysqlerr.ER_NO_SUCH_TABLE {
logger.Error("Query error", "error", err)
return errQueryFailed
return fmt.Errorf(("query failed - %s"), t.userError)
}
}
return err
}
var errQueryFailed = errors.New("query failed - please inspect Grafana server log for details")
func (t *mysqlQueryResultTransformer) GetConverterList() []sqlutil.StringConverter {
// For the MySQL driver , we have these possible data types:
// https://www.w3schools.com/sql/sql_datatypes.asp#:~:text=In%20MySQL%20there%20are%20three,numeric%2C%20and%20date%20and%20time.

View File

@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb/sqleng"
)
@ -74,7 +75,7 @@ func TestIntegrationMySQL(t *testing.T) {
rowTransformer := mysqlQueryResultTransformer{}
exe, err := sqleng.NewQueryDataHandler(config, &rowTransformer, newMysqlMacroEngine(logger), logger)
exe, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &rowTransformer, newMysqlMacroEngine(logger, setting.NewCfg()), logger)
require.NoError(t, err)
@ -1165,7 +1166,7 @@ func TestIntegrationMySQL(t *testing.T) {
queryResultTransformer := mysqlQueryResultTransformer{}
handler, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newMysqlMacroEngine(logger), logger)
handler, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newMysqlMacroEngine(logger, setting.NewCfg()), logger)
require.NoError(t, err)
t.Run("When doing a table query that returns 2 rows should limit the result to 1 row", func(t *testing.T) {

View File

@ -112,7 +112,7 @@ func (s *Service) newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFacto
queryResultTransformer := postgresQueryResultTransformer{}
handler, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newPostgresMacroEngine(dsInfo.JsonData.Timescaledb),
handler, err := sqleng.NewQueryDataHandler(cfg, config, &queryResultTransformer, newPostgresMacroEngine(dsInfo.JsonData.Timescaledb),
logger)
if err != nil {
logger.Error("Failed connecting to Postgres", "err", err)

View File

@ -225,7 +225,7 @@ func TestIntegrationPostgres(t *testing.T) {
queryResultTransformer := postgresQueryResultTransformer{}
exe, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newPostgresMacroEngine(dsInfo.JsonData.Timescaledb),
exe, err := sqleng.NewQueryDataHandler(cfg, config, &queryResultTransformer, newPostgresMacroEngine(dsInfo.JsonData.Timescaledb),
logger)
require.NoError(t, err)
@ -1267,7 +1267,7 @@ func TestIntegrationPostgres(t *testing.T) {
queryResultTransformer := postgresQueryResultTransformer{}
handler, err := sqleng.NewQueryDataHandler(config, &queryResultTransformer, newPostgresMacroEngine(false), logger)
handler, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newPostgresMacroEngine(false), logger)
require.NoError(t, err)
t.Run("When doing a table query that returns 2 rows should limit the result to 1 row", func(t *testing.T) {

View File

@ -20,7 +20,9 @@ import (
"xorm.io/xorm"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tsdb/intervalv2"
"github.com/grafana/grafana/pkg/util/errutil"
)
// XormDriverMu is used to allow safe concurrent registering and querying of drivers in xorm
@ -29,7 +31,7 @@ var XormDriverMu sync.RWMutex
// MetaKeyExecutedQueryString is the key where the executed query should get stored
const MetaKeyExecutedQueryString = "executedQueryString"
var ErrConnectionFailed = errors.New("failed to connect to server - please inspect Grafana server log for details")
var ErrConnectionFailed = errutil.NewBase(errutil.StatusInternal, "sqleng.connectionError")
// SQLMacroEngine interpolates macros into sql. It takes in the Query to have access to query context and
// timeRange to be able to generate queries that use from and to.
@ -100,6 +102,7 @@ type DataPluginConfiguration struct {
MetricColumnTypes []string
RowLimit int64
}
type DataSourceHandler struct {
macroEngine SQLMacroEngine
queryResultTransformer SqlQueryResultTransformer
@ -109,6 +112,7 @@ type DataSourceHandler struct {
log log.Logger
dsInfo DataSourceInfo
rowLimit int64
userError string
}
type QueryJson struct {
@ -128,13 +132,13 @@ func (e *DataSourceHandler) TransformQueryError(logger log.Logger, err error) er
var opErr *net.OpError
if errors.As(err, &opErr) {
logger.Error("Query error", "err", err)
return ErrConnectionFailed
return ErrConnectionFailed.Errorf("failed to connect to server - %s", e.userError)
}
return e.queryResultTransformer.TransformQueryError(logger, err)
}
func NewQueryDataHandler(config DataPluginConfiguration, queryResultTransformer SqlQueryResultTransformer,
func NewQueryDataHandler(cfg *setting.Cfg, config DataPluginConfiguration, queryResultTransformer SqlQueryResultTransformer,
macroEngine SQLMacroEngine, log log.Logger) (*DataSourceHandler, error) {
log.Debug("Creating engine...")
defer func() {
@ -148,6 +152,7 @@ func NewQueryDataHandler(config DataPluginConfiguration, queryResultTransformer
log: log,
dsInfo: config.DSInfo,
rowLimit: config.RowLimit,
userError: cfg.UserFacingDefaultError,
}
if len(config.TimeColumnNames) > 0 {
@ -242,7 +247,7 @@ func (e *DataSourceHandler) executeQuery(query backend.DataQuery, wg *sync.WaitG
} else if theErrString, ok := r.(string); ok {
queryResult.dataResponse.Error = fmt.Errorf(theErrString)
} else {
queryResult.dataResponse.Error = fmt.Errorf("unexpected error, see the server log for details")
queryResult.dataResponse.Error = fmt.Errorf("unexpected error - %s", e.userError)
}
ch <- queryResult
}