From df0e77d8113243b8d3f3d1ab1a2a00ea395a3385 Mon Sep 17 00:00:00 2001 From: John Schnake Date: Wed, 22 Sep 2021 13:03:49 -0500 Subject: [PATCH] Default our experimental features to true Added logic so users can turn them off if it breaks their logic in some way. Fixes #1436 Fixes #1437 Signed-off-by: John Schnake --- cmd/sonobuoy/app/featureGates.go | 20 +++++++- cmd/sonobuoy/app/featureGates_test.go | 50 +++++++++++++++++++ test/integration/sonobuoy_integration_test.go | 3 +- 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 cmd/sonobuoy/app/featureGates_test.go diff --git a/cmd/sonobuoy/app/featureGates.go b/cmd/sonobuoy/app/featureGates.go index e4a4d4df2..c8f304dc0 100644 --- a/cmd/sonobuoy/app/featureGates.go +++ b/cmd/sonobuoy/app/featureGates.go @@ -10,6 +10,24 @@ const ( FeatureWaitOutputProgressByDefault = "SONOBUOY_WAIT_PROGRESS" ) +var ( + featureDefaultMap = map[string]bool{ + FeaturePluginInstallation: true, + FeatureWaitOutputProgressByDefault: true, + } +) + +// featureEnabled returns if the named feature is enabled based on the current env and defaults. func featureEnabled(feature string) bool { - return os.Getenv(FeaturesAll) == "true" || os.Getenv(feature) == "true" + return featureEnabledCore(feature, os.Getenv(FeaturesAll), os.Getenv(feature), featureDefaultMap) +} + +// Extracted logic here for testing so we can modify the env and defaults easily. +func featureEnabledCore(featureName, allEnv, featureEnv string, defaultMap map[string]bool) bool { + // Allow features we default as true to be turned off while still relatively new so if major + // bugs are found we have workarounds. + if featureEnv == "false" { + return false + } + return defaultMap[featureName] || allEnv == "true" || featureEnv == "true" } diff --git a/cmd/sonobuoy/app/featureGates_test.go b/cmd/sonobuoy/app/featureGates_test.go new file mode 100644 index 000000000..16e10e313 --- /dev/null +++ b/cmd/sonobuoy/app/featureGates_test.go @@ -0,0 +1,50 @@ +package app + +import ( + "testing" +) + +func Test_FeatureEnabled(t *testing.T) { + // Choose arbitrary feature to test against + feature := "foo" + tests := []struct { + name string + want bool + allEnv string + featureEnv string + defaultVal bool + }{ + { + name: "All false", + want: false, + allEnv: "false", featureEnv: "false", defaultVal: false, + }, { + name: "Explicit false overrides all else", + want: false, + allEnv: "true", featureEnv: "false", defaultVal: true, + }, { + name: "Explicit true overrides all else", + want: true, + allEnv: "false", featureEnv: "true", defaultVal: false, + }, { + name: "FeaturesAll true overrides default", + want: true, + allEnv: "true", featureEnv: "", defaultVal: false, + }, { + name: "Can default to true even if all env is false", + want: true, + allEnv: "false", featureEnv: "", defaultVal: true, + }, { + name: "Default value used if others empty", + want: true, + allEnv: "", featureEnv: "", defaultVal: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := featureEnabledCore(feature, tt.allEnv, tt.featureEnv, map[string]bool{feature: tt.defaultVal}); got != tt.want { + t.Errorf("featureEnabled() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/test/integration/sonobuoy_integration_test.go b/test/integration/sonobuoy_integration_test.go index 3ab79fceb..cabb14dfb 100644 --- a/test/integration/sonobuoy_integration_test.go +++ b/test/integration/sonobuoy_integration_test.go @@ -812,7 +812,8 @@ func TestPluginLoading_LocalGolden(t *testing.T) { output = mustRunSonobuoyCommandWithContext(ctx, t, "gen -p hello-world.yaml --kubernetes-version=v123.456.789", envVars...) checkFileMatchesOrUpdate(t, output.String(), installedPluginFile, tmpDir) - envVars = append(envVars, "SONOBUOY_ALL_FEATURES=false") + // Disable the feature explicitly and ensure we aren't using it. + envVars = append(envVars, "SONOBUOY_PLUGIN_INSTALLATION=false") output = mustRunSonobuoyCommandWithContext(ctx, t, "gen -p hello-world.yaml --kubernetes-version=v123.456.789", envVars...) checkFileMatchesOrUpdate(t, output.String(), localPluginFile, tmpDir)