The DefaultStopSignal const has been deprecated, because the daemon already
handles a default value. The current code did not actually send the default
value unless the flag was set, which also made the flag description incorrect,
because in that case, the _daemon's_ default would be used, which could
potentially be different as was specified here.
This patch removes the default value from the flag, leaving it to the daemon
to set a default.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Locking was removed in https://github.com/docker/cli/pull/3025 which
allows for parallel calls to config.Load to modify global state.
The consequence in this case is innocuous, but it does trigger a
`DATA RACE` exception when tests run with `-race` option.
Signed-off-by: coryb <cbennett@netflix.com>
This allows us to drop the `//go:generate` and use of the github.com/mjibson/esc
utility.
worth noting that Go's native "embed" does not compress files. We could compress
these files as part of a build / validate step (which would add some complexity
when updating these files) if this is a concern, but not sure if the additional
complexity is warranted.
Comparing before/after sizes (see below);
macOS: 54125840 - 54005264 = 120576 (+120.58 kB)
Linux: 52393231 - 52277701 = 115530 (+115.53 kB)
Before:
ls -l build/
total 208736
lrwxr-xr-x 1 sebastiaan staff 19 Aug 15 09:36 docker@ -> docker-linux-amd64
-rwxr-xr-x 1 sebastiaan staff 54005264 Aug 15 09:35 docker-darwin-amd64*
-rwxr-xr-x 1 sebastiaan staff 52277701 Aug 15 09:36 docker-linux-amd64*
After:
ls -l build/
total 208960
lrwxr-xr-x 1 sebastiaan staff 18 Aug 15 09:32 docker@ -> docker-linux-amd64
-rwxr-xr-x 1 sebastiaan staff 54125840 Aug 15 09:31 docker-darwin-amd64*
-rwxr-xr-x 1 sebastiaan staff 52393231 Aug 15 09:32 docker-linux-amd64*
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 73aef6edfe
modified archive.ReplaceFileTarWrapper to set the Name field in the tar header,
if the field was not set.
That change exposed an issue in how a Dockerfile from stdin was sent to the daemon.
When attempting to build using a build-context, and a Dockerfile from stdin, the
following happened:
```bash
mkdir build-stdin && cd build-stdin && echo hello > hello.txt
DOCKER_BUILDKIT=0 docker build --no-cache -t foo -f- . <<'EOF'
FROM alpine
COPY . .
EOF
Sending build context to Docker daemon 2.607kB
Error response from daemon: dockerfile parse error line 1: unknown instruction: .DOCKERIGNORE
```
Removing the `-t foo`, oddly lead to a different failure:
```bash
DOCKER_BUILDKIT=0 docker build --no-cache -f- . <<'EOF'
FROM alpine
COPY . .
EOF
Sending build context to Docker daemon 2.581kB
Error response from daemon: Cannot locate specified Dockerfile: .dockerfile.701d0d71fb1497d6a7ce
```
From the above, it looks like the tar headers got mangled, causing (in the first
case) the daemon to use the build-context tar as a plain-text file, and therefore
parsing it as Dockerfile, and in the second case, causing it to not being able to
find the Dockerfile in the context.
I noticed that both TarModifierFuncs were using the same `hdrTmpl` struct, which
looks to caused them to step on each other's toes. Changing them to each initialize
their own struct made the issue go away.
After this change:
```bash
DOCKER_BUILDKIT=0 docker build --no-cache -t foo -f- . <<'EOF'
FROM alpine
COPY . .
EOF
Sending build context to Docker daemon 2.607kB
Step 1/2 : FROM alpine
---> d4ff818577bc
Step 2/2 : COPY . .
---> 556f745e6938
Successfully built 556f745e6938
Successfully tagged foo:latest
DOCKER_BUILDKIT=0 docker build --no-cache -f- . <<'EOF'
FROM alpine
COPY . .
EOF
Sending build context to Docker daemon 2.607kB
Step 1/2 : FROM alpine
---> d4ff818577bc
Step 2/2 : COPY . .
---> aaaee43bec5e
Successfully built aaaee43bec5e
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This warning will be moved to the daemon-side, similar to how it returns
other warnings. There's work in progress to change the name of the default
profile, so we may need to backport this change to prevent existing clients
from printing an incorrect warning if they're connecting to a newer daemon.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's the only use of this function, and it's better to check that
the client actually sends the header.
This also simplifies some asserts, and makes sure that "actual" and "expected"
are in the correct order.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Golang uses a `sync.Once` when determining the proxy to use. This means
that it's not possible to test the proxy configuration in unit tests,
because the proxy configuration will be "fixated" the first time Golang
detects the proxy configuration.
This patch changes TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e
test so that we can verify the CLI picks up the proxy configuration.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
> Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since
> it does not authenticate the ciphertext, it is vulnerable to padding oracle
> attacks that can let an attacker recover the plaintext
From https://go-review.googlesource.com/c/go/+/264159
> It's unfortunate that we don't implement PKCS#8 encryption so we can't
> recommend an alternative but PEM encryption is so broken that it's worth
> deprecating outright.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
From https://go-review.googlesource.com/c/go/+/264159
> It's unfortunate that we don't implement PKCS#8 encryption so we can't
> recommend an alternative but PEM encryption is so broken that it's worth
> deprecating outright.
When linting on Go 1.16:
cli/context/docker/load.go:69:6: SA1019: x509.IsEncryptedPEMBlock is deprecated: Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since it does not authenticate the ciphertext, it is vulnerable to padding oracle attacks that can let an attacker recover the plaintext. (staticcheck)
if x509.IsEncryptedPEMBlock(pemBlock) {
^
cli/context/docker/load.go:70:20: SA1019: x509.DecryptPEMBlock is deprecated: Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since it does not authenticate the ciphertext, it is vulnerable to padding oracle attacks that can let an attacker recover the plaintext. (staticcheck)
keyBytes, err = x509.DecryptPEMBlock(pemBlock, []byte(c.TLSPassword))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Support for ALL_PROXY as default build-arg was added recently in
buildkit and the classic builder.
This patch adds the `ALL_PROXY` environment variable to the list of
configurable proxy variables, and updates the documentation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Errors always need to go to stderr.
This also fixes a test in moby/moby's integration-cli which is checking
to see if errors connecting to the daemon are output on stderr.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The docker info output contains both "local" and "remote" (daemon-side) information.
The API endpoint to collect daemon information (`/info`) is known to be "heavy",
and (depending on what information is needed) not needed.
This patch checks if the template (`--format`) used requires information from the
daemon, and if not, omits making an API request.
This will improve performance if (for example), the current "context" is requested
from `docker info` or if only plugin information is requested.
Before:
time docker info --format '{{range .ClientInfo.Plugins}}Plugin: {{.Name}}, {{end}}'
Plugin: buildx, Plugin: compose, Plugin: scan,
________________________________________________________
Executed in 301.91 millis fish external
usr time 168.64 millis 82.00 micros 168.56 millis
sys time 113.72 millis 811.00 micros 112.91 millis
time docker info --format '{{json .ClientInfo.Plugins}}'
time docker info --format '{{.ClientInfo.Context}}'
default
________________________________________________________
Executed in 334.38 millis fish external
usr time 177.23 millis 93.00 micros 177.13 millis
sys time 124.90 millis 927.00 micros 123.97 millis
docker context use remote-ssh-daemon
time docker info --format '{{.ClientInfo.Context}}'
remote-ssh-daemon
________________________________________________________
Executed in 1.22 secs fish external
usr time 116.93 millis 110.00 micros 116.82 millis
sys time 144.36 millis 887.00 micros 143.47 millis
And daemon logs:
Jul 06 12:42:12 remote-ssh-daemon dockerd[14377]: time="2021-07-06T12:42:12.139529947Z" level=debug msg="Calling HEAD /_ping"
Jul 06 12:42:12 remote-ssh-daemon dockerd[14377]: time="2021-07-06T12:42:12.140772052Z" level=debug msg="Calling HEAD /_ping"
Jul 06 12:42:12 remote-ssh-daemon dockerd[14377]: time="2021-07-06T12:42:12.163832016Z" level=debug msg="Calling GET /v1.41/info"
After:
time ./build/docker info --format '{{range .ClientInfo.Plugins}}Plugin: {{.Name}}, {{end}}'
Plugin: buildx, Plugin: compose, Plugin: scan,
________________________________________________________
Executed in 139.84 millis fish external
usr time 76.53 millis 62.00 micros 76.46 millis
sys time 69.25 millis 723.00 micros 68.53 millis
time ./build/docker info --format '{{.ClientInfo.Context}}'
default
________________________________________________________
Executed in 136.94 millis fish external
usr time 74.61 millis 74.00 micros 74.54 millis
sys time 65.77 millis 858.00 micros 64.91 millis
docker context use remote-ssh-daemon
time ./build/docker info --format '{{.ClientInfo.Context}}'
remote-ssh-daemon
________________________________________________________
Executed in 1.02 secs fish external
usr time 74.25 millis 76.00 micros 74.17 millis
sys time 65.09 millis 643.00 micros 64.44 millis
And daemon logs:
Jul 06 12:42:55 remote-ssh-daemon dockerd[14377]: time="2021-07-06T12:42:55.313654687Z" level=debug msg="Calling HEAD /_ping"
Jul 06 12:42:55 remote-ssh-daemon dockerd[14377]: time="2021-07-06T12:42:55.314811624Z" level=debug msg="Calling HEAD /_ping"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 330a003533
introduced "synchronous" service update and rollback, using progress bars to show
current status for each task.
As part of that change, progress bars were "reversed" when doing a rollback, to
indicate that status was rolled back to a previous state.
Reversing direction is somewhat confusing, as progress bars now return to their
"initial" state to indicate it was "completed"; for an "automatic" rollback, this
may be somewhat clear (progress bars "move to the right", then "roll back" if the
update failed), but when doing a manual rollback, it feels counter-intuitive
(rolling back is the _expected_ outcome).
This patch removes the code to reverse the direction of progress-bars, and makes
progress-bars always move from left ("start") to right ("finished").
Before this patch
----------------------------------------
1. create a service with automatic rollback on failure
$ docker service create --update-failure-action=rollback --name foo --tty --replicas=5 nginx:alpine
9xi1w3mv5sqtyexsuh78qg0cb
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Waiting 2 seconds to verify that tasks are stable...
2. update the service, making it fail after 3 seconds
$ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo
overall progress: rolling back update: 2 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: starting [============================================> ]
4/5: running [==================================================>]
5/5: running [==================================================>]
3. Once the service starts failing, automatic rollback is started; progress-bars now move in the reverse direction;
overall progress: rolling back update: 3 out of 5 tasks
1/5: ready [===========> ]
2/5: ready [===========> ]
3/5: running [> ]
4/5: running [> ]
5/5: running [> ]
4. When the rollback is completed, the progressbars are at the "start" to indicate they completed;
overall progress: rolling back update: 5 out of 5 tasks
1/5: running [> ]
2/5: running [> ]
3/5: running [> ]
4/5: running [> ]
5/5: running [> ]
rollback: update rolled back due to failure or early termination of task bndiu8a998agr8s6sjlg9tnrw
verify: Service converged
After this patch
----------------------------------------
Progress bars always go from left to right; also in a rollback situation;
After updating to the "faulty" entrypoint, task are deployed:
$ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo
foo
overall progress: 1 out of 5 tasks
1/5:
2/5: running [==================================================>]
3/5: ready [======================================> ]
4/5:
5/5:
Once tasks start failing, rollback is started, and presented the same as a regular
update; progress bars go from left to right;
overall progress: rolling back update: 3 out of 5 tasks
1/5: ready [======================================> ]
2/5: starting [============================================> ]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
rollback: update rolled back due to failure or early termination of task c11dxd7ud3d5pq8g45qkb4rjx
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This extends #2929 to Darwin as well as Linux.
Running the example in https://github.com/golang/go/issues/37942
I see lots of:
```
dave@m1 sigurg % uname -ms
Darwin arm64
dave@m1 sigurg % go run main.go
received urgent I/O condition: 2021-05-21 16:03:03.482211 +0100 BST m=+0.014553751
received urgent I/O condition: 2021-05-21 16:03:03.507171 +0100 BST m=+0.039514459
```
Signed-off-by: David Scott <dave@recoil.org>
The kernel memory limit is deprecated in Docker 20.10.0,
and its support was removed in runc v1.0.0-rc94.
So, this warning can be safely removed.
Relevant: b8ca7de823
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
- use var/const blocks when declaring a list of variables
- use const where possible
TestCheckKubernetesConfigurationRaiseAnErrorOnInvalidValue:
- use keys when assigning values
- make sure test is dereferenced in the loop
- use subtests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some tests were using domain names that were intended to be "fake", but are
actually registered domain names (such as mycorp.com).
Even though we were not actually making connections to these domains, it's
better to use domains that are designated for testing/examples in RFC2606:
https://tools.ietf.org/html/rfc2606
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
New solution is not hardcoded to amd64 but integrates
with the cross toolchain and support creating arm binaries.
Go has been updated so that ASLR works
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Relates to the deprecation, added in 3c0a167ed5
The docker CLI up until v1.7.0 used the `~/.dockercfg` file to store credentials
after authenticating to a registry (`docker login`). Docker v1.7.0 replaced this
file with a new CLI configuration file, located in `~/.docker/config.json`. When
implementing the new configuration file, the old file (and file-format) was kept
as a fall-back, to assist existing users with migrating to the new file.
Given that the old file format encourages insecure storage of credentials
(credentials are stored unencrypted), and that no version of the CLI since
Docker v1.7.0 has created this file, the file is marked deprecated, and support
for this file will be removed in a future release.
This patch adds a deprecation warning, which is printed if the CLI falls back
to using the deprecated ~/.dockercfg file.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We refactorted `ForwardAllSignals` so it blocks but did not update the
call in `start` to call it in a goroutine.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Commit fff164c22e modified ForwardAllSignals to
take `SIGURG` signals into account, which can be generated by the Go runtime
on Go 1.14 and up as an interrupt to support pre-emptable system calls on Linux.
With the updated code, the signal (`s`) would sometimes be `nil`, causing spurious
(but otherwise harmless) warnings to be printed;
Unsupported signal: <nil>. Discarding.
To debug this issue, I patched v20.10.4 to handle `nil`, and added a debug line
to print the signal in all cases;
```patch
diff --git a/cli/command/container/signals.go b/cli/command/container/signals.go
index 06e4d9eb6..0cb53ef06 100644
--- a/cli/command/container/signals.go
+++ b/cli/command/container/signals.go
@@ -22,8 +22,9 @@ func ForwardAllSignals(ctx context.Context, cli command.Cli, cid string, sigc <-
case <-ctx.Done():
return
}
+ fmt.Fprintf(cli.Err(), "Signal: %v\n", s)
if s == signal.SIGCHLD || s == signal.SIGPIPE {
```
When running a cross-compiled macOS binary with Go 1.13 (`make -f docker.Makefile binary-osx`):
# regular "docker run" (note that the `<nil>` signal only happens "sometimes"):
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
Signal: <nil>
# when cancelling with CTRL-C:
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
^CSignal: interrupt
Cloning into 'getting-started'...
error: could not lock config file /git/getting-started/.git/config: No such file or directory
fatal: could not set 'core.repositoryformatversion' to '0'
Signal: <nil>
Signal: <nil>
When running a macOS binary built with Go 1.15 (`DISABLE_WARN_OUTSIDE_CONTAINER=1 make binary`):
# regular "docker run" (note that the `<nil>` signal only happens "sometimes"):
# this is the same as on Go 1.13
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
Signal: <nil>
# when cancelling with CTRL-C:
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
^CSignal: interrupt
Signal: urgent I/O condition
Signal: urgent I/O condition
fatal: --stdin requires a git repository
fatal: index-pack failed
Signal: <nil>
Signal: <nil>
This patch checks if the channel is closed, and removes the warning (to prevent warnings if new
signals are added that are not in our known list of signals)
We should also consider updating `notfiyAllSignals()`, which currently forwards
_all_ signals (`signal.Notify(sigc)` without passing a list of signals), and
instead pass it "all signals _minus_ the signals we don't want forwarded":
35f023a7c2/cli/command/container/signals.go (L55)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
On Windows, the os/exec.{Command,CommandContext,LookPath} functions
resolve command names that have neither path separators nor file extension
(e.g., "git") by first looking in the current working directory before
looking in the PATH environment variable.
Go maintainers intended to match cmd.exe's historical behavior.
However, this is pretty much never the intended behavior and as an abundance of precaution
this patch prevents that when executing commands.
Example of commands that docker.exe may execute: `git`, `docker-buildx` (or other cli plugin), `docker-credential-wincred`, `docker`.
Note that this was prompted by the [Go 1.15.7 security fixes](https://blog.golang.org/path-security), but unlike in `go.exe`,
the windows path lookups in docker are not in a code path allowing remote code execution, thus there is no security impact on docker.
Signed-off-by: Tibor Vass <tibor@docker.com>
Prior to this change, progressbars would sometimes be hidden, and the function
would return early. In addition, the direction of the progressbars would sometimes
be "incrementing" (similar to "docker service update"), and sometimes be "decrementing"
(to indicate a "rollback" is being performed).
This fix makes sure that we always proceed with the "verifying" step, and now
prints a message _after_ the verifying stage was completed;
$ docker service rollback foo
foo
overall progress: rolling back update: 5 out of 5 tasks
1/5: running [> ]
2/5: starting [===========> ]
3/5: starting [===========> ]
4/5: running [> ]
5/5: running [> ]
verify: Service converged
rollback: rollback completed
$ docker service rollback foo
foo
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
rollback: rollback completed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change:
--------------------------------------------
$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
service rolled back: rollback completed
$ echo $?
1
After this change:
--------------------------------------------
$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Waiting 1 seconds to verify that tasks are stable...
$ echo $?
0
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
service rolled back: rollback completed
$ echo $?
0
$ docker service ps foo
ID NAME IMAGE NODE DESIRED STATE CURRENT STATE ERROR PORTS
4dt4ms4c5qfb foo.1 nginx:alpine docker-desktop Running Running 2 minutes ago
Remaining issues with reconciliation
--------------------------------------------
Note that both before, and after this change, the command sometimes terminates
early, and does not wait for the service to reconcile; this is most apparent
when rolling back is scaling up (so more tasks are deployed);
$ docker service rollback foo
foo
service rolled back: rollback completed
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 5 tasks
1/5: pending [=================================> ]
2/5: running [> ]
3/5: pending [=================================> ]
4/5: pending [=================================> ]
5/5: pending [=================================> ]
service rolled back: rollback completed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit c2626a8270 replaced the use of
github.com/docker/docker/pkg/homedir with Golang's os.UserHomeDir().
This change was partially reverted in 7a279af43d
to account for situations where `$HOME` is not set.
In situations where no configuration file is present in `~/.config/`, the CLI
falls back to looking for the (deprecated) `~/.dockercfg` configuration file,
which was still using `os.UserHomeDir()`, which produces an error/warning if
`$HOME` is not set.
This patch introduces a helper function and a global variable to get the user's
home-directory. The global variable is used to prevent repeatedly looking up
the user's information (which, depending on the setup can be a costly operation).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit f32731f902 fixed a potential panic
when an error was returned while trying to get existing credentials.
However, other code paths currently use the result of `GetDefaultAuthConfig()`
even in an error condition; this resulted in a panic, because a `nil` was
returned.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: 75b288015a...c1f2f97bff
relevant changes:
- pkcs12: document that we use the wrong PEM type
- pkcs12: drop PKCS#12 attributes with unknown OIDs
- ocsp: Improve documentation for ParseResponse and ParseResponseForCert
other changes (not in vendor);
- ssh: improve error message for KeyboardInteractiveChallenge
- ssh: remove slow unnecessary diffie-hellman-group-exchange primality check
- ssh/terminal: replace with a golang.org/x/term wrapper
- Deprecates ssh/terminal in favor of golang.org/x/term
- ssh/terminal: add support for zos
- ssh/terminal: bump x/term dependency to fix js/nacl
- nacl/auth: use Size instead of KeySize for Sum output
- sha3: remove go:nocheckptr annotation
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Will display when user types `docker help` or `docker --help`, but not for `docker run --help`.
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
This hack was added in an attempt to continue supporting the experimental
(non-buildkit) `--platform` option, by dynamically updating the API version
required if buildkit isn't enabled.
This hack didn't work, however, because at the moment the override is
added, the command is not yet attached to the "root" (`docker`) command,
and because of that, the command itself is the `root` command;
`cmd.Root()` returned the `build` command.
As a result, validation steps defined as `PersistentPreRunE` on the root
command were not executed, causing invalid flags/options to not producing
an error.
Attempts to use an alternative approach (for example, cobra supports both
a `PersistentPreRun` and `PersistentPreRunE`) did not work either, because
`PersistentPreRunE` takes precedence over `PersistentPreRun`, and only one
will be executed.
Now that `--platform` should be supported for other cases than just for
experimental (LCOW), let's remove the 'experimental' check, and just assume
it's supported for API v1.32 and up.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While performance will be worse, we can safely ignore the --stream
option when used, and print a deprecation warning instead of failing
the build.
With this patch:
echo -e "FROM scratch\nLABEL foo=bar" | docker build --stream -
DEPRECATED: The experimental --stream flag has been removed and the build context
will be sent non-streaming. Enable BuildKit instead with DOCKER_BUILDKIT=1
to stream build context, see https://docs.docker.com/go/buildkit/
Sending build context to Docker daemon 2.048kB
Step 1/2 : FROM scratch
--->
Step 2/2 : LABEL foo=bar
---> Running in 99e4021085b6
Removing intermediate container 99e4021085b6
---> 1a7a41be241f
Successfully built 1a7a41be241f
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The CLI disabled experimental features by default, requiring users
to set a configuration option to enable them.
Disabling experimental features was a request from Enterprise users
that did not want experimental features to be accessible.
We are changing this policy, and now enable experimental features
by default. Experimental features may still change and/or removed,
and will be highlighted in the documentation and "usage" output.
For example, the `docker manifest inspect --help` output now shows:
EXPERIMENTAL:
docker manifest inspect is an experimental feature.
Experimental features provide early access to product functionality. These features
may change between releases without warning or can be removed entirely from a future
release. Learn more about experimental features: https://docs.docker.com/go/experimental/
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All output of the usage / --help output uses spaces, and having a tab
in the output can be somewhat cumbersome (e.g. our YAML docs generator
doesn't like them, and copy/pasing the output in iTerm produces a warning).
This patch changes the output to use two spaces instead.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When initializing the API client, the User-Agent was added to any custom
HTTPHeaders that were configured. However, because the map was not properly
dereferenced, the original map was modified, causing the User-Agent to also
be saved to config.json after `docker login` and `docker logout`:
Before this change;
$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory
$ docker login -u myusername
Password:
...
Login Succeeded
$ cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "<base64 auth>"
}
},
"HttpHeaders": {
"User-Agent": "Docker-Client/19.03.12 (linux)"
}
}
$ docker logout
{
"auths": {},
"HttpHeaders": {
"User-Agent": "Docker-Client/19.03.12 (linux)"
}
}
After this change:
$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory
$ docker login -u myusername
Password:
...
Login Succeeded
$ cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "<base64 auth>"
}
}
}
$ docker logout
Removing login credentials for https://index.docker.io/v1/
$ cat ~/.docker/config.json
{
"auths": {}
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This implements a special "RESET" value that can be used to reset the
list of capabilities to add/drop when updating a service.
Given the following service;
| CapDrop | CapAdd |
| -------------- | ------------- |
| CAP_SOME_CAP | |
When updating the service, and applying `--cap-drop RESET`, the "drop" list
is reset to its default:
| CapDrop | CapAdd |
| -------------- | ------------- |
| | |
When updating the service, and applying `--cap-drop RESET`, combined with
`--cap-add CAP_SOME_CAP` and `--cap-drop CAP_SOME_OTHER_CAP`:
| CapDrop | CapAdd |
| -------------- | ------------- |
| CAP_FOO_CAP | CAP_SOME_CAP |
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adding/removing capabilities when updating a service is considered a tri-state;
- if the capability was previously "dropped", then remove it from "CapabilityDrop",
but do NOT add it to "CapabilityAdd". However, if the capability was not yet in
the service's "CapabilityDrop", then simply add it to the service's "CapabilityAdd"
- likewise, if the capability was previously "added", then remove it from
"CapabilityAdd", but do NOT add it to "CapabilityDrop". If the capability was
not yet in the service's "CapabilityAdd", then simply add it to the service's
"CapabilityDrop".
In other words, given a service with the following:
| CapDrop | CapAdd |
| -------------- | ------------- |
| CAP_SOME_CAP | |
When updating the service, and applying `--cap-add CAP_SOME_CAP`, the previously
dropped capability is removed:
| CapDrop | CapAdd |
| -------------- | ------------- |
| | |
When updating the service a second time, applying `--cap-add CAP_SOME_CAP`,
capability is now added:
| CapDrop | CapAdd |
| -------------- | ------------- |
| | CAP_SOME_CAP |
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When creating and updating services, we need to avoid unneeded service churn.
The interaction of separate lists to "add" and "drop" capabilities, a special
("ALL") capability, as well as a "relaxed" format for accepted capabilities
(case-insensitive, `CAP_` prefix optional) make this rather involved.
This patch updates how we handle `--cap-add` / `--cap-drop` when _creating_ as
well as _updating_, with the following rules/assumptions applied:
- both existing (service spec) and new (values passed through flags or in
the compose-file) are normalized and de-duplicated before use.
- the special "ALL" capability is equivalent to "all capabilities" and taken
into account when normalizing capabilities. Combining "ALL" capabilities
and other capabilities is therefore equivalent to just specifying "ALL".
- adding capabilities takes precedence over dropping, which means that if
a capability is both set to be "dropped" and to be "added", it is removed
from the list to "drop".
- the final lists should be sorted and normalized to reduce service churn
- no validation of capabilities is handled by the client. Validation is
delegated to the daemon/server.
When deploying a service using a docker-compose file, the docker-compose file
is *mostly* handled as being "declarative". However, many of the issues outlined
above also apply to compose-files, so similar handling is applied to compose
files as well to prevent service churn.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The tabwriter was configured to have a min-width for columns of 20 positions.
This seemed quite wide, and caused smaller columns to be printed with a large
gap between.
Before:
docker container stats
CONTAINER ID NAME CPU % MEM USAGE / LIMIT MEM % NET I/O BLOCK I/O PIDS
29184b3ae391 amazing_shirley 0.00% 800KiB / 1.944GiB 0.04% 1.44kB / 0B 0B / 0B 1
403c101bad56 agitated_swartz 0.15% 34.31MiB / 1.944GiB 1.72% 10.2MB / 206kB 0B / 0B 51
0dc4b7f6c6be container2 0.00% 1.012MiB / 1.944GiB 0.05% 12.9kB / 0B 0B / 0B 5
2d99abcc6f62 container99 0.00% 972KiB / 1.944GiB 0.05% 13kB / 0B 0B / 0B 5
9f9aa90173ac foo 0.00% 820KiB / 1.944GiB 0.04% 13kB / 0B 0B / 0B 5
docker container ls
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
29184b3ae391 docker-cli-dev "ash" 4 hours ago Up 4 hours amazing_shirley
403c101bad56 docker-dev:master "hack/dind bash" 3 days ago Up 3 days agitated_swartz
0dc4b7f6c6be nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp container2
2d99abcc6f62 nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp container99
9f9aa90173ac nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp foo
docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
docker-cli-dev latest 5f603caa04aa 4 hours ago 610MB
docker-cli-native latest 9dd29f8d387b 4 hours ago 519MB
docker-dev master 8132bf7a199e 3 days ago 2.02GB
docker-dev improve-build-errors 69e208994b3f 11 days ago 2.01GB
docker-dev refactor-idtools 69e208994b3f 11 days ago 2.01GB
After:
docker container stats
CONTAINER ID NAME CPU % MEM USAGE / LIMIT MEM % NET I/O BLOCK I/O PIDS
29184b3ae391 amazing_shirley 0.14% 5.703MiB / 1.944GiB 0.29% 1.44kB / 0B 0B / 0B 10
403c101bad56 agitated_swartz 0.15% 56.97MiB / 1.944GiB 2.86% 10.2MB / 206kB 0B / 0B 51
0dc4b7f6c6be container2 0.00% 1016KiB / 1.944GiB 0.05% 12.9kB / 0B 0B / 0B 5
2d99abcc6f62 container99 0.00% 956KiB / 1.944GiB 0.05% 13kB / 0B 0B / 0B 5
9f9aa90173ac foo 0.00% 980KiB / 1.944GiB 0.05% 13kB / 0B 0B / 0B 5
docker container ls
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
29184b3ae391 docker-cli-dev "ash" 12 minutes ago Up 12 minutes amazing_shirley
403c101bad56 docker-dev:master "hack/dind bash" 3 days ago Up 3 days agitated_swartz
0dc4b7f6c6be nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp container2
2d99abcc6f62 nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp container99
9f9aa90173ac nginx:alpine "/docker-entrypoint.…" 4 days ago Up 4 days 80/tcp foo
docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
docker-cli-dev latest 5f603caa04aa 4 hours ago 610MB
docker-cli-native latest 9dd29f8d387b 4 hours ago 519MB
docker-dev master 8132bf7a199e 3 days ago 2.02GB
docker-dev improve-build-errors 69e208994b3f 11 days ago 2.01GB
docker-dev refactor-idtools 69e208994b3f 11 days ago 2.01GB
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The vanity domain is down, and the project has moved
to a new location.
vendor check started failing because of this:
Collecting initial packages
Download dependencies
unrecognized import path "vbom.ml/util" (https fetch: Get https://vbom.ml/util?go-get=1: dial tcp: lookup vbom.ml on 169.254.169.254:53: no such host)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When using `docker rm` / `docker container rm` with the `-f` / `--force` option, attempts to remove non-existing containers should print a warning, but should return a zero exit code ("successful").
Currently, a non-zero exit code is returned, marking the removal as "failed";
$ docker rm -fv 798c9471b695
Error: No such container: 798c9471b695
$ echo $?
1
The command should match the behavior of `rm` / `rm -f`, with the exception that
a warning is printed (instead of silently ignored):
Running `rm` with `-f` silences output and returns a zero exit code:
touch some-file && rm -f no-such-file some-file; echo exit code: $?; ls -la
# exit code: 0
# total 0
# drwxr-xr-x 2 sebastiaan staff 64 Aug 14 12:17 .
# drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 ..
mkdir some-directory && rm -rf no-such-directory some-directory; echo exit code: $?; ls -la
# exit code: 0
# total 0
# drwxr-xr-x 2 sebastiaan staff 64 Aug 14 12:17 .
# drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 ..
Note that other reasons for a delete to fail should still result in a non-zero
exit code, matching the behavior of `rm`. For instance, in the example below,
the `rm` failed because directories can only be removed if the `-r` option is used;
touch some-file && mkdir some-directory && rm -f some-directory no-such-file some-file; echo exit code: $?; ls -la
# rm: some-directory: is a directory
# exit code: 1
# total 0
# drwxr-xr-x 3 sebastiaan staff 96 Aug 14 14:15 .
# drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 ..
# drwxr-xr-x 2 sebastiaan staff 64 Aug 14 14:15 some-directory
This patch updates the `docker rm` / `docker container rm` command to not produce
an error when attempting to remove a missing containers, and instead only print
the error, but return a zero (0) exit code.
With this patch applied:
docker create --name mycontainer busybox \
&& docker rm nosuchcontainer mycontainer; \
echo exit code: $?; \
docker ps -a --filter name=mycontainer
# df23cc8573f00e97d6e948b48d9ea7d75ce3b4faaab4fe1d3458d3bfa451f39d
# mycontainer
# Error: No such container: nosuchcontainer
# exit code: 0
# CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Combining `-add` and `-rm` flags on `docker service update` should
be usable to explicitly replace existing options. The current order
of processing did not allow this, causing the `-rm` flag to remove
properties that were specified in `-add`. This behavior was inconsistent
with (for example) `--host-add` and `--host-rm`.
This patch updates the behavior to first remove properties, then
add new properties.
Note that there's still some improvements to make, to make the removal
more granulas (e.g. to make `--label-rm label=some-value` only remove
the label if value matches `some-value`); these changes are left for
a follow-up.
Before this change:
-----------------------------
Create a service with two env-vars
```bash
docker service create --env FOO=bar --env BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Env }}' test | jq .
[
"FOO=bar",
"BAR=baz"
]
```
Update the service, with the intent to replace the value of `FOO` for a new value
```bash
docker service update --env-rm FOO --env-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Env }}' test | jq .
[
"BAR=baz"
]
```
Create a service with two labels
```bash
docker service create --label FOO=bar --label BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "bar"
}
```
Update the service, with the intent to replace the value of `FOO` for a new value
```bash
docker service update --label-rm FOO --label-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.Labels }}' test | jq .
{
"BAR": "baz"
}
```
Create a service with two container labels
```bash
docker service create --container-label FOO=bar --container-label BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "bar"
}
```
Update the service, with the intent to replace the value of `FOO` for a new value
```bash
docker service update --container-label-rm FOO --container-label-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Labels }}' test | jq .
{
"BAR": "baz",
}
```
With this patch applied:
--------------------------------
Create a service with two env-vars
```bash
docker service create --env FOO=bar --env BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Env }}' test | jq .
[
"FOO=bar",
"BAR=baz"
]
```
Update the service, and replace the value of `FOO` for a new value
```bash
docker service update --env-rm FOO --env-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Env }}' test | jq .
[
"BAR=baz",
"FOO=updated-foo"
]
```
Create a service with two labels
```bash
docker service create --label FOO=bar --label BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "bar"
}
```
Update the service, and replace the value of `FOO` for a new value
```bash
docker service update --label-rm FOO --label-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "updated-foo"
}
```
Create a service with two container labels
```bash
docker service create --container-label FOO=bar --container-label BAR=baz --name=test nginx:alpine
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "bar"
}
```
Update the service, and replace the value of `FOO` for a new value
```bash
docker service update --container-label-rm FOO --container-label-add FOO=updated-foo test
docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.Labels }}' test | jq .
{
"BAR": "baz",
"FOO": "updated-foo"
}
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When doing `docker service inspect --pretty` on services without
`TaskTemplate.Resources` or `TaskTemplate.Resources.Limits`, the command
fails. This is due to a missing check on ResourceLimitPids().
This bug has been introduced by 395a6d560d
and produces following error message:
```
Template parsing error: template: :139:10: executing "" at <.ResourceLimitPids>: error calling ResourceLimitPids: runtime error: invalid memory address or nil pointer dereference
```
Signed-off-by: Albin Kerouanton <albin@akerouanton.name>
Both libaries provide similar functionality. We're currently using
Google Shlex in more places, so prefering that one for now, but we
could decide to switch to mattn/go-shellwords in future if that
library is considered better (it looks to be more actively maintained,
but that may be related to it providing "more features").
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In situations where `~/.docker/config.json` was a symlink, saving
the file would replace the symlink with a file, instead of updating
the target file location;
mkdir -p ~/.docker
touch ~/real-config.json
ln -s ~/real-config.json ~/.docker/config.json
ls -la ~/.docker/config.json
# lrwxrwxrwx 1 root root 22 Jun 23 12:34 /root/.docker/config.json -> /root/real-config.json
docker login
# Username: thajeztah
# Password:
# Login Succeeded
ls -la ~/.docker/config.json
-rw-r--r-- 1 root root 229 Jun 23 12:36 /root/.docker/config.json
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The wrapping made the code harder to read (and in some cases destracted
from the actual code flow).
Some of these functions take too many arguments; instead of hiding that,
it probably better to make it apparent that something needs to be done
(and fix it :-)).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- change `validateResolveImageFlag()` to only perform _validation_,
and not combine it with modifying the option.
- use a `switch` instead of `if` in `validateResolveImageFlag()`
- `deployServices()`: break up some `switch` cases to make them
easier to read/understand the logic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- TestParseRunAttach: use subtests to reduce cyclomatic complexity
- TestParseRunWithInvalidArgs: use subtests, and check if the expected
error is returned.
- Removed parseMustError() as it was mostly redundant
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These tests failed when running natively on macOS;
unknown server OS: darwin
Skipping them, like we do on Windows
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change, a warning would be printed if the `~/.docker/config.json`
file was empty:
mkdir -p ~/.docker && touch ~/.docker/config.json
docker pull busybox
WARNING: Error loading config file: /root/.docker/config.json: EOF
Using default tag: latest
....
Given that we also accept an empty "JSON" file (`{}`), it should be
okay to ignore an empty file, as it's effectively a configuration file
with no custom options set.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I'm not sure if this fixes anything, however I have seen some weird
behavior on Windows where temp config files are left around and there
doesn't seem to be any errors reported.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Previously, if a registry AuthInfo was not present in the CLI config file, docker logout could not be used
to ask the credential helper to forget about it. It causes problem for people working with
multiple alternative config files, and it causes problems for cases like Docker Desktop w/ WSL 2, as
it uses the same win32 credential helper as the Windows CLI, but a different config file, leading to
bugs where I cannot logout from a registry from wsl2 if I logged in from Windows and vice-versa.
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
There's no need to perform an `os.Stat()` first, because
`os.Open()` also returns the same errors if the file does
not exist, or couldn't be opened for other reasons.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This prevents inconsistent errors when using a symlink, or when renaming
the binary;
Before this change;
ln -s $(which docker) toto
./toto rune
docker: 'rune' is not a docker command.
./toto run daslkjadslkjdaslkj
Unable to find image 'adslkjadslakdsj:latest' locally
./toto: Error response from daemon: pull access denied for adslkjadslakdsj, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.
After this change:
ln -s $(which docker) toto
./toto rune
docker: 'rune' is not a docker command.
./toto run daslkjadslkjdaslkj
Unable to find image 'adslkjadslakdsj:latest' locally
docker: Error response from daemon: pull access denied for adslkjadslakdsj, repository does not exist or may require 'docker login': denied: requested access to the resource is den>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously we only set the platform when performing a pull, which is
only initiated if pull always is set, or if the image reference does not
exist in the daemon.
The daemon now supports specifying which platform you wanted on
container create so it can validate the image reference is the platform
you thought you were getting.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Sebastiaan van Stijn
CrazyMax
Samuel Karp
Sebastiaan van Stijn
coryb
Alex Couture-Beil
Brian Goff
Mathieu Champlon
Silvin Lubecki
David Scott
Akihiro Suda
Maximillian Fan Xavier
Tonis Tiigi
Chris Crone
Tibor Vass
Jim Lin
Alexey Igrychev
Tibor Vass
Djordje Lukic
Guillaume Tardif
bn4t
Rahul Kadyan
Jorge Vallecillo
Jennings Zhang
Albin Kerouanton
Olli Janatuinen
horpto
Wang Yumu
Brian Wieder
Dominik Braun
Simon Ferquel
Saswat Bhattacharya
devisions
Daniil Nikolenko
Silvin Lubecki