diff --git a/pkg/services/featuremgmt/models.go b/pkg/services/featuremgmt/models.go index 596cfcb2491..5a64eb78c06 100644 --- a/pkg/services/featuremgmt/models.go +++ b/pkg/services/featuremgmt/models.go @@ -129,6 +129,7 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error { type FeatureFlagType int const ( + // Boolean -- Type of a flag Boolean FeatureFlagType = iota Integer Float diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index b4da574c65b..22017b8de03 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -8,6 +8,7 @@ import ( clientauthmiddleware "github.com/grafana/grafana/pkg/clientauth/middleware" "github.com/grafana/grafana/pkg/setting" + "github.com/open-feature/go-sdk/openfeature/memprovider" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/open-feature/go-sdk/openfeature" @@ -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..7805032df84 100644 --- a/pkg/services/featuremgmt/service.go +++ b/pkg/services/featuremgmt/service.go @@ -47,7 +47,7 @@ func ProvideManagerService(cfg *setting.Cfg) (*FeatureManager, error) { } mgmt.warnings[key] = "unknown flag in config" } - mgmt.startup[key] = val.Value == true + mgmt.startup[key] = setting.IsEnabled(val) } // update the values diff --git a/pkg/services/featuremgmt/static_provider.go b/pkg/services/featuremgmt/static_provider.go index 1d59561bc2b..c44e7ab8fe6 100644 --- a/pkg/services/featuremgmt/static_provider.go +++ b/pkg/services/featuremgmt/static_provider.go @@ -1,9 +1,8 @@ package featuremgmt import ( - "encoding/json" "fmt" - "strconv" + "reflect" "github.com/grafana/grafana/pkg/setting" "github.com/open-feature/go-sdk/openfeature" @@ -33,15 +32,12 @@ 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 for _, flag := range standardFlags { - inMemFlag, err := createTypedFlag(flag) - if err != nil { - return nil, err - } + inMemFlag := setting.ParseFlag(flag.Name, flag.Expression) flags[flag.Name] = inMemFlag index[flag.Name] = flag @@ -49,60 +45,18 @@ func newStaticProvider(confFlags map[string]setting.FeatureToggle, standardFlags // Add flags from config.ini file for name, flag := range confFlags { - standard, exists := index[flag.Name] + standard, exists := flags[flag.Key] // 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) + standardValue, _ := setting.GetDefaultValue(standard) + flagValue, _ := setting.GetDefaultValue(flag) + + if exists && reflect.TypeOf(standardValue) != reflect.TypeOf(flagValue) { + return nil, fmt.Errorf("type mismatch for flag '%s' detected", flag.Key) } - flags[name] = createInMemoryFlag(flag) + flags[name] = flag } 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..3faba509790 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/setting" + "github.com/open-feature/go-sdk/openfeature/memprovider" "github.com/open-feature/go-sdk/openfeature" "github.com/stretchr/testify/assert" @@ -95,16 +96,17 @@ ABCD = true } func Test_StaticProvider_FailfastOnMismatchedType(t *testing.T) { - staticFlags := map[string]setting.FeatureToggle{"oldBooleanFlag": { - Type: setting.Boolean, - Name: "oldBooleanFlag", - Value: true, + staticFlags := map[string]memprovider.InMemoryFlag{"oldBooleanFlag": { + Key: "oldBooleanFlag", + DefaultVariant: setting.DefaultVariantName, + Variants: map[string]any{ + setting.DefaultVariantName: 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") @@ -201,7 +203,7 @@ func Test_StaticProvider_ConfigOverride(t *testing.T) { name: "int", typ: Integer, originalValue: "0", - configValue: int64(1), + configValue: 1, }, { name: "float", @@ -251,20 +253,26 @@ func makeFlags(tt struct { 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{ - tt.name: { - Name: tt.name, - Type: setting.FeatureFlagType(tt.typ.String()), - Value: tt.configValue, - }, + config := map[string]memprovider.InMemoryFlag{ + tt.name: makeInMemoryFlag(tt.name, tt.configValue), } return config, []FeatureFlag{orig} } + +func makeInMemoryFlag(name string, value any) memprovider.InMemoryFlag { + return memprovider.InMemoryFlag{ + Key: name, + DefaultVariant: setting.DefaultVariantName, + Variants: map[string]any{ + setting.DefaultVariantName: value, + }, + } +} diff --git a/pkg/services/updatemanager/plugins_test.go b/pkg/services/updatemanager/plugins_test.go index 1560a5725dd..dc63226caa9 100644 --- a/pkg/services/updatemanager/plugins_test.go +++ b/pkg/services/updatemanager/plugins_test.go @@ -9,7 +9,9 @@ import ( "sync" "testing" + "cuelang.org/go/pkg/strconv" "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 +380,8 @@ 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: setting.ParseFlag(featuremgmt.FlagPluginsAutoUpdate, strconv.FormatBool(flagValue)), }, }) require.NoError(t, err) diff --git a/pkg/setting/setting_feature_toggles.go b/pkg/setting/setting_feature_toggles.go index 8231317be15..092185f5fdf 100644 --- a/pkg/setting/setting_feature_toggles.go +++ b/pkg/setting/setting_feature_toggles.go @@ -2,9 +2,10 @@ package setting import ( "encoding/json" - "fmt" + "errors" "strconv" + "github.com/open-feature/go-sdk/openfeature/memprovider" "gopkg.in/ini.v1" "github.com/grafana/grafana/pkg/util" @@ -20,6 +21,8 @@ const ( String FeatureFlagType = "string" ) +const DefaultVariantName = "" + type FeatureToggle struct { Type FeatureFlagType `json:"type"` Name string `json:"name"` @@ -33,6 +36,7 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { if err != nil { return err } + // TODO IsFeatureToggleEnabled has been deprecated for 2 years now, we should remove this function completely // nolint:staticcheck cfg.IsFeatureToggleEnabled = func(key string) bool { @@ -41,22 +45,19 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { return false } - return toggle.Type == Boolean && toggle.Value == true + return IsEnabled(toggle) } + 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] = createFlag(feature, true) } // read all other settings under [feature_toggles]. If a toggle is @@ -66,36 +67,71 @@ func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[str continue } - b, err := ParseFlag(v.Name(), v.Value()) - if err != nil { - 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) - } - + b := ParseFlag(v.Name(), v.Value()) featureToggles[v.Name()] = b } return featureToggles, nil } -func ParseFlag(name, value string) (FeatureToggle, error) { +func ParseFlag(name, value string) memprovider.InMemoryFlag { var structure map[string]any if integer, err := strconv.Atoi(value); err == nil { - return FeatureToggle{Type: Integer, Name: name, Value: integer}, nil + return createFlag(name, integer) } if float, err := strconv.ParseFloat(value, 64); err == nil { - return FeatureToggle{Type: Float, Name: name, Value: float}, nil + return createFlag(name, float) } if err := json.Unmarshal([]byte(value), &structure); err == nil { - return FeatureToggle{Type: Structure, Name: name, Value: structure}, nil + return createFlag(name, structure) } if boolean, err := strconv.ParseBool(value); err == nil { - return FeatureToggle{Type: Boolean, Name: name, Value: boolean}, nil + return createFlag(name, boolean) } - return FeatureToggle{Type: String, Name: name, Value: value}, nil + return createFlag(name, value) +} + +func SerializeFlag(flag memprovider.InMemoryFlag) string { + value, _ := flag.Variants[DefaultVariantName] + + switch castedValue := value.(type) { + case bool: + return strconv.FormatBool(castedValue) + case int64: + return strconv.FormatInt(castedValue, 10) + case float64: + return strconv.FormatFloat(castedValue, 'f', -1, 64) + case string: + return castedValue + default: + val, _ := json.Marshal(value) + return string(val) + } +} + +func createFlag(name string, value any) memprovider.InMemoryFlag { + return memprovider.InMemoryFlag{ + Key: name, + DefaultVariant: DefaultVariantName, + Variants: map[string]any{ + DefaultVariantName: value, + }, + } +} + +func GetDefaultValue(flag memprovider.InMemoryFlag) (any, error) { + if value, ok := flag.Variants[flag.DefaultVariant]; !ok { + return nil, errors.New("no default variant found") + } else { + return value, nil + } +} + +func IsEnabled(flag memprovider.InMemoryFlag) bool { + if value, ok := flag.Variants[flag.DefaultVariant]; !ok { + return false + } else { + return value == true + } } diff --git a/pkg/setting/setting_feature_toggles_test.go b/pkg/setting/setting_feature_toggles_test.go index bd8adf73440..a21058df353 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" ) @@ -13,16 +13,16 @@ func TestFeatureToggles(t *testing.T) { 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": makeInMemoryFlag("feature1", true), + "feature2": makeInMemoryFlag("feature2", true), }, }, { @@ -31,10 +31,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": makeInMemoryFlag("feature1", true), + "feature2": makeInMemoryFlag("feature2", true), + "feature3": makeInMemoryFlag("feature3", true), }, }, { @@ -43,20 +43,20 @@ 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": makeInMemoryFlag("feature1", true), + "feature2": makeInMemoryFlag("feature2", 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: "conflict in type declaration is be detected", + // conf: map[string]string{ + // "enable": "feature1,feature2", + // "feature2": "invalid", + // }, + // expectedToggles: map[string]memprovider.InMemoryFlag{}, + // 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 +64,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": makeInMemoryFlag("feature1", 1), + "feature2": makeInMemoryFlag("feature2", 1.0), + "feature3": makeInMemoryFlag("feature3", map[string]any{"foo": "bar"}), + "feature4": makeInMemoryFlag("feature4", "bar"), + "feature5": makeInMemoryFlag("feature5", true), + "feature6": makeInMemoryFlag("feature6", true), }, }, } @@ -97,3 +97,13 @@ func TestFeatureToggles(t *testing.T) { } } } + +func makeInMemoryFlag(name string, value any) memprovider.InMemoryFlag { + return memprovider.InMemoryFlag{ + Key: name, + DefaultVariant: DefaultVariantName, + Variants: map[string]any{ + DefaultVariantName: value, + }, + } +}