The `Commit` type was introduced in 2790ac68b3,
to assist triaging issues that were reported with an incorrect version of
runc or containerd. At the time, both `runc` and `containerd` were not yet
stable, and had to be built from a specific commit to guarantee compatibility.
We encountered various situations where unexpected (and incompatible) versions
of those binaries were packaged, resulting in hard to trace bug-reports.
For those situations, a "expected" version was set at compile time, to
indicate if the version installed was different from the expected version;
docker info
...
runc version: a592beb5bc4c4092b1b1bac971afed27687340c5 (expected: 69663f0bd4b60df09991c08812a60108003fa340)
Both `runc` and `containerd` are stable now, and docker 19.03 and up set the
expected version to the actual version since c65f0bd13c
and 23.0 did the same for the `init` binary b585c64e2b,
to prevent the CLI from reporting "unexpected version".
In short; the `Expected` fields no longer serves a real purpose, so we should
no longer print it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This command was declaring that it requires at least 1 argument, when it
needs exactly 1 argument. This was causing the CLI to panic when the
command was invoked with no argument:
`docker volume update`
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Running `docker login` in a non-interactive environment sometimes errors
out if no username/pwd is provided. This handling is somewhat
inconsistent – this commit addresses that.
Before:
| `--username` | `--password` | Result |
|:------------:|:------------:| ------------------------------------------------------------------ |
| ✅ | ✅ | ✅ |
| ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ❌ | ✅ | hangs |
After:
| `--username` | `--password` | Result |
|:------------:|:------------:| ------------------------------------------------------------------ |
| ✅ | ✅ | ✅ |
| ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ❌ | ✅ | `Error: Cannot perform an interactive login from a non TTY device` |
It's worth calling out a separate scenario – if there are previous,
valid credentials, then running `docker login` with no username or
password provided will use the previously stored credentials, and not
error out.
```console
cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "xxxxxxxxxxx"
}
}
}
⭑ docker login 0>/dev/null
Authenticating with existing credentials...
Login Succeeded
```
This commit also applies the same non-interactive handling logic to the
new web-based login flow, which means that now, if there are no prior
credentials stored and a user runs `docker login`, instead of initiating
the new web-based login flow, an error is returned.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Normalization/converting the registry address to just a hostname happens
inside of `command.GetDefaultAuthConfig`. Use this value for the rest of
the login flow/storage.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This reverts commit e6624676e0.
Since e6624676e0, during login, we started
normalizing `registry-1.docker.io` to `index.docker.io`. This means that
if a user logs in with `docker login -u [username]
registry-1.docker.io`, the user's credentials get stored in
credhelpers/config.json under `https://index.docker.io/v1/`.
However, while the registry code normalizes an image reference without
registry (`docker pull alpine:latest`) and image references explicitly for
`index.docker.io` (`docker pull index.docker.io/library/alpine:latest`)
to the official index server (`https://index.docker.io/v1/`), and
fetches credentials for that auth key, it does not normalize
`registry-1.docker.io`, which means pulling explicitly from there
(`docker pull registry-1.docker.io/alpine:latest`) will not use
credentials stored under `https://index.docker.io/v1/`.
As such, until changes are made to the registry/pull/push code to
normalize `registry-1.docker.io` to `https://index.docker.io/v1/`, we
should not normalize this during login.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
cli/command/utils.go:225:29: printf: non-constant format string in call to github.com/pkg/errors.Wrapf (govet)
return errors.Wrapf(err, fmt.Sprintf("invalid output path: %q must be a directory or a regular file", path))
^
cli/command/manifest/cmd.go:21:33: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(dockerCli.Err(), "\n"+cmd.UsageString())
^
cli/command/service/remove.go:45:24: printf: non-constant format string in call to github.com/pkg/errors.Errorf (govet)
return errors.Errorf(strings.Join(errs, "\n"))
^
cli/command/service/scale.go:93:23: printf: non-constant format string in call to github.com/pkg/errors.Errorf (govet)
return errors.Errorf(strings.Join(errs, "\n"))
^
cli/command/stack/swarm/remove.go:74:24: printf: non-constant format string in call to github.com/pkg/errors.Errorf (govet)
return errors.Errorf(strings.Join(errs, "\n"))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/system/info.go:375:5: S1009: should omit nil check; len() for []github.com/docker/docker/api/types/system.NetworkAddressPool is defined as zero (gosimple)
if info.DefaultAddressPools != nil && len(info.DefaultAddressPools) > 0 {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
On `docker ps`, port bindings with an IPv6 HostIP should have their
addresses put into brackets when joining them to their ports.
RFC 3986 (Section 3.2.2) stipulates that IPv6 addresses should be
enclosed within square brackets. This RFC is only about URIs. However,
doing so here helps user identifier what's part of the IP address and
what's the port. It also makes it easier to copy/paste that
'[addr]:port' into other software (including browsers).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Don't output the extra spacing around the images when none of the
top-level image entries has any children.
This makes the list look better when ran against the graphdrivers image
store.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This commit adds support for the oauth [device-code](https://auth0.com/docs/get-started/authentication-and-authorization-flow/device-authorization-flow)
login flow when authenticating against the official registry.
This is achieved by adding `cli/internal/oauth`, which contains code to manage
interacting with the Docker OAuth tenant (`login.docker.com`), including launching
the device-code flow, refreshing access using the refresh-token, and logging out.
The `OAuthManager` introduced here is also made available through the `command.Cli`
interface method `OAuthManager()`.
In order to maintain compatibility with any clients manually accessing
the credentials through `~/.docker/config.json` or via credential
helpers, the added `OAuthManager` uses the retrieved access token to
automatically generate a PAT with Hub, and store that in the
credentials.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This test was just incorrect (and testing incorrect
behavior): it was checking that `docker run` exited with a `context
canceled` error after signalling the CLI/cancelling the command's
context, but this was incorrect (and was fixed in
991b1303da - which was when this test
started failing).
However, since this test assertion was happening inside of a goroutine,
it would sometimes pass if this assertion didn't get to run before the
test suite terminated. It was flaky because sometimes this assertion
inside the goroutine did get to execute, but after the test finished
execution, which is a big no-no.
As an aside, assertions inside goroutines are generally bad, and `govet`
even has a linter for this (but it only catches `t.Fatal` and `t.FailNow`
calls and not `assert.Xx`.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Such as with `docker run`, if a user CTRL-Cs while attached to a
container, we should forward the signal and wait for the exit from
`ContainerWait`, instead of just returning.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
In 3f0d90a2a9 we introduced a global
signal handler and made sure all the contexts passed into command
execution get (appropriately) cancelled when we get a SIGINT.
Due to that change, and how we use this context during `docker attach`,
we started to return the context cancelation error when a user signals
the running `docker attach`.
Since this is the intended behavior, we shouldn't return an error, so
this commit adds checks to ignore this specific error in this case.
Also adds a regression test.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This environment variable allows for setting additional headers
to be sent by the client. Headers set through this environment
variable are added to headers set through the config-file (through
the HttpHeaders field).
This environment variable can be used in situations where headers
must be set for a specific invocation of the CLI, but should not
be set by default, and therefore cannot be set in the config-file.
WARNING: If both config and environment-variable are set, the environment
variable currently overrides all headers set in the configuration file.
This behavior may change in a future update, as we are considering the
environment variable to be appending to existing headers (and to only
override headers with the same name).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The fileStore itself is aware that it's insecure, so we can make it
responsible for printing the warning. It's not "perfect", as we use
`os.Stderr` unconditionally (not `dockerCli.Err()`), but probably won't
make a difference in _most_ cases.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Add an empty line before the warning to separate it from the command's output
- Use the `/go/` redirect URL that we have available.
- Put quotes around the filename used for storage.
- Use present tense for the message, as the message is printed while saving.
- User "credentials" instead of "password" for consistency with "credentials-store"
Before:
docker login myregistry.example.com
Username: thajeztah
Password:
WARNING! Your password will be stored unencrypted in /root/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credential-stores
Login Succeeded
After:
docker login myregistry.example.com
Username: thajeztah
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
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If we fail to save credentials, make sure that the error about saving
doesn't get lost in the warning about credentials being stored unencrypted.
Also discard errors about printing the warning, as those would be unlikely,
and if they would occur, probably would fail to be printed as well.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `reportError` utility was present because cli.StatusError would print
the error decorated with `Status: <error-message>, Code: <exit-code>`.
That was not desirable in many cases as it would mess-up the output. To
prevent this, the CLI had code to check for an empty `Status` (error message)
in which case the error would be "ignored" (and only used for the exit-status),
and the `reportError` utility would be used to manually print a custom error
message before returning the error.
Now that bca2090061 fixed the output format
of `cli.StatusError`, and 3dd6fc365d and
350a0b68a9 no longer discard these error,
we can get rid of this utility, and just set the error-message for
the status-error.
This patch:
- Introduces a `withHelp` which takes care of decorating errors with
a "Run --help" hint for the user.
- Introduces a `toStatusError` utility that detects certain errors in
the container to assign a corresponding exit-code (these error-codes
can be used to distinguish "client" errors from "container" errors).
- Removes the `reportError` utility, and removes code that manually
printed errors before returning.
Behavior is mostly unmodified, with the exception of some slight reformatting
of the errors:
- `withHelp` adds a `docker:` prefix to the error, to indicate the error
is produced by the `docker` command. This prefix was already present
in most cases.
- The "--help" hint is slightly updated ("Run 'docker run --help' for
more information" instead of "See 'docker run --help'"), to make it
more clear that it's a "call to action".
- An empty is added before the "--help" hint to separate it better from
the error-message.
Before this patch:
$ docker run --pull=invalid-option alpine
docker: invalid pull option: 'invalid-option': must be one of "always", "missing" or "never".
See 'docker run --help'.
$ echo $?
125
$ docker run --rm alpine nosuchcommand
docker: Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "nosuchcommand": executable file not found in $PATH: unknown.
$ echo $?
127
With this patch:
$ docker run --pull=invalid-option alpine
docker: invalid pull option: 'invalid-option': must be one of "always", "missing" or "never"
Run 'docker run --help' for more information
$ echo $?
125
$ docker run --rm alpine nosuchcommand
docker: Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "nosuchcommand": executable file not found in $PATH: unknown.
Run 'docker run --help' for more information
$ echo $?
127
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this patch:
docker run --volumes-from amazing_nobel
amazing_cannon boring_wozniak determined_banzai
elegant_solomon reverent_booth amazing_nobel
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
"docker run" and "docker create" are mostly identical, so we can copy
the same completion functions,
We could possibly create a utility for this (similar to `addFlags()` which
configures both commands with the flags they share). I considered combining
his with `addFlags()`, but that utility is also used in various tests, in
which we don't need this feature, so keeping that for a future exercise.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's an alias for cobra.FixedCompletions but takes a variadic list
of strings, so that it's not needed to construct an array for this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
EnvVarNames offers completion for environment-variable names. This
completion can be used for "--env" and "--build-arg" flags, which
allow obtaining the value of the given environment-variable if present
in the local environment, so we only should complete the names of the
environment variables, and not their value. This also prevents the
completion script from printing values of environment variables
containing sensitive values.
For example;
export MY_VAR=hello
docker run --rm --env MY_VAR alpine printenv MY_VAR
hello
Before this patch:
docker run --env GO
GO111MODULE=auto GOLANG_VERSION=1.21.12 GOPATH=/go GOTOOLCHAIN=local
With this patch:
docker run --env GO<tab>
GO111MODULE GOLANG_VERSION GOPATH GOTOOLCHAIN
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is just a convenience function to allow defining completion to
use the default (complete with filenames and directories).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn
Laura Brehm
lentil32
Sebastiaan van Stijn
David Karlsson
Albin Kerouanton
Paweł Gronowski
plaurent