From d141eaf8cbf7b3f2c2583ecb73b14ed9e1bcf1a0 Mon Sep 17 00:00:00 2001 From: Balint Pato Date: Fri, 30 Aug 2019 17:47:22 -0700 Subject: [PATCH 1/2] Merge pull request #2774 from GoogleContainerTools/add_privacy_policy add privacy notice and command to set update check false --- cmd/skaffold/app/cmd/cmd.go | 8 +-- cmd/skaffold/app/cmd/config/set_test.go | 14 +++++ cmd/skaffold/app/cmd/flags.go | 11 ++-- cmd/skaffold/app/cmd/flags_test.go | 2 +- docs/config.toml | 4 +- .../content/en/docs/getting-started/_index.md | 10 +++ docs/content/en/docs/references/_index.md | 1 + .../en/docs/references/privacy/_index.md | 21 +++++++ pkg/skaffold/config/global_config.go | 1 + pkg/skaffold/config/options_test.go | 4 +- pkg/skaffold/config/util.go | 13 +++- pkg/skaffold/config/util_test.go | 63 ++++++++++++++++--- pkg/skaffold/update/dummyconfig | 0 pkg/skaffold/update/update.go | 15 ++++- pkg/skaffold/update/update_test.go | 63 +++++++++++++++++++ 15 files changed, 200 insertions(+), 30 deletions(-) create mode 100644 docs/content/en/docs/references/privacy/_index.md create mode 100644 pkg/skaffold/update/dummyconfig create mode 100644 pkg/skaffold/update/update_test.go diff --git a/cmd/skaffold/app/cmd/cmd.go b/cmd/skaffold/app/cmd/cmd.go index b83adc67f02..71971dedf8a 100644 --- a/cmd/skaffold/app/cmd/cmd.go +++ b/cmd/skaffold/app/cmd/cmd.go @@ -100,7 +100,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command { logrus.Debugf("Update check is disabled because of quiet mode") } else { go func() { - if err := updateCheck(updateMsg); err != nil { + if err := updateCheck(updateMsg, opts.GlobalConfig); err != nil { logrus.Infof("update check failed: %s", err) } }() @@ -121,8 +121,6 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command { }, } - SetUpFlags() - groups := templates.CommandGroups{ { Message: "End-to-end pipelines:", @@ -183,8 +181,8 @@ func NewCmdOptions() *cobra.Command { return cmd } -func updateCheck(ch chan string) error { - if !update.IsUpdateCheckEnabled() { +func updateCheck(ch chan string, configfile string) error { + if !update.IsUpdateCheckEnabled(configfile) { logrus.Debugf("Update check not enabled, skipping.") return nil } diff --git a/cmd/skaffold/app/cmd/config/set_test.go b/cmd/skaffold/app/cmd/config/set_test.go index d6e9ba72037..204c05698b9 100644 --- a/cmd/skaffold/app/cmd/config/set_test.go +++ b/cmd/skaffold/app/cmd/config/set_test.go @@ -146,6 +146,20 @@ func TestSetAndUnsetConfig(t *testing.T) { }, }, }, + { + description: "set global update-check", + key: "update-check", + value: "true", + global: true, + expectedSetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{UpdateCheck: util.BoolPtr(true)}, + ContextConfigs: []*config.ContextConfig{}, + }, + expectedUnsetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{}, + ContextConfigs: []*config.ContextConfig{}, + }, + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go index 565e24c7aab..ca29b0701de 100644 --- a/cmd/skaffold/app/cmd/flags.go +++ b/cmd/skaffold/app/cmd/flags.go @@ -250,8 +250,8 @@ var FlagRegistry = []Flag{ var commandFlags []*pflag.Flag -// SetUpFlags creates pflag.Flag for all registered flags -func SetUpFlags() { +// SetupFlags creates pflag.Flag for all registered flags +func SetupFlags() { commandFlags = make([]*pflag.Flag, len(FlagRegistry)) for i, fl := range FlagRegistry { fs := pflag.NewFlagSet(fl.Name, pflag.ContinueOnError) @@ -274,9 +274,6 @@ func SetUpFlags() { } func AddFlags(fs *pflag.FlagSet, cmdName string) { - if len(commandFlags) == 0 { - SetUpFlags() - } for _, f := range commandFlags { if hasCmdAnnotation(cmdName, f.Annotations["cmds"]) { fs.AddFlag(f) @@ -293,3 +290,7 @@ func hasCmdAnnotation(cmdName string, annotations []string) bool { } return false } + +func init() { + SetupFlags() +} diff --git a/cmd/skaffold/app/cmd/flags_test.go b/cmd/skaffold/app/cmd/flags_test.go index c82b36a7aa8..378de77f1c9 100644 --- a/cmd/skaffold/app/cmd/flags_test.go +++ b/cmd/skaffold/app/cmd/flags_test.go @@ -62,6 +62,6 @@ func TestAddFlagsSmoke(t *testing.T) { Use: "test", Short: "Test commanf for smoke testing", } - SetUpFlags() + SetupFlags() AddFlags(testCmd.Flags(), "test") } diff --git a/docs/config.toml b/docs/config.toml index 562c28d6eff..a879277be7d 100644 --- a/docs/config.toml +++ b/docs/config.toml @@ -67,8 +67,8 @@ weight = 1 # Everything below this are Site Params [params] -#copyright = "Skaffold" -#privacy_policy = "https://policies.google.com/privacy" +copyright = "Skaffold Authors" +privacy_policy = "https://policies.google.com/privacy" github_repo = "https://github.com/GoogleContainerTools/skaffold" skaffold_version = "skaffold/v1beta13" diff --git a/docs/content/en/docs/getting-started/_index.md b/docs/content/en/docs/getting-started/_index.md index 0c2e1877c20..7bbe845a553 100644 --- a/docs/content/en/docs/getting-started/_index.md +++ b/docs/content/en/docs/getting-started/_index.md @@ -145,6 +145,16 @@ For the latest **bleeding edge** build: {{% /tabs %}} +{{< alert title="Note" >}} +To keep Skaffold up to date, update checks are made to Google servers to see if a new version of +Skaffold is available. + +You can turn this update check off by following these instructions. + + +Your use of this software is subject to the Google Privacy Policy +{{< /alert >}} + ## Downloading the sample app 1. Clone the Skaffold repository: diff --git a/docs/content/en/docs/references/_index.md b/docs/content/en/docs/references/_index.md index 9f3f2714496..6d31be371fc 100755 --- a/docs/content/en/docs/references/_index.md +++ b/docs/content/en/docs/references/_index.md @@ -8,4 +8,5 @@ weight: 100 |----------| | [CLI](/docs/references/cli) | | [skaffold.yaml](/docs/references/yaml) | +| [Privacy Settings] (/docs/references/privacy) | diff --git a/docs/content/en/docs/references/privacy/_index.md b/docs/content/en/docs/references/privacy/_index.md new file mode 100644 index 00000000000..850532a4f04 --- /dev/null +++ b/docs/content/en/docs/references/privacy/_index.md @@ -0,0 +1,21 @@ +--- +title: "Privacy Settings" +linkTitle: "Privacy Settings" +weight: 300 +--- + +The privacy of our users is very important to us. +Your use of this software is subject to the Google Privacy Policy. + +## Update check + +To keep Skaffold up to date, update checks are made to Google servers to see if a new version of +Skaffold is available. By default, this behavior is enabled. As a side effect this request is logged. + +To disable the update check you have two options: + +1. set the `SKAFFOLD_UPDATE_CHECK` environment variable to `false` +2. turn it off in skaffold's global config with: +```bash + skaffold config set -g update-check false +``` \ No newline at end of file diff --git a/pkg/skaffold/config/global_config.go b/pkg/skaffold/config/global_config.go index c7ff914dbcd..57bba1777ad 100644 --- a/pkg/skaffold/config/global_config.go +++ b/pkg/skaffold/config/global_config.go @@ -30,4 +30,5 @@ type ContextConfig struct { DefaultRepo string `yaml:"default-repo,omitempty"` LocalCluster *bool `yaml:"local-cluster,omitempty"` InsecureRegistries []string `yaml:"insecure-registries,omitempty"` + UpdateCheck *bool `yaml:"update-check,omitempty"` } diff --git a/pkg/skaffold/config/options_test.go b/pkg/skaffold/config/options_test.go index bf18b468be6..9e66f484b6e 100644 --- a/pkg/skaffold/config/options_test.go +++ b/pkg/skaffold/config/options_test.go @@ -130,12 +130,12 @@ func TestIsTargetImage(t *testing.T) { expectedMatch: true, }, { - description: "match full name", + description: "match full description", targetImages: []string{"domain/image"}, expectedMatch: true, }, { - description: "match partial name", + description: "match partial description", targetImages: []string{"image"}, expectedMatch: true, }, diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 4596ec17be6..6c53e6fe981 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -49,7 +49,8 @@ var ( configErr error configOnce sync.Once - ReadConfigFile = readConfigFileCached + ReadConfigFile = readConfigFileCached + GetConfigForCurrentKubectx = getConfigForCurrentKubectx ) // readConfigFileCached reads the specified file and returns the contents @@ -96,7 +97,7 @@ func ReadConfigFileNoCache(configFile string) (*GlobalConfig, error) { // GetConfigForCurrentKubectx returns the specific config to be modified based on the kubeContext. // Either returns the config corresponding to the provided or current context, // or the global config. -func GetConfigForCurrentKubectx(configFile string) (*ContextConfig, error) { +func getConfigForCurrentKubectx(configFile string) (*ContextConfig, error) { configOnce.Do(func() { cfg, err := ReadConfigFile(configFile) if err != nil { @@ -191,3 +192,11 @@ func isDefaultLocal(kubeContext string) bool { func IsKindCluster(kubeContext string) bool { return strings.HasSuffix(kubeContext, "@kind") } + +func IsUpdateCheckEnabled(configfile string) bool { + cfg, err := GetConfigForCurrentKubectx(configfile) + if err != nil { + return true + } + return cfg == nil || cfg.UpdateCheck == nil || *cfg.UpdateCheck +} diff --git a/pkg/skaffold/config/util_test.go b/pkg/skaffold/config/util_test.go index 07d4274ac8d..269d4f38460 100644 --- a/pkg/skaffold/config/util_test.go +++ b/pkg/skaffold/config/util_test.go @@ -17,6 +17,7 @@ limitations under the License. package config import ( + "fmt" "path/filepath" "strings" "testing" @@ -108,13 +109,13 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { } tests := []struct { - name string + description string kubecontext string cfg *GlobalConfig expectedConfig *ContextConfig }{ { - name: "global config when kubecontext is empty", + description: "global config when kubecontext is empty", cfg: &GlobalConfig{ Global: &ContextConfig{ InsecureRegistries: []string{"mediocre.io"}, @@ -135,12 +136,12 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { }, }, { - name: "no global config and no kubecontext", + description: "no global config and no kubecontext", cfg: &GlobalConfig{}, expectedConfig: &ContextConfig{}, }, { - name: "config for unknown kubecontext", + description: "config for unknown kubecontext", kubecontext: someKubeContext, cfg: &GlobalConfig{}, expectedConfig: &ContextConfig{ @@ -148,7 +149,7 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { }, }, { - name: "config for kubecontext when globals are empty", + description: "config for kubecontext when globals are empty", kubecontext: someKubeContext, cfg: &GlobalConfig{ ContextConfigs: []*ContextConfig{sampleConfig2, sampleConfig1}, @@ -156,7 +157,7 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { expectedConfig: sampleConfig1, }, { - name: "config for kubecontext without merged values", + description: "config for kubecontext without merged values", kubecontext: someKubeContext, cfg: &GlobalConfig{ Global: sampleConfig2, @@ -165,7 +166,7 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { expectedConfig: sampleConfig1, }, { - name: "config for kubecontext with merged values", + description: "config for kubecontext with merged values", kubecontext: someKubeContext, cfg: &GlobalConfig{ Global: sampleConfig2, @@ -182,7 +183,7 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { }, }, { - name: "config for unknown kubecontext with merged values", + description: "config for unknown kubecontext with merged values", kubecontext: someKubeContext, cfg: &GlobalConfig{Global: sampleConfig2}, expectedConfig: &ContextConfig{ @@ -192,7 +193,7 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { }, }, { - name: "merge global and context-specific insecure-registries", + description: "merge global and context-specific insecure-registries", kubecontext: someKubeContext, cfg: &GlobalConfig{ Global: &ContextConfig{ @@ -210,9 +211,51 @@ func Test_getConfigForKubeContextWithGlobalDefaults(t *testing.T) { }, } for _, test := range tests { - testutil.Run(t, test.name, func(t *testutil.T) { + testutil.Run(t, test.description, func(t *testutil.T) { actual, err := getConfigForKubeContextWithGlobalDefaults(test.cfg, test.kubecontext) t.CheckErrorAndDeepEqual(false, err, test.expectedConfig, actual) }) } } + +func TestIsUpdateCheckEnabled(t *testing.T) { + tests := []struct { + description string + cfg *ContextConfig + readErr error + expected bool + }{ + { + description: "config update-check is nil returns true", + cfg: &ContextConfig{}, + expected: true, + }, + { + description: "config update-check is true", + cfg: &ContextConfig{UpdateCheck: util.BoolPtr(true)}, + expected: true, + }, + { + description: "config update-check is false", + cfg: &ContextConfig{UpdateCheck: util.BoolPtr(false)}, + }, + { + description: "config is nil", + cfg: nil, + expected: true, + }, + { + description: "config has err", + cfg: nil, + readErr: fmt.Errorf("error while reading"), + expected: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(&GetConfigForCurrentKubectx, func(string) (*ContextConfig, error) { return test.cfg, test.readErr }) + actual := IsUpdateCheckEnabled("dummyconfig") + t.CheckDeepEqual(test.expected, actual) + }) + } +} diff --git a/pkg/skaffold/update/dummyconfig b/pkg/skaffold/update/dummyconfig new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/skaffold/update/update.go b/pkg/skaffold/update/update.go index ccac116bde8..7fdbee0875e 100644 --- a/pkg/skaffold/update/update.go +++ b/pkg/skaffold/update/update.go @@ -22,6 +22,7 @@ import ( "os" "strings" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/version" "github.com/blang/semver" @@ -31,20 +32,28 @@ import ( // Fot testing var ( GetLatestAndCurrentVersion = getLatestAndCurrentVersion + isConfigUpdateCheckEnabled = config.IsUpdateCheckEnabled + getEnv = os.Getenv ) const latestVersionURL = "https://storage.googleapis.com/skaffold/releases/latest/VERSION" // IsUpdateCheckEnabled returns whether or not the update check is enabled // It is true by default, but setting it to any other value than true will disable the check -func IsUpdateCheckEnabled() bool { +func IsUpdateCheckEnabled(configfile string) bool { // Don't perform a version check on dirty trees if version.Get().GitTreeState == "dirty" { return false } - v := os.Getenv(constants.UpdateCheckEnvironmentVariable) - return v == "" || strings.ToLower(v) == "true" + return isUpdateCheckEnabledByEnvOrConfig(configfile) +} + +func isUpdateCheckEnabledByEnvOrConfig(configfile string) bool { + if v := getEnv(constants.UpdateCheckEnvironmentVariable); v != "" { + return strings.ToLower(v) == "true" + } + return 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 new file mode 100644 index 00000000000..8faf5d0e409 --- /dev/null +++ b/pkg/skaffold/update/update_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2019 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package update + +import ( + "testing" + + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestIsUpdateCheckEnabledByEnvOrConfig(t *testing.T) { + tests := []struct { + description string + envVariable string + 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: "env variable is set to random string", + envVariable: "foo", + }, + { + description: "env variable is empty and config is enabled", + configCheck: true, + expected: true, + }, + { + description: "env variable is false but Global update-check config is true", + envVariable: "false", + configCheck: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(&isConfigUpdateCheckEnabled, func(string) bool { return test.configCheck }) + t.Override(&getEnv, func(string) string { return test.envVariable }) + t.CheckDeepEqual(test.expected, isUpdateCheckEnabledByEnvOrConfig("dummyconfig")) + }) + } +} From b58aa4ec019c7af0599e0c85ade91f438c341008 Mon Sep 17 00:00:00 2001 From: balopat Date: Wed, 4 Sep 2019 00:10:25 -0700 Subject: [PATCH 2/2] cut v0.37.1 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93284350bbf..05e0f0821db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# v0.37.1 Release - 09/04/2019 + +This is a minor release for a privacy policy update: + +* add privacy notice and command to set update check false [#2774](https://github.com/GoogleContainerTools/skaffold/pull/2774) + # v0.37.0 Release - 08/29/2019 No new features in this release!