correctly handle configuration file saving when it is a relative symlink
fixes: #5281
- What I did Correctly hand the cases with config file being a relative symlink.
- How I did it
Check whether the result path of os.Readlink(cfgFile) is a relative path, if so, get the absolute path of it.
- How to verify it
mkdir ~/.docker
echo '{}' > ~/.docker/config-company.json
ln -sf ~/.docker/config-company.json ~/.docker/config.json
docker login -u <user>
grep auths ~/.docker/config.json && echo OK || echo Failed
- Description for the changelog
correctly handle configuration file saving when it is a relative symlink
- A picture of a cute animal (not mandatory but encouraged)
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 55.04%. Comparing base (
ae922ec) to head (1015d15). Report is 6 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #5282 +/- ##
==========================================
+ Coverage 55.01% 55.04% +0.02%
==========================================
Files 361 361
Lines 30137 30142 +5
==========================================
+ Hits 16581 16591 +10
+ Misses 12598 12594 -4
+ Partials 958 957 -1
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
The TestLoadDanglingSymlink test failed.
According to that test's comment, Docker CLI currently doesn't treat a dangling symlink as an error. However, filepath.EvalSymlinks does treat a dangling symlink as an error. Consequently, when the symlink is dangling, the value of cfgFile isn't assigned to the "resolved" path of the symlink, causing the last assertion in TestLoadDanglingSymlink to fail.
After conducting local tests, I've compiled the following table:
os.Readlink() |
filepath.EvalSymlinks() |
|
|---|---|---|
| Real file | ❌ | ✅ |
| Symlink | ✅ | ✅ |
| Dangling symlink | ✅ | ❌ |
| Resolve nested symlink | ❌ | ✅ |
In summary:
os.Readlinkis to check whether a file is a link or not and simply returns the raw target of that link without trying to resolve the pathfilepath.EvalSymlinkschecks if the file is valid (regardless of whether it's a symlink) and returns its resolved path.
~It appears that os.Readlink is the only suitable solution in this case. I've reset the code back to os.Readlink and see if it can pass the test.~
Update:
After more investigation, I found a limitation of os.Readlink: it only dereferences one layer of symlinks and doesn't handle "nested" symlinks (i.e., a symlink pointing to another symlink). So filetype.EvalSymlinks is better suited for supporting nested symlinks. To allow for dangling symlinks with filepath.EvalSymlinks, I've added some code to get around this issue, by extracting the path from the "NotExist" error. I've run docker buildx bake test on my local machine, and it passed successfully.
@vvoland I rebased and squashed this one; also adjusted the error-matching; https://github.com/docker/cli/pull/5282#discussion_r2102051077