diff --git a/config/samples/example_flags.json b/config/samples/example_flags.json index 785f793a4..847604301 100644 --- a/config/samples/example_flags.json +++ b/config/samples/example_flags.json @@ -1,12 +1,7 @@ { "flags": { "myBoolFlag": { - "state": "ENABLED", - "variants": { - "on": true, - "off": false - }, - "defaultVariant": "on" + "defaultVariant": "true" }, "myStringFlag": { "state": "ENABLED", diff --git a/go.mod b/go.mod index b8f092191..52667256c 100644 --- a/go.mod +++ b/go.mod @@ -106,3 +106,5 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect ) + +replace github.com/open-feature/schemas => ../schemas diff --git a/go.sum b/go.sum index 0808f1b3c..65dd9a8b8 100644 --- a/go.sum +++ b/go.sum @@ -57,8 +57,6 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/bufbuild/connect-go v1.5.1 h1:ORhrSiu63hWxtuMmC/V1mKySSRhEySsW5RkHJcyJXBk= -github.com/bufbuild/connect-go v1.5.1/go.mod h1:9iNvh/NOsfhNBUH5CtvXeVUskQO1xsrEviH7ZArwZ3I= github.com/bufbuild/connect-go v1.5.2 h1:G4EZd5gF1U1ZhhbVJXplbuUnfKpBZ5j5izqIwu2g2W8= github.com/bufbuild/connect-go v1.5.2/go.mod h1:GmMJYR6orFqD0Y6ZgX8pwQ8j9baizDrIQMm1/a6LnHk= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -284,8 +282,6 @@ github.com/onsi/ginkgo/v2 v2.6.0 h1:9t9b9vRUbFq3C4qKFCGkVuq/fIHji802N1nrtkh1mNc= github.com/onsi/gomega v1.24.1 h1:KORJXNNTzJXzu4ScJWssJfJMnJ+2QJqhoQSRwNlze9E= github.com/open-feature/open-feature-operator v0.2.28 h1:qzzVq8v9G7aXO7luocO/wQCGnTJjtcQh75mDOqjnFxo= github.com/open-feature/open-feature-operator v0.2.28/go.mod h1:bQncVK7hvhj5QStPwexxQ1aArPwox2Y1vWrVei/qIFg= -github.com/open-feature/schemas v0.2.8 h1:oA75hJXpOd9SFgmNI2IAxWZkwzQPUDm7Jyyh3q489wM= -github.com/open-feature/schemas v0.2.8/go.mod h1:vj+rfTsOLlh5PtGGkAbitnJmFPYuTHXTjOy13kzNgKQ= github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU= github.com/pelletier/go-toml/v2 v2.0.6/go.mod h1:eumQOmlWiOPt5WriQQqoM5y18pDHwha2N+QD+EUNTek= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -472,8 +468,6 @@ golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96b golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= -golang.org/x/net v0.6.0 h1:L4ZwwTvKW9gr0ZMS1yrHD9GZhIuVjOBBnaKH+SPQK0Q= -golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= diff --git a/pkg/eval/json_evaluator.go b/pkg/eval/json_evaluator.go index b8668ae54..11d26d155 100644 --- a/pkg/eval/json_evaluator.go +++ b/pkg/eval/json_evaluator.go @@ -291,15 +291,20 @@ func (je *JSONEvaluator) configToFlags(config string, newFlags *Flags) error { if err != nil { return fmt.Errorf("unmarshalling provided configurations: %w", err) } - if err := validateDefaultVariants(newFlags); err != nil { + + if err := loadDefaultVariants(newFlags, schemaLoader); err != nil { + return fmt.Errorf("loading default variants: %w", err) + } + + if err := validateDefaultVariant(newFlags); err != nil { return err } return nil } -// validateDefaultVariants returns an error if any of the default variants aren't valid -func validateDefaultVariants(flags *Flags) error { +// validateDefaultVariant returns an error if any of the default variants aren't valid +func validateDefaultVariant(flags *Flags) error { for name, flag := range flags.Flags { if _, ok := flag.Variants[flag.DefaultVariant]; !ok { return fmt.Errorf( @@ -311,6 +316,78 @@ func validateDefaultVariants(flags *Flags) error { return nil } +// loadDefaultVariants loads the default variants of the boolean type flag schema if variants aren't explicitly given +// (defaults only make sense for boolean type) +// e.g. the following flag structure +// +// "myBoolFlag": { +// "defaultVariant": "true" +// } +// +// becomes +// +// "myBoolFlag": { +// "defaultVariant": "true", +// "variants": defaultVariantsObjectAsDefinedInJSONSchema +// } +func loadDefaultVariants(flags *Flags, schemaLoader gojsonschema.JSONLoader) error { + flagKeysToLoadVariants := make([]string, 0) + + for flagKey, flag := range flags.Flags { + if flag.Variants == nil { + flagKeysToLoadVariants = append(flagKeysToLoadVariants, flagKey) + } + } + + if len(flagKeysToLoadVariants) == 0 { + // all variants are explicitly given + return nil + } + + schemaJSON, err := schemaLoader.LoadJSON() + if err != nil { + return fmt.Errorf("load JSON from schema: %w", err) + } + + schemaJSONMap, ok := schemaJSON.(map[string]interface{}) + if !ok { + return errors.New("schemaJSON is of unexpected type") + } + defs := schemaJSONMap["$defs"] + defsMap, ok := defs.(map[string]interface{}) + if !ok { + return errors.New("$defs is of unexpected type") + } + booleanVariants := defsMap["booleanVariants"] + booleanVariantsMap, ok := booleanVariants.(map[string]interface{}) + if !ok { + return errors.New("booleanVariants is of unexpected type") + } + properties := booleanVariantsMap["properties"] + propertiesMap, ok := properties.(map[string]interface{}) + if !ok { + return errors.New("properties is of unexpected type") + } + variants := propertiesMap["variants"] + variantsMap, ok := variants.(map[string]interface{}) + if !ok { + return errors.New("variants is of unexpected type") + } + defaultVariants := variantsMap["default"] + defaultVariantsMap, ok := defaultVariants.(map[string]any) + if !ok { + return errors.New("default is of unexpected type") + } + + for _, flagKey := range flagKeysToLoadVariants { + flag := flags.Flags[flagKey] + flag.Variants = defaultVariantsMap + flags.Flags[flagKey] = flag + } + + return nil +} + func (je *JSONEvaluator) transposeEvaluators(state string) (string, error) { var evaluators Evaluators if err := json.Unmarshal([]byte(state), &evaluators); err != nil { diff --git a/pkg/eval/json_evaluator_test.go b/pkg/eval/json_evaluator_test.go index 87c52918f..521d62e40 100644 --- a/pkg/eval/json_evaluator_test.go +++ b/pkg/eval/json_evaluator_test.go @@ -295,26 +295,6 @@ func TestGetState_Valid_ContainsFlag(t *testing.T) { } } -func TestSetState_Invalid_Error(t *testing.T) { - evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false)) - - // set state with an invalid flag definition - _, err := evaluator.SetState(sync.DataSync{FlagData: InvalidFlags}) - if err == nil { - t.Fatalf("expected error") - } -} - -func TestSetState_Valid_NoError(t *testing.T) { - evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false)) - - // set state with a valid flag definition - _, err := evaluator.SetState(sync.DataSync{FlagData: ValidFlags}) - if err != nil { - t.Fatalf("expected no error") - } -} - func TestResolveAllValues(t *testing.T) { evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false)) _, err := evaluator.SetState(sync.DataSync{FlagData: Flags}) @@ -786,12 +766,17 @@ func BenchmarkResolveObjectValue(b *testing.B) { } } -func TestSetState_DefaultVariantValidation(t *testing.T) { +func TestSetState(t *testing.T) { tests := map[string]struct { - jsonFlags string - valid bool + jsonFlags string + expectedState *string + valid bool }{ - "is valid": { + "explicitly enabled": { + jsonFlags: ValidFlags, + valid: true, + }, + "valid default variant": { jsonFlags: ` { "flags": { @@ -816,7 +801,7 @@ func TestSetState_DefaultVariantValidation(t *testing.T) { `, valid: true, }, - "is not valid": { + "invalid default variant": { jsonFlags: ` { "flags": { @@ -833,6 +818,47 @@ func TestSetState_DefaultVariantValidation(t *testing.T) { `, valid: false, }, + "invalid flags": { + jsonFlags: InvalidFlags, + valid: false, + }, + "implicitly enabled": { + jsonFlags: `{ + "flags": { + "implicitlyEnabledFlag": { + "variants": { + "on": true, + "off": false + }, + "defaultVariant": "on" + } + } + }`, + valid: true, + }, + "default variants": { + jsonFlags: `{ + "flags": { + "defaultVariants": { + "defaultVariant": "true" + } + } + }`, + valid: true, + expectedState: strPtr(`{ + "flags": { + "defaultVariants": { + "state": "", + "defaultVariant": "true", + "variants": { + "false": false, + "true": true + }, + "source": "" + } + } + }`), + }, } for name, tt := range tests { @@ -844,6 +870,17 @@ func TestSetState_DefaultVariantValidation(t *testing.T) { if tt.valid && err != nil { t.Error(err) } + + if tt.expectedState == nil { + return + } + + gotState, err := jsonEvaluator.GetState() + if err != nil { + t.Error(err) + } + + assert.Equal(t, strings.Join(strings.Fields(*tt.expectedState), ""), gotState) }) } } @@ -1194,3 +1231,7 @@ func TestFlagStateSafeForConcurrentReadWrites(t *testing.T) { }) } } + +func strPtr(s string) *string { + return &s +}