docker / docker-credential-helpers

Programs to keep Docker login credentials safe by storing in platform keystores
MIT License
1.05k stars 166 forks source link

Assorted improvements, and add "--version, -v", and "--help, -h" flags #284

Closed thaJeztah closed 1 year ago

thaJeztah commented 1 year ago

See individual commits for details. Some outlined below:

client: use os/exec/Cmd.Environ() instead of os.Environ()

Don't set Env if not set; the default is already handled if it's nil; from the documentation: https://pkg.go.dev/os/exec@go1.20.4#Cmd.Env

// If Env is nil, the new process uses the current process's
// environment.

Use os/exec/Cmd.Environ() instead of os.Environ(), which was added in go1.19, and handles additional environment variables, such as PWD on POSIX systems, and SYSTEMROOT on Windows. https://pkg.go.dev/os/exec@go1.20.4#Cmd.Environ

Also remove a redundant fmt.Sprintf(), as we're only concatenating strings.

credentials: Serve(): use "Name instead of "os.Args[0]" for usage output

GNU guidelines describes; https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html#g_t_002d_002dversion

The program’s name should be a constant string; don’t compute it from argv[0].
The idea is to state the standard or canonical name for the program, not its
file name.

Although the above recommendation is for --version output, it probably makes sense to do the same for the "usage" output.

Before this change:

/usr/local/bin/docker-credential-osxkeychain invalid command
Usage: /usr/local/bin/docker-credential-osxkeychain <store|get|erase|list|version>

/Applications/Docker.app/Contents/Resources/bin/docker-credential-osxkeychain invalid command
Usage: /Applications/Docker.app/Contents/Resources/bin/docker-credential-osxkeychain <store|get|erase|list|version>

With this patch:

/usr/local/bin/docker-credential-osxkeychain invalid command
Usage: docker-credential-osxkeychain <store|get|erase|list|version>

/Applications/Docker.app/Contents/Resources/bin/docker-credential-osxkeychain invalid command
Usage: docker-credential-osxkeychain <store|get|erase|list|version>

credentials: Serve(): simplify error-handling logic

Don't use an err if we can print the error immediately :)

credentials: HandleCommand(): improve error for unknown command/action

Before this change:

docker-credential-osxkeychain nosuchaction
Unknown credential action `nosuchaction`

After this change:

docker-credential-osxkeychain nosuchaction
docker-credential-osxkeychain: unknown action: nosuchaction

credentials: Serve(): implement "--version, -v", and "--help, -h" flags

As recommended in the GNU documentation;

With this patch:

$ docker-credential-osxkeychain --version
docker-credential-osxkeychain (github.com/docker/docker-credential-helpers) v0.7.0-51-g26c426e.m

$ docker-credential-osxkeychain -v
docker-credential-osxkeychain (github.com/docker/docker-credential-helpers) v0.7.0-51-g26c426e.m

$ docker-credential-osxkeychain --help
Usage: docker-credential-osxkeychain <store|get|erase|list|version>

$ docker-credential-osxkeychain -h
Usage: docker-credential-osxkeychain <store|get|erase|list|version>

credentials: define consts for supported actions (sub-commands)

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 24.24% and project coverage change: -0.56 :warning:

Comparison is base (f8e94d9) 55.23% compared to head (129017a) 54.68%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #284 +/- ## ========================================== - Coverage 55.23% 54.68% -0.56% ========================================== Files 9 9 Lines 668 673 +5 ========================================== - Hits 369 368 -1 - Misses 256 262 +6 Partials 43 43 ``` | [Impacted Files](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker) | Coverage Δ | | |---|---|---| | [client/command.go](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker#diff-Y2xpZW50L2NvbW1hbmQuZ28=) | `0.00% <0.00%> (ø)` | | | [credentials/credentials.go](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker#diff-Y3JlZGVudGlhbHMvY3JlZGVudGlhbHMuZ28=) | `48.67% <14.28%> (-3.67%)` | :arrow_down: | | [client/client.go](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker#diff-Y2xpZW50L2NsaWVudC5nbw==) | `67.53% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.