SKAFFOLD_UPDATE_CHECK should also be a global flag

Signed-off-by: David Gageot <david@gageot.net>
GoogleContainerTools · Jul 21, 2020 · 6a373d8 · 6a373d8
1 parent ac11549
commit 6a373d8
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 30 deletions.
diff --git a/cmd/skaffold/app/cmd/cmd.go b/cmd/skaffold/app/cmd/cmd.go
@@ -45,6 +45,7 @@ var (
 	forceColors       bool
 	overwrite         bool
 	interactive       bool
+	checkForUpdate    bool
 	shutdownAPIServer func() error
 )
 
@@ -167,6 +168,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 	rootCmd.PersistentFlags().IntVar(&defaultColor, "color", int(color.DefaultColorCode), "Specify the default output color in ANSI escape codes")
 	rootCmd.PersistentFlags().BoolVar(&forceColors, "force-colors", false, "Always print color codes (hidden)")
 	rootCmd.PersistentFlags().BoolVar(&interactive, "interactive", true, "Allow user prompts for more information")
+	rootCmd.PersistentFlags().BoolVar(&update.EnableCheck, "update-check", true, "Check for a more recent version of Skaffold")
 	rootCmd.PersistentFlags().MarkHidden("force-colors")
 
 	setFlagsFromEnvVariables(rootCmd)

diff --git a/docs/content/en/docs/references/cli/_index.md b/docs/content/en/docs/references/cli/_index.md
@@ -99,6 +99,7 @@ Env vars:
 
 * `SKAFFOLD_COLOR` (same as `--color`)
 * `SKAFFOLD_INTERACTIVE` (same as `--interactive`)
+* `SKAFFOLD_UPDATE_CHECK` (same as `--update-check`)
 * `SKAFFOLD_VERBOSITY` (same as `--verbosity`)
 
 ### skaffold build
@@ -697,6 +698,7 @@ The following options can be passed to any command:
 
       --color=34: Specify the default output color in ANSI escape codes
       --interactive=true: Allow user prompts for more information
+      --update-check=true: Check for a more recent version of Skaffold
   -v, --verbosity='warning': Log level (debug, info, warn, error, fatal, panic)
 
 

diff --git a/pkg/skaffold/constants/constants.go b/pkg/skaffold/constants/constants.go
@@ -56,8 +56,6 @@ const (
 	// DefaultDebugHelpersRegistry is the default location used for the helper images for `debug`.
 	DefaultDebugHelpersRegistry = "gcr.io/gcp-dev-tools/duct-tape"
 
-	UpdateCheckEnvironmentVariable = "SKAFFOLD_UPDATE_CHECK"
-
 	DefaultSkaffoldDir = ".skaffold"
 	DefaultCacheFile   = "cache"
 

diff --git a/pkg/skaffold/update/update.go b/pkg/skaffold/update/update.go
@@ -20,22 +20,22 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"os"
 	"strings"
 
 	"github.com/blang/semver"
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
 )
 
+// EnableCheck enabled the check for a more recent version of Skaffold.
+var EnableCheck bool
+
 // For testing
 var (
 	GetLatestAndCurrentVersion = getLatestAndCurrentVersion
 	isConfigUpdateCheckEnabled = config.IsUpdateCheckEnabled
-	getEnv                     = os.Getenv
 )
 
 const LatestVersionURL = "https://storage.googleapis.com/skaffold/releases/latest/VERSION"
@@ -48,14 +48,7 @@ func IsUpdateCheckEnabled(configfile string) bool {
 		return false
 	}
 
-	return isUpdateCheckEnabledByEnvOrConfig(configfile)
-}
-
-func isUpdateCheckEnabledByEnvOrConfig(configfile string) bool {
-	if v := getEnv(constants.UpdateCheckEnvironmentVariable); v != "" {
-		return strings.ToLower(v) == "true"
-	}
-	return isConfigUpdateCheckEnabled(configfile)
+	return EnableCheck && isConfigUpdateCheckEnabled(configfile)
 }
 
 // getLatestAndCurrentVersion uses a VERSION file stored on GCS to determine the latest released version

diff --git a/pkg/skaffold/update/update_test.go b/pkg/skaffold/update/update_test.go
@@ -22,42 +22,46 @@ import (
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
-func TestIsUpdateCheckEnabledByEnvOrConfig(t *testing.T) {
+func TestIsUpdateCheckEnabled(t *testing.T) {
 	tests := []struct {
 		description string
-		envVariable string
+		enabled     bool
 		configCheck bool
 		expected    bool
 	}{
 		{
-			description: "env variable is set to true",
-			envVariable: "true",
-			expected:    true,
-		},
-		{
-			description: "env variable is set to false",
-			envVariable: "false",
+			description: "globally disabled - disabled in config -> disabled",
+			enabled:     false,
+			configCheck: false,
+			expected:    false,
 		},
 		{
-			description: "env variable is set to random string",
-			envVariable: "foo",
+			description: "globally enabled - disabled in config -> disabled",
+			enabled:     true,
+			configCheck: false,
+			expected:    false,
 		},
 		{
-			description: "env variable is empty and config is enabled",
+			description: "globally disabled - enabled in config -> disabled",
+			enabled:     false,
 			configCheck: true,
-			expected:    true,
+			expected:    false,
 		},
 		{
-			description: "env variable is false but Global update-check config is true",
-			envVariable: "false",
+			description: "globally enabled - enabled in config -> enabled",
+			enabled:     true,
 			configCheck: true,
+			expected:    true,
 		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Override(&EnableCheck, test.enabled)
 			t.Override(&isConfigUpdateCheckEnabled, func(string) bool { return test.configCheck })
-			t.Override(&getEnv, func(string) string { return test.envVariable })
-			t.CheckDeepEqual(test.expected, isUpdateCheckEnabledByEnvOrConfig("dummyconfig"))
+
+			isEnabled := IsUpdateCheckEnabled("dummyconfig")
+
+			t.CheckDeepEqual(test.expected, isEnabled)
 		})
 	}
 }