Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecation: config: remove support for old ~/.dockercfg #2504

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Deprecation: config: remove support for old ~/.dockercfg #2504

merged 1 commit into from
Apr 6, 2022

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented May 7, 2020 •
edited
Loading

depends on:

The ~/.dockercfg file was replaced by ~/.docker/config.json in 2015
(moby/moby@18c9b6c (moby/moby#12009)), but the CLI still falls back to checking if this file exists if no current (~/.docker/config.json) file was found.

Given that no version of the CLI since Docker v1.7.0 has created this file, and if such a file exists, it means someone hasn't re authenticated for 5 years, it's probably safe to remove this fallback.

- Description for the changelog

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

@thaJeztah thaJeztah added this to the 20.03.0 milestone May 7, 2020
@thaJeztah
Copy link
Member Author

We need to check Kubernetes; I found some code using ~/.dockercfg for authentication. I don't know if that's the "main" method of storing authentication info, or if it's used for some legacy paths here; https://github.com/kubernetes/api/blob/df4a449f30f251f38d0292ca48ecaf81692be07c/core/v1/types.go#L5542-L5549

@thaJeztah thaJeztah force-pushed the remove_old_config_file_support branch from de3fd82 to b7206d9 Compare May 9, 2020 17:04
silvin-lubecki
silvin-lubecki approved these changes May 11, 2020
View reviewed changes
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Looks good and safe to me, it removes lots of crappy logic 👍

@thaJeztah
Copy link
Member Author

Was discussing this in the maintainers meeting, and (similar to the kubernetes code I linked to), there might actually be other systems that use this format to store credentials. For that reason, we should officially deprecate it (and be sure to call it out in the release notes), and not immediately remove it (at least not for the next 20.0x release)

@thaJeztah thaJeztah mentioned this pull request May 13, 2020
Merged
@thaJeztah thaJeztah modified the milestones: 20.10.0, 20.10.1, 20.10.2 Dec 9, 2020
@thaJeztah thaJeztah modified the milestones: 20.10.2, 20.10.3 Jan 5, 2021
@thaJeztah thaJeztah modified the milestones: 20.10.3, 20.10.4, 21.xx Feb 2, 2021
@thaJeztah thaJeztah force-pushed the remove_old_config_file_support branch 2 times, most recently from c80b780 to 0af0d51 Compare March 3, 2021 12:51
@thaJeztah thaJeztah changed the title config: remove support for old ~/.dockercfg Deprecation: config: remove support for old ~/.dockercfg Mar 3, 2021
@codecov-io
Copy link

Codecov Report

Merging #2504 (0af0d51) into master (70a0015) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2504      +/-   ##
==========================================
+ Coverage   57.06%   57.13%   +0.07%     
==========================================
  Files         299      299              
  Lines       18683    18665      -18     
==========================================
+ Hits        10662    10665       +3     
+ Misses       7155     7133      -22     
- Partials      866      867       +1

@thaJeztah thaJeztah force-pushed the remove_old_config_file_support branch from 0af0d51 to 4517757 Compare March 8, 2021 16:30
@tao12345666333
Copy link
Contributor

SGTM. 👍

@thaJeztah thaJeztah force-pushed the remove_old_config_file_support branch from 4517757 to ad273a9 Compare May 21, 2021 11:19
@codecov-commenter
Copy link

codecov-commenter commented May 21, 2021 •
edited
Loading

Codecov Report

Merging #2504 (ee218fa) into master (59449a5) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2504      +/-   ##
==========================================
+ Coverage   59.00%   59.06%   +0.06%     
==========================================
  Files         284      284              
  Lines       23839    23799      -40     
==========================================
- Hits        14066    14057       -9     
+ Misses       8914     8883      -31     
  Partials      859      859

@thaJeztah
Copy link
Member Author

Ah, yes, forgot about this one; rebased, and let me move out of draft

@thaJeztah thaJeztah marked this pull request as ready for review May 21, 2021 11:20
@tao12345666333
Copy link
Contributor 

 docker: Error response from daemon: cgroups: cgroup mountpoint does not exist: unknown.

This seems to come from an error reported by containerd when processing cgroups.

@thaJeztah
Copy link
Member Author

Hmm.. yes, definitely not related; let me restart CI

tao12345666333
tao12345666333 approved these changes May 22, 2021
View reviewed changes
Copy link
Contributor

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

👍

@thaJeztah
Deprecation: config: remove support for old ~/.dockercfg
ee218fa 
The `~/.dockercfg` file was replaced by `~/.docker/config.json` in 2015
(github.com/docker/docker/commit/18c9b6c6455f116ae59cde8544413b3d7d294a5e),
but the CLI still falls back to checking if this file exists if no current
(`~/.docker/config.json`) file was found.

Given that no version of the CLI since Docker v1.7.0 has created this file,
and if such a file exists, it means someone hasn't re-authenticated for
5 years, it's probably safe to remove this fallback.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the remove_old_config_file_support branch from ad273a9 to ee218fa Compare April 6, 2022 19:36
@thaJeztah
Copy link
Member Author

rebased; still green; let's bring this one in

@thaJeztah thaJeztah merged commit 2f1b094 into docker:master Apr 6, 2022
@thaJeztah thaJeztah deleted the remove_old_config_file_support branch April 6, 2022 20:01
@thaJeztah thaJeztah mentioned this pull request May 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hc0503 hc0503 hc0503 approved these changes

@silvin-lubecki silvin-lubecki silvin-lubecki approved these changes

@tao12345666333 tao12345666333 tao12345666333 approved these changes

Assignees
No one assigned
Labels
impact/changelog impact/deprecation status/2-code-review
Projects
None yet
Milestone
23.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@thaJeztah @codecov-io @tao12345666333 @codecov-commenter @silvin-lubecki @hc0503