From 8a13780c488fdbf6ada20bbef355489eeca50500 Mon Sep 17 00:00:00 2001 From: drichelson Date: Tue, 4 Jun 2024 13:43:06 -0700 Subject: [PATCH] Change schema for % rollout flag: validate and populate missing fields --- internal/dorkly/flag.go | 37 ++++++ internal/dorkly/flag_test.go | 111 ++++++++++++++++++ internal/dorkly/project.go | 8 +- internal/dorkly/project_test.go | 4 +- .../environments/production/rollout1.yml | 3 +- .../environments/staging/rollout1.yml | 3 +- 6 files changed, 159 insertions(+), 7 deletions(-) diff --git a/internal/dorkly/flag.go b/internal/dorkly/flag.go index 3967009..1ce4e52 100644 --- a/internal/dorkly/flag.go +++ b/internal/dorkly/flag.go @@ -2,6 +2,7 @@ package dorkly import ( "encoding/base64" + "errors" "github.com/launchdarkly/go-sdk-common/v3/ldvalue" "github.com/launchdarkly/go-server-sdk-evaluation/v3/ldmodel" ) @@ -30,22 +31,58 @@ const ( // when combined with a FlagBase, it can be converted to a LaunchDarkly FeatureFlag type FlagConfigForEnv interface { ToLdFlag(flagBase FlagBase) ldmodel.FeatureFlag + Validate(flagBase FlagBase) error } +var _ FlagConfigForEnv = &FlagBoolean{} + // FlagBoolean is a boolean flag that is either on (true) or off (false) type FlagBoolean struct { Variation bool `yaml:"variation"` } +func (f *FlagBoolean) Validate(flagBase FlagBase) error { + return nil +} + func (f *FlagBoolean) ToLdFlag(flagBase FlagBase) ldmodel.FeatureFlag { return flagBase.ldFeatureFlagBoolean(f.Variation) } +var _ FlagConfigForEnv = &FlagBooleanRollout{} + // FlagBooleanRollout is a boolean flag that is on (true) for a percentage of users based on the id field type FlagBooleanRollout struct { PercentRollout BooleanRolloutVariation `yaml:"percentRollout"` } +func (f *FlagBooleanRollout) Validate(flagBase FlagBase) error { + if f.PercentRollout.True < 0.0 { + return errors.New("percentRollout.true must be >= 0") + } + if f.PercentRollout.False < 0.0 { + return errors.New("percentRollout.false must be >= 0") + } + + if f.PercentRollout.True+f.PercentRollout.False > 100 { + return errors.New("sum of percentRollout values must be <= 100") + } + + if f.PercentRollout.True == 0.0 && f.PercentRollout.False == 0 { + return errors.New("at least one of percentRollout.true or percentRollout.false must be > 0") + } + + if f.PercentRollout.True == 0.0 { + f.PercentRollout.True = 100.0 - f.PercentRollout.False + } + + if f.PercentRollout.False == 0.0 { + f.PercentRollout.False = 100.0 - f.PercentRollout.True + } + + return nil +} + type BooleanRolloutVariation struct { True float64 `yaml:"true"` False float64 `yaml:"false"` diff --git a/internal/dorkly/flag_test.go b/internal/dorkly/flag_test.go index b205121..db3523d 100644 --- a/internal/dorkly/flag_test.go +++ b/internal/dorkly/flag_test.go @@ -7,6 +7,117 @@ import ( "testing" ) +func TestFlagBooleanRollout_Validate(t *testing.T) { + tests := []struct { + name string + input FlagBooleanRollout + expected *FlagBooleanRollout + wantErr bool + }{ + { + name: "ValidPercentages", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + False: 70.0, + }, + }, + expected: &FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + False: 70.0, + }, + }, + wantErr: false, + }, + { + name: "ValidTruePercentage", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + }, + }, + expected: &FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + False: 70.0, + }, + }, + wantErr: false, + }, + { + name: "ValidFalsePercentage", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + False: 70.0, + }, + }, + expected: &FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + False: 70.0, + }, + }, + wantErr: false, + }, + { + name: "NegativeTruePercentage", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: -10.0, + False: 70.0, + }, + }, + expected: nil, + wantErr: true, + }, + { + name: "NegativeFalsePercentage", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 30.0, + False: -10.0, + }, + }, + expected: nil, + wantErr: true, + }, + { + name: "SumGreaterThan100", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 60.0, + False: 50.0, + }, + }, + wantErr: true, + }, + { + name: "BothPercentagesZero", + input: FlagBooleanRollout{ + PercentRollout: BooleanRolloutVariation{ + True: 0.0, + False: 0.0, + }, + }, + expected: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.input.Validate(FlagBase{}) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, &tt.input) + } + }) + } +} + func Test_FlagBoolean_ToLdFlag(t *testing.T) { cases := []struct { name string diff --git a/internal/dorkly/project.go b/internal/dorkly/project.go index b07e130..7afd16b 100644 --- a/internal/dorkly/project.go +++ b/internal/dorkly/project.go @@ -110,10 +110,16 @@ func (p *Project) loadFlagYamlFile(filePath string) (*Flag, error) { flag.key = getFileNameNoSuffix(f.Name()) flag.envConfigs = make(map[string]FlagConfigForEnv) for _, env := range p.environments { - flag.envConfigs[env], err = p.loadFlagConfigForEnvYamlFile(flag, filepath.Join(p.path, "environments", env, flag.key+".yml")) + flagEnvConfig, err := p.loadFlagConfigForEnvYamlFile(flag, filepath.Join(p.path, "environments", env, flag.key+".yml")) if err != nil { return nil, err } + err = flagEnvConfig.Validate(flag.FlagBase) + if err != nil { + return nil, err + } + flag.envConfigs[env] = flagEnvConfig + } return &flag, nil diff --git a/internal/dorkly/project_test.go b/internal/dorkly/project_test.go index e06bcee..c8b023a 100644 --- a/internal/dorkly/project_test.go +++ b/internal/dorkly/project_test.go @@ -38,8 +38,8 @@ var ( EnableMobileKey: true, }, envConfigs: map[string]FlagConfigForEnv{ - "production": &FlagBooleanRollout{PercentRollout: 31.0}, - "staging": &FlagBooleanRollout{PercentRollout: 100.0}, + "production": &FlagBooleanRollout{PercentRollout: BooleanRolloutVariation{True: 31.0, False: 69.0}}, + "staging": &FlagBooleanRollout{PercentRollout: BooleanRolloutVariation{True: 100.0, False: 0.0}}, }, }, }, diff --git a/internal/dorkly/testdata/testProject1/inputYaml/environments/production/rollout1.yml b/internal/dorkly/testdata/testProject1/inputYaml/environments/production/rollout1.yml index 4f09d2d..43a007c 100644 --- a/internal/dorkly/testdata/testProject1/inputYaml/environments/production/rollout1.yml +++ b/internal/dorkly/testdata/testProject1/inputYaml/environments/production/rollout1.yml @@ -1,3 +1,2 @@ percentRollout: - true: 31.0 - false: 69.0 \ No newline at end of file + true: 31.0 \ No newline at end of file diff --git a/internal/dorkly/testdata/testProject1/inputYaml/environments/staging/rollout1.yml b/internal/dorkly/testdata/testProject1/inputYaml/environments/staging/rollout1.yml index 6585f4d..98f48ce 100644 --- a/internal/dorkly/testdata/testProject1/inputYaml/environments/staging/rollout1.yml +++ b/internal/dorkly/testdata/testProject1/inputYaml/environments/staging/rollout1.yml @@ -1,3 +1,2 @@ percentRollout: - true: 100.0 - false: 0.0 \ No newline at end of file + true: 100.0 \ No newline at end of file