Commit Graph

2231 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 0ab0eca8bd
Merge pull request #5550 from thaJeztah/login_minor_refactor
cli/command: PromptUserForCredentials: assorted minor improvements and (linting) fixes
2024-10-21 23:23:06 +02:00
Sebastiaan van Stijn abb8e9b78a
Merge pull request #5546 from thaJeztah/hints_coverage
cli/hints: add tests
2024-10-21 18:08:28 +02:00
Laura Brehm 7029147458
Merge pull request #5557 from thaJeztah/minor_linting_issues 2024-10-21 17:00:40 +01:00
Paweł Gronowski d2b87a0a3b
Merge pull request #5553 from thaJeztah/login_idempotent
cli/config/credentials: skip saving config-file if credentials didn't change
2024-10-21 15:23:26 +02:00
Sebastiaan van Stijn d3f6867e4d
cli/config/credentials: skip saving config-file if credentials didn't change
Before this change, the config-file was always updated, even if there
were no changes to save. This could cause issues when the config-file
already had credentials set and was read-only for the current user.

For example, on NixOS, this poses a problem because `config.json` is a
symlink to a write-protected file;

    $ readlink ~/.docker/config.json
    /home/username/.config/sops-nix/secrets/ghcr_auth

    $ readlink -f ~/.docker/config.json
    /run/user/1000/secrets.d/28/ghcr_auth

Which causes `docker login` to fail, even if no changes were to be made;

    Error saving credentials: rename /home/derek/.docker/config.json2180380217 /home/username/.config/sops-nix/secrets/ghcr_auth: invalid cross-device link

This patch updates the code to only update the config file if changes
were detected. It there's nothing to save, it skips updating the file,
as well as skips printing the warning about credentials being stored
insecurely.

With this patch applied:

    $ docker login -u yourname
    Password:

    WARNING! Your credentials are stored unencrypted in '/root/.docker/config.json'.
    Configure a credential helper to remove this warning. See
    https://docs.docker.com/go/credential-store/

    Login Succeeded

    $ docker login -u yourname
    Password:
    Login Succeeded

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-21 00:19:52 +02:00
Sebastiaan van Stijn 6b9083776f
cli/command: AddPlatformFlag: suppress unhandled error
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-20 17:51:36 +02:00
Sebastiaan van Stijn fb61156b05
cli/command/registry: fix minor linting issues
- fix camelCase naming of verifyLoginOptions
- suppress unhandled errors that can be ignored

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-20 17:51:12 +02:00
Sebastiaan van Stijn 4b7a1e4613
cli/command: PromptUserForCredentials: suppress unhandled errors
Keep the linters (and my IDE) happy; these errors should be safe to ignore.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 13:24:19 +02:00
Sebastiaan van Stijn 378a3d7d36
cli/command: PromptUserForCredentials: use consts for all hints
This message resulted in code-lines that were too long; move it to a
const together with the other hint. While at it, also suppress unhandled
error, and touch-up the code-comment.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 13:23:29 +02:00
Sebastiaan van Stijn 54e3685bcd
cli/command: ConfigureAuth: fix deprecation comment
Deprecation comments must have an empty line before them, otherwise tools
and linters may not recognise them. While fixing this, also updated the
reference to PromptUserForCredentials to be a docs-link to make it clickable.

Updates 6e4818e7d6.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 13:05:31 +02:00
Sebastiaan van Stijn 3d8b49523d
cli/command: PromptUserForCredentials: print error on terminal restore fail
If restoring the terminal state fails, "echo" no longer works, which means
that anything the user types is no longer shown. The login itself may already
have succeeded, so we should not fail the command, but it's good to inform
the user that this happened, which may give them a clue why things no longer
work as they expect them to work.

With this patch:

    docker login -u yourname
    Password:
    Error: failed to restore terminal state to echo input: something bad happened

    Login Succeeded

We should consider printing instructions how  to restore this manually (other
than restarting the shell). e.g., 'run stty echo' when in a Linux or macOS shell,
but PowerShell and CMD.exe may need different instructions.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 12:49:44 +02:00
Sebastiaan van Stijn a21a5f4243
cli/command: PromptUserForCredentials: always trim password
we don't support empty passwords; when prompting the user for a password,
we already trim the result, but we didn't do the same for a password that's
passed through stdin or through the `-p` / `--password` flag.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 12:10:46 +02:00
Sebastiaan van Stijn eda78e9cdc
cli/command: PromptUserForCredentials: move trimming where it's used
- move trimming defaultUsername inside the if-branch, as it's the only
  location where the result of the trimmed username is use.
- do the reverse for trimming argUser, because the result of trimming
  argUser is used outside of the if-branch (not just for the condition).
  putting it inside the condition makes it easy to assume the result is
  only used locally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 12:07:51 +02:00
Sebastiaan van Stijn 581cf36bd4
cli/command: PromptUserForCredentials: move "post" check for empty name
move the "post" check for username being empty inside the branch
that's handling the username, as it's the only branch where username
is mutated after checking if it's empty.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 12:06:49 +02:00
Sebastiaan van Stijn a55cfe5f82
cli/command: PromptUserForCredentials: inline isDefaultRegistry
remove isDefaultRegistry and inline it where it's used; the code-comment
already outlines what we're looking for, so the intermediate var didn't
add much currently.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 11:58:42 +02:00
Sebastiaan van Stijn 3a8485085d
cli/command: PromptUserForCredentials: remove named output variables
This function has multiple conditional branches, which makes it harder
to see at a glance whether authConfig may be partially populated. This
patch instead returns a fresh instance for error returns to prevent any
confusion.

It also removes the named output variables, as they're now no longer used,
and the returned types should already be descriptive enough to understand
what's returned.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 11:46:21 +02:00
Sebastiaan van Stijn 87acf77aef
cli/hints: add tests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-19 00:48:16 +02:00
Sebastiaan van Stijn 35d7b1a7a6
cli/command/container: TestWaitExitOrRemoved use subtests
=== RUN   TestWaitExitOrRemoved
    === RUN   TestWaitExitOrRemoved/normal-container
    === RUN   TestWaitExitOrRemoved/give-me-exit-code-42
    === RUN   TestWaitExitOrRemoved/i-want-a-wait-error
    time="2024-10-13T18:48:14+02:00" level=error msg="Error waiting for container: removal failed"
    === RUN   TestWaitExitOrRemoved/non-existent-container-id
    time="2024-10-13T18:48:14+02:00" level=error msg="error waiting for container: no such container: non-existent-container-id"
    --- PASS: TestWaitExitOrRemoved (0.00s)
        --- PASS: TestWaitExitOrRemoved/normal-container (0.00s)
        --- PASS: TestWaitExitOrRemoved/give-me-exit-code-42 (0.00s)
        --- PASS: TestWaitExitOrRemoved/i-want-a-wait-error (0.00s)
        --- PASS: TestWaitExitOrRemoved/non-existent-container-id (0.00s)
    PASS

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-16 12:03:17 +02:00
Sebastiaan van Stijn 3b38dc67be
cli/command/container: set empty args in tests and discard output
Prevent some tests from failing when running from a pre-compiled
testbinary, and discard output to make the output less noisy.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-16 12:01:25 +02:00
Sebastiaan van Stijn e1c472a436
completion: add test for VolumeNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 19:08:56 +02:00
Sebastiaan van Stijn 302d73f990
completion: add test for NetworkNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 19:08:52 +02:00
Sebastiaan van Stijn ab418a38d8
completion: add test for ImageNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 19:08:26 +02:00
Sebastiaan van Stijn f3b4094eb0
completion: add test for ContainerNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 19:07:52 +02:00
Sebastiaan van Stijn be197da6b8
completion: add test for NoComplete
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 17:54:40 +02:00
Sebastiaan van Stijn 51713196c9
completion: add test for FromList
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 17:53:19 +02:00
Sebastiaan van Stijn a5ca5b33f1
completion: add test for FileNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 17:52:50 +02:00
Sebastiaan van Stijn 8f2e5662e7
completion: add test for EnvVarNames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 17:48:49 +02:00
Sebastiaan van Stijn b8cddc63ad
completion: ContainerNames: don't panic on nil filter
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-13 17:47:07 +02:00
Harald Albers 147630a309 Only complete removable containers if --force is not given
Signed-off-by: Harald Albers <github@albersweb.de>
2024-10-10 21:34:38 +00:00
Paweł Gronowski d085e2445c
image/history: Add `--platform` flag
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-10 17:00:43 +02:00
Paweł Gronowski b0bb4ba7f2
image/load: Add `--platform`
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-10 16:35:07 +02:00
Paweł Gronowski a20eb45b26
image/save: Add `--platform`
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-10 16:35:03 +02:00
Stavros Panakakis 0319795d4f cli/compose: implement the ports validation method
This commit implements a validation
method for the port mappings.

Also, it removes the ports validation
method from the expose property
since they do not accept the
same type of values.

Signed-off-by: Stavros Panakakis <stavrospanakakis@gmail.com>
2024-10-10 11:50:11 +03:00
Sebastiaan van Stijn 839dbbcf27
cli/command/images: set cmd.Args to prevent test-failures
When running tests from my IDE, it compiles the tests before running,
then executes the compiled binary to run the tests. Cobra doesn't like that,
because in that situation os.Args is taken as argument for the command that's
executed. The command that's tested now sees the `test-` flags as arguments
(`-test.v -test.run ..`), which causes various tests to fail ("Command XYZ
does not accept arguments").

    # compile the tests:
    go test -c -o foo.test

    # execute the test:
    ./foo.test -test.v -test.run TestFoo
    === RUN   TestFoo
    Error: "foo" accepts no arguments.

Set arguments to an empty slice to make sure it doesn't inherit arguments
from the test-binary.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-08 20:26:15 +02:00
Sebastiaan van Stijn 19eeb10155
cli/command/image: fix TestNewSaveCommandSuccess to actually test
This test was added in [moby@b2551c6] as part of a larger PR that implemented
unit tests in various packages. In this specific test, it looks like the
`imageSaveFunc` that's defined in the test-table was forgotten to be wired
up, causing all tests to effectively be skipped.

This patch wires up the function so that it's used in the test.

[moby@b2551c6]: b2551c619d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-08 19:41:05 +02:00
Sebastiaan van Stijn 185622986e
Merge pull request #5506 from Stavrospanakakis/cli-container-testing
command: add tests for container kill, commit, and pause
2024-10-08 16:15:26 +02:00
Sebastiaan van Stijn d42cf96e15
cli/command/image: add shell completion for --platform flags
With this patch, completion is provided for `--platform` flags:

    docker pull --platform<TAB>
    linux           linux/amd64     linux/arm/v5    linux/arm/v7    linux/arm64/v8  linux/riscv64   wasip1          windows
    linux/386       linux/arm       linux/arm/v6    linux/arm64     linux/ppc64le   linux/s390x     wasip1/wasm     windows/amd64

Note that `docker buildx build` (with BuildKit) does not yet provide completion;
it's provided through buildx, and uses a different format (accepting multiple
comma-separated platforms). Interestingly, tab-completion for `docker build`
currently uses completion for non-buildkit, and has some other issues that may
have to be looked into.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-08 13:15:28 +02:00
Sebastiaan van Stijn 8c7f713db6
cli/command/container: add shell completion for --platform flags
With this patch, completion is provided for `--platform` flags:

    docker run --platform<TAB>
    linux           linux/amd64     linux/arm/v5    linux/arm/v7    linux/arm64/v8  linux/riscv64   wasip1          windows
    linux/386       linux/arm       linux/arm/v6    linux/arm64     linux/ppc64le   linux/s390x     wasip1/wasm     windows/amd64

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-08 13:14:45 +02:00
Sebastiaan van Stijn ce1aebcc30
cli/command/completion: add Platforms
Add a utility for completing platform strings.

Platforms offers completion for platform-strings. It provides a non-exhaustive
list of platforms to be used for completion. Platform-strings are based on
[runtime.GOOS] and [runtime.GOARCH], but with (optional) variants added. A
list of recognised os/arch combinations from the Go runtime can be obtained
through "go tool dist list".

Some noteworthy exclusions from this list:

  - arm64 images ("windows/arm64", "windows/arm64/v8") do not yet exist for windows.
  - we don't (yet) include `os-variant` for completion (as can be used for Windows images)
  - we don't (yet) include platforms for which we don't build binaries, such as
    BSD platforms (freebsd, netbsd, openbsd), android, macOS (darwin).
  - we currently exclude architectures that may have unofficial builds,
    but don't have wide adoption (and no support), such as loong64, mipsXXX,
    ppc64 (non-le) to prevent confusion.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-08 13:12:31 +02:00
Stavros Panakakis 442c38636f command: add tests for container kill, commit, and pause
This commit adds tests for the commands
docker kill, docker commit, and docker
pause. Also, it creates the mock methods
of the docker client ContainerCommit and
ContainerPause so they can
be used in the tests.

For docker kill, it covers the
cases that:
 - the command runs successfully
 - the client returns an error

For docker commit, it covers
the cases that:
 - the command runs successfully
 - the client returns an error

For docker pause, it covers
the cases that:
 - the command runs successfully
 - the client returns an error

Signed-off-by: Stavros Panakakis <stavrospanakakis@gmail.com>
2024-10-06 20:00:49 +03:00
Sebastiaan van Stijn 7908982543
Merge pull request #5467 from Stavrospanakakis/cli-container-tests
command: add tests for container diff and rename
2024-10-04 13:49:32 +02:00
Nicolas De Loof 9ecfe4f5a7
move parsing key-value files to a separate package
Move the code for parsing key-value files, such as used for
env-files and label-files to a separate package. This allows
other projects (such as compose) to use the same parsing
logic, but provide custom lookup functions for their situation
(which is slightly different).

The new package provides utilities for parsing key-value files
for either a file or an io.Reader. Most tests for EnvFile were
now testing functionality that's already tested in the new package,
so were (re)moved.

Co-authored-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-04 12:27:10 +02:00
Sebastiaan van Stijn d49e72c0ac
cli/command/container: add unit tests for completion helpers
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-02 10:38:25 +02:00
Sebastiaan van Stijn 462e08219d
cli/container: use github.com/moby/sys/capability for completions
We used a hard-coded list of capabilities that we copied from containerd,
but the new "capability" package allows use to have a maintained list
of capabilities.

There's likely still some improvements to be made;

First of all, the capability package could provide a function to get the list
of strings.

On the completion-side, we need to consider what format is most convenient;
currently we use the canonical name (uppercase and "CAP_" prefix), however,
tab-completion is case-sensitive by default, so requires the user to type
uppercase letters to filter the list of options.

Bash completion provides a `completion-ignore-case on` option to make completion
case-insensitive (https://askubuntu.com/a/87066), but it looks to be a global
option; the current cobra.CompletionOptions also don't provide this as an option
to be used in the generated completion-script.

Fish completion has `smartcase` (by default?) which matches any case if
all of the input is lowercase.

Zsh does not have a dedicated option, but allows setting matching-rules
(see https://superuser.com/a/1092328).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-01 14:01:02 +02:00
Sebastiaan van Stijn bd96bdaf1b
align "conflicting options" errors for consistency
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-01 12:08:56 +02:00
Sebastiaan van Stijn df8b34595b
cli/command/container: stop, restart: rename "--time" to "--timeout"
This renames the `--time` flag as used on `docker stop` and `docker restart`
to `--timeout`,  bringing it in line with other uses for this property,
such as `--stop-timeout` on `docker run`.

The `--time` option is deprecated and hidden, but will be kept for
backward compatibility, as these options existed for a long time.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-30 09:43:54 +02:00
Rob Murray 8fca0a1f28 Check that --ip-range is a CIDR address
Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-09-26 20:38:33 +01:00
Sebastiaan van Stijn ac502b5909
cli/command/container: add unit tests for container stop
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-26 18:58:40 +02:00
Sebastiaan van Stijn 16aa994255
cli/command/container: add unit tests for container restart
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-26 18:57:27 +02:00
Sebastiaan van Stijn b12ac897fb
vendor: github.com/docker/docker 164cae56ed95 (master, v-next)
full diff: 2269acc7a3...164cae56ed

Co-authored-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-24 16:30:30 +02:00