Settings: Encapsulate settings within an extensible provider (#32219)

* Encapsulate settings with a provider with support for runtime reloads

* SettingsProvider: reload is controlled by the services

* naive impl of reload handlers for settings

* working naive detection on new changes

* Trigger settings reload from API endpoint

* validation step added

* validation of settings

* Fix linting errors

* Replace DB_Varchar by DB_NVarchar

* Reduce settings columns (section, key) lenghts

* wip db update logic

* Db Settings: separate updates and removals

* Fix: removes incorrectly added code

* Minor code improvements

* Runtime settings: moved oss -> ee

* Remove no longer used setting.Cfg SAML-related fields

* Rename file setting/settings.go => setting/provider.go

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* Minor code improvements on OSS settings provider

* Fix some login API tests

* Correct some GoDoc comments

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

Co-authored-by: Leonard Gram <leo@xlson.com>
Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

pkg/api/api.go

@@ -5,7 +5,6 @@ import (
 	"time"
 
 	"github.com/go-macaron/binding"
-
 	"github.com/grafana/grafana/pkg/api/avatar"
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/frontendlogging"
@@ -15,7 +14,6 @@ import (
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-
 	acmiddleware "github.com/grafana/grafana/pkg/services/accesscontrol/middleware"
 )
 

pkg/api/http_server.go

@@ -73,6 +73,7 @@ type HTTPServer struct {
 	Bus                    bus.Bus                                 `inject:""`
 	RenderService          rendering.Service                       `inject:""`
 	Cfg                    *setting.Cfg                            `inject:""`
+	SettingsProvider       setting.Provider                        `inject:""`
 	HooksService           *hooks.HooksService                     `inject:""`
 	CacheService           *localcache.CacheService                `inject:""`
 	DatasourceCache        datasources.CacheService                `inject:""`

pkg/api/login.go

@@ -96,7 +96,7 @@ func (hs *HTTPServer) LoginView(c *models.ReqContext) {
 	}
 
 	viewData.Settings["oauth"] = enabledOAuths
-	viewData.Settings["samlEnabled"] = hs.License.HasValidLicense() && hs.Cfg.SAMLEnabled
+	viewData.Settings["samlEnabled"] = hs.samlEnabled()
 
 	if loginError, ok := tryGetEncryptedCookie(c, loginErrorCookieName); ok {
 		// this cookie is only set whenever an OAuth login fails
@@ -278,7 +278,7 @@ func (hs *HTTPServer) loginUserWithUser(user *models.User, c *models.ReqContext)
 }
 
 func (hs *HTTPServer) Logout(c *models.ReqContext) {
-	if hs.Cfg.SAMLEnabled && hs.Cfg.SAMLSingleLogoutEnabled && hs.License.HasValidLicense() {
+	if hs.samlSingleLogoutEnabled() {
 		c.Redirect(hs.Cfg.AppSubURL + "/logout/saml")
 		return
 	}
@@ -342,6 +342,14 @@ func (hs *HTTPServer) RedirectResponseWithError(ctx *models.ReqContext, err erro
 	return response.Redirect(hs.Cfg.AppSubURL + "/login")
 }
 
+func (hs *HTTPServer) samlEnabled() bool {
+	return hs.SettingsProvider.KeyValue("auth.saml", "enabled").MustBool(false) && hs.License.HasValidLicense()
+}
+
+func (hs *HTTPServer) samlSingleLogoutEnabled() bool {
+	return hs.SettingsProvider.KeyValue("auth.saml", "single_logout").MustBool(false) && hs.samlEnabled()
+}
+
 func getLoginExternalError(err error) string {
 	var createTokenErr *models.CreateTokenErr
 	if errors.As(err, &createTokenErr) {

pkg/api/login_test.go

@@ -94,8 +94,9 @@ func TestLoginErrorCookieAPIEndpoint(t *testing.T) {
 	sc := setupScenarioContext(t, "/login")
 	cfg := setting.NewCfg()
 	hs := &HTTPServer{
-		Cfg:     cfg,
-		License: &licensing.OSSLicensingService{},
+		Cfg:              cfg,
+		SettingsProvider: &setting.OSSImpl{Cfg: cfg},
+		License:          &licensing.OSSLicensingService{},
 	}
 
 	sc.defaultHandler = routing.Wrap(func(w http.ResponseWriter, c *models.ReqContext) {
@@ -154,9 +155,11 @@ func TestLoginViewRedirect(t *testing.T) {
 
 	fakeViewIndex(t)
 	sc := setupScenarioContext(t, "/login")
+	cfg := setting.NewCfg()
 	hs := &HTTPServer{
-		Cfg:     setting.NewCfg(),
-		License: &licensing.OSSLicensingService{},
+		Cfg:              cfg,
+		SettingsProvider: &setting.OSSImpl{Cfg: cfg},
+		License:          &licensing.OSSLicensingService{},
 	}
 	hs.Cfg.CookieSecure = true
 
@@ -485,9 +488,11 @@ func TestLoginOAuthRedirect(t *testing.T) {
 	fakeSetIndexViewData(t)
 
 	sc := setupScenarioContext(t, "/login")
+	cfg := setting.NewCfg()
 	hs := &HTTPServer{
-		Cfg:     setting.NewCfg(),
-		License: &licensing.OSSLicensingService{},
+		Cfg:              cfg,
+		SettingsProvider: &setting.OSSImpl{Cfg: cfg},
+		License:          &licensing.OSSLicensingService{},
 	}
 
 	sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) {
@@ -577,6 +582,7 @@ func setupAuthProxyLoginTest(t *testing.T, enableLoginToken bool) *scenarioConte
 	sc.cfg.LoginCookieName = "grafana_session"
 	hs := &HTTPServer{
 		Cfg:              sc.cfg,
+		SettingsProvider: &setting.OSSImpl{Cfg: sc.cfg},
 		License:          &licensing.OSSLicensingService{},
 		AuthTokenService: auth.NewFakeUserAuthTokenService(),
 		log:              log.New("hello"),

pkg/extensions/main.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
 	"github.com/grafana/grafana/pkg/services/licensing"
 	"github.com/grafana/grafana/pkg/services/validations"
+	"github.com/grafana/grafana/pkg/setting"
 	_ "github.com/grafana/loki/pkg/logproto"
 	_ "github.com/grafana/loki/pkg/promtail/client"
 	_ "github.com/grpc-ecosystem/go-grpc-middleware"
@@ -31,6 +32,7 @@ func init() {
 	registry.RegisterService(&licensing.OSSLicensingService{})
 	registry.RegisterService(&validations.OSSPluginRequestValidator{})
 	registry.RegisterService(&ossaccesscontrol.OSSAccessControlService{})
+	registry.RegisterService(&setting.OSSImpl{})
 }
 
 var IsEnterprise bool = false

pkg/setting/provider.go

@@ -0,0 +1,141 @@
+package setting
+
+import (
+	"errors"
+	"strings"
+	"time"
+
+	"gopkg.in/ini.v1"
+)
+
+var (
+	ErrOperationNotPermitted = errors.New("operation not permitted")
+)
+
+type ValidationError struct {
+	Errors []error
+}
+
+func (v ValidationError) Error() string {
+	builder := strings.Builder{}
+
+	for i, e := range v.Errors {
+		builder.WriteString(e.Error())
+		if i != len(v.Errors)-1 {
+			builder.WriteString(", ")
+		}
+	}
+
+	return builder.String()
+}
+
+// Provider is a settings provider abstraction
+// with thread-safety and runtime updates.
+type Provider interface {
+	// Update
+	Update(updates SettingsBag, removals SettingsRemovals) error
+	// KeyValue returns a key-value abstraction
+	// for the given pair of section and key.
+	KeyValue(section, key string) KeyValue
+	// Section returns a settings section
+	// abstraction for the given section name.
+	Section(section string) Section
+	// RegisterReloadHandler registers a handler for validation and reload
+	// of configuration updates tied to a specific section
+	RegisterReloadHandler(section string, handler ReloadHandler)
+}
+
+// Section is a settings section copy
+// with all of its pairs of keys-values.
+type Section interface {
+	// KeyValue returns a key-value
+	// abstraction for the given key.
+	KeyValue(key string) KeyValue
+}
+
+// KeyValue represents a settings key-value
+// for a given pair of section and key.
+type KeyValue interface {
+	// Key returns pair's key.
+	Key() string
+	// Value returns pair's value.
+	Value() string
+
+	// MustString returns the value's string representation
+	// If empty, then it returns the given default.
+	MustString(defaultVal string) string
+	// MustBool returns the value's boolean representation
+	// Otherwise returns the given default.
+	MustBool(defaultVal bool) bool
+	// MustDuration returns the value's time.Duration
+	// representation. Otherwise returns the given default.
+	MustDuration(defaultVal time.Duration) time.Duration
+}
+
+// ReloadHandler defines the expected behaviour from a
+// service that have support for configuration reloads.
+type ReloadHandler interface {
+	// Reload handles reloading of configuration changes.
+	Reload(section Section) error
+
+	// Validate validates the configuration, if the validation
+	// fails the configuration will not be updated neither reloaded.
+	Validate(section Section) error
+}
+
+type SettingsBag map[string]map[string]string
+type SettingsRemovals map[string][]string
+
+type OSSImpl struct {
+	Cfg *Cfg `inject:""`
+}
+
+func (o OSSImpl) Init() error {
+	return nil
+}
+
+func (OSSImpl) Update(SettingsBag, SettingsRemovals) error {
+	return errors.New("oss settings provider do not have support for settings updates")
+}
+
+func (o *OSSImpl) KeyValue(section, key string) KeyValue {
+	return o.Section(section).KeyValue(key)
+}
+
+func (o *OSSImpl) Section(section string) Section {
+	return &sectionImpl{section: o.Cfg.Raw.Section(section)}
+}
+
+func (OSSImpl) RegisterReloadHandler(string, ReloadHandler) {}
+
+type keyValImpl struct {
+	key *ini.Key
+}
+
+func (k *keyValImpl) Key() string {
+	return k.key.Name()
+}
+
+func (k *keyValImpl) Value() string {
+	return k.key.Value()
+}
+
+func (k *keyValImpl) MustString(defaultVal string) string {
+	return k.key.MustString(defaultVal)
+}
+
+func (k *keyValImpl) MustBool(defaultVal bool) bool {
+	return k.key.MustBool(defaultVal)
+}
+
+func (k *keyValImpl) MustDuration(defaultVal time.Duration) time.Duration {
+	return k.key.MustDuration(defaultVal)
+}
+
+type sectionImpl struct {
+	section *ini.Section
+}
+
+func (s *sectionImpl) KeyValue(key string) KeyValue {
+	return &keyValImpl{s.section.Key(key)}
+}

pkg/setting/setting.go

@@ -297,10 +297,6 @@ type Cfg struct {
 	// OAuth
 	OAuthCookieMaxAge int
 
-	// SAML Auth
-	SAMLEnabled             bool
-	SAMLSingleLogoutEnabled bool
-
 	// JWT Auth
 	JWTAuthEnabled       bool
 	JWTAuthHeaderName    string
@@ -1175,10 +1171,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
 	SigV4AuthEnabled = auth.Key("sigv4_auth_enabled").MustBool(false)
 	cfg.SigV4AuthEnabled = SigV4AuthEnabled
 
-	// SAML auth
-	cfg.SAMLEnabled = iniFile.Section("auth.saml").Key("enabled").MustBool(false)
-	cfg.SAMLSingleLogoutEnabled = iniFile.Section("auth.saml").Key("single_logout").MustBool(false)
-
 	// anonymous access
 	AnonymousEnabled = iniFile.Section("auth.anonymous").Key("enabled").MustBool(false)
 	cfg.AnonymousEnabled = AnonymousEnabled