From 48a5c1e8509980b6ffe8cf5c5e486d096eb7058c Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 16 Jan 2024 13:18:25 -0800 Subject: [PATCH] FeatureFlags: Remove the unsupported/undocumented option to read flags from a file (#79959) --- pkg/services/featuremgmt/manager.go | 21 +----------- pkg/services/featuremgmt/service.go | 14 -------- pkg/services/featuremgmt/settings.go | 34 ------------------- pkg/services/featuremgmt/settings_test.go | 25 -------------- .../featuremgmt/testdata/features.yaml | 33 ------------------ .../featuremgmt/testdata/included.yaml | 13 ------- 6 files changed, 1 insertion(+), 139 deletions(-) delete mode 100644 pkg/services/featuremgmt/settings.go delete mode 100644 pkg/services/featuremgmt/settings_test.go delete mode 100644 pkg/services/featuremgmt/testdata/features.yaml delete mode 100644 pkg/services/featuremgmt/testdata/included.yaml diff --git a/pkg/services/featuremgmt/manager.go b/pkg/services/featuremgmt/manager.go index 82cccf06360..9482bbde01c 100644 --- a/pkg/services/featuremgmt/manager.go +++ b/pkg/services/featuremgmt/manager.go @@ -19,9 +19,7 @@ type FeatureManager struct { allowEditing bool licensing licensing.Licensing flags map[string]*FeatureFlag - enabled map[string]bool // only the "on" values - config string // path to config file - vars map[string]any + enabled map[string]bool // only the "on" values startup map[string]bool // the explicit values registered at startup warnings map[string]string // potential warnings about the flag log log.Logger @@ -112,23 +110,6 @@ func (fm *FeatureManager) update() { fm.enabled = enabled } -// Run is called by background services -func (fm *FeatureManager) readFile() error { - if fm.config == "" { - return nil // not configured - } - - cfg, err := readConfigFile(fm.config) - if err != nil { - return err - } - - fm.registerFlags(cfg.Flags...) - fm.vars = cfg.Vars - - return nil -} - // IsEnabled checks if a feature is enabled func (fm *FeatureManager) IsEnabled(ctx context.Context, flag string) bool { return fm.enabled[flag] diff --git a/pkg/services/featuremgmt/service.go b/pkg/services/featuremgmt/service.go index e424779f9f2..3b654b9cfcf 100644 --- a/pkg/services/featuremgmt/service.go +++ b/pkg/services/featuremgmt/service.go @@ -1,9 +1,6 @@ package featuremgmt import ( - "os" - "path/filepath" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -59,17 +56,6 @@ func ProvideManagerService(cfg *setting.Cfg, licensing licensing.Licensing) (*Fe mgmt.startup[key] = val } - // Load config settings - configfile := filepath.Join(cfg.HomePath, "conf", "features.yaml") - if _, err := os.Stat(configfile); err == nil { - mgmt.log.Info("[experimental] loading features from config file", "path", configfile) - mgmt.config = configfile - err = mgmt.readFile() - if err != nil { - return mgmt, err - } - } - // update the values mgmt.update() diff --git a/pkg/services/featuremgmt/settings.go b/pkg/services/featuremgmt/settings.go deleted file mode 100644 index 3c33979e2d7..00000000000 --- a/pkg/services/featuremgmt/settings.go +++ /dev/null @@ -1,34 +0,0 @@ -package featuremgmt - -import ( - "os" - - "gopkg.in/yaml.v3" -) - -type configBody struct { - // define variables that can be used in expressions - Vars map[string]any `yaml:"vars"` - - // Define and override feature flag properties - Flags []FeatureFlag `yaml:"flags"` - - // keep track of where the fie was loaded from - filename string -} - -// will read a single configfile -func readConfigFile(filename string) (*configBody, error) { - cfg := &configBody{} - - // Can ignore gosec G304 because the file path is forced within config subfolder - //nolint:gosec - yamlFile, err := os.ReadFile(filename) - if err != nil { - return cfg, err - } - - err = yaml.Unmarshal(yamlFile, cfg) - cfg.filename = filename - return cfg, err -} diff --git a/pkg/services/featuremgmt/settings_test.go b/pkg/services/featuremgmt/settings_test.go deleted file mode 100644 index 7b8e6c83b42..00000000000 --- a/pkg/services/featuremgmt/settings_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package featuremgmt - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" -) - -func TestReadingFeatureSettings(t *testing.T) { - config, err := readConfigFile("testdata/features.yaml") - require.NoError(t, err, "No error when reading feature configs") - - assert.Equal(t, map[string]any{ - "level": "free", - "stack": "something", - "valA": "value from features.yaml", - }, config.Vars) - - out, err := yaml.Marshal(config) - require.NoError(t, err) - fmt.Printf("%s", string(out)) -} diff --git a/pkg/services/featuremgmt/testdata/features.yaml b/pkg/services/featuremgmt/testdata/features.yaml deleted file mode 100644 index dd737494580..00000000000 --- a/pkg/services/featuremgmt/testdata/features.yaml +++ /dev/null @@ -1,33 +0,0 @@ -include: - - included.yaml # not yet supported - -vars: - stack: something - level: free - valA: value from features.yaml - -flags: - - name: feature1 - description: feature1 - expression: "false" - - - name: feature3 - description: feature3 - expression: "true" - - - name: feature3 - description: feature3 - expression: env.level == 'free' - - - name: displaySwedishTheme - description: enable swedish background theme - expression: | - // restrict to users allowing swedish language - req.locale.contains("sv") - - name: displayFrenchFlag - description: sho background theme - expression: | - // only admins - user.id == 1 - // show to users allowing french language - && req.locale.contains("fr") \ No newline at end of file diff --git a/pkg/services/featuremgmt/testdata/included.yaml b/pkg/services/featuremgmt/testdata/included.yaml deleted file mode 100644 index 322b5c7972f..00000000000 --- a/pkg/services/featuremgmt/testdata/included.yaml +++ /dev/null @@ -1,13 +0,0 @@ -include: - - features.yaml # make sure we avoid recusion! - -# variables that can be used in expressions -vars: - stack: something - deep: 1 - valA: value from included.yaml - -flags: - - name: featureFromIncludedFile - description: an inlcuded file - expression: invalid expression string here