IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

FeatureFlags: Avoid using cfg.IsFeatureToggleEnabled (#81407)

Browse Source
This commit is contained in:
Ryan McKinley
2024-01-28 15:22:45 -08:00
committed by GitHub
parent 1a1531ca5e
commit 1fab107e79
21 changed files with 248 additions and 387 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
pkg/services/sqlstore/database_config.go
View File

@@ -10,6 +10,7 @@ import (
"strings"
"github.com/go-sql-driver/mysql"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/setting"
@@ -45,7 +46,7 @@ type DatabaseConfig struct {
TransactionRetries int
}
func NewDatabaseConfig(cfg *setting.Cfg) (*DatabaseConfig, error) {
func NewDatabaseConfig(cfg *setting.Cfg, features featuremgmt.FeatureToggles) (*DatabaseConfig, error) {
if cfg == nil {
return nil, errors.New("cfg cannot be nil")
}
@@ -55,7 +56,7 @@ func NewDatabaseConfig(cfg *setting.Cfg) (*DatabaseConfig, error) {
return nil, err
}
if err := dbCfg.buildConnectionString(cfg); err != nil {
if err := dbCfg.buildConnectionString(cfg, features); err != nil {
return nil, err
}
@@ -120,7 +121,7 @@ func (dbCfg *DatabaseConfig) readConfig(cfg *setting.Cfg) error {
return nil
}
func (dbCfg *DatabaseConfig) buildConnectionString(cfg *setting.Cfg) error {
func (dbCfg *DatabaseConfig) buildConnectionString(cfg *setting.Cfg, features featuremgmt.FeatureToggles) error {
if dbCfg.ConnectionString != "" {
return nil
}
@@ -154,8 +155,7 @@ func (dbCfg *DatabaseConfig) buildConnectionString(cfg *setting.Cfg) error {
cnnstr += fmt.Sprintf("&transaction_isolation=%s", val)
}
// nolint:staticcheck
if cfg.IsFeatureToggleEnabled(featuremgmt.FlagMysqlAnsiQuotes) {
if features != nil && features.IsEnabledGlobally(featuremgmt.FlagMysqlAnsiQuotes) {
cnnstr += "&sql_mode='ANSI_QUOTES'"
}

7
pkg/services/sqlstore/database_config_test.go
View File

@@ -5,10 +5,11 @@ import (
"net/url"
"testing"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
)
type databaseConfigTest struct {
@@ -109,7 +110,7 @@ func TestIntegrationSQLConnectionString(t *testing.T) {
for _, testCase := range databaseConfigTestCases {
t.Run(testCase.name, func(t *testing.T) {
cfg := makeDatabaseTestConfig(t, testCase)
dbCfg, err := NewDatabaseConfig(cfg)
dbCfg, err := NewDatabaseConfig(cfg, testCase.features)
require.Equal(t, testCase.err, err)
if testCase.expConnStr != "" {
assert.Equal(t, testCase.expConnStr, dbCfg.ConnectionString)

7
pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go
View File

@@ -154,9 +154,7 @@ func TestMigrations(t *testing.T) {
desc: "with editors can admin",
config: &setting.Cfg{
EditorsCanAdmin: true,
// nolint:staticcheck
IsFeatureToggleEnabled: func(key string) bool { return key == "accesscontrol" },
Raw: ini.Empty(),
Raw: ini.Empty(),
},
expectedRolePerms: map[string][]rawPermission{
"managed:users:1:permissions": {{Action: "teams:read", Scope: team1Scope}},
@@ -262,9 +260,6 @@ func setupTestDB(t *testing.T) *xorm.Engine {
mg := migrator.NewMigrator(x, &setting.Cfg{
Logger: log.New("acmigration.test"),
Raw: ini.Empty(),
IsFeatureToggleEnabled: func(key string) bool {
return true
},
})
migrations := &migrations.OSSMigrations{}
migrations.AddMigration(mg)

23
pkg/services/sqlstore/migrations/migrations.go
View File

@@ -21,13 +21,14 @@ import (
// specifically added
type OSSMigrations struct {
features featuremgmt.FeatureToggles
}
func ProvideOSSMigrations() *OSSMigrations {
return &OSSMigrations{}
func ProvideOSSMigrations(features featuremgmt.FeatureToggles) *OSSMigrations {
return &OSSMigrations{features}
}
func (*OSSMigrations) AddMigration(mg *Migrator) {
func (oss *OSSMigrations) AddMigration(mg *Migrator) {
mg.AddCreateMigration()
addUserMigrations(mg)
addTempUserMigrations(mg)
@@ -94,12 +95,8 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
AddExternalAlertmanagerToDatasourceMigration(mg)
addFolderMigrations(mg)
// nolint:staticcheck
if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil {
// nolint:staticcheck
if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagExternalServiceAuth) {
oauthserver.AddMigration(mg)
}
if oss.features != nil && oss.features.IsEnabledGlobally(featuremgmt.FlagExternalServiceAuth) {
oauthserver.AddMigration(mg)
}
anonservice.AddMigration(mg)
@@ -115,12 +112,8 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
ualert.CreateOrgMigratedKVStoreEntries(mg)
// https://github.com/grafana/identity-access-team/issues/546: tracks removal of the feature toggle from the annotation permission migration
// nolint:staticcheck
if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil {
// nolint:staticcheck
if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAnnotationPermissionUpdate) {
accesscontrol.AddManagedDashboardAnnotationActionsMigration(mg)
}
if oss.features != nil && oss.features.IsEnabledGlobally(featuremgmt.FlagAnnotationPermissionUpdate) {
accesscontrol.AddManagedDashboardAnnotationActionsMigration(mg)
}
addKVStoreMySQLValueTypeLongTextMigration(mg)

74
pkg/services/sqlstore/sqlstore.go
View File

@@ -38,6 +38,7 @@ type ContextSessionKey struct{}
type SQLStore struct {
Cfg *setting.Cfg
features featuremgmt.FeatureToggles
sqlxsession *session.SessionDB
bus bus.Bus
@@ -52,7 +53,10 @@ type SQLStore struct {
recursiveQueriesMu sync.Mutex
}
func ProvideService(cfg *setting.Cfg, migrations registry.DatabaseMigrator, bus bus.Bus, tracer tracing.Tracer) (*SQLStore, error) {
func ProvideService(cfg *setting.Cfg,
features featuremgmt.FeatureToggles,
migrations registry.DatabaseMigrator,
bus bus.Bus, tracer tracing.Tracer) (*SQLStore, error) {
// This change will make xorm use an empty default schema for postgres and
// by that mimic the functionality of how it was functioning before
// xorm's changes above.
@@ -61,9 +65,9 @@ func ProvideService(cfg *setting.Cfg, migrations registry.DatabaseMigrator, bus
if err != nil {
return nil, err
}
s.features = features
// nolint:staticcheck
if err := s.Migrate(cfg.IsFeatureToggleEnabled(featuremgmt.FlagMigrationLocking)); err != nil {
if err := s.Migrate(features.IsEnabledGlobally(featuremgmt.FlagMigrationLocking)); err != nil {
return nil, err
}
@@ -87,8 +91,8 @@ func ProvideService(cfg *setting.Cfg, migrations registry.DatabaseMigrator, bus
return s, nil
}
func ProvideServiceForTests(cfg *setting.Cfg, migrations registry.DatabaseMigrator) (*SQLStore, error) {
return initTestDB(cfg, migrations, InitTestDBOpt{EnsureDefaultOrgAndUser: true})
func ProvideServiceForTests(cfg *setting.Cfg, features featuremgmt.FeatureToggles, migrations registry.DatabaseMigrator) (*SQLStore, error) {
return initTestDB(cfg, features, migrations, InitTestDBOpt{EnsureDefaultOrgAndUser: true})
}
func newSQLStore(cfg *setting.Cfg, engine *xorm.Engine,
@@ -239,7 +243,7 @@ func (ss *SQLStore) initEngine(engine *xorm.Engine) error {
return nil
}
dbCfg, err := NewDatabaseConfig(ss.Cfg)
dbCfg, err := NewDatabaseConfig(ss.Cfg, ss.features)
if err != nil {
return err
}
@@ -402,15 +406,11 @@ type InitTestDBOpt struct {
FeatureFlags []string
}
var featuresEnabledDuringTests = []string{
featuremgmt.FlagPanelTitleSearch,
featuremgmt.FlagUnifiedStorage,
}
// InitTestDBWithMigration initializes the test DB given custom migrations.
func InitTestDBWithMigration(t ITestDB, migration registry.DatabaseMigrator, opts ...InitTestDBOpt) *SQLStore {
t.Helper()
store, err := initTestDB(setting.NewCfg(), migration, opts...)
features := getFeaturesForTesting(opts...)
store, err := initTestDB(setting.NewCfg(), features, migration, opts...)
if err != nil {
t.Fatalf("failed to initialize sql store: %s", err)
}
@@ -420,7 +420,9 @@ func InitTestDBWithMigration(t ITestDB, migration registry.DatabaseMigrator, opt
// InitTestDB initializes the test DB.
func InitTestDB(t ITestDB, opts ...InitTestDBOpt) *SQLStore {
t.Helper()
store, err := initTestDB(setting.NewCfg(), &migrations.OSSMigrations{}, opts...)
features := getFeaturesForTesting(opts...)
store, err := initTestDB(setting.NewCfg(), features, migrations.ProvideOSSMigrations(features), opts...)
if err != nil {
t.Fatalf("failed to initialize sql store: %s", err)
}
@@ -432,8 +434,26 @@ func InitTestDBWithCfg(t ITestDB, opts ...InitTestDBOpt) (*SQLStore, *setting.Cf
return store, store.Cfg
}
func getFeaturesForTesting(opts ...InitTestDBOpt) featuremgmt.FeatureToggles {
featureKeys := []any{
featuremgmt.FlagPanelTitleSearch,
featuremgmt.FlagUnifiedStorage,
}
for _, opt := range opts {
if len(opt.FeatureFlags) > 0 {
for _, f := range opt.FeatureFlags {
featureKeys = append(featureKeys, f)
}
}
}
return featuremgmt.WithFeatures(featureKeys...)
}
//nolint:gocyclo
func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts ...InitTestDBOpt) (*SQLStore, error) {
func initTestDB(testCfg *setting.Cfg,
features featuremgmt.FeatureToggles,
migration registry.DatabaseMigrator,
opts ...InitTestDBOpt) (*SQLStore, error) {
testSQLStoreMutex.Lock()
defer testSQLStoreMutex.Unlock()
@@ -441,14 +461,6 @@ func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts
opts = []InitTestDBOpt{{EnsureDefaultOrgAndUser: false, FeatureFlags: []string{}}}
}
features := make([]string, len(featuresEnabledDuringTests))
copy(features, featuresEnabledDuringTests)
for _, opt := range opts {
if len(opt.FeatureFlags) > 0 {
features = append(features, opt.FeatureFlags...)
}
}
if testSQLStore == nil {
dbType := migrator.SQLite
@@ -460,14 +472,7 @@ func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts
// set test db config
cfg := setting.NewCfg()
// nolint:staticcheck
cfg.IsFeatureToggleEnabled = func(key string) bool {
for _, enabledFeature := range features {
if enabledFeature == key {
return true
}
}
return false
}
cfg.IsFeatureToggleEnabled = features.IsEnabledGlobally
sec, err := cfg.Raw.NewSection("database")
if err != nil {
@@ -555,14 +560,7 @@ func initTestDB(testCfg *setting.Cfg, migration registry.DatabaseMigrator, opts
}
// nolint:staticcheck
testSQLStore.Cfg.IsFeatureToggleEnabled = func(key string) bool {
for _, enabledFeature := range features {
if enabledFeature == key {
return true
}
}
return false
}
testSQLStore.Cfg.IsFeatureToggleEnabled = features.IsEnabledGlobally
if err := testSQLStore.Dialect.TruncateDBTables(testSQLStore.GetEngine()); err != nil {
return nil, err