Docker 25.0 (08e4e88482) deprecated API versions
older than v1.24, and support was removed in Docker 26.0.
As part of this deprecation, support for plain-text errors was also removed
in commit ffd877f948.
So while we no longer support API versions older 1.24 [api.MinSupportedAPIVersion],
a client may try to connect using an older version and expect a plain-text error
instead of a JSON error. This would result in an "API version too old" error
formatted in JSON being printed as-is.
DOCKER_API_VERSION=v1.10 docker info --format '{{.ID}}'
Error response from daemon: {"message":"client version 1.10 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version"}
curl --unix-socket /var/run/docker.sock http://localhost/v1.10/info
{"message":"client version 1.10 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version"}
Note that this was only a problem for old API versions; unsupported API versions
that were higher than the maximum version were already handled as JSON;
DOCKER_API_VERSION=v1.99 docker info --format '{{.ID}}'
Error response from daemon: client version 1.99 is too new. Maximum supported API version is 1.48
curl --unix-socket /var/run/docker.sock http://localhost/v1.99/info
{"message":"client version 1.99 is too new. Maximum supported API version is 1.48"}
Let's be nice, and return errors in plain-text to provide a more readable error
to help the user understand the API version they're using is no longer supported.
With this patch applied:
DOCKER_API_VERSION=v1.10 docker info --format '{{.ID}}'
Error response from daemon: client version 1.10 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version
curl --unix-socket /var/run/docker.sock http://localhost/v1.10/info
client version 1.10 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
request.ReadBody already closes the body;
time="2025-03-20T19:08:25Z" level=error msg="subsequent attempt to close ReadCloserWrapper"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Split the `ErrConflictHostNetwork` error into two distinct errors:
1. `ErrConflictConnectToHostNetwork` when attempting to change the
network mode of a running container from a different mode to `host`
2. `ErrConflictDisconnectFromHostNetwork` when the network mode of a
running container is `host` and attempting to disconnect from `host`
This commit clarifies error messaging by differentiating between the two
errors, making it clearer which operation failed and how to fix it.
Signed-off-by: Brendon Smith <bws@bws.bio>
Remove output variables, and use explicit returns
integration-cli/check_test.go:86:3: naked return in func `testRun` with 43 lines of code (nakedret)
return
^
integration-cli/check_test.go:97:3: naked return in func `testRun` with 43 lines of code (nakedret)
return
^
integration-cli/docker_cli_cp_utils_test.go:167:2: naked return in func `makeTestContainer` with 44 lines of code (nakedret)
return
^
integration-cli/docker_api_attach_test.go:299:3: naked return in func `readTimeout` with 12 lines of code (nakedret)
return
^
integration-cli/docker_cli_cp_utils_test.go:215:2: naked return in func `startContainerGetOutput` with 11 lines of code (nakedret)
return
^
integration-cli/docker_cli_logs_test.go:276:4: naked return in func `ConsumeWithSpeed` with 18 lines of code (nakedret)
return
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In 28.0.0, Docker appended to the FORWARD chain - breaking other
applications that had appended their own rules that needed to execute
after Docker's rules.
Move most of Docker's rules out of the filter-FORWARD chain into a
new DOCKER-FORWARD chain, so that Docker can append to DOCKER-FORWARD
without affecting the order of rules in the FORWARD chain.
After daemon startup inserts jumps to DOCKER-USER and DOCKER-FORWARD,
the bridge driver will not touch the FORWARD chain again. DOCKER-INGRESS
is still added to the FORWARD chain, if used, as it was in 27.x and
earlier.
Signed-off-by: Rob Murray <rob.murray@docker.com>
integration-cli/docker_cli_external_volume_driver_test.go:116:4: Error return value of `(*encoding/json.Encoder).Encode` is not checked: unsafe type `interface{}` found (errchkjson)
json.NewEncoder(w).Encode(&data)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In situations where an empty ID was passed, the client would construct an
invalid API endpoint URL, which either resulted in the "not found" handler
being hit (resulting in a "page not found" error), or even the wrong endpoint
being hit if the client follows redirects.
For example, `/containers/<empty id>/json` (inspect) redirects to `/containers/json`
(docker ps))
Given that empty IDs should never be expected (especially if they're part of
the API URL path), we can validate these and return early.
Its worth noting that a few methods already had an error in place; those
methods were related to the situation mentioned above, where (e.g.) an
"inspect" would redirect to a "list" endpoint. The existing errors, for
convenience, mimicked a "not found" error; this patch changes such errors
to an "Invalid Parameter" instead, which is more correct, but it could be
a breaking change for some edge cases where users parsed the output;
git grep 'objectNotFoundError{'
client/config_inspect.go: return swarm.Config{}, nil, objectNotFoundError{object: "config", id: id}
client/container_inspect.go: return container.InspectResponse{}, nil, objectNotFoundError{object: "container", id: containerID}
client/container_inspect.go: return container.InspectResponse{}, objectNotFoundError{object: "container", id: containerID}
client/distribution_inspect.go: return distributionInspect, objectNotFoundError{object: "distribution", id: imageRef}
client/image_inspect.go: return image.InspectResponse{}, nil, objectNotFoundError{object: "image", id: imageID}
client/network_inspect.go: return network.Inspect{}, nil, objectNotFoundError{object: "network", id: networkID}
client/node_inspect.go: return swarm.Node{}, nil, objectNotFoundError{object: "node", id: nodeID}
client/plugin_inspect.go: return nil, nil, objectNotFoundError{object: "plugin", id: name}
client/secret_inspect.go: return swarm.Secret{}, nil, objectNotFoundError{object: "secret", id: id}
client/service_inspect.go: return swarm.Service{}, nil, objectNotFoundError{object: "service", id: serviceID}
client/task_inspect.go: return swarm.Task{}, nil, objectNotFoundError{object: "task", id: taskID}
client/volume_inspect.go: return volume.Volume{}, nil, objectNotFoundError{object: "volume", id: volumeID}
Two such errors are still left, as "ID or name" would probably be confusing,
but perhaps we can use a more generic error to include those as well (e.g.
"invalid <object> reference: value is empty");
client/distribution_inspect.go: return distributionInspect, objectNotFoundError{object: "distribution", id: imageRef}
client/image_inspect.go: return image.InspectResponse{}, nil, objectNotFoundError{object: "image", id: imageID}
Before this patch:
docker container start ""
Error response from daemon: page not found
Error: failed to start containers:
docker container start " "
Error response from daemon: No such container:
Error: failed to start containers:
With this patch:
docker container start ""
invalid container name or ID: value is empty
Error: failed to start containers:
docker container start " "
invalid container name or ID: value is empty
Error: failed to start containers:
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now a gratuitous/unsolicted ARP is sent, there's no need to
use an IPv4-based MAC address to preserve arp-cache mappings
between an endpoint's IP addresses and its MAC addresses.
Because a random MAC address is used for the default bridge,
it no longer makes sense to derive container IPv6 addresses
from the MAC address. This "postIPv6" behaviour was needed
before IPv6 addresses could be configured, but not now. So,
IPv6 addresses will now be IPAM-allocated on the default
bridge network, just as they are for user-defined bridges.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The StatsResponse type was a compatibility-wrapper introduced in
d3379946ec to differentiate responses
for API < 1.21 and API >= 1.21. API versions lower than 1.24 are
deprecated, and we should merge StatsResponse and Stats, but let's
start with using the StatsResponse in our tests.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was testing errors produced by runc; both the "maximum" and
"minimum" values originate from the OCI runtime;
d48d9cfefc/libcontainer/cgroups/fs/cpu.go (L66-L83)
docker run --cpu-shares=1 alpine
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error setting cgroup config for procHooks process: the minimum allowed cpu-shares is 2: unknown.
Happy path for this setting is covered by TestRunWithCPUShares, and
various other tests, so we validate that the options take effect;
f5af46d4d5/integration-cli/docker_cli_run_unix_test.go (L494-L503)
This patch:
- removes the test and migrates it to an integration test
- removes the checks for errors that might be produced by runc
- updates our validation for invalid (negative) values to happen
when creating the contaienr; the existing check that happened when
creating the OCI spec is preserved, so that configs of existing containers
are still validated.
- updates validateResources to return the correct error-type
- updated unit-test to validate
With this patch:
make TEST_FILTER='TestCreateInvalidHostConfig' TEST_SKIP_INTEGRATION_CLI=1 test-integration
--- PASS: TestCreateInvalidHostConfig (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_IpcMode (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_CPUShares (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_PidMode (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_PidMode_without_container_ID (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_Annotations (0.00s)
--- PASS: TestCreateInvalidHostConfig/invalid_UTSMode (0.00s)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test originally added in 4352da7803,
and was a bit involved as it involved building an image, and had some
dubious test-cases, such as using `wrongimage:<ID of other image>` as
reference, and expecting that to produce a "not found" error. Possibly
this format was supported in the past, but currently it fails equally with
`correctimage:<ID of image>`.
This patch rewrites the test to an integration test, and removes the test
from integration-cli. It also removes TestCreate64ByteHexID, as it was
duplicated by this test.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The use of bufio for writing without flushing can lead to an incomplete
writing of the tar and subsequent unexpected EOF when importing.
Signed-off-by: Derek McGowan <derek@mcg.dev>
The netfilter module is now loaded on-demand, and no longer during daemon
startup, making these fields obsolete. These fields are now always `false`
and will be removed in the next relase.
This patch deprecates:
- the `BridgeNfIptables` field in `api/types/system.Info`
- the `BridgeNfIp6tables` field in `api/types/system.Info`
- the `BridgeNFCallIPTablesDisabled` field in `pkg/sysinfo.SysInfo`
- the `BridgeNFCallIP6TablesDisabled` field in `pkg/sysinfo.SysInfo`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 3ec19ff62b introduced a GC goroutine to delete files where netns
were mounted. It was primarly added to work around a race in kernel
3.18-4.0.1. Since no distros we support are using such old kernels,
there's no need to keep this code around.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Non-distributable artifacts (foreign layers) were introduced in commit
05bd04350b to accommodate Windows images,
for which the EULA did not allow layers to be distributed through registries
other than those hosted by Microsoft. The concept of foreign / non-distributable
layers was adopted by the OCI distribution spec in [oci#233].
These restrictions were relaxed later to allow distributing these images
through non-public registries, for which a configuration was added in the
daemon in 67fdf574d5. In 2022, Microsoft updated
the EULA and [removed these restrictions altogether][1], and the OCI distribution
spec deprecated the functionality in [oci#965].
In 2023, Microsoft [removed the use of foreign data layers][2] for their images,
making this functionality obsolete.
This patch:
- Deprecates the `--allow-nondistributable-artifacts` daemon flag and corresponding
`allow-nondistributable-artifacts` field in `daemon.json`. Setting either
option will no longer take an effect, but a deprecation warning log is added
to raise awareness about the deprecation. This warning is planned to become
an error in the next release.
- Deprecates the `RegistryConfig.AllowNondistributableArtifactsCIDRs` and
`RegistryConfig.AllowNondistributableArtifactsHostnames` fields in the
`GET /info` API response. For API version v1.48 and lower, the fields are
still included in the response, but always `null`. In API version v1.49 and
higher, the field will be omitted entirely.
- Deprecates the `api/types/registry/ServiceConfig.AllowNondistributableArtifactsCIDRs`
field.
- Deprecates the `api/types/registry/ServiceConfig.AllowNondistributableArtifactsHostnames`
field.
- Deprecates the `registry.ServiceOptions.AllowNondistributableArtifacts` field.
[oci#233]: https://github.com/opencontainers/image-spec/pull/233
[oci#965]: https://github.com/opencontainers/image-spec/pull/965
[1]: https://techcommunity.microsoft.com/blog/containers/announcing-windows-container-base-image-redistribution-rights-change/3645201
[2]: https://techcommunity.microsoft.com/blog/containers/announcing-removal-of-foreign-layers-from-windows-container-images/3846833
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This partially reverts e440831802 ("fix and skip some tests based on
API version"), which caused the integration-cli tests to skip all
AppArmor-related tests on SUSE.
It's not really clear why this was done originally, but I have verified
that on modern SLE 12 and SLE 15 systems the AppArmor tests pass without
any adjustments needed.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Test 'TestDockerCLIRunSuite/TestRunAttachFailedNoLeak' does this ...
- start a container that exits immediately, its comment says:
- "Run a dummy container to ensure all goroutines are up and running
before we get a count"
- wait for the number of goroutines to be stable for 400ms, and remember
that number
- start a container
- start another container, expecting it to fail with a port-mapping clash
- stop the running container
- wait for up to 30s for the number of goroutines to fall back to the
remembered number.
In a CI run - hacking in some debug to count goroutines once a second,
before waiting for the number to stablilise for 400ms, showed that the
initial (dummy) container run had no immediate effect. But, three more
goroutines appeared within a few seconds. For example:
=== RUN TestDockerCLIRunSuite/TestRunAttachFailedNoLeak
docker_cli_run_test.go:3822: goroutines before container run: 47 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 47 i 0 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 1 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 2 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 3 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 4 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 5 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 6 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 7 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 8 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 9 err <nil>
That means a delay while running the rest of the test risks finding the
extra goroutines that are going to start anyway and not go away (regardless
of whether more containers are started).
So - wait for the goroutine count to be stable for 7s, rather than 400ms.
Signed-off-by: Rob Murray <rob.murray@docker.com>
If we have an error type that we're checking a substring against, we
should really be checking using ErrorContains to indicate the right
semantics to assert.
Mostly done using these transforms:
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Assert(t, is.ErrorContains(e, s)) -> assert.ErrorContains(t, e, s)'
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Assert(t, is.Contains(err.Error(), s)) -> assert.ErrorContains(t, err, s)'
find . -type f -name "*_test.go" | \
xargs gofmt -w -r 'assert.Check(t, is.Contains(err.Error(), s)) -> assert.Check(t, is.ErrorContains(err, s))'
As well as some small fixups to helpers that were doing
strings.Contains explicitly.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If a values is non-nil when we don't expect it, it would be quite
helpful to get an error message explaining what happened.
find . -type f -name "*_test.go" | \
xargs gofmt -w -r "assert.Assert(t, a == nil) -> assert.Assert(t, is.Nil(a))"
find . -type f -name "*_test.go" | \
xargs gofmt -w -r "assert.Check(t, a == nil) -> assert.Check(t, is.Nil(a))"
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Migrated using
find . -type f -name "*_test.go" |
xargs gofmt -w \
-r "assert.Check(t, strings.Contains(a, b)) -> assert.Check(t, is.Contains(a, b))"
find . -type f -name "*_test.go" |
xargs gofmt -w \
-r "assert.Assert(t, strings.Contains(a, b)) -> assert.Assert(t, is.Contains(a, b))"
Using a boolean in assert.Assert or assert.Check results in error
messages that don't contain the actual problematic string, and when
running the integration suite on an actual machine (where the source
code parsing doesn't work) this makes it almost impossible to figure out
what the actual error is.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
integration-cli/docker_api_containers_test.go:1748:3: The copy of the 'for' variable "x" can be deleted (Go 1.22+) (copyloopvar)
x := x
^
integration-cli/docker_api_containers_test.go:1916:3: The copy of the 'for' variable "x" can be deleted (Go 1.22+) (copyloopvar)
x := x
^
integration-cli/docker_cli_build_test.go:6203:3: The copy of the 'for' variable "builder" can be deleted (Go 1.22+) (copyloopvar)
builder := builder
^
integration-cli/docker_cli_build_test.go:6227:4: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
integration-cli/docker_utils_test.go:492:3: shadow: declaration of "line" shadows declaration at line 491 (govet)
line := strings.TrimSpace(line)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These already had a comment, so let's make it a ignore-comment
integration-cli/docker_cli_exec_test.go:409:13: G302: Expect file permissions to be 0600 or less (gosec)
f, err := os.OpenFile(netFilePath, os.O_WRONLY|os.O_SYNC|os.O_APPEND, 0o644)
^
integration-cli/docker_cli_run_test.go:3050:12: G302: Expect file permissions to be 0600 or less (gosec)
if err := os.Chmod(filename, 0o646); err != nil {
^
integration-cli/docker_cli_run_test.go:3072:12: G302: Expect file permissions to be 0600 or less (gosec)
if err := os.Chmod(filename, 0o646); err != nil {
^
integration-cli/docker_cli_run_test.go:3094:12: G302: Expect file permissions to be 0600 or less (gosec)
if err := os.Chmod(filename, 0o646); err != nil {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
integration-cli/docker_cli_run_test.go:401:12: G302: Expect file permissions to be 0600 or less (gosec)
f, err := os.OpenFile(filepath.Join(dir, "test"), os.O_CREATE, 0o700)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
integration-cli/docker_utils_test.go:187:12: G302: Expect file permissions to be 0600 or less (gosec)
f, err := os.OpenFile(dst, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o700)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
integration-cli/docker_utils_test.go:149:8: G601: Implicit memory aliasing in for loop. (gosec)
m = &c
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A recent change to the vishvananda/netlink package exposes
NLM_F_DUMP_INTR in some netlink responses as an EINTR (with
no data).
Retry the requests when that happens, up to five times, before
returning the error. The limit of five is arbitrary, on most
systems a single retry will be rare but, there's no guarantee
that a retry will succeed. So, on a very busy or misbehaving
system the error may still be returned. In most cases, this
will lead to failure of the operation being attempted (which
may lead to daemon startup failure, network initialisation
failure etc).
Signed-off-by: Rob Murray <rob.murray@docker.com>
Add `Platform` parameter that allows to select a specific platform to
show the history for.
This is a breaking change to the Go client as it changes the signature
of `ImageHistory`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
