Settings: Actually deprecate access to feature flags (#78073)

This commit is contained in:
Ryan McKinley 2023-11-13 11:39:01 -08:00 committed by GitHub
parent 4a3c148298
commit dec9a07738
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 37 additions and 21 deletions

View File

@ -33,6 +33,7 @@ import (
func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features *featuremgmt.FeatureManager, pstore pluginstore.Store, psettings pluginsettings.Service) (*web.Mux, *HTTPServer) { func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features *featuremgmt.FeatureManager, pstore pluginstore.Store, psettings pluginsettings.Service) (*web.Mux, *HTTPServer) {
t.Helper() t.Helper()
db.InitTestDB(t) db.InitTestDB(t)
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = features.IsEnabled cfg.IsFeatureToggleEnabled = features.IsEnabled
{ {

View File

@ -73,6 +73,7 @@ func ProvideManagerService(cfg *setting.Cfg, licensing licensing.Licensing) (*Fe
mgmt.update() mgmt.update()
// Minimum approach to avoid circular dependency // Minimum approach to avoid circular dependency
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = mgmt.IsEnabled cfg.IsFeatureToggleEnabled = mgmt.IsEnabled
return mgmt, nil return mgmt, nil
} }

View File

@ -27,7 +27,7 @@ type config struct {
logLevel int logLevel int
} }
func newConfig(cfg *setting.Cfg) *config { func newConfig(cfg *setting.Cfg, features featuremgmt.FeatureToggles) *config {
defaultLogLevel := 0 defaultLogLevel := 0
ip := net.ParseIP(cfg.HTTPAddr) ip := net.ParseIP(cfg.HTTPAddr)
apiURL := cfg.AppURL apiURL := cfg.AppURL
@ -46,7 +46,7 @@ func newConfig(cfg *setting.Cfg) *config {
host := fmt.Sprintf("%s:%d", ip, port) host := fmt.Sprintf("%s:%d", ip, port)
return &config{ return &config{
enabled: cfg.IsFeatureToggleEnabled(featuremgmt.FlagGrafanaAPIServer), enabled: features.IsEnabled(featuremgmt.FlagGrafanaAPIServer),
devMode: cfg.Env == setting.Dev, devMode: cfg.Env == setting.Dev,
dataPath: filepath.Join(cfg.DataPath, "grafana-apiserver"), dataPath: filepath.Join(cfg.DataPath, "grafana-apiserver"),
ip: ip, ip: ip,

View File

@ -11,8 +11,7 @@ import (
) )
func TestNewConfig(t *testing.T) { func TestNewConfig(t *testing.T) {
// nolint:staticcheck cfg := setting.NewCfg()
cfg := setting.NewCfgWithFeatures(featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServer).IsEnabled)
cfg.Env = setting.Prod cfg.Env = setting.Prod
cfg.DataPath = "/tmp/grafana" cfg.DataPath = "/tmp/grafana"
cfg.HTTPAddr = "10.0.0.1" cfg.HTTPAddr = "10.0.0.1"
@ -23,7 +22,7 @@ func TestNewConfig(t *testing.T) {
section.Key("log_level").SetValue("5") section.Key("log_level").SetValue("5")
section.Key("etcd_servers").SetValue("http://localhost:2379") section.Key("etcd_servers").SetValue("http://localhost:2379")
actual := newConfig(cfg) actual := newConfig(cfg, featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServer))
expected := &config{ expected := &config{
enabled: true, enabled: true,

View File

@ -34,6 +34,7 @@ import (
"github.com/grafana/grafana/pkg/modules" "github.com/grafana/grafana/pkg/modules"
"github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/registry"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/featuremgmt"
filestorage "github.com/grafana/grafana/pkg/services/grafana-apiserver/storage/file" filestorage "github.com/grafana/grafana/pkg/services/grafana-apiserver/storage/file"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
@ -113,12 +114,13 @@ type service struct {
func ProvideService( func ProvideService(
cfg *setting.Cfg, cfg *setting.Cfg,
features featuremgmt.FeatureToggles,
rr routing.RouteRegister, rr routing.RouteRegister,
authz authorizer.Authorizer, authz authorizer.Authorizer,
tracing *tracing.TracingService, tracing *tracing.TracingService,
) (*service, error) { ) (*service, error) {
s := &service{ s := &service{
config: newConfig(cfg), config: newConfig(cfg, features),
rr: rr, rr: rr,
stopCh: make(chan struct{}), stopCh: make(chan struct{}),
builders: []APIGroupBuilder{}, builders: []APIGroupBuilder{},

View File

@ -38,12 +38,14 @@ type gPRCServerService struct {
logger log.Logger logger log.Logger
server *grpc.Server server *grpc.Server
address string address string
enabled bool
} }
func ProvideService(cfg *setting.Cfg, authenticator interceptors.Authenticator, tracer tracing.Tracer, registerer prometheus.Registerer) (Provider, error) { func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, authenticator interceptors.Authenticator, tracer tracing.Tracer, registerer prometheus.Registerer) (Provider, error) {
s := &gPRCServerService{ s := &gPRCServerService{
cfg: cfg, cfg: cfg,
logger: log.New("grpc-server"), logger: log.New("grpc-server"),
enabled: features.IsEnabled(featuremgmt.FlagGrpcServer),
} }
// Register the metric here instead of an init() function so that we do // Register the metric here instead of an init() function so that we do
@ -123,10 +125,7 @@ func (s *gPRCServerService) Run(ctx context.Context) error {
} }
func (s *gPRCServerService) IsDisabled() bool { func (s *gPRCServerService) IsDisabled() bool {
if s.cfg == nil { return !s.enabled
return true
}
return !s.cfg.IsFeatureToggleEnabled(featuremgmt.FlagGrpcServer)
} }
func (s *gPRCServerService) GetServer() *grpc.Server { func (s *gPRCServerService) GetServer() *grpc.Server {

View File

@ -41,7 +41,6 @@ func NewTestMigrationStore(t *testing.T, sqlStore *sqlstore.SQLStore, cfg *setti
cfg.UnifiedAlerting.BaseInterval = time.Second * 10 cfg.UnifiedAlerting.BaseInterval = time.Second * 10
} }
features := featuremgmt.WithFeatures() features := featuremgmt.WithFeatures()
cfg.IsFeatureToggleEnabled = features.IsEnabled
alertingStore := store.DBstore{ alertingStore := store.DBstore{
SQLStore: sqlStore, SQLStore: sqlStore,
Cfg: cfg.UnifiedAlerting, Cfg: cfg.UnifiedAlerting,

View File

@ -44,6 +44,7 @@ func SetupTestEnv(tb testing.TB, baseInterval time.Duration) (*ngalert.AlertNG,
tb.Helper() tb.Helper()
cfg := setting.NewCfg() cfg := setting.NewCfg()
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled
cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{ cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{
BaseInterval: setting.SchedulerBaseInterval, BaseInterval: setting.SchedulerBaseInterval,

View File

@ -69,10 +69,12 @@ func TestIntegrationPluginManager(t *testing.T) {
features := featuremgmt.WithFeatures() features := featuremgmt.WithFeatures()
cfg := &setting.Cfg{ cfg := &setting.Cfg{
Raw: raw, Raw: raw,
StaticRootPath: staticRootPath, StaticRootPath: staticRootPath,
BundledPluginsPath: bundledPluginsPath, BundledPluginsPath: bundledPluginsPath,
Azure: &azsettings.AzureSettings{}, Azure: &azsettings.AzureSettings{},
// nolint:staticcheck
IsFeatureToggleEnabled: features.IsEnabled, IsFeatureToggleEnabled: features.IsEnabled,
} }

View File

@ -153,7 +153,8 @@ func TestMigrations(t *testing.T) {
{ {
desc: "with editors can admin", desc: "with editors can admin",
config: &setting.Cfg{ config: &setting.Cfg{
EditorsCanAdmin: true, EditorsCanAdmin: true,
// nolint:staticcheck
IsFeatureToggleEnabled: func(key string) bool { return key == "accesscontrol" }, IsFeatureToggleEnabled: func(key string) bool { return key == "accesscontrol" },
Raw: ini.Empty(), Raw: ini.Empty(),
}, },

View File

@ -94,7 +94,9 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
AddExternalAlertmanagerToDatasourceMigration(mg) AddExternalAlertmanagerToDatasourceMigration(mg)
addFolderMigrations(mg) addFolderMigrations(mg)
// nolint:staticcheck
if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil { if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil {
// nolint:staticcheck
if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagExternalServiceAuth) { if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagExternalServiceAuth) {
oauthserver.AddMigration(mg) oauthserver.AddMigration(mg)
} }

View File

@ -68,6 +68,7 @@ func ProvideService(cfg *setting.Cfg, cacheService *localcache.CacheService, mig
return nil, err return nil, err
} }
// nolint:staticcheck
if err := s.Migrate(cfg.IsFeatureToggleEnabled(featuremgmt.FlagMigrationLocking)); err != nil { if err := s.Migrate(cfg.IsFeatureToggleEnabled(featuremgmt.FlagMigrationLocking)); err != nil {
return nil, err return nil, err
} }
@ -306,10 +307,12 @@ func (ss *SQLStore) buildConnectionString() (string, error) {
cnnstr += fmt.Sprintf("&transaction_isolation=%s", val) cnnstr += fmt.Sprintf("&transaction_isolation=%s", val)
} }
// nolint:staticcheck
if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagMysqlAnsiQuotes) || ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagNewDBLibrary) { if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagMysqlAnsiQuotes) || ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagNewDBLibrary) {
cnnstr += "&sql_mode='ANSI_QUOTES'" cnnstr += "&sql_mode='ANSI_QUOTES'"
} }
// nolint:staticcheck
if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagNewDBLibrary) { if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagNewDBLibrary) {
cnnstr += "&parseTime=true" cnnstr += "&parseTime=true"
} }
@ -638,6 +641,7 @@ func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts
// set test db config // set test db config
cfg := setting.NewCfg() cfg := setting.NewCfg()
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = func(key string) bool { cfg.IsFeatureToggleEnabled = func(key string) bool {
for _, enabledFeature := range features { for _, enabledFeature := range features {
if enabledFeature == key { if enabledFeature == key {
@ -732,6 +736,7 @@ func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts
return testSQLStore, nil return testSQLStore, nil
} }
// nolint:staticcheck
testSQLStore.Cfg.IsFeatureToggleEnabled = func(key string) bool { testSQLStore.Cfg.IsFeatureToggleEnabled = func(key string) bool {
for _, enabledFeature := range features { for _, enabledFeature := range features {
if enabledFeature == key { if enabledFeature == key {

View File

@ -144,7 +144,9 @@ func (o *OSSImpl) Section(section string) Section {
func (*OSSImpl) RegisterReloadHandler(string, ReloadHandler) {} func (*OSSImpl) RegisterReloadHandler(string, ReloadHandler) {}
// Deprecated: use feature toggles
func (o *OSSImpl) IsFeatureToggleEnabled(name string) bool { func (o *OSSImpl) IsFeatureToggleEnabled(name string) bool {
// nolint:staticcheck
return o.Cfg.IsFeatureToggleEnabled(name) return o.Cfg.IsFeatureToggleEnabled(name)
} }

View File

@ -361,7 +361,7 @@ type Cfg struct {
ApiKeyMaxSecondsToLive int64 ApiKeyMaxSecondsToLive int64
// Check if a feature toggle is enabled // Check if a feature toggle is enabled
// @deprecated // Deprecated: use featuremgmt.FeatureFlags
IsFeatureToggleEnabled func(key string) bool // filled in dynamically IsFeatureToggleEnabled func(key string) bool // filled in dynamically
AnonymousEnabled bool AnonymousEnabled bool
@ -1168,6 +1168,7 @@ func (cfg *Cfg) Load(args CommandLineArgs) error {
return err return err
} }
// nolint:staticcheck
if err := cfg.readFeatureToggles(iniFile); err != nil { if err := cfg.readFeatureToggles(iniFile); err != nil {
return err return err
} }

View File

@ -8,13 +8,14 @@ import (
"github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util"
) )
// @deprecated -- should use `featuremgmt.FeatureToggles` // Deprecated: should use `featuremgmt.FeatureToggles`
func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error {
section := iniFile.Section("feature_toggles") section := iniFile.Section("feature_toggles")
toggles, err := ReadFeatureTogglesFromInitFile(section) toggles, err := ReadFeatureTogglesFromInitFile(section)
if err != nil { if err != nil {
return err return err
} }
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = func(key string) bool { return toggles[key] } cfg.IsFeatureToggleEnabled = func(key string) bool { return toggles[key] }
return nil return nil
} }