diff --git a/pkg/services/featuremgmt/models.go b/pkg/services/featuremgmt/models.go index f47c0484127..abcbf838933 100644 --- a/pkg/services/featuremgmt/models.go +++ b/pkg/services/featuremgmt/models.go @@ -126,15 +126,63 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error { return nil } -type FeatureFlagType string +type FeatureFlagType int const ( - Boolean = "boolean" - Number = "number" - Object = "object" - String = "string" + Boolean FeatureFlagType = iota + Integer + Float + Object + String ) +func (t FeatureFlagType) String() string { + switch t { + case Boolean: + return "boolean" + case Integer: + return "number" + case Float: + return "float" + case Object: + return "object" + case String: + return "string" + } + + 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 "number": + *t = Integer + case "float": + *t = Float + case "object": + *t = Object + case "string": + *t = String + } + 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 diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index cd3b77322fb..9e516c06ed7 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -117,7 +117,7 @@ func createProvider( } } - return newStaticProvider(staticFlags) + return newStaticProvider(staticFlags, standardFeatureFlags) } func createHTTPClient(m *clientauthmiddleware.TokenExchangeMiddleware) (*http.Client, error) { diff --git a/pkg/services/featuremgmt/static_evaluator.go b/pkg/services/featuremgmt/static_evaluator.go index c3d46837d28..fdeef7a5858 100644 --- a/pkg/services/featuremgmt/static_evaluator.go +++ b/pkg/services/featuremgmt/static_evaluator.go @@ -29,7 +29,7 @@ func CreateStaticEvaluator(cfg *setting.Cfg) (StaticFlagEvaluator, error) { return nil, fmt.Errorf("failed to read feature flags from config: %w", err) } - staticProvider, err := newStaticProvider(staticFlags) + staticProvider, err := newStaticProvider(staticFlags, standardFeatureFlags) if err != nil { return nil, fmt.Errorf("failed to create static provider: %w", err) } diff --git a/pkg/services/featuremgmt/static_provider.go b/pkg/services/featuremgmt/static_provider.go index 22f6f3ff3db..17d85efc406 100644 --- a/pkg/services/featuremgmt/static_provider.go +++ b/pkg/services/featuremgmt/static_provider.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/open-feature/go-sdk/openfeature" "github.com/open-feature/go-sdk/openfeature/memprovider" + "strconv" ) // inMemoryBulkProvider is a wrapper around memprovider.InMemoryProvider that @@ -29,8 +30,8 @@ func (p *inMemoryBulkProvider) ListFlags() ([]string, error) { return keys, nil } -func newStaticProvider(confFlags map[string]bool) (openfeature.FeatureProvider, error) { - flags := make(map[string]memprovider.InMemoryFlag, len(standardFeatureFlags)) +func newStaticProvider(confFlags map[string]bool, standardFlags []FeatureFlag) (openfeature.FeatureProvider, error) { + flags := make(map[string]memprovider.InMemoryFlag, len(standardFlags)) // Add flags from config.ini file for name, value := range confFlags { @@ -38,11 +39,11 @@ func newStaticProvider(confFlags map[string]bool) (openfeature.FeatureProvider, } // Add standard flags - for _, flag := range standardFeatureFlags { + for _, flag := range standardFlags { _, exists := flags[flag.Name] if exists && flag.Type != Boolean { - return nil, fmt.Errorf("flag %s already exists", flag.Name) + return nil, fmt.Errorf("flag %s already declared as boolean", flag.Name) } inMemFlag, err := createFlag(flag) @@ -76,19 +77,26 @@ func createFlag(flag FeatureFlag) (memprovider.InMemoryFlag, error) { defaultVariant := "default" var value any + var err error switch flag.Type { case Boolean: value = flag.Expression == "true" case String: value = flag.Expression - case Number: - value = f + case Integer: + value, err = strconv.Atoi(flag.Expression) + case Float: + value, err = strconv.ParseFloat(flag.Expression, 64) case Object: value = 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, @@ -97,5 +105,3 @@ func createFlag(flag FeatureFlag) (memprovider.InMemoryFlag, error) { }, }, nil } - -func diff --git a/pkg/services/featuremgmt/static_provider_test.go b/pkg/services/featuremgmt/static_provider_test.go index 29610baa8c4..40127a08fbb 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -93,3 +93,86 @@ ABCD = true enabledFeatureManager := mgr.GetEnabled(ctx) assert.Equal(t, openFeatureEnabledFlags, enabledFeatureManager) } + +func Test_StaticProvider_DifferentTypeFlagsNoSilentIgnore(t *testing.T) { + staticFlags := map[string]bool{"oldBooleanFlag": true} + + flag := FeatureFlag{ + Name: "oldBooleanFlag", + Expression: "1.0", + Type: Integer, + } + _, err := newStaticProvider(staticFlags, []FeatureFlag{flag}) + assert.EqualError(t, err, "flag oldBooleanFlag already declared as boolean") +} + +func Test_StaticProvider_DifferentType(t *testing.T) { + tests := []struct { + flags FeatureFlag + defaultValue any + expectedValue any + test int + }{ + { + flags: FeatureFlag{ + Name: "Flag", + Expression: "true", + Type: Boolean, + }, + defaultValue: false, + expectedValue: true, + test: 0, + }, + { + flags: FeatureFlag{ + Name: "Flag", + Expression: "1.0", + Type: Float, + }, + defaultValue: 0.0, + expectedValue: 1.0, + test: 1, + }, + { + flags: FeatureFlag{ + Name: "Flag", + Expression: "blue", + Type: String, + }, + defaultValue: "red", + expectedValue: "blue", + test: 2, + }, + { + flags: FeatureFlag{ + Name: "Flag", + Expression: "1", + Type: Integer, + }, + defaultValue: int64(0), + expectedValue: int64(1), + test: 3, + }, + } + + for _, tt := range tests { + provider, err := newStaticProvider(nil, []FeatureFlag{tt.flags}) + assert.NoError(t, err) + + var result any + switch tt.test { + case 0: + result = provider.BooleanEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(bool), openfeature.FlattenedContext{}).Value + case 1: + result = provider.FloatEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(float64), openfeature.FlattenedContext{}).Value + case 2: + result = provider.StringEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(string), openfeature.FlattenedContext{}).Value + case 3: + result = provider.IntEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(int64), openfeature.FlattenedContext{}).Value + case 4: + result = provider.ObjectEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(map[string]any), openfeature.FlattenedContext{}).Value + } + + assert.Equal(t, tt.expectedValue, result) + } +}