Use platform MatchComparer when checking for matching platforms.
Also, add unit test to ensure the merging of defined and host-supported
platforms works correctly.
Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
The legacy `docker.pkg.github.com` registry provided by GitHub was not
OCI compliant, and did not suport pull by digest (among others). Commit
495d623ae5 added fallback code to detect
"not found" errors when pulling by digest, in which case we would fall
back to pulling by `name:tag`.
GitHub deprecated the legacy registry, and it was [sunset on Feb 24th, 2025][1]
in favor of GitHub Container Registry (GHCR) (ghcr.io).
This reverts commit 495d623ae5, removing the
fallback logic.
[1]: https://github.blog/changelog/2025-01-23-legacy-docker-registry-closing-down/
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The netip types are really useful for tracking state in the overlay
driver as they are hashable, unlike net.IP and friends, making them
directly useable as map keys. Converting between netip and net types is
fairly trivial, but fewer conversions is more ergonomic.
The NetworkDB entries for the overlay peer table encode the IP addresses
as strings. We need to parse them to some representation before
processing them further. Parse directly into netip types and pass those
values around to cut down on the number of conversions needed.
The peerDB needs to marshal the keys and entries to structs of hashable
values to be able to insert them into the SetMatrix. Use netip.Addr in
peerEntry so that peerEntry values can be directly inserted into the
SetMatrix without conversions. Use a hashable struct type as the
SetMatrix key to avoid having to marshal the whole struct to a string
and parse it back out.
Use netip.Addr as the map key for the driver's encryption map so the
values do not need to be converted to and from strings. Change the
encryption configuration methods to take netip types so the peerDB code
can pass netip values directly.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Because nftables tables/chain aren't fixed, like they are
in iptables - this change makes an assumption about the
bridge driver's naming.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The Namespace keeps some state for each inserted neighbor-table entry
which is used to delete the entry (and any related entries) given only
the IP and MAC address of the entry to delete. This state is not
strictly required as the retained data is a pure function of the
parameters passed to AddNeighbor(), and the kernel can inform us whether
an attempt to add a neighbor entry would conflict with an existing
entry. Get rid of the neighbor state in Namespace. It's just one more
piece of state that can cause lots of grief if it falls out of sync with
ground truth. Require callers to call DeleteNeighbor() with the same
aguments as they had passed to AddNeighbor(). Push the responsibility
for detecting attempts to insert conflicting entries into the neighbor
table onto the kernel by using (*netlink.Handle).NeighAdd() instead of
NeighSet().
Modernize the error messages and logging in DeleteNeighbor() and
AddNeighbor().
Signed-off-by: Cory Snider <csnider@mirantis.com>
func (*Namespace) AddNeighbor is only ever called with the force
parameter set to false. Remove the parameter and eliminate dead code.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Add checks in both containerd-based and distribution-based image pull
code paths to detect and AI model images early in the pull process.
These are not yet supported directly by the Engine and need to be
handled by the `docker model` CLI plugin.
For distribution-based pull, reject the AI models pulls.
For containerd image service only emit a warning.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
I'm horrible at vim, and use nano as my go-to editor for quick changes.
Let's install nano in the dev-container as alternative to vim for those
like me, who are not so vim-savvy ^O^C^C:wq:
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The (*Sandbox).joinLeaveStart() and .joinLeaveEnd() methods implement an
exclusive lock which is almost functionally identical to
(*sync.Mutex).Lock() and .Unlock(), respectively. The only notable
differences are that joinLeaveStart allocates, and calling
joinLeaveEnd() more times than joinLeaveStart() is a silent no-op
instead of a fatal error.
The construction of the joinLeaveStart/End methods is shaped like a
condition variable which uses channels for waiting and broadcasting.
The condition being waited for is that the joinLeaveDone struct field is
nil, i.e. that the lock has not been acquired by another goroutine.
As the condition is being checked and set while in a critical section,
it is a mutex implemented in terms of mutexes and channels. Replace the
home-grown mutex with a plain sync.Mutex.
Signed-off-by: Cory Snider <csnider@mirantis.com>
When deleting specific platforms from a multi-platform image, there's
potential risk as platform-specific content might be shared across
images/containers.
For now, require `force` parameter to make the user aware.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This change adds the ability to delete a specific platform from a
multi-platform image.
Previously, image deletion was an all-or-nothing operation - when
deleting a multi-platform image, all platforms would be removed
together. This change allows users to selectively remove individual
platforms from a multi-architecture image while keeping other platforms
intact.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
commit af0cdc36c7 marked these fields as
deprecated and to be removed in API v1.47 (which was targeted for v28.0).
We shipped v1.47 with the v27.2 release, but did not yet remove the erroneous
fields, so the version to deprecate was updated to v1.48 through
3df03d8e66
This patch removes fields that are not part of the image by replacing the
type with the Config struct from the docker image-spec.
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.50/images/alpine/json | jq .Config
{
"Env": [
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
],
"Cmd": [
"/bin/sh"
]
}
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.49/images/alpine/json | jq .Config
{
"Hostname": "",
"Domainname": "",
"User": "",
"AttachStdin": false,
"AttachStdout": false,
"AttachStderr": false,
"Tty": false,
"OpenStdin": false,
"StdinOnce": false,
"Env": [
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
],
"Cmd": [
"/bin/sh"
],
"Image": "",
"Volumes": null,
"WorkingDir": "",
"Entrypoint": null,
"OnBuild": null,
"Labels": null
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were originally added in 7b9ecb9700.
The `StartLimitBurst` and `StartLimitInterval` options used to be
options for `[Service]`, but were moved to the `[Unit]` in systemd 229,
and systemd 230 respectively. The `StartLimitInterval` was renamed to
`StartLimitIntervalSec`, which either takes a number (using "seconds" as
default unit) or an optional unit;
- 6bf0f408e4
- f0367da7d1
We kept these deprecated options to provide compatibility with older distros
that not yet had systemd 230, but all distros we support are now on versions
of systemd that support the new options;
docker run --rm registry.access.redhat.com/ubi8/ubi systemctl --version
systemd 239 (239-82.el8_10.5)
+PAM +AUDIT +SELINUX +IMA -APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ +LZ4 +SECCOMP +BLKID +ELFUTILS +KMOD +IDN2 -IDN +PCRE2 default-hierarchy=legacy
docker run -it --rm debian:
apt-get update && apt-get install -y systemd
...
systemctl --version
systemd 247 (247.3-7+deb11u6)
+PAM +AUDIT +SELINUX +IMA +APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ +LZ4 +ZSTD +SECCOMP +BLKID +ELFUTILS +KMOD +IDN2 -IDN +PCRE2 default-hierarchy=unified
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The "fluentd-async-connect" option was deprecated in 20.10 through
cc1f3c750e, and removed in 28.0 trough
49ec488036, which added migration code
on daemon startup.
However, the migration code _copied_ the deprecated option to the new
("fluentd-async") option, preserving the old field. Doing so could cause
an issue if a user would downgrade the daemon to a previous release, as
the changes in cc1f3c750e invalidate a config
that has both fields set (see [daemon/logger/fluentd/fluentd.go#L198-L200]);
if cfg[asyncKey] != "" && cfg[asyncConnectKey] != "" {
return config, errors.Errorf("conflicting options: cannot specify both '%s' and '%s", asyncKey, asyncConnectKey)
}
This patch updates the migration code to remove the deprecated option.
[daemon/logger/fluentd/fluentd.go#L198-L200]: cc1f3c750e/daemon/logger/fluentd/fluentd.go (L198-L200)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 7b153b9e28 changed the daemon to
advertise the recommended builder to use to V2 (BuildKit) for Linux
daemons, and V1 (Legacy Builder) for Windows daemons. For Linux daemons
we allowed the default to be overridden through the "features" field
in the daemon config (daemon.json), but for Windows we hard-coded it
to be V1, and no option to override.
With work in progress on implementing support for Windows in BuildKit,
we should remove this hardcoded assumption, and allow the default to
be overridden to advertise that BuildKit is supported.
Note that BuildKit on Windows is still very much a "work in progress",
and enabling it in the daemon may not even work, so users should not
try to enable this feature; a warning-level log is added to make it
visible that the feature is enabled.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`opts.params.Platform` is always nil inside this branch so we can omit
it from the `GetImage` call to make it less confusing.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Previously, we were using our own `FromStatusCode` function to map HTTP
status codes to Docker error types. Switch to the containerd code.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
the core/remotes/docker/schema1 package is deprecated and removed
in containerd v2.1; remove its use as we already deprecated these
images for "docker pull" and elsewhere.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
WCOW support on Buildkit is now coming to maturity. As part
of making this generally available, integrating it in
Docker Engine is critical for it's adoption.
This commit adds the buildkit execuitor for WCOW as the
next-builder (backend) for building Windows containers.
This will be an opt-in feature, with the end users setting
DOCKER_BUILDKIT=1 environment variable to use it.
The integration tests bit has also been handled.
https://github.com/moby/buildkit/pull/5956,
BUILDKIT_REF has been set to `master` for now, so
that the tests can run successfully. On the next
release, we will revert this back to using releases.
Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
The `BridgeNfIptables` and `BridgeNfIp6tables` fields in the
`GET /info` response were deprecated in API v1.48, and are now omitted
in API v1.50.
With this patch, old API version continue to return the field:
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.48/info | jq .BridgeNfIp6tables
false
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.48/info | jq .BridgeNfIptables
false
Omitting the field in API v1.50 and above
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.50/info | jq .BridgeNfIp6tables
null
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.50/info | jq .BridgeNfIptables
null
This reverts commit eacbbdeec6, and re-applies
a variant of 5d2006256f
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Manifest v2 schema1 was deprecated in 4866f5139a1 and this commit
removes the push code for v2 schema1.
This reverts commit f695e98cb7,
adjusted for changes that were made since
daemon: do not mkdir trust directory
Remove push tests and move UUID tests to integration
Partial revert of f23a51a860.
Only the schema1 push tests are removed but the schema1 pull tests
are still desired.
The UUID test is moved from integration-cli to integration.
Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add ability for the device driver to implement a device discovery
mechanism and expose discovered devices in the `docker info` output.
Currently it's only implemented for CDI devices.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Prevent the daemon spawned for integration tests from sourcing the
daemon configuration intended interactive dev shell usage.
Before this change, integration tests would fail to create a daemon
with different configuration provided via cli flags (like `--feature`)
if they're already specified in the default daemon.json.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Refactor the system CPU usage testing approach for improved maintainability:
1. Extract the core CPU usage parsing logic into a new `readSystemCPUUsage`
function that accepts an io.Reader, making it more testable and modular.
2. Use go:embed directive to embed the test data file at compile time,
eliminating runtime file operations and making tests more reliable.
3. Simplify the test by removing global variable mocking in favor of a more
direct approach with the new reader-based function.
4. Maintain full test coverage for the long "intr" line edge case which was
crucial for the original bug fix, while making the test more maintainable.
This change preserves the original test behavior while improving code quality,
testability, and making the tests self-contained.
Signed-off-by: Lee Gaines <leetgaines@gmail.com>
- rename the error-return to prevent accidental shadowing
- remove some intermediate variables
- usee a struct-literal for specs.Process
- optimize logging-code to not use chained "WithField"
- remove punctuation from error-message
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit ec65f2d21b has a typo: it switches
indexes[byTable] with indexes[byNetwork]. The indexes are not
equivalent. Switch it back.
Signed-off-by: Cory Snider <csnider@mirantis.com>
When a node leaves a network or the cluster, or memberlist considers the
node as failed, NetworkDB atomically deletes all table entries (for the
left network) owned by the node. This maintains the invariant that table
entries owned by a node are present in the local database indices iff
that node is an active cluster member which is participating in the
network the entries pertain to.
(*NetworkDB).handleTableEvent() is written in a way which attempts to
minimize the amount of time it is in a critical section with the mutex
locked for writing. It first checks under a read-lock whether both the
local node and the node where the event originated are participating in
the network which the event pertains to. If the check passes, the mutex
is unlocked for reading and locked for writing so the local database
state is mutated in a critical section. That leaves a window of time
between the participation check the write-lock being acquired for a
network or node event to arrive and be processed. If a table event for a
node+network races a node or network event which triggers the purge of
all table entries for the same node+network, the invariant could be
violated. The table entry described by the table event may be reinserted
into the local database state after being purged by the node's leaving,
resulting in an orphaned table entry which the local node will bulk-sync
to other nodes indefinitely.
It's not completely wrong to perform a pre-flight check outside of the
critical section. It allows for an early return in the no-op case
without having to bear the cost of synchronization. But such optimistic
concurrency control is only sound if the condition is double-checked
inside the critical section. It is tricky to get right, and this
instance of optimistic concurrency control smells like a case of
premature optimization. Move the pre-flight check into the critical
section to ensure that the invariant is maintained.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Use the errdefs resolver to ensure the status code is chosen from the
first outermost error. Remove duplicate containerd errdefs checks.
Signed-off-by: Derek McGowan <derek@mcg.dev>
The `Cause()` function is not part of the standard library and should
not be used or relied on. `pkg/errors` already has compatibility with
the standard library.
Signed-off-by: Derek McGowan <derek@mcg.dev>
The present content size is included in the image size usage and should
be included in the total size that the layer takes up on disk.
This prevents an issue where the reclaimable amount reported by the CLI
was a negative number.
This also updates the `/system/df` endpoint to use a new type that
computes information that was previously computed by the CLI. Computing
these in the server should require less work from the CLI and ensure
the calculations are more accurate because the CLI doesn't have to
reconstruct the numbers.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
daemon/network.go:156:3: S1000: should use for range instead of for { select {} } (staticcheck)
for {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
container.HealthStatus is a pseudo-type (alias for string) that was
introduced in 1e4bb14bcd.
Changing this field to use that type as a potential stepping-stone
towards making that type a distinct type.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ContainerState is currently an alias for string, so this should not
be a disruptive change, but is a stepping-stone to make it a distinct
type in future.
Relates to b811829595
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test is failing frequently on Windows;
=== FAIL: github.com/docker/docker/integration/networking TestPortMappedHairpinWindows (12.37s)
nat_windows_test.go:108: assertion failed: error is not nil: Post "http://%2F%2F.%2Fpipe%2Fdocker_engine/v1.49/containers/1181d6510a2f55c742b7b183aa7324eddbc213cd15797428c4062dcb031fb825/start": context deadline exceeded
panic.go:636: assertion failed: error is not nil: Error response from daemon: error while removing network: network clientnet has active endpoints (name:"laughing_lederberg" id:"8605ebbc2c7c")
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(*NetworkDB).SetPrimaryKey() acquires a read lock on the NetworkDB
instance. That seems sound on the surface as it is only reading from the
NetworkDB struct, not mutating it. However, concurrent calls to
(*memberlist.Keyring).UseKey() would get flagged by Go's race detector
due to some questionable locking in its implementation. Acquire an
exclusive lock in SetPrimaryKey so concurrent calls don't race each
other.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The bridge driver should figure out whether it's running in
a mirrored WSL2 setup, and tell the firewaller.
So, move the WSL2-deciding code back into the bridge driver
and unit test it there. Use TestIptabler to check the rules
are constructed properly.
Signed-off-by: Rob Murray <rob.murray@docker.com>
integration/internal/network/ops.go:137:1: ST1020: comment on exported function WithIPAMRange should be of the form "WithIPAMRange ..." (staticcheck)
// WithIPAM adds an IPAM with the specified Subnet, IPRange and Gateway to the network
^
libnetwork/bitmap/sequence.go:50:1: ST1020: comment on exported function New should be of the form "New ..." (staticcheck)
// NewHandle returns a new Bitmap of ordinals in the interval [0, n).
^
libnetwork/diagnostic/server.go:61:1: ST1020: comment on exported method HandleFunc should be of the form "HandleFunc ..." (staticcheck)
// Handle registers the handler function for the given pattern,
^
libnetwork/endpoint.go:388:1: ST1020: comment on exported method Key should be of the form "Key ..." (staticcheck)
// endpoint Key structure : endpoint/network-id/endpoint-id
^
libnetwork/network.go:574:1: ST1020: comment on exported method MarshalJSON should be of the form "MarshalJSON ..." (staticcheck)
// TODO : Can be made much more generic with the help of reflection (but has some golang limitations)
^
libnetwork/network.go:633:1: ST1020: comment on exported method UnmarshalJSON should be of the form "UnmarshalJSON ..." (staticcheck)
// TODO : Can be made much more generic with the help of reflection (but has some golang limitations)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
NetworkDB's Watch() facility is problematic to use in practice. The
stream of events begins when the watch is started, so the watch cannot
be used to process table entries that existed beforehand. Either option
to process existing table entries is racy: walking the table before
starting the watch leaves a race window where events could be missed,
and walking the table after starting the watch leaves a race window
where created/updated entries could be processed twice.
Modify Watch() to initialize the channel with synthetic CREATE events
for all existing entries owned by remote nodes before hooking it up to
the live event stream. This way watchers observe an equivalent sequence
of events irrespective of whether the watch was started before or after
entries from remote nodes are added to the database. Remove the bespoke
and racy synthetic event replay logic for driver watches from the
libnetwork agent.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The gossip protocol which powers NetworkDB does not guarantee in-order
reception of events. This poses a problem with deleting entries: without
some mechanism to discard stale CREATE or UPDATE events received after a
DELETE, out-of-order reception of events could result in a deleted entry
being spuriously resurrected in the local NetworkDB state! NetworkDB
handles this situation by storing "tombstone" entries for a period of
time with the Lamport timestamps of the entries' respective DELETE
events. Out-of-order CREATE or UPDATE events will be ignored by virtue
of having older timestmaps than the tombstone entry, just like how it
works for entries that have not yet been deleted.
NetworkDB was only storing a tombstone if the entry was already present
in the local database at the time of the DELETE event. If the first
event received for an entry is a DELETE, no tombstone is stored. If a
stale CREATE/UPDATE event for the entry (with an older timestamp than
the DELETE) is subsequently received, NetworkDB erroneously creates a
live entry in the local state with stale data. Modify NetworkDB to store
tombstones for DELETE events irrespective of whether the entry was known
to NetworkDB beforehand so that it correctly discards out-of-order
CREATEs and UPDATEs in all cases.
Signed-off-by: Cory Snider <csnider@mirantis.com>
NetworkDB gossips changes to table entries to other nodes using distinct
CREATE, UPDATE and DELETE events. It is unfortunate that the wire
protocol distinguishes CREATEs from UPDATEs as nothing useful can be
done with this information. Newer events for an entry invalidate older
ones, so there is no guarantee that a CREATE event is broadcast to any
node before an UPDATE is broadcast. And due to the nature of gossip
protocols, even if the CREATE event is broadcast from the originating
node, there is no guarantee that any particular node will receive the
CREATE before an UPDATE. Any code which handles an UPDATE event
differently from a CREATE event is therefore going to behave in
unexpected ways in less than perfect conditions.
NetworkDB table watchers also receive CREATE, UPDATE and DELETE events.
Since the watched tables are local to the node, the events could all
have well-defined meanings that are actually useful. Unfortunately
NetworkDB is just bubbling up the wire-protocol event types to the
watchers. Redefine the table-watch events such that a CREATE event is
broadcast when an entry pops into existence in the local NetworkDB, an
UPDATE event is broadcast when an entry which was already present in the
NetworkDB state is modified, and a DELETE event is broadcast when an
entry which was already present in the NetworkDB state is marked for
deletion. DELETE events are broadcast with the same value as the most
recent CREATE or UPDATE event for the entry.
The handler for endpoint table events in the libnetwork agent assumed
incorrectly that CREATE events always correspond to adding a new active
endpoint and that UPDATE events always correspond to disabling an
endpoint. Fix up the handler to handle CREATE and UPDATE events using
the same code path, checking the table entry's ServiceDisabled flag to
determine which action to take.
Signed-off-by: Cory Snider <csnider@mirantis.com>
State.RemovalInProgress was originally non-exported when it was added in
[40945fc186][1], adding a comment that the
field should not be persisted to disk.
But when moved to a separate package in [6bb0d1816a][2],
it was was exported, without adding `json:"-"`. As a result, it's now persisted
to disk;
cat /var/lib/docker/containers/e493924a99cad918cda8048f967032729105ee072d563d734125cec46e1b5885/config.v2.json | jq .State
{
"Running": true,
"Paused": false,
"Restarting": false,
"OOMKilled": false,
"RemovalInProgress": false,
"Dead": false,
"Pid": 5053,
"ExitCode": 0,
"Error": "",
"StartedAt": "2025-05-13T12:12:15.115512564Z",
"FinishedAt": "0001-01-01T00:00:00Z",
"Health": null
}
Note that this type is used internally, and (while similar) is not used for
API responses;
docker inspect e493924a99cad918cda8048f967032729105ee072d563d734125cec46e1b5885 | jq .[].State
{
"Status": "running",
"Running": true,
"Paused": false,
"Restarting": false,
"OOMKilled": false,
"Dead": false,
"Pid": 5053,
"ExitCode": 0,
"Error": "",
"StartedAt": "2025-05-13T12:12:15.115512564Z",
"FinishedAt": "0001-01-01T00:00:00Z"
}
However, interestingly, [`daemon.restore`][3] does take this field into account
while restoring containers, which seems that it depends on the field being
persisted to disk. That logic was added in [ce72473197][4].
That logic may be redundant if we no longer persist to disk, as the `State.Dead`
is already set when cleaning up a container in [`daemon.cleanupContainer`][5].
[1]: 40945fc186
[2]: 6bb0d1816a (diff-60173e67d15f3085dd09956b3ffa83566ae25fec61cfe08ddd2e1c37223e3be7R24)
[3]: d42d79dceb/daemon/daemon.go (L498-L514)
[4]: ce72473197
[5]: 294f0c36e4/daemon/delete.go (L124-L126)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The swarmPlugin type does not implement the Swarm plugin.AddrPlugin
interface because it embeds an interface value which does not include
that method in its method set. (You can type-assert an interface value
to another interface which the concrete type implements, but a struct
embedding an interface value is not itself an interface value.) Wrap the
plugin with a different adapter type which exposes the Addr() method if
the concrete plugin implements it.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Makes the code slightly more idiomatic. These paths avoided uses of
defer because they came with an overhead in older versions of Go,
but this overhead should now be neglectible.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Currently, it's needed to set `BIND_DIR=.` when starting the dev-container
in order to bind-mount the current source-code into the container. Omitting
that option only bind-mounts the "bundles" directory and copies the source
into the image before starting.
While there are some cases where (e.g.) a remote server is used for development,
and bind-mounting is not possible, those scenarios should be considered more
advanced, so let's make the common scenario easier to use.
With this patch, running `make shell` should build and run the dev-container
with the source-code mounted.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was decorating errors with the container name, but within its
own context wouldn't be aware how the delete was referenced. This could
result in a container deleted by "ID" to produce an error with the container
Name. Some errors were also decorated before storing as "removalError" on
the container object itself.
The removalError was originally added in f963500c54,
before which the error was returned. Now that it's part of the container's
state itself, adding the container's ID is probably not very useful.
This patch reduces the scope of decorating the errors to the error-condition
itself, leaving it to the caller to decorate them further with the container
ID or Name (if any).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code only needed to know whether the container was paused; for other
states ("restarting", "running"), it's still used to be included in the
error string.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Currently, there's no mechanism to specify a write timeout value for
fluentd connections. This means that writes can forever be blocked if
the downstream connections is unhealthy. This commit makes this value
configurable via a new fluentd log option called "fluentd-write-timeout".
Signed-off-by: Anirudh Aithal <aithal@amazon.com>
Logic was added to the Swarm executor in commit 0d9b0ed678
to clean up managed networks whenever the node's load-balancer IP
address is removed or changed in order to free up the address in the
case where the container fails to start entirely. Unfortunately, due to
an oversight the function returns early if the Swarm is lacking
an ingress network. Remove the early return so that load-balancer IP
addresses for all the other networks are freed as appropriate,
irrespective of whether an ingress network exists in the Swarm.
Signed-off-by: Cory Snider <csnider@mirantis.com>
If kernel module br_netfilter isn't loaded on the test host
before rootless test-integration tests that need it, the
rootless daemon hits a permission error when trying to do
the load. So, try to load the module before going rootless.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The container.WaitCondition type was deprecated in
100102108b, but this use of the
deprecated alias was left behind.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Part of https://github.com/docker/docker-ce-packaging/issues/1186
Adds the required systemd-sysusers configuration to the
moby/moby repo. This will be used by downstream package
maintainers (RPM, DEB etc) as a replacement to the
`groupadd` postinstall commands.
It's generally recommended to use sysusers since it is
more of a declarative method of defining these service
accounts and groups.
This configuration file specifies a group ("g") named
"docker" should be created with an automatic GID
allocation ("-").
Signed-off-by: Robert Sturla <robertsturla@outlook.com>
Introduce a ValidateHealthStatus utility in api/types/container to
validate if a given HealthState is valid.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Rename the ouput variable to prevent accidental shadowing, and simplify how
we check for the `syscall.ENOTDIR` error; `errors.Is()` will already unwrap
the error, so no type-casting is needed;
package main
import (
"errors"
"fmt"
"os"
"syscall"
)
func main() {
err := &os.PathError{Op: "mkdir", Path: "/hello/world", Err: syscall.ENOTDIR}
if errors.Is(err, syscall.ENOTDIR) {
fmt.Println(err)
}
}
While at it, also improve the code-comment that outlines the intent.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing these errors, which are used in defers, and
while at it, also fixed some linting warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The function used a `cmdErr` error, but it was discarded when returning an
error. Also removing the named "err" return variable.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- remove named err-returns to prevent accidental shadowing
- fix some minor linting issues (unhandled errors)
- update code depending on "errors.Wrap" behavior. which ignores
nil-errors, which can be easily overlooked when rewriting to
native go error-wrapping (fmt.Errorf()).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test normally fails due to a known issue, but it
has recently started passing in CI ... not sure why, it
still fails locally - so, allow it to unexpeectedly
pass, but collect some debug info to try to undertand
why.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Skip tests that have recently started failing in rootless
mode with error:
failed to start daemon: Error initializing network controller:
error creating default "bridge" network:
cannot restrict inter-container communication or run
without the userland proxy:
stat /proc/sys/net/bridge/bridge-nf-call-iptables:
no such file or directory:
set environment variable DOCKER_IGNORE_BR_NETFILTER_ERROR=1 to ignore
(Perhaps we can ensure the module is loaded before starting
the rootless env - or work out why the failures have only
recently started, and put-back whatever changed. But, for
now, I think we need to skip.)
Signed-off-by: Rob Murray <rob.murray@docker.com>
NetworkDB uses a hierarchy of queues to prioritize messages for
broadcast. Unfortunately the logic to pull from multiple queues is
flawed. The length of the messages pulled from the first queue is not
taken into account when pulling messages from the second queue. A list
of messages up to tiwce as long as the limit could be returned! Messages
beyond the limit will be truncated unceremoniously by memberlist.
Memberlist broadcast queues assume that all messages returned from a
GetBroadcasts call will be broadcasted to other nodes in the cluster.
Messages are popped from the queue once they have hit their retransmit
limit. On a busy system messages may be broadcast fewer times than
intended, possibly even being dropped without ever being broadcast!
Subtract the length of messages pulled from the first queue from the
broadcast size limit so the limit is not exceeded when pulling from the
second queue.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The loopback-test fixes seem to be sufficient to resolve the flakiness
of all the tests aside from TestFlakyNetworkDBIslands.
Signed-off-by: Cory Snider <csnider@mirantis.com>
NetworkDB defaults to binding to the unspecified address for gossip
communications, with no advertise address set. In this configuration,
the memberlist instance listens on all network interfaces and picks one
of the host's public IP addresses as the advertise address.
The NetworkDB unit tests don't override this default, leaving them
vulnerable to flaking out as a result of rogue network traffic
perturbing the test, or the inferred advertise address not being useable
for loopback testing. And macOS prompts for permission to allow the test
executable to listen on public interfaces every time it is rebuilt.
Modify the NetworkDB tests to explicitly bind to, advertise, and join
ports on 127.0.0.1 to make the tests more robust to flakes in CI and
more convenient to run locally.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The NetworkDB unit tests instantiate clusters which communicate over
loopback where every "node" listens on a distinct localhost port. The
tests make use of a NetworkDB configuration knob to set the port. When
the NetworkDB configuration's BindPort field is set to a nonzero value,
its memberlist instance is configured to bind to the specified port
number. However, the advertise port is left at the
memberlist.DefaultLANConfig() default value of 7946. Because of this,
nodes would be unable to contact any of the other nodes in the cluster
learned by gossip as the gossiped addresseses specify the wrong ports!
The flaky tests passed as often as they did thanks to the robustness of
the memberlist module: NetworkDB gossip and and memberlist node
liveness-probe pings to unreachable nodes can all be relayed through
the reachable nodes, the nodes on the bootstrap join list.
Make the NetworkDB unit tests less flaky by setting each node's
advertise port to the bind port.
The daemon is unaffected by this oversight as it unconditionally uses
the default listen port of 7946, which aligns with the advertise port.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The function was accessing the index map without holding the mutex, so
it would race any mutation to the database indexes. Fetch the reference
to the tree's root while holding a read lock. Since the radix tree is
immutable, taking a reference to the root is equivalent to starting a
read-only database transaction, providing a consistent view of the data
at a snapshot in time, even as the live state is mutated concurrently.
Also optimize the WalkTable function by leveraging the immutability of
the radix tree.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The image api already defines the backend used and does
not directly use the reference store backend. It also should not
directly use the reference store backend.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Per-network option com.docker.network.bridge.trusted-host-interfaces
accepts a list of interfaces that are allowed to route
directly to a container's published ports in a bridge
network with nat enabled.
This daemon level option disables direct access filtering,
enabling direct access to published ports on container
addresses in all bridge networks, via all host interfaces.
It overlaps with short-term env-var workaround:
DOCKER_INSECURE_NO_IPTABLES_RAW=1
- it does not allow packets sent from outside the host to reach
ports published only to 127.0.0.1
- it will outlive iptables (the workaround was initially intended
for hosts that do not have kernel support for the "raw" iptables
table).
Signed-off-by: Rob Murray <rob.murray@docker.com>
trusted_host_interface have access to published ports on container
addresses - enabling direct routing to the container via those
interfaces.
Signed-off-by: Rob Murray <rob.murray@docker.com>
On Sandbox restore if an Endpoint (or Network) can't be loaded from
the store and the container is to be preserved (live-restore), stub
Network and Endpoint objects are created and added to the cache. But
then the Endpoint is just dropped - leaving the stub objects in
cache.
If not-live-restore, the Sandbox is reconstructed do that it can be
deleted by sb.delete(force=true). But, the only thing sb.delete does
with the Endpoint in this case is call ep.Delete. And, ep.Delete
doesn't do anything if the Network and Endpoint can't both be loaded
from the store. So, again, the stub objects are left in the in-memory
cache and nothing that might have needed tidying up got tidied up.
So, whether live-restoring or not, just log an error and ignore an
Endpoint if it can't be loaded from disk.
Signed-off-by: Rob Murray <rob.murray@docker.com>
- Use structured logging.
- Which means ids are logged consistently.
- Use variable 'isRestore' instead of extra map lookups.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This will use the default settings for buildkit gc unless explicitly
disabled by setting `enabled: false` in the gc configuration.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
- remove named error-returns
- make error-handling slightly more idiomatic (check for non-nil errors)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
builder/builder-next/adapters/containerimage/pull.go:912:3: shadow: declaration of "now" shadows declaration at line 905 (govet)
now := time.Now()
^
builder/builder-next/adapters/snapshot/layer.go:52:4: shadow: declaration of "diffIDs" shadows declaration at line 28 (govet)
diffIDs, err := s.EnsureLayer(gctx, info.Parent)
^
builder/builder-next/worker/worker.go:577:3: shadow: declaration of "now" shadows declaration at line 570 (govet)
now := time.Now()
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/plugins/plugins.go:325:4: shadow: declaration of "pl" shadows declaration at line 315 (govet)
pl, err := loadWithRetry(name, false)
^
pkg/plugins/pluginrpc-gen/parser.go:153:4: shadow: declaration of "iface" shadows declaration at line 135 (govet)
iface, ok := spec.Type.(*ast.InterfaceType)
^
pkg/plugins/pluginrpc-gen/parser_test.go:61:2: shadow: declaration of "arg" shadows declaration at line 40 (govet)
arg := f.Args[0]
^
pkg/plugins/pluginrpc-gen/parser_test.go:165:2: shadow: declaration of "arg" shadows declaration at line 40 (govet)
arg := f.Args[0]
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
daemon/logger/loggertest/logreader.go:548:14: shadow: declaration of "open" shadows declaration at line 545 (govet)
case err, open := <-lw.Err:
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
api/server/router/network/network_routes.go:148:14: shadow: declaration of "nwv" shadows declaration at line 146 (govet)
} else if nwv, ok := listByFullName[nwk.ID]; ok {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
libnetwork/network.go:637:6: shadow: declaration of "v" shadows declaration at line 634 (govet)
if v, ok := n.generic[netlabel.GenericData]; ok {
^
libnetwork/service_linux.go:125:7: shadow: declaration of "ep" shadows declaration at line 86 (govet)
if ep := sb.getGatewayEndpoint(); ep != nil {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
libnetwork/portallocator/portallocator_test.go:151:7: shadow: declaration of "i" shadows declaration at line 150 (govet)
for i := 0; i <= p.end-p.begin; i++ {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
also moved the variables closer to where they're used
libnetwork/networkdb/cluster.go:414:4: shadow: declaration of "nid" shadows declaration at line 397 (govet)
nid := params[0]
^
libnetwork/networkdb/networkdb.go:514:4: shadow: declaration of "nid" shadows declaration at line 506 (govet)
nid := params[0]
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing these errors, which are used in defers, and
while at it, also fixed some linting warnings about unhandled errors, and
defers created in a loop.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing the error, which is used in a defer, and
while at it, also fixed some linting warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing these errors, which are used in defers, and
while at it, also fixed some linting warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing these errors, which are used in defers, and
while at it, also fixed some linting warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Update referencesin hack/README.md from outdated docker URLs to the correct location at /docs/contributing/test.md.
Signed-off-by: ChengyuZhu6 <hudson@cyzhu.com>
Alias and deprecate the status types and constants from the root
container package. The root container package is intended for use
within the daemon and no the api package.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Prevent accidentally shadowing these errors, which are used in defers, and
while at it, also fixed some linting warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also renamed some vars for clarity, renamed a error-returns to prevent
shadowing, and fixed some linter warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also renamed some vars for clarity, renamed a error-returns to prevent
shadowing, and fixed some linter warnings about unhandled errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent accidentally shadowing the error, which is used in a defers, and
while at it, also fixed some linting warnings about unhandled errors and
shadowed vars.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The build exporter now clears the image tags and always exported to a
dangling image. It then uses the image tagger to perform the tagging
which causes the dangling image to be removed and the naming message to
be sent correctly.
An additional progress message is sent to indicate the renaming.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Before this patch, and error would be produced when starting the exec,
but the CLI would wait for the exec to complete, timing out after 10
seconds (default). With this change, an error is returned immediately
when creating the exec.
Note that "technically" this check may have some TOCTOU issues, because
'/etc/passwd' and '/etc/groups' may be mutated by the container in between
creating the exec and starting it.
This is very likely a corner-case, but something we can consider changing
in future (either allow creating an invalid exec, and checking before
starting, or checking both before create and before start).
With this patch:
printf 'FROM alpine\nRUN rm -f /etc/group' | docker build -t nogroup -
ID=$(docker run -dit nogroup)
time docker exec -u 0:root $ID echo hello
Error response from daemon: unable to find group root: no matching entries in group file
real 0m0.014s
user 0m0.010s
sys 0m0.003s
# numericc uid/gid (should not require lookup);
time docker exec -u 0:0 $ID echo hello
hello
real 0m0.059s
user 0m0.007s
sys 0m0.008s
# no user specified (should not require lookup);
time docker exec $ID echo hello
hello
real 0m0.057s
user 0m0.013s
sys 0m0.008s
docker rm -fv $ID
# container that does have a valid /etc/groups
ID=$(docker run -dit alpine)
time docker exec -u 0:root $ID echo hello
hello
real 0m0.063s
user 0m0.010s
sys 0m0.009s
# non-existing user or group
time docker exec -u 0:blabla $ID echo hello
Error response from daemon: unable to find group blabla: no matching entries in group file
real 0m0.013s
user 0m0.004s
sys 0m0.009s
docker rm -fv $ID
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When a network is created with "-o com.docker.network.enable_ipv4'
(including via "default-network-opts" in daemon config), and
EnableIPv4 is present in the API request (including when CLI option
"--ipv4" is used) - the top-level API value is used and the '-o'
is ignored.
But, the "-o" still shows up in Options in inspect output, which is
confusing if the values are different.
So, drop the "-o" if the top-level API option is set. Ditto IPv6.
Signed-off-by: Rob Murray <rob.murray@docker.com>
`moby/vpnkit` now officially pushes a binary image to `moby/vpnkit-bin`
repository on Docker Hub.
Use it to fetch the vpnkit binary.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Commit 27adcd5 ("libnet/d/bridge: drop connections to lo mappings, and
direct remote connections") introduced an iptables rule to drop 'direct'
remote connections made to the container's IP address - for each
published port on the container.
The normal filter-FORWARD rules would then drop packets sent directly to
unpublished ports. This rule was only created along with the rest of port
publishing (when a container's endpoint was selected as its gateway). Until
then, all packets addressed directly to the container's ports were dropped
by the filter-FORWARD rules.
But, the rule doesn't need to be per-port. Just drop packets sent
directly to a container's address unless they originate on the host.
That means fewer rules, that can be created along with the endpoint (then
directly-routed get dropped at the same point whether or not the endpoint
is currently the gateway - very slightly earlier than when it's not the
gateway).
Signed-off-by: Rob Murray <rob.murray@docker.com>
gotest.tools v3.0.1 and up support Go's native test.Cleanup(), which
means that manually calling the cleanup functions in a defer is no
longer needed.
Some of these could probably be replaced by Go's native `t.TempDir()`,
but keeping that for a follow-up exercise.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was added in 684633f734, because
Go stdlib's filepath.Rel did not support Windows UUID paths (and UNC paths).
UNC paths were fixed in https://golang.org/cl/253197 (go1.17), and I think
UUID paths were also supported now.
To verify the behavior I temporarily added a unit-test to compare between
stdlib and this implementation, and it all looks to work, so we can deprecate
and remove this function. Deprecating first, but there's no (known) external
users of this, so we can likely remove in an upcoming minor (or patch) release.
func TestRel(t *testing.T) {
testCases := []struct {
doc string
base string
target string
expected string
}{
{
doc: "UNC path",
base: `\\server\share`,
target: `\\server\share\folder\file.txt`,
expected: `folder\file.txt`,
},
{
doc: "UUID path",
base: `\\?\Volume{b75e2c83-0000-0000-0000-602f00000000}\data`,
target: `\\?\Volume{b75e2c83-0000-0000-0000-602f00000000}\data\file.txt`,
expected: `file.txt`,
},
{
doc: "subdirectory",
base: `C:\Projects`,
target: `C:\Projects\Go\main.go`,
expected: `Go\main.go`,
},
{
doc: "same directory",
base: `C:\Projects`,
target: `C:\Projects`,
expected: `.`,
},
{
doc: "parent directory",
base: `C:\Projects\Go`,
target: `C:\Projects`,
expected: `..`,
},
}
for _, tc := range testCases {
t.Run(tc.doc, func(t *testing.T) {
actual, err := remotecontext.Rel(tc.base, tc.target)
if err != nil {
t.Fatal(err)
}
if actual != tc.expected {
t.Errorf("expected: %q, got: %q", tc.expected, actual)
}
// Try with stdlib
actual, err = filepath.Rel(tc.base, tc.target)
if err != nil {
t.Fatal(err)
}
if actual != tc.expected {
t.Errorf("expected: %q, got: %q", tc.expected, actual)
}
})
}
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The syntax was pinned for 1.7 in f696e0d2a7
possibly because it was not yet promoted as "latest stable" at the
time.
I don't think we need to pin to an old version, and just go with
the latest, so that we can use the latest features provided.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- We now only use winresources for the dockerd binary, so we can
reduce some uses of wildcards
- Use explicit ("/") to indicate these should only be ignored
relative to the root of the repository and build-context
- Remove remnant ignore for rpm builds
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Fix a logical error in the rootless setup tool where the iptables error
handling was incorrectly placed. The code was setting
`faced_iptables_error=1` even when iptables was available, causing
unnecessary error messages and setup suggestions.
This patch correctly moves the error handling into the `else` branch so
that we only report iptables errors when the command is actually
unavailable.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;
> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.
This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since commit 925b484 ("No fallback nameservers for internal
resolver"), if the host's resolv.conf has no nameservers and
no servers are supplied via config, the internal resolver will
not use Google's DNS - so the container will not be able to
resolve external DNS requests.
That can happen when container's are "restart-always" and the
docker daemon starts before the host's DNS is configured.
So, to highlight the issue (which may not be an error, but
probably is), include a warning in the container's resolv.conf
file.
Also, log a warning - logs currently say "No non-localhost DNS
nameservers are left in resolv.conf. Using default external
servers". But, that's misleading because it's from an initial
resolv.conf setup, before the internal resolver configured without
those fallbacks - we'll drop the fallbacks completely once the
default bridge has an internal resolver).
Signed-off-by: Rob Murray <rob.murray@docker.com>
CI runs a docker daemon, and some tests run their own. That tramples
the CI daemon's default bridge.
If a test leaves the bridge with only a link-local IPv6 address (for
example), subsequent tests are likely to fail - even if they also
start their own daemon because, by default the default bridge
network's config is based on an existing bridge device.
So, remember the bridge's addresses at the start of each test, and
restore them afterwards.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Since commit 51d7f95 ("libnet: remove struct endpointCnt") an
endpoint count for networks has not been persisted.
But, on downgrade to a version older than that commit, the
missing field caused daemon startup to fail.
So, create the count in the store - it only needs to exist, it's
no longer maintained as a count of endpoints. On downgrade, the
count is probably zero anyway (the daemon is stopped), but the
older daemon fixes it up on startup if necessary.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The existing code was more complicated than needed. By default, the daemon
runs "unconfined", but we try to detect the current profile that's set.
When failing to do so (error, or detected profile is empty), we assume
the default ("unconfined").
This patch simplifies the logic;
- Set the default ("unconfined")
- Only update the default when we successfully found the current profile
(no error occurred, and the profile is not empty).
While updating, also;
- Replaced use of `strings.SplitN` for `strings.Cut`, which is more
efficient, and doesn't allocate.
- Move constructing the profileData closer to where it's used.
- Remove intermediate var.
- Combine defers and change the order (close file first, before removing),
and suppress errors to keep linters happy.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`exec.Cmd.Path` already contains the command that was executed, so we
were printing the command twice. However, `exec.Cmd` implements a stringer
interface, which provides a readable version of the command that was
executed, so use that instead. While updating, lso change backticks in
the error for regular quotes.
Before:
running `/usr/sbin/apparmor_parser apparmor_parser -Kr /no/such/file` failed with output: Cache read/write disabled: interface file missing. (Kernel needs AppArmor 2.4 compatibility patch.)
Warning: unable to find a suitable fs in /proc/mounts, is it mounted?
Use --subdomainfs to override.
error: exit status 1
After:
running '/usr/sbin/apparmor_parser -Kr /no/such/file' failed with output: Cache read/write disabled: interface file missing. (Kernel needs AppArmor 2.4 compatibility patch.)
Warning: unable to find a suitable fs in /proc/mounts, is it mounted?
Use --subdomainfs to override.
error: exit status 1
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We are considering moving the apparmor profile to a separate module,
so removing these comments in preparation. These comments are ignored
already when building in go module mode, so have little benefits.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We are considering moving the seccomp profile to a separate module,
so removing these comments in preparation. These comments are ignored
already when building in go module mode, so have little benefits.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We are considering moving the seccomp profile to a separate module,
so reducing the list of dependencies for this package.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit 5d2006256f, which
caused some issues in the docker/cli formatting code that needs some
investigating.
Let's (temporarily) revert this while we look what's wrong.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy the swagger / OpenAPI file to the documentation. This is the API
version used by the upcoming v28.1.0 release.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Like for containerCreate, filter the list of container labels based on
`DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS` and put that list in the
OTel span.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This commit adds a new OTel span to `(*Daemon).containerCreate()` and
puts filtered container labels in the span attributes.
The filter is based on a comma-separated list of labels provided through
the `DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS` environment variable.
This label might be removed at any point in time if we figure out a
better way to filter labels, or if that span becomes unecessary.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
It's possible to remove a legacy link from running containers.
When that happens, the Sandbox's Endpoints are removed and
re-added.
Since commit 65120d5 ("Create bridge veth in container netns")
the veth device has been created in the container's netns. When
that happens, a flag is set on the Endpoint to note that it
does not need to be moved into the netns.
But, during the Leave/Join (Sandbox.Refresh) the veth is moved
out of the netns. So, clear the flag during the Leave, to note
that it needs to be moved back in during the Join.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The `BridgeNfIptables` and `BridgeNfIp6tables` fields in the
`GET /info` response were deprecated in API v1.48, and are now omitted
in API v1.49.
With this patch, old API version continue to return the field:
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.48/info | jq .BridgeNfIp6tables
false
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.48/info | jq .BridgeNfIptables
false
Omitting the field in API v1.49 and above
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.49/info | jq .BridgeNfIp6tables
null
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.49/info | jq .BridgeNfIptables
null
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was introduced in c817ea2159, but overlooked
that the logs were now produced before (for POST requests) the form-data was
handled and added to the "fields" variable.
This patch changes the logic to wrap the handler and make sure logs are
created before, and after the handler is called (the "form-data" is always
added before the handler is called).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This updates the Docker CLI version used for integration-cli tests from
v17.06.2-ce to v18.06.3-ce.
v18.06 is the first one that supports enabling BuildKit.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This option was deprecated in 1932091e21, and
is no longer used. It was only kept to allow priniting a deprecation warning
if the config would happen to have the field set.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There have been numerous reports of the "has active endpoints" error
over the years. Historically, there were some faulty code paths that
could lead to this error, but we believe they all have been fixed by
now.
However, users are still facing this error from time to time. Either
because they forgot that some containers are still running, or because
we still have bugs lying around.
To help users figure whether this error is legitimate, and what triggers
it, add endpoint names (which are just container names) to the error
message.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Add an OTel span processor copying the 'trigger' baggage member
propagated through contexts to all children spans. It's used to identify
what triggered a trace / span (API call, libnet init, etc...)
All code paths that call libnet's `NewNetwork` set this baggage member
with a unique value.
For instance, this can be used to distinguish bridge's `createNetwork`
spans triggered by daemon / libnet initialization from custom network
creation triggerd by an API call.
Two util functions are added to wrap `baggage.New` and
`baggage.NewMemberRaw` to make it easier to deal with baggage and
members by panicking on error. These should not be used with dynamic
values.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Plumb context from the API down to libnet driver method `CreateNetwork`,
and add an OTel span to the bridge driver's `createNetwork` method.
Include a few attributes describing the network configuration (e.g.
IPv4/IPv6, ICC, internal and MTU).
A new util function, `RecordStatus`, is added to the `otelutil` package
to easily record any error, and update the span status accordingly.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Since commit `eaa84bc`, when a container joins a network, we broadcast
unsolicited ARP/NAs to other containers connected to that network. The
initial advertisement is sent synchronously, and then remaining
advertisements (by default, 2) are sent asynchronously at a regular
interval (by default, 1 second).
An OTel span records the time spent sending subsequent advertisements,
but it's attached to the parent span `libnetwork.osl.AddInterface` so it
affects the overall latency reported by the trace.
The real latency (i.e. excluding the latency of that asynchronous
process), is pretty much what CLI users perceive, so it's an important
measure. It's available on the top-most span in the trace, but limiting
the trace to the real time taken by the Engine to process the HTTP
request seems better.
With this change, the initial advertisement is now tracked by a
dedicated span, and a new independent trace (containing a single span)
is created and linked to the 'initial' span / HTTP trace.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
- pkg/archive: deprecate, and add aliases
Keeping the tests in this commit; also moves various utilities
into a _test.go file, as they were now only used in tests.
- pkg/chrootarchive: deprecate and add aliase
deprecate pkg/archive and add aliases
keeping the tests in this commit
- Add temporary exceptions for deprecation linting errors, because
this commit is to verify everything works with the aliases.
- remove tests that depend on un-exported types
=== RUN TestDisablePigz
--- FAIL: TestDisablePigz (0.00s)
panic: interface conversion: io.Reader is *archive.readCloserWrapper, not *archive.readCloserWrapper (types from different packages) [recovered]
- pkg/archive, pkg/chrootarchive: remove test files
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Using iptables.OnReloaded to restore individual per-network rules
on firewalld reload means rules for deleted networks pop back in
to existence (because there was no way to delete the callbacks on
network-delete).
So, on firewalld reload, walk over current networks and ask them
to restore their iptables rules.
Signed-off-by: Rob Murray <rob.murray@docker.com>
- registry.ServiceConfig: add a "ExtraFields" for outputting deprecated
fields.
- remove uses of AllowNondistributableArtifactsCIDRs and AllowNondistributableArtifactsHostnames
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Report FirewallBackend in "docker info".
It's currently "iptables" or "iptables+firewalld" on Linux, and
omitted on Windows.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This fix address issues where the scanner was unable to properly parse longer outputs from /proc/stat. This could happen on an ARM machine with large amount of CPU cores (and interrupts). By switching to reader we have more control over data parsing and dump unnecessary data
Signed-off-by: Patrik Leifert <patrikleifert@hotmail.com>
Removes all tests, except for TestGetRootUIDGID and TestToContainer, which
are the only once that have a local implementation that's not covered.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Update idtools to use Mkdir funcs from moby sys/user package
Add deprecation exception to golanci until move off idtools is complete
Signed-off-by: Derek McGowan <derek@mcg.dev>
- Split the implementation from the exported function (exported
function is still used by the CLI for Docker Content Trust).
- Pass through context to allow handling context-cancellation
once wired up in callers.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
endpointCnt is a refcounter used to track how many endpoints use a
network, and how many networks references a config-only network. It's
stored separately from the network.
This is only used to determine if a network can be removed.
This commit removes the `endpointCnt` struct and all its references. The
refcounter is replaced by two lookups in the newly introduced `networks`
and `endpoints` caches added to the `Controller`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The `Controller`'s store is used by:
- `deleteFromStore`
- `getNetworks`
- `getNetworksFromStore`
- `updateToStore`
- … and other methods that can't store / delete / retrieve a Network
Calls to `updateToStore` and `deleteFromStore` have been replaced with
`upsertNetwork` and `deleteNetwork`.
Both `getNetworks` and `getNetworksFromStore` call `cacheNetwork` to
ensure networks loaded from the datastore are kept in-memory.
Finally, `sandboxRestore` was instantiating `Network` itself. These are
cached too.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The `Controller`'s store is used by:
- `deleteFromStore`
- `getEndpointFromStore`
- `getEndpointsFromStore`
- `updateToStore`
- … and other methods that can't store / delete / retrieve an Endpoint
Calls to `updateToStore` and `deleteFromStore` have been replaced with
`upsertEndpoint` and `deleteEndpoint`.
Both `getEndpointFromStore` and `getEndpointsFromStore` call
`cacheEndpoint` to ensure endpoints loaded from the datastore are kept
in-memory.
Finally, `sandboxRestore` was instantiating `Endpoint` itself. These are
cached too.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Rewrite `validateDestination` to first check if the destination path
exists. This slightly simplifies the logic (allowing returning
early in each step of the validation) and slightly improves the
error produced.
Before this, the error confusingly would mention the full path
not being a directory. While this _does_ match what `os.Writefile`
would return, it's .. confusing:
failed to stat output path: lstat ./not-a-dir/new-file.txt: not a directory
After this, the error would mention the directory that doesn't exist:
invalid output path: stat ./not-a-dir: not a directory
A slight optimization is made as well, now checking for _both_ "."
and ".." as special case, as either path should exist given any current
working directory (unless the working directory has been deleted, but we'd
fail further down the line).
With this change in order, we can also merge `validateFileMode` into
`validateDestination`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While the target-file does not have to exist, its parent must, and must
be a directory. This adds a test-case to verify the behavior if the
parent is not a directory.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, we were silently discarding this situation and hoping that
it would work; let's produce an error instead (we can add additional
filemodes when they arrive and if we need them)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The implementation uses "os.Rename" to move the temporary file to
the destination, which does not follow symlinks, and because of this
would replace a symlink with a file.
We can consider adding support for symlinked files in future, so that
WriteFile can be used as a drop-in replacement for `os.WriteFile()`
but in the meantime, let's produce an error so that nobody can depend
on this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
CentOS 10 uses iptables-nft by default, and doesn't have
kernel module ip_tables - so dockerd-rootless-setuptool.sh
reports that the module is missing. It suggests installing
it (which isn't needed), or using --skip-iptables (which
disables iptables in daemon config).
So, unless "iptables --version" command reports "legacy",
check for kernel module "nf_tables" instead of "ip_tables".
Signed-off-by: Rob Murray <rob.murray@docker.com>
`GET /image/{name}/json` now supports `platform` parameter allowing to
specify which platform variant of a multi-platform image to inspect.
For servers that do not use containerd image store integration, this
option will cause an error if the requested platform doesn't match the
image's actual platform
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
full diff: https://github.com/golang/go/compare/go1.23.7...go1.23.8
release notes: https://go.dev/doc/devel/release#go1.24.2
go1.23.8 (released 2025-04-01) includes security fixes to the net/http package,
as well as bug fixes to the runtime and the go command. See the Go 1.23.8
milestone on our issue tracker for details;
https://github.com/golang/go/issues?q=milestone%3AGo1.23.8+label%3ACherryPickApproved
From the mailing list:
Hello gophers,
We have just released Go versions 1.24.2 and 1.23.8, minor point releases.
These minor releases include 1 security fixes following the security policy:
- net/http: request smuggling through invalid chunked data
The net/http package accepted data in the chunked transfer encoding
containing an invalid chunk-size line terminated by a bare LF.
When used in conjunction with a server or proxy which incorrectly
interprets a bare LF in a chunk extension as part of the extension,
this could permit request smuggling.
The net/http package now rejects chunk-size lines containing a bare LF.
Thanks to Jeppe Bonde Weikop for reporting this issue.
This is CVE-2025-22871 and Go issue https://go.dev/issue/71988.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since commit f2a183a99, `getEndpointPortMapInfo` is called for all the
endpoints of a container to get its complete list of port mappings. This
is required as multiple endpoints might publish different ports (e.g.
IPv4-only and IPv6-only endpoints mapping an IPv4 and an IPv6 port).
`getEndpointPortMapInfo` calls `(*Endpoint).DriverInfo()` which has a
dodgy behavior: if the endpoint is part of a sandbox that also has an
endpoint for the `docker_gwbridge` network, then `(*Endpoint).DriverInfo()`
returns the DriverInfo of that `docker_gwbridge` endpoint in place of
the current Endpoint's DriverInfo.
On overlay networks, host port-mappings are made through the
`docker_gwbridge` network (which is automatically attached to all Swarm
tasks). This results in duplicated port mappings reported for all Swarm
containers.
Since `getEndpointPortMapInfo` is the only place where
`(*Endpoint).DriverInfo()` is called, just remove that dodgy behavior.
`(*Endpoint).DriverInfo()` has an OS-specific implementation. Unlike the
Linux implementation, on Windows, `DriverInfo()` returns the DriverInfo
of the gateway endpoint _in addition_ to the current Endpoint's
DriverInfo. So it shouldn't be affected by this bug -- don't touch it.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Tests will run in the host's namspace not the rootlesskit
namespace. So, just duplicating the non-rootless unit tests.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Unlike the equivalent for pulling images, [Service.LookupPushEndpoints]
never returns mirror endpoints, as it calls [Service.lookupV2Endpoints]
with "includeMirrors=false", so we should not use this field, and
unconditionally handle errors without the additional fallbacks that
we consider for pulling images from a mirror.
[Service.LookupPushEndpoints]: cea56c1d9c/registry/service.go (L134-L139)
[Service.lookupV2Endpoints]: cea56c1d9c/registry/service_v2.go (L10-L40)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The github action running bake expected FIREWALLD to be set, but
DOCKER_FIREWALLD was set instead, so firewalld wasn't installed
in the dev image.
The dind-systemd script expected DOCKER_FIREWALLD to be set if it
needed to run firewalld, and it was. But it had no effect.
In CI, bake builds the image then make runs it - and the use the
same env. So, align on FIREWALLD (as it's not a docker feature).
Signed-off-by: Rob Murray <rob.murray@docker.com>
Create an iptablesNetwork containing all the info needed to
set up per-network iptables rules, give it methods to do
create the rules, and use it instead of per-rule-type calls
from driver.createNetwork().
Signed-off-by: Rob Murray <rob.murray@docker.com>
While it's generally better to define interfaces locally, this one
now duplicated distribution.RegistryResolver, and it's passed on
to other types which expect that interface.
Remove this (un-exported) interface to make it easier to discover
what's used where.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
inline a simplified version of "newIndexInfo" without handling of
insecure registries and mirrors, as we don't need that information
to resolve the auth-config.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Simplify how we lookup auth-config, as we don't need the
additional information provided by RepositoryInfo. There's
still more layers to peel off, which will be done in follow-ups.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.
[Service.ResolveRepository]: ecb03c4cda/registry/service.go (L106-L111)
[newRepositoryInfo]: ecb03c4cda/registry/config.go (L392-L408)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.
[Service.ResolveRepository]: ecb03c4cda/registry/service.go (L106-L111)
[newRepositoryInfo]: ecb03c4cda/registry/config.go (L392-L408)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.
[Service.ResolveRepository]: ecb03c4cda/registry/service.go (L106-L111)
[newRepositoryInfo]: ecb03c4cda/registry/config.go (L392-L408)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This type only uses the name / reference, and doesn't use any of the
other properties provided in RepositoryInfo.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was introduced in 19515a7ad8 when
the registry code was replaced for code vendored from docker/distribution.
It was used for v1 registries to update the "TrustStore" for signed manifests;
19515a7ad8/graph/pull.go (L89-L93)
Before that, it used the IndexInfo.Official field;
276c640be4/graph/pull.go (L94-L97)
And related to "v2" registries;
88fdcfef02
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Doesn't look like it would ever have worked, but:
- init the dbus connection to avoid a segv
- include the chain name when creating the rule
- remove the test rule if it's created
Signed-off-by: Rob Murray <rob.murray@docker.com>
Firewalld runs in the host netns. So, tests running in
their own netns can't check firewalld's iptables rules.
Signed-off-by: Rob Murray <rob.murray@docker.com>
With firewalld enabled in CI, TestAccessPublishedPortFromHost/userland-proxy=true/IPv6=true
consistently fails when trying to use a link-local address on
eth0 (it's ok for the ULL added by the test).
In a local moby dev container, it passes - although it sometimes
fails when making its request to the host's ::1.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The daemon runs in a separate netns, but when it wants to create
an iptables rule it sends a dbus message to firewalld - which is
running in the host's netns.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Since commit cfc049c the dev container has been using iptables-nft,
but the Dockerfile configured firewalld to use its iptables backend
(if firewalld is enabled, which it accidentally hasn't been in CI).
The integration tests run ok for a while, then some things start
failing with the less than obvious message:
Error: COMMAND_FAILED: UNKNOWN_ERROR: nonexistent or underflow of priority count
So, let firewalld use its default nftables backend.
Signed-off-by: Rob Murray <rob.murray@docker.com>
During a load, ensure any existing images are left as dangling images.
This affects the `docker load` command with the containerd snapshotter
backend.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The pkg/archive/archive_linux_test.go tests create a test archive
content in a temporary directory. This also includes device nodes (using
mknod).
Running these tests in Docker-in-Docker (dind) with systemd was failing
with "operation not permitted" because the rootfs is remounted with
`nodev`.
This change aligns `hack/dind-systemd` with `hack/dind` by
conditionally mounting a `tmpfs` on `/tmp` (with dev enabled),
to make the mknod work.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Commit 9a6e96f ("Before sending ARPs/NAs, check the bridge is ready")
introduced a check that, if a container's veth is connected to a bridge,
the port is forwarding before sending unsolicited ARP/NA messages to
advertise the container's MAC/IP addresses.
It waited for up-to 200ms, but it seems that's not always enough.
If the port isn't forwarding, the ARP/NA messages will be silently
dropped. But, if it comes up within two seconds, one or both of the
resends will succeed.
So - keep the wait for 200ms but, if anything goes wrong with checking
the bridge port's state, or it's not forwarding after that time, just
log an info/warning level message and continue.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This package imports all "importable" packages, i.e., packages that:
- are not applications ("main")
- are not internal
- and that have non-test go-files
We do this to verify that our code can be consumed as a dependency
in "module mode". When using a dependency that does not have a go.mod
(i.e.; is not a "module"), go implicitly generates a go.mod. Lacking
information from the dependency itself, it assumes "go1.16" language
(see [DefaultGoModVersion]). Starting with Go1.21, go downgrades the
language version used for such dependencies, which means that any
language feature used that is not supported by go1.16 results in a
compile error;
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
These errors do NOT occur when using GOPATH mode, nor do they occur
when using "pseudo module mode" (the "-mod=mod -modfile=vendor.mod"
approach used in this repository).
As a workaround for this situation, we must include "//go:build" comments
in any file that uses newer go-language features (such as the "any" type
or the "min()", "max()" builtins).
From the go toolchain docs (https://go.dev/doc/toolchain):
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a go.mod file with a go line such as go 1.21 or go 1.21.3.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding //go:build go1.22 to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This file is a generated module that imports all packages provided in
the repository, which replicates an external consumer using our code
as a dependency in go-module mode, and verifies all files in those
packages have the correct "//go:build <go language version>" set.
To test this package:
make -C ./internal/gocompat/
GO111MODULE=off go generate .
go mod tidy
go test -v
# github.com/docker/docker/libnetwork/options
../../libnetwork/options/options.go:45:25: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/libnetwork/internal/setmatrix
../../libnetwork/internal/setmatrix/setmatrix.go:13:16: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:13:18: predeclared comparable requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:14:20: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:20:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:31:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:43:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:59:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:80:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:93:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:104:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:104:10: too many errors
# github.com/docker/docker/libnetwork/config
../../libnetwork/config/config.go:35:47: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:47:41: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:63:55: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:95:63: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/testutil
../../testutil/helpers.go:80:9: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/builder/builder-next/adapters/containerimage
../../builder/builder-next/adapters/containerimage/pull.go:72:4: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../builder/builder-next/adapters/containerimage/pull.go:200:19: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
FAIL gocompat [build failed]
make: *** [Makefile:5: verify] Error 1
[DefaultGoModVersion]: 58c28ba286/src/cmd/go/internal/gover/version.go (L15-L24)
[2]: https://go.dev/doc/toolchain
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The builder `Named` callback was not called with the containerd image
store integration enabled and BuildKit due to wrong imageexporter output
key being used.
We have a test `TestBuildEmitsEvents` that should have detected the bug,
but it wasn't actually working because the CLI version used in the
`integration-cli` didn't support BuildKit yet.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When listing images, prioritize the tagged version. When computing the
unique images, the last returned image would be used as a base and repo
tags would be merged into it.
This makes it so `RepoDigests` has the correct value when there is both
a dangling and non-dangling version of the same image.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
JSON errors were introduced in API 1.24, and daemons running older versions of
the API would return errors as plain-text. However, such API versions would
also send the corresponding content-type header (text/plain), so we don't
really need to make the code version-dependent; there's already fallbacks
in place to handle JSON-responses that don't use the expected format, in
which case we produce a generic status-code error.
Before this patch, the client would print JSON-responses as-is when the
daemon returned an "API version too old" error;
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"}
With this patch, the client detects that the response is JSON, and prints
a friendlier error-message to help the user discover their client is too
old;
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
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Using "familiarname" (e.g. "ubuntu") should be mostly done for presenting
image refernces to the user, but internally, we should use the canonical
format where possible ("docker.io/library/ubuntu").
There's still many places where we use the familiar (short) form, but
let's start with not converting references in the client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-09 16:52:21 +01:00
1634 changed files with 68603 additions and 20572 deletions
- G204 # G204:Subprocess launched with variable; too many false positives.
- G301 # G301:Expect directory permissions to be 0750 or less (also EXC0009); too restrictive
- G302 # G302:Expect file permissions to be 0600 or less (also EXC0009); too restrictive
- G304 # G304:Potential file inclusion via variable.
- G306 # G306:Expect WriteFile permissions to be 0600 or less (too restrictive; also flags "0o644" permissions)
- G307 # G307:Deferring unsafe method "*os.File" on type "Close" (also EXC0008); (TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close")
- G504 # G504: Blocklisted import net/http/cgi: Go versions < 1.6.3 are vulnerable to Httpoxy attack:(CVE-2016-5386); (only affects go < 1.6.3)
govet:
enable-all:true
disable:
- fieldalignment # TODO:evaluate which ones should be updated.
importas:
# Do not allow unaliased imports of aliased packages.
no-unaliased:true
alias:
# Enforce alias to prevent it accidentally being used instead of our
- G204 # G204:Subprocess launched with variable; too many false positives.
- G301 # G301:Expect directory permissions to be 0750 or less (also EXC0009); too restrictive
- G302 # G302:Expect file permissions to be 0600 or less (also EXC0009); too restrictive
- G304 # G304:Potential file inclusion via variable.
- G306 # G306:Expect WriteFile permissions to be 0600 or less (too restrictive; also flags "0o644" permissions)
- G307 # G307:Deferring unsafe method "*os.File" on type "Close" (also EXC0008); (TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close")
- G504 # G504: Blocklisted import net/http/cgi: Go versions < 1.6.3 are vulnerable to Httpoxy attack:(CVE-2016-5386); (only affects go < 1.6.3)
govet:
enable-all:true
disable:
- fieldalignment # TODO:evaluate which ones should be updated.
importas:
# Do not allow unaliased imports of aliased packages.
no-unaliased:true
alias:
# Enforce alias to prevent it accidentally being used instead of our
# This allows the test suite to be able to run without worrying about the underlying fs used by the container running the daemon (e.g. aufs-on-aufs), so long as the host running the container is running a supported fs.
// TODO(thaJeztah): unify this with Server.makeHTTPHandler, which also logs internal server errors as error-log. See https://github.com/moby/moby/pull/48740#discussion_r1816675574
fields["error-response"]=retErr
fields["status"]=httpstatus.FromError(retErr)
fields["error-response"]=err
fields["status"]=httpstatus.FromError(err)
logger.WithFields(fields).Debugf("error response for %s request",r.Method)
}
}()
returnerr
}
ifr.Method!=http.MethodPost{
returnhandler(ctx,w,r,vars)
returnhandleWithLogs(ctx,w,r,vars)
}
iferr:=httputils.CheckForJSON(r);err!=nil{
returnhandler(ctx,w,r,vars)
returnhandleWithLogs(ctx,w,r,vars)
}
maxBodySize:=4096// 4KB
ifr.ContentLength>int64(maxBodySize){
returnhandler(ctx,w,r,vars)
returnhandleWithLogs(ctx,w,r,vars)
}
body:=r.Body
@@ -56,7 +58,7 @@ func DebugRequestMiddleware(handler func(ctx context.Context, w http.ResponseWri
b,err:=bufReader.Peek(maxBodySize)
iferr!=io.EOF{
// either there was an error reading, or the buffer is full (in which case the request is too large)
returnhandler(ctx,w,r,vars)
returnhandleWithLogs(ctx,w,r,vars)
}
varpostFormmap[string]interface{}
@@ -74,7 +76,7 @@ func DebugRequestMiddleware(handler func(ctx context.Context, w http.ResponseWri
// TODO(thaJeztah); we only use this to produce a nice error, but as a result, we can't remove libtrust as dependency - see if we can reduce the dependencies, but still able to detect it's a deprecated manifest
// FIXME(thaJeztah): this is a copy of dockerOCIImageConfigToContainerConfig in daemon/containerd: https://github.com/moby/moby/blob/6b617699c500522aa6526cfcae4558333911b11f/daemon/containerd/imagespec.go#L107-L128
description:"Do not delete untagged parent images"
type:"boolean"
default:false
- name:"platforms"
in:"query"
description:|
Select platform-specific content to delete.
Multiple values are accepted.
Each platform is a OCI platform encoded as a JSON string.
type:"array"
items:
# This should be OCIPlatform
# but $ref is not supported for array in query in Swagger 2.0
# $ref: "#/definitions/OCIPlatform"
type:"string"
tags:["Image"]
/images/search:
get:
@@ -10491,13 +10387,9 @@ paths:
### Image tarball format
An image tarball contains one directory per image layer (named using its long ID), each containing these files:
An image tarball contains [Content as defined in the OCI Image Layout Specification](https://github.com/opencontainers/image-spec/blob/v1.1.1/image-layout.md#content).
- `VERSION`: currently `1.0` - the file format version
- `json`: detailed layer information, similar to `docker inspect layer_id`
- `layer.tar`: A tarfile containing the filesystem changes in this layer
The `layer.tar` file contains `aufs` style `.wh..wh.aufs` files and directories for storing attribute changes and deletions.
Additionally, includes the manifest.json file associated with a backwards compatible docker save format.
If the tarball defines a repository, the tarball should also include a `repositories` file at the root that contains a list of repository and tag names mapped to layer IDs.
@@ -10537,6 +10429,7 @@ paths:
If not provided, the full multi-platform image will be saved.
// it's part of ContainerJSONBase and returned by "inspect" command
typeStatestruct{
Statusstring// String representation of the container state. Can be one of "created", "running", "paused", "restarting", "removing", "exited", or "dead"
StatusContainerState// String representation of the container state. Can be one of "created", "running", "paused", "restarting", "removing", "exited", or "dead"
// ContainerState is a string representation of the container's current state.
//
// It currently is an alias for string, but may become a distinct type in the future.
typeContainerState=string
const(
StateCreatedContainerState="created"// StateCreated indicates the container is created, but not (yet) started.
StateRunningContainerState="running"// StateRunning indicates that the container is running.
StatePausedContainerState="paused"// StatePaused indicates that the container's current state is paused.
StateRestartingContainerState="restarting"// StateRestarting indicates that the container is currently restarting.
StateRemovingContainerState="removing"// StateRemoving indicates that the container is being removed.
StateExitedContainerState="exited"// StateExited indicates that the container exited.
StateDeadContainerState="dead"// StateDead indicates that the container failed to be deleted. Containers in this state are attempted to be cleaned up when the daemon restarts.
{state:"invalid-state-string",expectedErr:`invalid value for state (invalid-state-string): must be one of created, running, paused, restarting, removing, exited, dead`},
BridgeNfIptablesbool`json:"BridgeNfIptables"`// Deprecated: netfilter module is now loaded on-demand and no longer during daemon startup, making this field obsolete. This field is always false and will be removed in the next release.
BridgeNfIP6tablesbool`json:"BridgeNfIp6tables"`// Deprecated: netfilter module is now loaded on-demand and no longer during daemon startup, making this field obsolete. This field is always false and will be removed in the next release.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
