REMOVE MULTIPLE CREDENTIAL ERASES #1917
Conversation
When starting to erase docker credentials on logout, the slice "regsToTry" is initialised with the given server address. Afterwards the server address is normalized and the possible, different formats are being built for the actual credential erase. If the given server address is already normalized, it would be checked twice, issuing when checking the second time. This bug is fixed here. Signed-off-by: Robert Zastrau <robert.zastrau@icloud.com>
Fix problem that no logout is performed, when dealing with the default registry. Also shortened the whole func to avoid appending to a second slice. This also saves a second for loop, because auth credentials can be removed as they are found. Not sure how the errors shall be handled in here. Signed-off-by: Robert Zastrau <robert.zastrau@icloud.com>
Fix problem with hostname address. If default registry is handled, the hostname address would be empty and error logs would not be clear to the user. Signed-off-by: Robert Zastrau <robert.zastrau@icloud.com>
Codecov Report
@@ Coverage Diff @@
## master #1917 +/- ##
==========================================
+ Coverage 56.71% 56.72% +0.01%
==========================================
Files 310 310
Lines 21792 21787 -5
==========================================
Hits 12359 12359
+ Misses 8518 8513 -5
Partials 915 915 |
|
@thaJeztah After you pointed out that the case with the default registry is not handled, I observed that the only distinction between the default and a private registry is made, when checking the server address for an empty string. I rewrote most of the function accordingly. See the commit messages for further details. Furthermore, I do have a question. How should we handle errors in the runLogout func? |
|
|
||
| var ( | ||
| loggedIn bool // is set later, when checking for credentials |
There was a problem hiding this comment.
nit: can you move this var declaration closer to where it is actually used?
There was a problem hiding this comment.
Hello @Rapix-x thank you for your contribution 🦁
The code itself looks good, but as it is a fix + a refactoring, I would be more confident if it was backed with some unit tests 😅
I think you could get inspired with the login tests. Let me know if you need some help 👍
- What I did
Fixed the problem of
docker logouttrying to erase credentials multiple times, which leads to an error when credentials no longer exist. This happened with the macOS keychain as well as with pass on Ubuntu 18.04. Fixes #204- How I did it
When starting to arrange the server addresses to be checked, the program initialized the slice (regsToTry) with the original server address. If this address is already blank (without any prefix or suffix, e.g. "8.8.8.8"), it is inserted twice in the slice. I simply removed the original server address from the initialization of the slice.
- How to verify it
Since it is pure string manipulation, it is pretty obvious. However, all server addresses are already cleaned up and reduced to the host name, when registering the credentials on
docker login. Therefore, uncleaned server addresses shall not occur and they don't need to be checked/erased on logout.- Description for the changelog
Remove duplicate credential erases on
docker logout.- A picture of a cute animal
