diff --git a/pkg/services/featuremgmt/models.go b/pkg/services/featuremgmt/models.go index 596cfcb2491..d59dff63c37 100644 --- a/pkg/services/featuremgmt/models.go +++ b/pkg/services/featuremgmt/models.go @@ -126,63 +126,6 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error { return nil } -type FeatureFlagType int - -const ( - Boolean FeatureFlagType = iota - Integer - Float - String - Structure -) - -func (t FeatureFlagType) String() string { - switch t { - case Boolean: - return "boolean" - case Integer: - return "integer" - case Float: - return "float" - case String: - return "string" - case Structure: - return "structure" - } - - return "unknown" -} - -// MarshalJSON marshals the enum as a quoted json string -func (t FeatureFlagType) MarshalJSON() ([]byte, error) { - buffer := bytes.NewBufferString(`"`) - buffer.WriteString(t.String()) - buffer.WriteString(`"`) - return buffer.Bytes(), nil -} - -func (t *FeatureFlagType) UnmarshalJSON(b []byte) error { - var j string - err := json.Unmarshal(b, &j) - if err != nil { - return err - } - - switch j { - case "boolean": - *t = Boolean - case "integer": - *t = Integer - case "float": - *t = Float - case "string": - *t = String - case "structure": - *t = Structure - } - return nil -} - // These are properties about the feature, but not the current state or value for it type FeatureFlag struct { Name string `json:"name" yaml:"name"` // Unique name @@ -192,8 +135,6 @@ type FeatureFlag struct { // CEL-GO expression. Using the value "true" will mean this is on by default Expression string `json:"expression,omitempty"` - // Type of the feature flag (boolean, number, string, structure), - Type FeatureFlagType `json:"type,omitempty"` // Special behavior properties RequiresDevMode bool `json:"requiresDevMode,omitempty"` // can not be enabled in production diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index b4da574c65b..8364f1c1a74 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -11,6 +11,7 @@ import ( sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" ) const ( @@ -26,7 +27,7 @@ type OpenFeatureConfig struct { // HTTPClient is a pre-configured HTTP client (optional, used by features-service + OFREP providers) HTTPClient *http.Client // StaticFlags are the feature flags to use with static provider - StaticFlags map[string]setting.FeatureToggle + StaticFlags map[string]memprovider.InMemoryFlag // TargetingKey is used for evaluation context TargetingKey string // ContextAttrs are additional attributes for evaluation context @@ -100,7 +101,7 @@ func InitOpenFeatureWithCfg(cfg *setting.Cfg) error { func createProvider( providerType string, u *url.URL, - staticFlags map[string]setting.FeatureToggle, + staticFlags map[string]memprovider.InMemoryFlag, httpClient *http.Client, ) (openfeature.FeatureProvider, error) { if providerType == setting.FeaturesServiceProviderType || providerType == setting.OFREPProviderType { diff --git a/pkg/services/featuremgmt/service.go b/pkg/services/featuremgmt/service.go index fd7ddbd91cf..2c97666d9e9 100644 --- a/pkg/services/featuremgmt/service.go +++ b/pkg/services/featuremgmt/service.go @@ -47,7 +47,8 @@ func ProvideManagerService(cfg *setting.Cfg) (*FeatureManager, error) { } mgmt.warnings[key] = "unknown flag in config" } - mgmt.startup[key] = val.Value == true + + mgmt.startup[key] = val.Variants[val.DefaultVariant] == true } // update the values diff --git a/pkg/services/featuremgmt/static_provider.go b/pkg/services/featuremgmt/static_provider.go index 1d59561bc2b..f6fe14d7de9 100644 --- a/pkg/services/featuremgmt/static_provider.go +++ b/pkg/services/featuremgmt/static_provider.go @@ -1,13 +1,13 @@ package featuremgmt import ( - "encoding/json" "fmt" - "strconv" + "maps" - "github.com/grafana/grafana/pkg/setting" "github.com/open-feature/go-sdk/openfeature" "github.com/open-feature/go-sdk/openfeature/memprovider" + + "github.com/grafana/grafana/pkg/setting" ) // inMemoryBulkProvider is a wrapper around memprovider.InMemoryProvider that @@ -33,76 +33,21 @@ func (p *inMemoryBulkProvider) ListFlags() ([]string, error) { return keys, nil } -func newStaticProvider(confFlags map[string]setting.FeatureToggle, standardFlags []FeatureFlag) (openfeature.FeatureProvider, error) { +func newStaticProvider(confFlags map[string]memprovider.InMemoryFlag, standardFlags []FeatureFlag) (openfeature.FeatureProvider, error) { flags := make(map[string]memprovider.InMemoryFlag, len(standardFlags)) - index := make(map[string]FeatureFlag, len(standardFlags)) - // Add standard flags + + // Parse and add standard flags for _, flag := range standardFlags { - inMemFlag, err := createTypedFlag(flag) + inMemFlag, err := setting.ParseFlag(flag.Name, flag.Expression) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse flag %s: %w", flag.Name, err) } flags[flag.Name] = inMemFlag - index[flag.Name] = flag } // Add flags from config.ini file - for name, flag := range confFlags { - standard, exists := index[flag.Name] - - // Fail fast if a flag is declared with a mismatched type - if exists && standard.Type.String() != string(flag.Type) { - return nil, fmt.Errorf("type mismatch for flag '%s' detected", flag.Name) - } - - flags[name] = createInMemoryFlag(flag) - } + maps.Copy(flags, confFlags) return newInMemoryBulkProvider(flags), nil } - -func createInMemoryFlag(flag setting.FeatureToggle) memprovider.InMemoryFlag { - variant := "default" - - return memprovider.InMemoryFlag{ - Key: flag.Name, - DefaultVariant: variant, - Variants: map[string]any{ - variant: flag.Value, - }, - } -} - -func createTypedFlag(flag FeatureFlag) (memprovider.InMemoryFlag, error) { - defaultVariant := "default" - - var value any - var err error - switch flag.Type { - case Boolean: - value = flag.Expression == "true" - case Integer: - value, err = strconv.Atoi(flag.Expression) - case Float: - value, err = strconv.ParseFloat(flag.Expression, 64) - case String: - value = flag.Expression - case Structure: - err = json.Unmarshal([]byte(flag.Expression), &value) - default: - return memprovider.InMemoryFlag{}, fmt.Errorf("unsupported flag type %s", flag.Type) - } - - if err != nil { - return memprovider.InMemoryFlag{}, err - } - - return memprovider.InMemoryFlag{ - Key: flag.Name, - DefaultVariant: defaultVariant, - Variants: map[string]any{ - defaultVariant: value, - }, - }, nil -} diff --git a/pkg/services/featuremgmt/static_provider_test.go b/pkg/services/featuremgmt/static_provider_test.go index b314475c244..8032d73166f 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/setting" "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -94,22 +95,6 @@ ABCD = true assert.Equal(t, openFeatureEnabledFlags, enabledFeatureManager) } -func Test_StaticProvider_FailfastOnMismatchedType(t *testing.T) { - staticFlags := map[string]setting.FeatureToggle{"oldBooleanFlag": { - Type: setting.Boolean, - Name: "oldBooleanFlag", - Value: true, - }} - - flag := FeatureFlag{ - Name: "oldBooleanFlag", - Expression: "1.0", - Type: Float, - } - _, err := newStaticProvider(staticFlags, []FeatureFlag{flag}) - assert.EqualError(t, err, "type mismatch for flag 'oldBooleanFlag' detected") -} - func Test_StaticProvider_TypedFlags(t *testing.T) { tests := []struct { flags FeatureFlag @@ -120,7 +105,6 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { flags: FeatureFlag{ Name: "Flag", Expression: "true", - Type: Boolean, }, defaultValue: false, expectedValue: true, @@ -129,7 +113,6 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { flags: FeatureFlag{ Name: "Flag", Expression: "1.0", - Type: Float, }, defaultValue: 0.0, expectedValue: 1.0, @@ -138,7 +121,6 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { flags: FeatureFlag{ Name: "Flag", Expression: "blue", - Type: String, }, defaultValue: "red", expectedValue: "blue", @@ -147,7 +129,6 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { flags: FeatureFlag{ Name: "Flag", Expression: "1", - Type: Integer, }, defaultValue: int64(0), expectedValue: int64(1), @@ -156,9 +137,7 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { flags: FeatureFlag{ Name: "Flag", Expression: `{ "foo": "bar" }`, - Type: Structure, }, - defaultValue: nil, expectedValue: map[string]any{"foo": "bar"}, }, } @@ -168,16 +147,16 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { assert.NoError(t, err) var result any - switch tt.flags.Type { - case Boolean: + switch tt.expectedValue.(type) { + case bool: result = provider.BooleanEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(bool), openfeature.FlattenedContext{}).Value - case Float: + case float64: result = provider.FloatEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(float64), openfeature.FlattenedContext{}).Value - case String: + case string: result = provider.StringEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(string), openfeature.FlattenedContext{}).Value - case Integer: + case int64: result = provider.IntEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(int64), openfeature.FlattenedContext{}).Value - case Structure: + case map[string]any: result = provider.ObjectEvaluation(t.Context(), tt.flags.Name, tt.defaultValue, openfeature.FlattenedContext{}).Value } @@ -187,37 +166,31 @@ func Test_StaticProvider_TypedFlags(t *testing.T) { func Test_StaticProvider_ConfigOverride(t *testing.T) { tests := []struct { name string - typ FeatureFlagType originalValue string configValue any }{ { name: "bool", - typ: Boolean, originalValue: "false", configValue: true, }, { name: "int", - typ: Integer, originalValue: "0", configValue: int64(1), }, { name: "float", - typ: Float, originalValue: "0.0", configValue: 1.0, }, { name: "string", - typ: String, originalValue: "foo", configValue: "bar", }, { name: "structure", - typ: Structure, originalValue: "{}", configValue: make(map[string]any), }, @@ -229,16 +202,16 @@ func Test_StaticProvider_ConfigOverride(t *testing.T) { assert.NoError(t, err) var result any - switch tt.typ { - case Boolean: + switch tt.configValue.(type) { + case bool: result = provider.BooleanEvaluation(t.Context(), tt.name, false, openfeature.FlattenedContext{}).Value - case Float: + case float64: result = provider.FloatEvaluation(t.Context(), tt.name, 0.0, openfeature.FlattenedContext{}).Value - case String: + case string: result = provider.StringEvaluation(t.Context(), tt.name, "foo", openfeature.FlattenedContext{}).Value - case Integer: + case int64: result = provider.IntEvaluation(t.Context(), tt.name, 1, openfeature.FlattenedContext{}).Value - case Structure: + case map[string]any: result = provider.ObjectEvaluation(t.Context(), tt.name, make(map[string]any), openfeature.FlattenedContext{}).Value } @@ -248,21 +221,18 @@ func Test_StaticProvider_ConfigOverride(t *testing.T) { func makeFlags(tt struct { name string - typ FeatureFlagType originalValue string configValue any -}) (map[string]setting.FeatureToggle, []FeatureFlag) { +}) (map[string]memprovider.InMemoryFlag, []FeatureFlag) { orig := FeatureFlag{ Name: tt.name, Expression: tt.originalValue, - Type: tt.typ, } - config := map[string]setting.FeatureToggle{ + config := map[string]memprovider.InMemoryFlag{ tt.name: { - Name: tt.name, - Type: setting.FeatureFlagType(tt.typ.String()), - Value: tt.configValue, + Key: tt.name, + Variants: map[string]any{"": tt.configValue}, }, } diff --git a/pkg/services/updatemanager/plugins_test.go b/pkg/services/updatemanager/plugins_test.go index 1560a5725dd..93b3d54b321 100644 --- a/pkg/services/updatemanager/plugins_test.go +++ b/pkg/services/updatemanager/plugins_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" @@ -378,8 +379,10 @@ func setupOpenFeatureProvider(t *testing.T, flagValue bool) { err := featuremgmt.InitOpenFeature(featuremgmt.OpenFeatureConfig{ ProviderType: setting.StaticProviderType, - StaticFlags: map[string]setting.FeatureToggle{ - featuremgmt.FlagPluginsAutoUpdate: {Value: flagValue, Type: setting.Boolean}, + StaticFlags: map[string]memprovider.InMemoryFlag{ + featuremgmt.FlagPluginsAutoUpdate: { + Key: featuremgmt.FlagPluginsAutoUpdate, Variants: map[string]any{"": flagValue}, + }, }, }) require.NoError(t, err) diff --git a/pkg/setting/setting_feature_toggles.go b/pkg/setting/setting_feature_toggles.go index 8231317be15..33c12136bae 100644 --- a/pkg/setting/setting_feature_toggles.go +++ b/pkg/setting/setting_feature_toggles.go @@ -2,30 +2,15 @@ package setting import ( "encoding/json" - "fmt" "strconv" "gopkg.in/ini.v1" + "github.com/open-feature/go-sdk/openfeature/memprovider" + "github.com/grafana/grafana/pkg/util" ) -type FeatureFlagType string - -const ( - Structure FeatureFlagType = "structure" - Integer FeatureFlagType = "integer" - Float FeatureFlagType = "float" - Boolean FeatureFlagType = "boolean" - String FeatureFlagType = "string" -) - -type FeatureToggle struct { - Type FeatureFlagType `json:"type"` - Name string `json:"name"` - Value any `json:"value"` -} - // Deprecated: should use `featuremgmt.FeatureToggles` func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { section := iniFile.Section("feature_toggles") @@ -41,22 +26,19 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { return false } - return toggle.Type == Boolean && toggle.Value == true + val, ok := toggle.Variants[toggle.DefaultVariant].(bool) + return ok && val } return nil } -func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]FeatureToggle, error) { - featureToggles := make(map[string]FeatureToggle, 10) +func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]memprovider.InMemoryFlag, error) { + featureToggles := make(map[string]memprovider.InMemoryFlag, 10) // parse the comma separated list in `enable`. featuresTogglesStr := valueAsString(featureTogglesSection, "enable", "") for _, feature := range util.SplitString(featuresTogglesStr) { - featureToggles[feature] = FeatureToggle{ - Type: Boolean, - Name: feature, - Value: true, - } + featureToggles[feature] = memprovider.InMemoryFlag{Key: feature, Variants: map[string]any{"": true}} } // read all other settings under [feature_toggles]. If a toggle is @@ -71,31 +53,28 @@ func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[str return featureToggles, err } - flag, exists := featureToggles[v.Name()] - if exists && flag.Type != b.Type { - return nil, fmt.Errorf("type mismatch during flag declaration '%s': %s, %s", v.Name(), flag.Type, b.Type) - } - featureToggles[v.Name()] = b } return featureToggles, nil } -func ParseFlag(name, value string) (FeatureToggle, error) { - var structure map[string]any - +func ParseFlag(name, value string) (memprovider.InMemoryFlag, error) { if integer, err := strconv.Atoi(value); err == nil { - return FeatureToggle{Type: Integer, Name: name, Value: integer}, nil - } - if float, err := strconv.ParseFloat(value, 64); err == nil { - return FeatureToggle{Type: Float, Name: name, Value: float}, nil - } - if err := json.Unmarshal([]byte(value), &structure); err == nil { - return FeatureToggle{Type: Structure, Name: name, Value: structure}, nil - } - if boolean, err := strconv.ParseBool(value); err == nil { - return FeatureToggle{Type: Boolean, Name: name, Value: boolean}, nil + return memprovider.InMemoryFlag{Key: name, Variants: map[string]any{"": integer}}, nil } - return FeatureToggle{Type: String, Name: name, Value: value}, nil + if float, err := strconv.ParseFloat(value, 64); err == nil { + return memprovider.InMemoryFlag{Key: name, Variants: map[string]any{"": float}}, nil + } + + var structure map[string]any + if err := json.Unmarshal([]byte(value), &structure); err == nil { + return memprovider.InMemoryFlag{Key: name, Variants: map[string]any{"": structure}}, nil + } + + if boolean, err := strconv.ParseBool(value); err == nil { + return memprovider.InMemoryFlag{Key: name, Variants: map[string]any{"": boolean}}, nil + } + + return memprovider.InMemoryFlag{Key: name, Variants: map[string]any{"": value}}, nil } diff --git a/pkg/setting/setting_feature_toggles_test.go b/pkg/setting/setting_feature_toggles_test.go index bd8adf73440..9904e4c5ccd 100644 --- a/pkg/setting/setting_feature_toggles_test.go +++ b/pkg/setting/setting_feature_toggles_test.go @@ -1,9 +1,9 @@ package setting import ( - "errors" "testing" + "github.com/open-feature/go-sdk/openfeature/memprovider" "github.com/stretchr/testify/require" "gopkg.in/ini.v1" ) @@ -12,17 +12,16 @@ func TestFeatureToggles(t *testing.T) { testCases := []struct { name string conf map[string]string - err error - expectedToggles map[string]FeatureToggle + expectedToggles map[string]memprovider.InMemoryFlag }{ { name: "can parse feature toggles passed in the `enable` array", conf: map[string]string{ "enable": "feature1,feature2", }, - expectedToggles: map[string]FeatureToggle{ - "feature1": {Name: "feature1", Type: Boolean, Value: true}, - "feature2": {Name: "feature2", Type: Boolean, Value: true}, + expectedToggles: map[string]memprovider.InMemoryFlag{ + "feature1": {Key: "feature1", Variants: map[string]any{"": true}}, + "feature2": {Key: "feature2", Variants: map[string]any{"": true}}, }, }, { @@ -31,10 +30,10 @@ func TestFeatureToggles(t *testing.T) { "enable": "feature1,feature2", "feature3": "true", }, - expectedToggles: map[string]FeatureToggle{ - "feature1": {Name: "feature1", Type: Boolean, Value: true}, - "feature2": {Name: "feature2", Type: Boolean, Value: true}, - "feature3": {Name: "feature3", Type: Boolean, Value: true}, + expectedToggles: map[string]memprovider.InMemoryFlag{ + "feature1": {Key: "feature1", Variants: map[string]any{"": true}}, + "feature2": {Key: "feature2", Variants: map[string]any{"": true}}, + "feature3": {Key: "feature3", Variants: map[string]any{"": true}}, }, }, { @@ -43,20 +42,11 @@ func TestFeatureToggles(t *testing.T) { "enable": "feature1,feature2", "feature2": "false", }, - expectedToggles: map[string]FeatureToggle{ - "feature1": {Name: "feature1", Type: Boolean, Value: true}, - "feature2": {Name: "feature2", Type: Boolean, Value: false}, + expectedToggles: map[string]memprovider.InMemoryFlag{ + "feature1": {Key: "feature1", Variants: map[string]any{"": true}}, + "feature2": {Key: "feature2", Variants: map[string]any{"": false}}, }, }, - { - name: "conflict in type declaration is be detected", - conf: map[string]string{ - "enable": "feature1,feature2", - "feature2": "invalid", - }, - expectedToggles: map[string]FeatureToggle{}, - err: errors.New("type mismatch during flag declaration 'feature2': boolean, string"), - }, { name: "type of the feature flag is handled correctly", conf: map[string]string{ @@ -64,13 +54,13 @@ func TestFeatureToggles(t *testing.T) { "feature3": `{"foo":"bar"}`, "feature4": "bar", "feature5": "t", "feature6": "T", }, - expectedToggles: map[string]FeatureToggle{ - "feature1": {Name: "feature1", Type: Integer, Value: 1}, - "feature2": {Name: "feature2", Type: Float, Value: 1.0}, - "feature3": {Name: "feature3", Type: Structure, Value: map[string]any{"foo": "bar"}}, - "feature4": {Name: "feature4", Type: String, Value: "bar"}, - "feature5": {Name: "feature5", Type: Boolean, Value: true}, - "feature6": {Name: "feature6", Type: Boolean, Value: true}, + expectedToggles: map[string]memprovider.InMemoryFlag{ + "feature1": {Key: "feature1", Variants: map[string]any{"": 1}}, + "feature2": {Key: "feature2", Variants: map[string]any{"": 1.0}}, + "feature3": {Key: "feature3", Variants: map[string]any{"": map[string]any{"foo": "bar"}}}, + "feature4": {Key: "feature4", Variants: map[string]any{"": "bar"}}, + "feature5": {Key: "feature5", Variants: map[string]any{"": true}}, + "feature6": {Key: "feature6", Variants: map[string]any{"": true}}, }, }, } @@ -85,15 +75,11 @@ func TestFeatureToggles(t *testing.T) { } featureToggles, err := ReadFeatureTogglesFromInitFile(toggles) - if tc.err != nil { - require.EqualError(t, err, tc.err.Error()) - } + require.NoError(t, err) - if err == nil { - for k, v := range featureToggles { - toggle := tc.expectedToggles[k] - require.Equal(t, toggle, v, tc.name) - } + for k, v := range featureToggles { + toggle := tc.expectedToggles[k] + require.Equal(t, toggle, v, tc.name) } } }