fix rebase issues

Co-authored-by: Dave Henderson <dave.henderson@grafana.com>

pkg/services/featuremgmt/models.go

@@ -126,6 +126,63 @@ 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
@@ -135,6 +192,8 @@ 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

pkg/services/featuremgmt/openfeature.go

@@ -26,7 +26,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]bool
+	StaticFlags map[string]setting.FeatureToggle
 	// TargetingKey is used for evaluation context
 	TargetingKey string
 	// ContextAttrs are additional attributes for evaluation context
@@ -100,7 +100,7 @@ func InitOpenFeatureWithCfg(cfg *setting.Cfg) error {
 func createProvider(
 	providerType string,
 	u *url.URL,
-	staticFlags map[string]bool,
+	staticFlags map[string]setting.FeatureToggle,
 	httpClient *http.Client,
 ) (openfeature.FeatureProvider, error) {
 	if providerType == setting.FeaturesServiceProviderType || providerType == setting.OFREPProviderType {
@@ -117,7 +117,7 @@ func createProvider(
 		}
 	}
 
-	return newStaticProvider(staticFlags)
+	return newStaticProvider(staticFlags, standardFeatureFlags)
 }
 
 func createHTTPClient(m *clientauthmiddleware.TokenExchangeMiddleware) (*http.Client, error) {

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
+		mgmt.startup[key] = val.Value == true
 	}
 
 	// update the values

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)
 	}

pkg/services/featuremgmt/static_provider.go

@@ -1,6 +1,11 @@
 package featuremgmt
 
 import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/open-feature/go-sdk/openfeature"
 	"github.com/open-feature/go-sdk/openfeature/memprovider"
 )
@@ -28,37 +33,76 @@ 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]setting.FeatureToggle, 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
+		}
 
-	// Add flags from config.ini file
-	for name, value := range confFlags {
-		flags[name] = createInMemoryFlag(name, value)
+		flags[flag.Name] = inMemFlag
+		index[flag.Name] = flag
 	}
 
-	// Add standard flags
-	for _, flag := range standardFeatureFlags {
-		if _, exists := flags[flag.Name]; !exists {
-			enabled := flag.Expression == "true"
-			flags[flag.Name] = createInMemoryFlag(flag.Name, enabled)
+	// 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)
 	}
 
 	return newInMemoryBulkProvider(flags), nil
 }
 
-func createInMemoryFlag(name string, enabled bool) memprovider.InMemoryFlag {
-	variant := "disabled"
-	if enabled {
-		variant = "enabled"
-	}
+func createInMemoryFlag(flag setting.FeatureToggle) memprovider.InMemoryFlag {
+	variant := "default"
 
 	return memprovider.InMemoryFlag{
-		Key:            name,
+		Key:            flag.Name,
 		DefaultVariant: variant,
-		Variants: map[string]interface{}{
-			"enabled":  true,
-			"disabled": false,
+		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
+}

pkg/services/featuremgmt/static_provider_test.go

@@ -93,3 +93,178 @@ ABCD = true
 	enabledFeatureManager := mgr.GetEnabled(ctx)
 	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
+		defaultValue  any
+		expectedValue any
+	}{
+		{
+			flags: FeatureFlag{
+				Name:       "Flag",
+				Expression: "true",
+				Type:       Boolean,
+			},
+			defaultValue:  false,
+			expectedValue: true,
+		},
+		{
+			flags: FeatureFlag{
+				Name:       "Flag",
+				Expression: "1.0",
+				Type:       Float,
+			},
+			defaultValue:  0.0,
+			expectedValue: 1.0,
+		},
+		{
+			flags: FeatureFlag{
+				Name:       "Flag",
+				Expression: "blue",
+				Type:       String,
+			},
+			defaultValue:  "red",
+			expectedValue: "blue",
+		},
+		{
+			flags: FeatureFlag{
+				Name:       "Flag",
+				Expression: "1",
+				Type:       Integer,
+			},
+			defaultValue:  int64(0),
+			expectedValue: int64(1),
+		},
+		{
+			flags: FeatureFlag{
+				Name:       "Flag",
+				Expression: `{ "foo": "bar" }`,
+				Type:       Structure,
+			},
+			defaultValue:  nil,
+			expectedValue: map[string]any{"foo": "bar"},
+		},
+	}
+
+	for _, tt := range tests {
+		provider, err := newStaticProvider(nil, []FeatureFlag{tt.flags})
+		assert.NoError(t, err)
+
+		var result any
+		switch tt.flags.Type {
+		case Boolean:
+			result = provider.BooleanEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(bool), openfeature.FlattenedContext{}).Value
+		case Float:
+			result = provider.FloatEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(float64), openfeature.FlattenedContext{}).Value
+		case String:
+			result = provider.StringEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(string), openfeature.FlattenedContext{}).Value
+		case Integer:
+			result = provider.IntEvaluation(t.Context(), tt.flags.Name, tt.defaultValue.(int64), openfeature.FlattenedContext{}).Value
+		case Structure:
+			result = provider.ObjectEvaluation(t.Context(), tt.flags.Name, tt.defaultValue, openfeature.FlattenedContext{}).Value
+		}
+
+		assert.Equal(t, tt.expectedValue, result)
+	}
+}
+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),
+		},
+	}
+
+	for _, tt := range tests {
+		configFlags, standardFlags := makeFlags(tt)
+		provider, err := newStaticProvider(configFlags, standardFlags)
+		assert.NoError(t, err)
+
+		var result any
+		switch tt.typ {
+		case Boolean:
+			result = provider.BooleanEvaluation(t.Context(), tt.name, false, openfeature.FlattenedContext{}).Value
+		case Float:
+			result = provider.FloatEvaluation(t.Context(), tt.name, 0.0, openfeature.FlattenedContext{}).Value
+		case String:
+			result = provider.StringEvaluation(t.Context(), tt.name, "foo", openfeature.FlattenedContext{}).Value
+		case Integer:
+			result = provider.IntEvaluation(t.Context(), tt.name, 1, openfeature.FlattenedContext{}).Value
+		case Structure:
+			result = provider.ObjectEvaluation(t.Context(), tt.name, make(map[string]any), openfeature.FlattenedContext{}).Value
+		}
+
+		assert.Equal(t, tt.configValue, result)
+	}
+}
+
+func makeFlags(tt struct {
+	name          string
+	typ           FeatureFlagType
+	originalValue string
+	configValue   any
+}) (map[string]setting.FeatureToggle, []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,
+		},
+	}
+
+	return config, []FeatureFlag{orig}
+}

pkg/services/updatemanager/plugins_test.go

@@ -378,8 +378,8 @@ func setupOpenFeatureProvider(t *testing.T, flagValue bool) {
 
 	err := featuremgmt.InitOpenFeature(featuremgmt.OpenFeatureConfig{
 		ProviderType: setting.StaticProviderType,
-		StaticFlags: map[string]bool{
-			featuremgmt.FlagPluginsAutoUpdate: flagValue,
+		StaticFlags: map[string]setting.FeatureToggle{
+			featuremgmt.FlagPluginsAutoUpdate: {Value: flagValue, Type: setting.Boolean},
 		},
 	})
 	require.NoError(t, err)

pkg/setting/setting_feature_toggles.go

@@ -1,6 +1,8 @@
 package setting
 
 import (
+	"encoding/json"
+	"fmt"
 	"strconv"
 
 	"gopkg.in/ini.v1"
@@ -8,6 +10,22 @@ import (
 	"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")
@@ -15,18 +33,30 @@ 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 { return toggles[key] }
+	cfg.IsFeatureToggleEnabled = func(key string) bool {
+		toggle, ok := toggles[key]
+		if !ok {
+			return false
+		}
+
+		return toggle.Type == Boolean && toggle.Value == true
+	}
 	return nil
 }
 
-func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]bool, error) {
-	featureToggles := make(map[string]bool, 10)
+func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[string]FeatureToggle, error) {
+	featureToggles := make(map[string]FeatureToggle, 10)
 
 	// parse the comma separated list in `enable`.
 	featuresTogglesStr := valueAsString(featureTogglesSection, "enable", "")
 	for _, feature := range util.SplitString(featuresTogglesStr) {
-		featureToggles[feature] = true
+		featureToggles[feature] = FeatureToggle{
+			Type:  Boolean,
+			Name:  feature,
+			Value: true,
+		}
 	}
 
 	// read all other settings under [feature_toggles]. If a toggle is
@@ -36,12 +66,36 @@ func ReadFeatureTogglesFromInitFile(featureTogglesSection *ini.Section) (map[str
 			continue
 		}
 
-		b, err := strconv.ParseBool(v.Value())
+		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)
+		}
+
 		featureToggles[v.Name()] = b
 	}
 	return featureToggles, nil
 }
+
+func ParseFlag(name, value string) (FeatureToggle, error) {
+	var structure map[string]any
+
+	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 FeatureToggle{Type: String, Name: name, Value: value}, nil
+}

pkg/setting/setting_feature_toggles_test.go

@@ -1,7 +1,7 @@
 package setting
 
 import (
-	"strconv"
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -13,16 +13,16 @@ func TestFeatureToggles(t *testing.T) {
 		name            string
 		conf            map[string]string
 		err             error
-		expectedToggles map[string]bool
+		expectedToggles map[string]FeatureToggle
 	}{
 		{
 			name: "can parse feature toggles passed in the `enable` array",
 			conf: map[string]string{
 				"enable": "feature1,feature2",
 			},
-			expectedToggles: map[string]bool{
-				"feature1": true,
-				"feature2": true,
+			expectedToggles: map[string]FeatureToggle{
+				"feature1": {Name: "feature1", Type: Boolean, Value: true},
+				"feature2": {Name: "feature2", Type: Boolean, Value: true},
 			},
 		},
 		{
@@ -31,10 +31,10 @@ func TestFeatureToggles(t *testing.T) {
 				"enable":   "feature1,feature2",
 				"feature3": "true",
 			},
-			expectedToggles: map[string]bool{
-				"feature1": true,
-				"feature2": true,
-				"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},
 			},
 		},
 		{
@@ -43,19 +43,35 @@ func TestFeatureToggles(t *testing.T) {
 				"enable":   "feature1,feature2",
 				"feature2": "false",
 			},
-			expectedToggles: map[string]bool{
-				"feature1": true,
-				"feature2": false,
+			expectedToggles: map[string]FeatureToggle{
+				"feature1": {Name: "feature1", Type: Boolean, Value: true},
+				"feature2": {Name: "feature2", Type: Boolean, Value: false},
 			},
 		},
 		{
-			name: "invalid boolean value should return syntax error",
+			name: "conflict in type declaration is be detected",
 			conf: map[string]string{
 				"enable":   "feature1,feature2",
 				"feature2": "invalid",
 			},
-			expectedToggles: map[string]bool{},
-			err:             strconv.ErrSyntax,
+			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{
+				"feature1": "1", "feature2": "1.0",
+				"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},
+			},
 		},
 	}
 
@@ -69,11 +85,14 @@ func TestFeatureToggles(t *testing.T) {
 		}
 
 		featureToggles, err := ReadFeatureTogglesFromInitFile(toggles)
-		require.ErrorIs(t, err, tc.err)
+		if tc.err != nil {
+			require.EqualError(t, err, tc.err.Error())
+		}
 
 		if err == nil {
 			for k, v := range featureToggles {
-				require.Equal(t, tc.expectedToggles[k], v, tc.name)
+				toggle := tc.expectedToggles[k]
+				require.Equal(t, toggle, v, tc.name)
 			}
 		}
 	}

pkg/tests/apis/features/features_test.go

@@ -44,6 +44,6 @@ func TestIntegrationFeatures(t *testing.T) {
 			"value": true,
 			"key":"`+flag+`",
 			"reason":"static provider evaluation result",
-			"variant":"enabled"}`, string(rsp.Body))
+			"variant":"default"}`, string(rsp.Body))
 	})
 }