Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set default variants of boolean flag if not given #420

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions config/samples/example_flags.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
{
"flags": {
"myBoolFlag": {
"state": "ENABLED",
"variants": {
"on": true,
"off": false
},
"defaultVariant": "on"
"defaultVariant": "true"
},
"myStringFlag": {
"state": "ENABLED",
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 0 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
83 changes: 80 additions & 3 deletions pkg/eval/json_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
91 changes: 66 additions & 25 deletions pkg/eval/json_evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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": {
Expand All @@ -816,7 +801,7 @@ func TestSetState_DefaultVariantValidation(t *testing.T) {
`,
valid: true,
},
"is not valid": {
"invalid default variant": {
jsonFlags: `
{
"flags": {
Expand All @@ -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 {
Expand All @@ -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)
})
}
}
Expand Down Expand Up @@ -1194,3 +1231,7 @@ func TestFlagStateSafeForConcurrentReadWrites(t *testing.T) {
})
}
}

func strPtr(s string) *string {
return &s
}