Config: Show user-friendly error message instead of stack trace (#16564)

Fixes #16283

pkg/setting/setting.go

@@ -5,6 +5,7 @@ package setting
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -259,8 +260,11 @@ func init() {
 	IsWindows = runtime.GOOS == "windows"
 }
 
-func parseAppUrlAndSubUrl(section *ini.Section) (string, string) {
-	appUrl := section.Key("root_url").MustString("http://localhost:3000/")
+func parseAppUrlAndSubUrl(section *ini.Section) (string, string, error) {
+	appUrl, err := valueAsString(section, "root_url", "http://localhost:3000/")
+	if err != nil {
+		return "", "", err
+	}
 	if appUrl[len(appUrl)-1] != '/' {
 		appUrl += "/"
 	}
@@ -272,7 +276,7 @@ func parseAppUrlAndSubUrl(section *ini.Section) (string, string) {
 	}
 	appSubUrl := strings.TrimSuffix(url.Path, "/")
 
-	return appUrl, appSubUrl
+	return appUrl, appSubUrl, nil
 }
 
 func ToAbsUrl(relativeUrl string) string {
@@ -479,7 +483,10 @@ func (cfg *Cfg) loadConfiguration(args *CommandLineArgs) (*ini.File, error) {
 	// load specified config file
 	err = loadSpecifedConfigFile(args.Config, parsedFile)
 	if err != nil {
-		cfg.initLogging(parsedFile)
+		err = cfg.initLogging(parsedFile)
+		if err != nil {
+			return nil, err
+		}
 		log.Fatal(3, err.Error())
 	}
 
@@ -496,8 +503,15 @@ func (cfg *Cfg) loadConfiguration(args *CommandLineArgs) (*ini.File, error) {
 	evalConfigValues(parsedFile)
 
 	// update data path and logging config
-	cfg.DataPath = makeAbsolute(parsedFile.Section("paths").Key("data").String(), HomePath)
-	cfg.initLogging(parsedFile)
+	dataPath, err := valueAsString(parsedFile.Section("paths"), "data", "")
+	if err != nil {
+		return nil, err
+	}
+	cfg.DataPath = makeAbsolute(dataPath, HomePath)
+	err = cfg.initLogging(parsedFile)
+	if err != nil {
+		return nil, err
+	}
 
 	return parsedFile, err
 }
@@ -570,34 +584,68 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 		ApplicationName = APP_NAME_ENTERPRISE
 	}
 
-	Env = iniFile.Section("").Key("app_mode").MustString("development")
-	InstanceName = iniFile.Section("").Key("instance_name").MustString("unknown_instance_name")
-	PluginsPath = makeAbsolute(iniFile.Section("paths").Key("plugins").String(), HomePath)
-	cfg.ProvisioningPath = makeAbsolute(iniFile.Section("paths").Key("provisioning").String(), HomePath)
+	Env, err = valueAsString(iniFile.Section(""), "app_mode", "development")
+	if err != nil {
+		return err
+	}
+	InstanceName, err = valueAsString(iniFile.Section(""), "instance_name", "unknown_instance_name")
+	if err != nil {
+		return err
+	}
+	plugins, err := valueAsString(iniFile.Section("paths"), "plugins", "")
+	if err != nil {
+		return err
+	}
+	PluginsPath = makeAbsolute(plugins, HomePath)
+	Provisioning, err := valueAsString(iniFile.Section("paths"), "provisioning", "")
+	if err != nil {
+		return err
+	}
+	cfg.ProvisioningPath = makeAbsolute(Provisioning, HomePath)
 	server := iniFile.Section("server")
-	AppUrl, AppSubUrl = parseAppUrlAndSubUrl(server)
+	AppUrl, AppSubUrl, err = parseAppUrlAndSubUrl(server)
+	if err != nil {
+		return err
+	}
 	cfg.AppUrl = AppUrl
 	cfg.AppSubUrl = AppSubUrl
 
 	Protocol = HTTP
-	if server.Key("protocol").MustString("http") == "https" {
+	protocolStr, err := valueAsString(server, "protocol", "http")
+	if err != nil {
+		return err
+	}
+	if protocolStr == "https" {
 		Protocol = HTTPS
 		CertFile = server.Key("cert_file").String()
 		KeyFile = server.Key("cert_key").String()
 	}
-	if server.Key("protocol").MustString("http") == "socket" {
+	if protocolStr == "socket" {
 		Protocol = SOCKET
 		SocketPath = server.Key("socket").String()
 	}
 
-	Domain = server.Key("domain").MustString("localhost")
-	HttpAddr = server.Key("http_addr").MustString(DEFAULT_HTTP_ADDR)
-	HttpPort = server.Key("http_port").MustString("3000")
+	Domain, err = valueAsString(server, "domain", "localhost")
+	if err != nil {
+		return err
+	}
+	HttpAddr, err = valueAsString(server, "http_addr", DEFAULT_HTTP_ADDR)
+	if err != nil {
+		return err
+	}
+	HttpPort, err = valueAsString(server, "http_port", "3000")
+	if err != nil {
+		return err
+	}
 	RouterLogging = server.Key("router_logging").MustBool(false)
 
 	EnableGzip = server.Key("enable_gzip").MustBool(false)
 	EnforceDomain = server.Key("enforce_domain").MustBool(false)
-	StaticRootPath = makeAbsolute(server.Key("static_root_path").String(), HomePath)
+	staticRoot, err := valueAsString(server, "static_root_path", "")
+	if err != nil {
+		return err
+	}
+	StaticRootPath = makeAbsolute(staticRoot, HomePath)
 
 	if err := cfg.validateStaticRootPath(); err != nil {
 		return err
@@ -611,7 +659,10 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 
 	// read security settings
 	security := iniFile.Section("security")
-	SecretKey = security.Key("secret_key").String()
+	SecretKey, err = valueAsString(security, "secret_key", "")
+	if err != nil {
+		return err
+	}
 	DisableGravatar = security.Key("disable_gravatar").MustBool(true)
 	cfg.DisableBruteForceLoginProtection = security.Key("disable_brute_force_login_protection").MustBool(false)
 	DisableBruteForceLoginProtection = cfg.DisableBruteForceLoginProtection
@@ -619,7 +670,10 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	CookieSecure = security.Key("cookie_secure").MustBool(false)
 	cfg.CookieSecure = CookieSecure
 
-	samesiteString := security.Key("cookie_samesite").MustString("lax")
+	samesiteString, err := valueAsString(security, "cookie_samesite", "lax")
+	if err != nil {
+		return err
+	}
 	validSameSiteValues := map[string]http.SameSite{
 		"lax":    http.SameSiteLaxMode,
 		"strict": http.SameSiteStrictMode,
@@ -636,8 +690,14 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 
 	// read snapshots settings
 	snapshots := iniFile.Section("snapshots")
-	ExternalSnapshotUrl = snapshots.Key("external_snapshot_url").String()
-	ExternalSnapshotName = snapshots.Key("external_snapshot_name").String()
+	ExternalSnapshotUrl, err = valueAsString(snapshots, "external_snapshot_url", "")
+	if err != nil {
+		return err
+	}
+	ExternalSnapshotName, err = valueAsString(snapshots, "external_snapshot_name", "")
+	if err != nil {
+		return err
+	}
 	ExternalEnabled = snapshots.Key("external_enabled").MustBool(true)
 	SnapShotRemoveExpired = snapshots.Key("snapshot_remove_expired").MustBool(true)
 
@@ -647,13 +707,23 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 
 	//  read data source proxy white list
 	DataProxyWhiteList = make(map[string]bool)
-	for _, hostAndIp := range util.SplitString(security.Key("data_source_proxy_whitelist").String()) {
+	securityStr, err := valueAsString(security, "data_source_proxy_whitelist", "")
+	if err != nil {
+		return err
+	}
+	for _, hostAndIp := range util.SplitString(securityStr) {
 		DataProxyWhiteList[hostAndIp] = true
 	}
 
 	// admin
-	AdminUser = security.Key("admin_user").String()
-	AdminPassword = security.Key("admin_password").String()
+	AdminUser, err = valueAsString(security, "admin_user", "")
+	if err != nil {
+		return err
+	}
+	AdminPassword, err = valueAsString(security, "admin_password", "")
+	if err != nil {
+		return err
+	}
 
 	// users
 	users := iniFile.Section("users")
@@ -663,20 +733,41 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	AutoAssignOrgId = users.Key("auto_assign_org_id").MustInt(1)
 	AutoAssignOrgRole = users.Key("auto_assign_org_role").In("Editor", []string{"Editor", "Admin", "Viewer"})
 	VerifyEmailEnabled = users.Key("verify_email_enabled").MustBool(false)
-	LoginHint = users.Key("login_hint").String()
-	PasswordHint = users.Key("password_hint").String()
-	DefaultTheme = users.Key("default_theme").String()
-	ExternalUserMngLinkUrl = users.Key("external_manage_link_url").String()
-	ExternalUserMngLinkName = users.Key("external_manage_link_name").String()
-	ExternalUserMngInfo = users.Key("external_manage_info").String()
+	LoginHint, err = valueAsString(users, "login_hint", "")
+	if err != nil {
+		return err
+	}
+	PasswordHint, err = valueAsString(users, "password_hint", "")
+	if err != nil {
+		return err
+	}
+	DefaultTheme, err = valueAsString(users, "default_theme", "")
+	if err != nil {
+		return err
+	}
+	ExternalUserMngLinkUrl, err = valueAsString(users, "external_manage_link_url", "")
+	if err != nil {
+		return err
+	}
+	ExternalUserMngLinkName, err = valueAsString(users, "external_manage_link_name", "")
+	if err != nil {
+		return err
+	}
+	ExternalUserMngInfo, err = valueAsString(users, "external_manage_info", "")
+	if err != nil {
+		return err
+	}
 	ViewersCanEdit = users.Key("viewers_can_edit").MustBool(false)
 	cfg.EditorsCanAdmin = users.Key("editors_can_admin").MustBool(false)
 
 	// auth
 	auth := iniFile.Section("auth")
 
-	LoginCookieName = auth.Key("login_cookie_name").MustString("grafana_session")
+	LoginCookieName, err = valueAsString(auth, "login_cookie_name", "grafana_session")
 	cfg.LoginCookieName = LoginCookieName
+	if err != nil {
+		return err
+	}
 	cfg.LoginMaxInactiveLifetimeDays = auth.Key("login_maximum_inactive_lifetime_days").MustInt(7)
 
 	LoginMaxLifetimeDays = auth.Key("login_maximum_lifetime_days").MustInt(30)
@@ -690,24 +781,46 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	DisableLoginForm = auth.Key("disable_login_form").MustBool(false)
 	DisableSignoutMenu = auth.Key("disable_signout_menu").MustBool(false)
 	OAuthAutoLogin = auth.Key("oauth_auto_login").MustBool(false)
-	SignoutRedirectUrl = auth.Key("signout_redirect_url").String()
+	SignoutRedirectUrl, err = valueAsString(auth, "signout_redirect_url", "")
+	if err != nil {
+		return err
+	}
 
 	// anonymous access
 	AnonymousEnabled = iniFile.Section("auth.anonymous").Key("enabled").MustBool(false)
-	AnonymousOrgName = iniFile.Section("auth.anonymous").Key("org_name").String()
-	AnonymousOrgRole = iniFile.Section("auth.anonymous").Key("org_role").String()
+	AnonymousOrgName, err = valueAsString(iniFile.Section("auth.anonymous"), "org_name", "")
+	if err != nil {
+		return err
+	}
+	AnonymousOrgRole, err = valueAsString(iniFile.Section("auth.anonymous"), "org_role", "")
+	if err != nil {
+		return err
+	}
 
 	// auth proxy
 	authProxy := iniFile.Section("auth.proxy")
 	AuthProxyEnabled = authProxy.Key("enabled").MustBool(false)
-	AuthProxyHeaderName = authProxy.Key("header_name").String()
-	AuthProxyHeaderProperty = authProxy.Key("header_property").String()
+	AuthProxyHeaderName, err = valueAsString(authProxy, "header_name", "")
+	if err != nil {
+		return err
+	}
+	AuthProxyHeaderProperty, err = valueAsString(authProxy, "header_property", "")
+	if err != nil {
+		return err
+	}
 	AuthProxyAutoSignUp = authProxy.Key("auto_sign_up").MustBool(true)
 	AuthProxyLdapSyncTtl = authProxy.Key("ldap_sync_ttl").MustInt()
-	AuthProxyWhitelist = authProxy.Key("whitelist").String()
+	AuthProxyWhitelist, err = valueAsString(authProxy, "whitelist", "")
+	if err != nil {
+		return err
+	}
 
 	AuthProxyHeaders = make(map[string]string)
-	for _, propertyAndHeader := range util.SplitString(authProxy.Key("headers").String()) {
+	headers, err := valueAsString(authProxy, "headers", "")
+	if err != nil {
+		return err
+	}
+	for _, propertyAndHeader := range util.SplitString(headers) {
 		split := strings.SplitN(propertyAndHeader, ":", 2)
 		if len(split) == 2 {
 			AuthProxyHeaders[split[0]] = split[1]
@@ -720,8 +833,14 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 
 	// Rendering
 	renderSec := iniFile.Section("rendering")
-	cfg.RendererUrl = renderSec.Key("server_url").String()
-	cfg.RendererCallbackUrl = renderSec.Key("callback_url").String()
+	cfg.RendererUrl, err = valueAsString(renderSec, "server_url", "")
+	if err != nil {
+		return err
+	}
+	cfg.RendererCallbackUrl, err = valueAsString(renderSec, "callback_url", "")
+	if err != nil {
+		return err
+	}
 	if cfg.RendererCallbackUrl == "" {
 		cfg.RendererCallbackUrl = AppUrl
 	} else {
@@ -737,26 +856,47 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	cfg.PhantomDir = filepath.Join(HomePath, "tools/phantomjs")
 	cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration(time.Second * 3600 * 24)
 	cfg.MetricsEndpointEnabled = iniFile.Section("metrics").Key("enabled").MustBool(true)
-	cfg.MetricsEndpointBasicAuthUsername = iniFile.Section("metrics").Key("basic_auth_username").String()
-	cfg.MetricsEndpointBasicAuthPassword = iniFile.Section("metrics").Key("basic_auth_password").String()
+	cfg.MetricsEndpointBasicAuthUsername, err = valueAsString(iniFile.Section("metrics"), "basic_auth_username", "")
+	if err != nil {
+		return err
+	}
+	cfg.MetricsEndpointBasicAuthPassword, err = valueAsString(iniFile.Section("metrics"), "basic_auth_password", "")
+	if err != nil {
+		return err
+	}
 
 	analytics := iniFile.Section("analytics")
 	ReportingEnabled = analytics.Key("reporting_enabled").MustBool(true)
 	CheckForUpdates = analytics.Key("check_for_updates").MustBool(true)
-	GoogleAnalyticsId = analytics.Key("google_analytics_ua_id").String()
-	GoogleTagManagerId = analytics.Key("google_tag_manager_id").String()
+	GoogleAnalyticsId, err = valueAsString(analytics, "google_analytics_ua_id", "")
+	if err != nil {
+		return err
+	}
+	GoogleTagManagerId, err = valueAsString(analytics, "google_tag_manager_id", "")
+	if err != nil {
+		return err
+	}
 
 	ldapSec := iniFile.Section("auth.ldap")
 	LdapEnabled = ldapSec.Key("enabled").MustBool(false)
-	LdapConfigFile = ldapSec.Key("config_file").String()
+	LdapConfigFile, err = valueAsString(ldapSec, "config_file", "")
+	if err != nil {
+		return err
+	}
 	LdapAllowSignup = ldapSec.Key("allow_sign_up").MustBool(true)
 
 	alerting := iniFile.Section("alerting")
 	AlertingEnabled = alerting.Key("enabled").MustBool(true)
 	ExecuteAlerts = alerting.Key("execute_alerts").MustBool(true)
 	AlertingRenderLimit = alerting.Key("concurrent_render_limit").MustInt(5)
-	AlertingErrorOrTimeout = alerting.Key("error_or_timeout").MustString("alerting")
-	AlertingNoDataOrNullValues = alerting.Key("nodata_or_nullvalues").MustString("no_data")
+	AlertingErrorOrTimeout, err = valueAsString(alerting, "error_or_timeout", "alerting")
+	if err != nil {
+		return err
+	}
+	AlertingNoDataOrNullValues, err = valueAsString(alerting, "nodata_or_nullvalues", "no_data")
+	if err != nil {
+		return err
+	}
 
 	AlertingEvaluationTimeout = alerting.Key("evaluation_timeout_seconds").MustDuration(time.Second * 30)
 	AlertingNotificationTimeout = alerting.Key("notification_timeout_seconds").MustDuration(time.Second * 30)
@@ -786,26 +926,56 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	}
 
 	// check old key  name
-	GrafanaComUrl = iniFile.Section("grafana_net").Key("url").MustString("")
+	GrafanaComUrl, err = valueAsString(iniFile.Section("grafana_net"), "url", "")
+	if err != nil {
+		return err
+	}
 	if GrafanaComUrl == "" {
-		GrafanaComUrl = iniFile.Section("grafana_com").Key("url").MustString("https://grafana.com")
+		GrafanaComUrl, err = valueAsString(iniFile.Section("grafana_com"), "url", "https://grafana.com")
+		if err != nil {
+			return err
+		}
 	}
 
 	imageUploadingSection := iniFile.Section("external_image_storage")
-	ImageUploadProvider = imageUploadingSection.Key("provider").MustString("")
+	ImageUploadProvider, err = valueAsString(imageUploadingSection, "provider", "")
+	if err != nil {
+		return err
+	}
 
 	enterprise := iniFile.Section("enterprise")
-	cfg.EnterpriseLicensePath = enterprise.Key("license_path").MustString(filepath.Join(cfg.DataPath, "license.jwt"))
+	cfg.EnterpriseLicensePath, err = valueAsString(enterprise, "license_path", filepath.Join(cfg.DataPath, "license.jwt"))
+	if err != nil {
+		return err
+	}
 
 	cacheServer := iniFile.Section("remote_cache")
+	dbName, err := valueAsString(cacheServer, "type", "database")
+	if err != nil {
+		return err
+	}
+	connStr, err := valueAsString(cacheServer, "connstr", "")
+	if err != nil {
+		return err
+	}
 	cfg.RemoteCacheOptions = &RemoteCacheOptions{
-		Name:    cacheServer.Key("type").MustString("database"),
-		ConnStr: cacheServer.Key("connstr").MustString(""),
+		Name:    dbName,
+		ConnStr: connStr,
 	}
 
 	return nil
 }
 
+func valueAsString(section *ini.Section, keyName string, defaultValue string) (value string, err error) {
+	defer func() {
+		if err_ := recover(); err_ != nil {
+			err = errors.New("Invalid value for key '" + keyName + "' in configuration file")
+		}
+	}()
+
+	return section.Key(keyName).MustString(defaultValue), nil
+}
+
 type RemoteCacheOptions struct {
 	Name    string
 	ConnStr string
@@ -821,15 +991,24 @@ func (cfg *Cfg) readSessionConfig() {
 	}
 }
 
-func (cfg *Cfg) initLogging(file *ini.File) {
+func (cfg *Cfg) initLogging(file *ini.File) error {
+	logModeStr, err := valueAsString(file.Section("log"), "mode", "console")
+	if err != nil {
+		return err
+	}
 	// split on comma
-	logModes := strings.Split(file.Section("log").Key("mode").MustString("console"), ",")
+	logModes := strings.Split(logModeStr, ",")
 	// also try space
 	if len(logModes) == 1 {
-		logModes = strings.Split(file.Section("log").Key("mode").MustString("console"), " ")
+		logModes = strings.Split(logModeStr, " ")
 	}
-	cfg.LogsPath = makeAbsolute(file.Section("paths").Key("logs").String(), HomePath)
+	logsPath, err := valueAsString(file.Section("paths"), "logs", "")
+	if err != nil {
+		return err
+	}
+	cfg.LogsPath = makeAbsolute(logsPath, HomePath)
 	log.ReadLoggingConfig(logModes, cfg.LogsPath, file)
+	return nil
 }
 
 func (cfg *Cfg) LogConfigSources() {

pkg/setting/setting_test.go

@@ -1,7 +1,9 @@
 package setting
 
 import (
+	"gopkg.in/ini.v1"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
 	"testing"
@@ -189,4 +191,29 @@ func TestLoadingSettings(t *testing.T) {
 			So(cfg.RendererCallbackUrl, ShouldEqual, "http://myserver/renderer/")
 		})
 	})
+
+	Convey("Test reading string values from .ini file", t, func() {
+
+		iniFile, err := ini.Load(path.Join(HomePath, "pkg/setting/testdata/invalid.ini"))
+		So(err, ShouldBeNil)
+
+		Convey("If key is found - should return value from ini file", func() {
+			value, err := valueAsString(iniFile.Section("server"), "alt_url", "")
+			So(err, ShouldBeNil)
+			So(value, ShouldEqual, "https://grafana.com/")
+		})
+
+		Convey("If key is not found - should return default value", func() {
+			value, err := valueAsString(iniFile.Section("server"), "extra_url", "default_url_val")
+			So(err, ShouldBeNil)
+			So(value, ShouldEqual, "default_url_val")
+		})
+
+		Convey("In case of panic - should return user-friendly error", func() {
+			value, err := valueAsString(iniFile.Section("server"), "root_url", "")
+			So(err.Error(), ShouldEqual, "Invalid value for key 'root_url' in configuration file")
+			So(value, ShouldEqual, "")
+		})
+
+	})
 }

pkg/setting/testdata/invalid.ini

@@ -0,0 +1,3 @@
+[server]
+root_url = %(protocol)s://%(domain)s:%(port)s/grafana/
+alt_url = https://grafana.com/