Skip to content

document deprecation of legacy ~/.dockercfg config-file - #2526

Merged
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:deprecate_old_config
Aug 5, 2020
Merged

document deprecation of legacy ~/.dockercfg config-file#2526
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:deprecate_old_config

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

relates to #2504

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.

Signed-off-by: Sebastiaan van Stijn github@gone.nl

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

@silvin-lubecki@tiborvass ptal

Comment threaddocs/deprecated.md
@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 5e0c7a0 to aadfe59CompareJune 11, 2020 10:56
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

rebased

@thaJeztahthaJeztah added this to the 20.03.0 milestone Jun 11, 2020
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Discussing in the maintainers meeting with @tonistiigi, and perhaps we should print a warning if the CLI falls back to the old config file

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from aadfe59 to 6f37302CompareJune 15, 2020 08:49
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

updated; now printing a warning if we fall back to the deprecated ~/.dockercfg PTAL

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 6f37302 to 21e06edCompareJune 15, 2020 08:50
@codecov-commenter

codecov-commenter commented Jun 15, 2020

Copy link
Copy Markdown

Codecov Report

Merging #2526 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #2526 +/- ##
=======================================
Coverage 58.13% 58.13% =======================================
Files 295 295 Lines 21207 21207 =======================================
Hits 12328 12328 Misses 7970 7970 Partials 909 909 

Comment threadcli/config/config.go Outdated
if err != nil {
fmt.Fprintf(stderr, "WARNING: Error loading config file: %v\n", err)
}
if filepath.Base(configFile.Filename) == oldConfigfile {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 this doesn't work; I originally had it set the filename to ~/.dockercfg, but when loading from a legacy file, I think we write to the new file. Let me double check this

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 21e06ed to f94ebf2CompareJune 17, 2020 12:20
Comment threadcli/config/config.go Outdated
}

// TODO remove this temporary hack, which is used to warn about the deprecated ~/.dockercfg file
var printLegacyFileWarning bool

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutex

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, 🤦 I was thinking about that initially (and check if there's multiple paths to this code). Yeah, let me add one

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch 2 times, most recently from d6bd321 to f12d448CompareJune 17, 2020 12:57
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

@AkihiroSuda updated; PTAL

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

hmmm.. why is this failing 🤔

=== Failed
=== FAIL: cli/config TestOldJSONFallbackDeprecationWarning (0.01s)
config_test.go:245: assertion failed: --- expected
+++ configFile
{*configfile.ConfigFile}.Filename:
-: "/tmp/TestOldJSONFallbackDeprecationWarning-734712734/.docker/config.json"
+: "/root/.docker/config.json"

Perhaps DOCKER_CONFIG is set? 🤔

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch 2 times, most recently from 2cbeb64 to 0961028CompareJune 17, 2020 13:39

@silvin-lubeckisilvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 84c14e0 to 4e4c4aaCompareJune 25, 2020 13:22
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

🤦 I know what's happening. The initial value for configDir is set once;

initConfigDir.Do(setConfigDir)
, but because the unit tests run in a single suite, it means the first test that hits that part of the code will set its value, and tests after it will ignore any new values of HOME or CONFIG_DIR for the "default" value.

@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 4e4c4aa to bfd6882CompareJune 25, 2020 15:55
@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch 2 times, most recently from 069f04e to 35df132CompareJuly 16, 2020 10:02
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.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the deprecate_old_config branch from 35df132 to 3c0a167CompareAugust 3, 2020 09:46
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Having some issues getting the deprecation warning working properly; I'll have a look at that separately. I removed the code changes from this PR, and it's now back to only the deprecation documentation changes

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Bringing this one in; I'll have another look if I have some time to get the warning working in #2666

@thaJeztah
thaJeztah merged commit ac2ebac into docker:masterAug 5, 2020
@thaJeztah
thaJeztah deleted the deprecate_old_config branch August 5, 2020 07:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah@codecov-commenter@AkihiroSuda@silvin-lubecki