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.

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

Fix ConfigFile.Save() replacing symlink with file #2627

Merged
merged 1 commit into from Jul 16, 2020

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 15, 2020

As noticed in #2595 (comment)

In situations where ~/.docker/config.json was a symlink, saving the file would replace the symlink with a file, instead of updating the target file location;

mkdir -p ~/.docker
touch ~/real-config.json
ln -s ~/real-config.json ~/.docker/config.json

ls -la ~/.docker/config.json
# lrwxrwxrwx 1 root root 22 Jun 23 12:34 /root/.docker/config.json -> /root/real-config.json

docker login
# Username: thajeztah
# Password:

# Login Succeeded

ls -la ~/.docker/config.json
-rw-r--r-- 1 root root 229 Jun 23 12:36 /root/.docker/config.json
@thaJeztah
Copy link
Member Author

@thaJeztah thaJeztah commented Jul 15, 2020

@thaJeztah
Copy link
Member Author

@thaJeztah thaJeztah commented Jul 15, 2020

Ah, crap, looks like CircleCI disabled the "old" configs;

#!/bin/sh -eo pipefail
# No configuration was found in your project. Please refer to https://circleci.com/docs/2.0/ to get started with your configuration.
# 
# -------
# Warning: This configuration was auto-generated to show you the message above.
# Don't rerun this job. Rerunning will have no effect.
false

Exited with code exit status 1
@thaJeztah thaJeztah force-pushed the thaJeztah:dont_overwrite_symlinks branch from de62549 to f716e43 Jul 15, 2020
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

LGTM

In situations where `~/.docker/config.json` was a symlink, saving
the file would replace the symlink with a file, instead of updating
the target file location;

    mkdir -p ~/.docker
    touch ~/real-config.json
    ln -s ~/real-config.json ~/.docker/config.json

    ls -la ~/.docker/config.json
    # lrwxrwxrwx 1 root root 22 Jun 23 12:34 /root/.docker/config.json -> /root/real-config.json

    docker login
    # Username: thajeztah
    # Password:

    # Login Succeeded

    ls -la ~/.docker/config.json
    -rw-r--r-- 1 root root 229 Jun 23 12:36 /root/.docker/config.json

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the thaJeztah:dont_overwrite_symlinks branch from f716e43 to 75ab44a Jul 16, 2020
@thaJeztah
Copy link
Member Author

@thaJeztah thaJeztah commented Jul 16, 2020

Rebased to trigger CI

@codecov-commenter
Copy link

@codecov-commenter codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #2627 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2627   +/-   ##
=======================================
  Coverage   58.12%   58.13%           
=======================================
  Files         295      295           
  Lines       21191    21194    +3     
=======================================
+ Hits        12318    12321    +3     
  Misses       7966     7966           
  Partials      907      907           
@tiborvass tiborvass merged commit 8ff4f03 into docker:master Jul 16, 2020
8 checks passed
8 checks passed
DCO DCO
Details
ci/circleci: cross Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
codecov/patch 100.00% of diff hit (target 50.00%)
Details
codecov/project 58.13% (+0.00%) compared to 03716c0
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@thaJeztah thaJeztah deleted the thaJeztah:dont_overwrite_symlinks branch Jul 16, 2020
@thaJeztah thaJeztah added this to the 20.03.0 milestone Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.