From 4571d90f20876255387238e44230b69f12ccdf3b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 16 Jul 2020 12:01:54 +0200 Subject: [PATCH 1/2] config: print deprecation warning when falling back to ~/.dockercfg Relates to the deprecation, added in 3c0a167ed51ba1ed4ad93bbeaca43490f807e492 The docker CLI up until v1.7.0 used the `~/.dockercfg` file to store credentials after authenticating to a registry (`docker login`). Docker v1.7.0 replaced this file with a new CLI configuration file, located in `~/.docker/config.json`. When implementing the new configuration file, the old file (and file-format) was kept as a fall-back, to assist existing users with migrating to the new file. Given that the old file format encourages insecure storage of credentials (credentials are stored unencrypted), and that no version of the CLI since Docker v1.7.0 has created this file, the file is marked deprecated, and support for this file will be removed in a future release. This patch adds a deprecation warning, which is printed if the CLI falls back to using the deprecated ~/.dockercfg file. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit b83bc67136e1edce2bdc225863af83d45d5472c9) Signed-off-by: Sebastiaan van Stijn --- cli/config/config.go | 29 +++++++++++++++++++++++++++-- cli/config/config_test.go | 27 +++++++++++++++++++++++++++ cmd/docker/docker_test.go | 4 +++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/cli/config/config.go b/cli/config/config.go index 98147e270a1f..3f208f9c8832 100644 --- a/cli/config/config.go +++ b/cli/config/config.go @@ -24,12 +24,12 @@ const ( ) var ( - initConfigDir sync.Once + initConfigDir = new(sync.Once) configDir string homeDir string ) -// resetHomeDir is used in testing to resets the "homeDir" package variable to +// resetHomeDir is used in testing to reset the "homeDir" package variable to // force re-lookup of the home directory between tests. func resetHomeDir() { homeDir = "" @@ -42,6 +42,13 @@ func getHomeDir() string { return homeDir } +// resetConfigDir is used in testing to reset the "configDir" package variable +// and its sync.Once to force re-lookup between tests. +func resetConfigDir() { + configDir = "" + initConfigDir = new(sync.Once) +} + func setConfigDir() { if configDir != "" { return @@ -97,10 +104,20 @@ func LoadFromReader(configData io.Reader) (*configfile.ConfigFile, error) { return &configFile, err } +// TODO remove this temporary hack, which is used to warn about the deprecated ~/.dockercfg file +var ( + mutex sync.RWMutex + printLegacyFileWarning bool +) + // Load reads the configuration files in the given directory, and sets up // the auth config information and returns values. // FIXME: use the internal golang config parser func Load(configDir string) (*configfile.ConfigFile, error) { + mutex.Lock() + printLegacyFileWarning = false + mutex.Unlock() + if configDir == "" { configDir = Dir() } @@ -125,6 +142,9 @@ func Load(configDir string) (*configfile.ConfigFile, error) { // Can't find latest config file so check for the old one filename = filepath.Join(getHomeDir(), oldConfigfile) if file, err := os.Open(filename); err == nil { + mutex.Lock() + printLegacyFileWarning = true + mutex.Unlock() defer file.Close() if err := configFile.LegacyLoadFromReader(file); err != nil { return configFile, errors.Wrap(err, filename) @@ -140,6 +160,11 @@ func LoadDefaultConfigFile(stderr io.Writer) *configfile.ConfigFile { if err != nil { fmt.Fprintf(stderr, "WARNING: Error loading config file: %v\n", err) } + mutex.RLock() + defer mutex.RUnlock() + if printLegacyFileWarning { + _, _ = fmt.Fprintln(stderr, "WARNING: Support for the legacy ~/.dockercfg configuration file and file-format is deprecated and will be removed in an upcoming release") + } if !configFile.ContainsAuth() { configFile.CredentialsStore = credentials.DetectDefaultStore(configFile.CredentialsStore) } diff --git a/cli/config/config_test.go b/cli/config/config_test.go index c7787bb5d17f..26197454e6ee 100644 --- a/cli/config/config_test.go +++ b/cli/config/config_test.go @@ -12,9 +12,11 @@ import ( "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/config/credentials" + "github.com/docker/cli/cli/config/types" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/env" + "gotest.tools/v3/fs" ) var homeKey = "HOME" @@ -223,6 +225,31 @@ func TestOldJSON(t *testing.T) { } } +func TestOldJSONFallbackDeprecationWarning(t *testing.T) { + js := `{"https://index.docker.io/v1/":{"auth":"am9lam9lOmhlbGxv","email":"user@example.com"}}` + tmpHome := fs.NewDir(t, t.Name(), fs.WithFile(oldConfigfile, js)) + defer tmpHome.Remove() + defer env.PatchAll(t, map[string]string{homeKey: tmpHome.Path(), "DOCKER_CONFIG": ""})() + + // reset the homeDir, configDir, and its sync.Once, to force them being resolved again + resetHomeDir() + resetConfigDir() + + buffer := new(bytes.Buffer) + configFile := LoadDefaultConfigFile(buffer) + expected := configfile.New(tmpHome.Join(configFileDir, ConfigFileName)) + expected.AuthConfigs = map[string]types.AuthConfig{ + "https://index.docker.io/v1/": { + Username: "joejoe", + Password: "hello", + Email: "user@example.com", + ServerAddress: "https://index.docker.io/v1/", + }, + } + assert.Assert(t, strings.Contains(buffer.String(), "WARNING: Support for the legacy ~/.dockercfg configuration file and file-format is deprecated and will be removed in an upcoming release")) + assert.Check(t, is.DeepEqual(expected, configFile)) +} + func TestNewJSON(t *testing.T) { tmpHome, err := ioutil.TempDir("", "config-test") assert.NilError(t, err) diff --git a/cmd/docker/docker_test.go b/cmd/docker/docker_test.go index 388eb2af85e5..0acca1b6b138 100644 --- a/cmd/docker/docker_test.go +++ b/cmd/docker/docker_test.go @@ -17,7 +17,9 @@ import ( func TestClientDebugEnabled(t *testing.T) { defer debug.Disable() - tcmd := newDockerCommand(&command.DockerCli{}) + cli, err := command.NewDockerCli() + assert.NilError(t, err) + tcmd := newDockerCommand(cli) tcmd.SetFlag("debug", "true") cmd, _, err := tcmd.HandleGlobalFlags() assert.NilError(t, err) From 4c3b87d92249114fdaa651b1dbd37bb18699e869 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 25 Mar 2021 21:45:14 +0100 Subject: [PATCH 2/2] config.Load() remove unneeded locks These were added in b83bc67136e1edce2bdc225863af83d45d5472c9, but I'm not sure why I added these; they're likely not needed. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 09ddcffb2f5d8455744e46e91a502f7fbac9e882) Signed-off-by: Sebastiaan van Stijn --- cli/config/config.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/cli/config/config.go b/cli/config/config.go index 3f208f9c8832..93275f3d9858 100644 --- a/cli/config/config.go +++ b/cli/config/config.go @@ -105,18 +105,13 @@ func LoadFromReader(configData io.Reader) (*configfile.ConfigFile, error) { } // TODO remove this temporary hack, which is used to warn about the deprecated ~/.dockercfg file -var ( - mutex sync.RWMutex - printLegacyFileWarning bool -) +var printLegacyFileWarning bool // Load reads the configuration files in the given directory, and sets up // the auth config information and returns values. // FIXME: use the internal golang config parser func Load(configDir string) (*configfile.ConfigFile, error) { - mutex.Lock() printLegacyFileWarning = false - mutex.Unlock() if configDir == "" { configDir = Dir() @@ -142,9 +137,7 @@ func Load(configDir string) (*configfile.ConfigFile, error) { // Can't find latest config file so check for the old one filename = filepath.Join(getHomeDir(), oldConfigfile) if file, err := os.Open(filename); err == nil { - mutex.Lock() printLegacyFileWarning = true - mutex.Unlock() defer file.Close() if err := configFile.LegacyLoadFromReader(file); err != nil { return configFile, errors.Wrap(err, filename) @@ -160,8 +153,6 @@ func LoadDefaultConfigFile(stderr io.Writer) *configfile.ConfigFile { if err != nil { fmt.Fprintf(stderr, "WARNING: Error loading config file: %v\n", err) } - mutex.RLock() - defer mutex.RUnlock() if printLegacyFileWarning { _, _ = fmt.Fprintln(stderr, "WARNING: Support for the legacy ~/.dockercfg configuration file and file-format is deprecated and will be removed in an upcoming release") }