go-winio now defines this function, so we can consume that.
Note that there's a difference between the old implementation and the original
one (added in 1cb9e9b44e). The old implementation
had special handling for win32 error codes, which was removed in the go-winio
implementation in 0966e1ad56
As `go-winio.GetFileSystemType()` calls `filepath.VolumeName(path)` internally,
this patch also removes the `string(home[0])`, which is redundant, and could
potentially panic if an empty string would be passed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 90431d1857)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 955c1f881a (Docker v17.12.0) replaced
detection of support for multiple lowerdirs (as required by overlay2) to not
depend on the kernel version. The `overlay2.override_kernel_check` was still
used to print a warning that older kernel versions may not have full support.
After this, commit e226aea280 (Docker v20.10.0,
backported to v19.03.7) removed uses of the `overlay2.override_kernel_check`
option altogether, but we were still parsing it.
This patch changes the `parseOptions()` function to not parse the option,
printing a deprecation warning instead. We should change this to be an error,
but the `overlay2.override_kernel_check` option was not deprecated in the
documentation, so keeping it around for one more release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e35700eb50)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The pkg/fsutils package was forked in containerd, and later moved to
containerd/continuity/fs. As we're moving more bits to containerd, let's also
use the same implementation to reduce code-duplication and to prevent them from
diverging.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5b6b42162b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
daemon/graphdriver/aufs/aufs.go:239:80: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/graphtest/graphbench_unix.go:249:27: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/graphtest/testutil.go:271:30: empty-lines: extra empty line at the end of a block (revive)
daemon/graphdriver/graphtest/graphbench_unix.go:179:32: empty-block: this block is empty, you can remove it (revive)
daemon/graphdriver/zfs/zfs.go:375:48: empty-lines: extra empty line at the end of a block (revive)
daemon/graphdriver/overlay/overlay.go:248:89: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/devmapper/deviceset.go:636:21: empty-lines: extra empty line at the end of a block (revive)
daemon/graphdriver/devmapper/deviceset.go:1150:70: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/devmapper/deviceset.go:1613:30: empty-lines: extra empty line at the end of a block (revive)
daemon/graphdriver/devmapper/deviceset.go:1645:65: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/btrfs/btrfs.go:53:101: empty-lines: extra empty line at the start of a block (revive)
daemon/graphdriver/devmapper/deviceset.go:1944:89: empty-lines: extra empty line at the start of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9d9cca49b4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove the "deadcode", "structcheck", and "varcheck" linters, as they are
deprecated:
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2f1c382a6d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
doCopyXattrs() never reached due to copyXattrs boolean being false, as
a result file capabilities not being copied.
moved copyXattr() out of doCopyXattrs()
Signed-off-by: Illo Abdulrahim <abdulrahim.illo@nokia.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 31f654a704)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 52c1a2fae8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These seemed to prevent cleaning up directories;
On arm64:
=== RUN TestSysctlOverride
testing.go:1090: TempDir RemoveAll cleanup: unlinkat /tmp/TestSysctlOverride2860094781/001/mounts/shm: device or resource busy
--- FAIL: TestSysctlOverride (0.00s)
On Windows:
=== Failed
=== FAIL: github.com/docker/docker/daemon TestLoadOrCreateTrustKeyInvalidKeyFile (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestLoadOrCreateTrustKeyInvalidKeyFile2014634395\001\keyfile4156691647: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/daemon/graphdriver TestIsEmptyDir (0.01s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestIsEmptyDir1962964337\001\dir-with-empty-file\file2523853824: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/pkg/directory TestSizeEmptyFile (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSizeEmptyFile1562416712\001\file16507846: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/pkg/directory TestSizeNonemptyFile (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSizeNonemptyFile1240832785\001\file3265662846: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/pkg/directory TestSizeFileAndNestedDirectoryEmpty (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSizeFileAndNestedDirectoryEmpty2163416550\001\file3715413181: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/pkg/directory TestSizeFileAndNestedDirectoryNonempty (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSizeFileAndNestedDirectoryNonempty878205470\001\file3280422273: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/volume/service TestSetGetMeta (0.01s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSetGetMeta3332268057\001\db: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/volume/service TestList (0.03s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestList2846947953\001\volumes\metadata.db: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/volume/service TestRestore (0.02s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestRestore3368254142\001\volumes\metadata.db: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/daemon/graphdriver TestIsEmptyDir (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestIsEmptyDir2823795693\001\dir-with-empty-file\file2625561089: The process cannot access the file because it is being used by another process.
=== FAIL: github.com/docker/docker/pkg/directory TestSizeFileAndNestedDirectoryNonempty (0.00s)
testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\CONTAI~1\AppData\Local\Temp\TestSizeFileAndNestedDirectoryNonempty4246252950\001\nested3442260313\file21164327: The process cannot access the file because it is being used by another process.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Support for overlay on a backing filesystem without d_type was deprecated in
0abb8dec3f (Docker 17.12), with an exception
for existing installations (0a4e793a3d).
That deprecation was nearly 5 years ago, and running without d_type is known to
cause serious issues (so users will likely already have run into other problems).
This patch removes support for running overlay and overlay2 on these filesystems,
returning the error instead of logging it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, we only printed a warning if a storage driver was deprecated. The
intent was to continue supporting these drivers, to allow users to migrate
to a different storage driver.
This patch changes the behavior; if the user has no storage driver specified
in the daemon configuration (so if we try to detect the previous storage
driver based on what's present in /var/lib/docker), we now produce an error,
informing the user that the storage driver is deprecated (and to be removed),
as well as instructing them to change the daemon configuration to explicitly
select the storage driver (to allow them to migrate).
This should make the deprecation more visible; this will be disruptive, but
it's better to have the failure happening *now* (while the drivers are still
there), than for users to discover the storage driver is no longer there
(which would require them to *downgrade* the daemon in order to migrate
to a different driver).
With this change, `docker info` includes a link in the warnings that:
/ # docker info
Client:
Context: default
Debug Mode: false
Server:
...
Live Restore Enabled: false
WARNING: The overlay storage-driver is deprecated, and will be removed in a future release.
Refer to the documentation for more information: https://docs.docker.com/go/storage-driver/
When starting the daemon without a storage driver configured explicitly, but
previous state was using a deprecated driver, the error is both logged and
printed:
...
ERRO[2022-03-25T14:14:06.032014013Z] [graphdriver] prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information
...
failed to start daemon: error initializing graphdriver: prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information
When starting the daemon and explicitly configuring it with a deprecated storage
driver:
WARN[2022-03-25T14:15:59.042335412Z] [graphdriver] WARNING: the overlay storage-driver is deprecated and will be removed in a future release; visit https://docs.docker.com/go/storage-driver/ for more information
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use pkg/errors for errors and fix error-capitalisation
- remove one redundant call to logDeprecatedWarning() (we're already skipping
deprecated drivers in that loop).
- rename `list` to `priorityList` for readability.
- remove redundant "skip" for the vfs storage driver, as it's already
excluded by `scanPriorDrivers()`
- change one debug log to an "info", so that the daemon logs contain the driver
that was configured, and include "multiple prior states found" error in the
daemon logs, to assist in debugging failed daemon starts.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Finish the refactor which was partially completed with commit
34536c498d, passing around IdentityMapping structs instead of pairs of
[]IDMap slices.
Existing code which uses []IDMap relies on zero-valued fields to be
valid, empty mappings. So in order to successfully finish the
refactoring without introducing bugs, their replacement therefore also
needs to have a useful zero value which represents an empty mapping.
Change IdentityMapping to be a pass-by-value type so that there are no
nil pointers to worry about.
The functionality provided by the deprecated NewIDMappingsFromMaps
function is required by unit tests to to construct arbitrary
IdentityMapping values. And the daemon will always need to access the
mappings to pass them to the Linux kernel. Accommodate these use cases
by exporting the struct fields instead. BuildKit currently depends on
the UIDs and GIDs methods so we cannot get rid of them yet.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Trying to build Docker images with buildkit using a ZFS-backed storage
was unreliable due to apparent race condition between adding and
removing layers to the storage (see: https://github.com/moby/buildkit/issues/1758).
The issue describes a similar problem with the BTRFS driver that was
resolved by adding additional locking based on the scheme used in the
OverlayFS driver. This commit replicates the scheme to the ZFS driver
which makes the problem as reported in the issue stop happening.
Signed-off-by: Tomasz Mańko <hi@jaen.me>
pkg/system historically has been a bit of a kitchen-sink of things that were
somewhat "system" related, but didn't have a good place for. EnsureRemoveAll()
is one of those utilities. EnsureRemoveAll() is used to both unmount and remove
a path, for which it depends on both github.com/moby/sys/mount, which in turn
depends on github.com/moby/sys/mountinfo.
pkg/system is imported in the CLI, but neither EnsureRemoveAll(), nor any of its
moby/sys dependencies are used on the client side, so let's move this function
somewhere else, to remove those dependencies from the CLI.
I looked for plausible locations that were related; it's used in:
- daemon
- daemon/graphdriver/XXX/
- plugin
I considered moving it into a (e.g.) "utils" package within graphdriver (but not
a huge fan of "utils" packages), and given that it felt (mostly) related to
cleaning up container filesystems, I decided to move it there.
Some things to follow-up on after this:
- Verify if this function is still needed (it feels a bit like a big hammer in
a "YOLO, let's try some things just in case it fails")
- Perhaps it should be integrated in `containerfs.Remove()` (so that it's used
automatically)
- Look if there's other implementations (and if they should be consolidated),
although (e.g.) the one in containerd is a copy of ours:
https://github.com/containerd/containerd/blob/v1.5.9/pkg/cri/server/helpers_linux.go#L200
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
daemon/graphdriver/fuse-overlayfs/fuseoverlayfs.go:101:63: SA9002: file mode '700' evaluates to 01274; did you mean '0700'? (staticcheck)
if err := idtools.MkdirAllAndChown(path.Join(home, linkDir), 700, currentID); err != nil {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use the syscall method instead of repeating the type conversions for
the syscall.Stat_t Atim/Mtim members. This also allows to drop the
//nolint: unconvert comments.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Do not use 0701 perms.
0701 dir perms allows anyone to traverse the docker dir.
It happens to allow any user to execute, as an example, suid binaries
from image rootfs dirs because it allows traversal AND critically
container users need to be able to do execute things.
0701 on lower directories also happens to allow any user to modify
things in, for instance, the overlay upper dir which neccessarily
has 0755 permissions.
This changes to use 0710 which allows users in the group to traverse.
In userns mode the UID owner is (real) root and the GID is the remapped
root's GID.
This prevents anyone but the remapped root to traverse our directories
(which is required for userns with runc).
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit ef7237442147441a7cadcda0600be1186d81ac73)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 93ac040bf0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Add hints for "Failed to destroy btrfs snapshot <DIR> for <ID>: operation not permitted" on rootless
Related to issue 41762
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
The daemon uses a priority list to automatically select the best-matching storage
driver for the backing filesystem that is used.
Historically, overlay2 was not supported on Btrfs and ZFS, and the daemon would
automatically pick the `btrfs` or `zfs` storage driver if that was the Backing
File System.
Commits 649e4c8889 and e226aea280
improved our detection to check if overlay2 was supported on the backing file-
system, allowing overlay2 to be used on top of Btrfs or ZFS, but did not change
the priority list.
While both Btrfs and ZFS have advantages for certain use-cases, and provide
advanced features that are not available to overlay2, they also are known
to require more "handholding", and are generally considered to be mostly
useful for "advanced" users.
This patch changes the storage-driver priority list, to prefer overlay2 (if
supported by the backing filesystem), and effectively makes btrfs and zfs
opt-in storage drivers.
This change does not affect existing installations; the daemon will detect
the storage driver that was previously in use (based on the presence of
storage directories in `/var/lib/docker`).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Probably needs a similar change as c208f03fbd,
but this code makes my head spin, so for now suppressing, and created a
tracking issue:
daemon/graphdriver/graphtest/graphtest_unix.go:305:12: unsafeptr: possible misuse of reflect.SliceHeader (govet)
header := *(*reflect.SliceHeader)(unsafe.Pointer(&buf))
^
daemon/graphdriver/graphtest/graphtest_unix.go:308:36: unsafeptr: possible misuse of reflect.SliceHeader (govet)
data := *(*[]byte)(unsafe.Pointer(&header))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The LCOW implementation in dockerd has been deprecated in favor of re-implementation
in containerd (in progress). Microsoft started removing the LCOW V1 code from the
build dependencies we use in Microsoft/opengcs (soon to be part of Microsoft/hcshhim),
which means that we need to start removing this code.
This first step removes the lcow graphdriver, the LCOW initialization code, and
some LCOW-related utilities.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A temporary directory was created but not removed at the end of the test.
The missing remove directory call is added now.
Signed-off-by: Muhammad Zohaib Aslam <zohaibse011@gmail.com>
Kernel 5.11 introduced support for rootless overlayfs, but incompatible with SELinux.
On the other hand, fuse-overlayfs is compatible.
Close issue 42333
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Before this change, cleanup of the btrfs driver (occuring on each daemon
shutdown) resulted in disabling quotas. It was done with an assumption
that quotas can be enabled or disabled on a subvolume level, which is
not true - enabling or disabling quota is always done on a filesystem
level.
That was leading to disabling quota on btrfs filesystems on each daemon
shutdown.
This change fixes that behavior and removes misleading `subvol` prefix
from functions and methods which set up quota (on a filesystem level).
Fixes: #34593
Fixes: 401c8d1767 ("Add disk quota support for btrfs")
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
overlay2 no longer sets `archive.OverlayWhiteoutFormat` when
running in UserNS, so we can remove the complicated logic in the
archive package.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
When running in userns, returns error (i.e. "use naive, not native")
immediately.
No substantial change to the logic.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Fix issue 41762
Cherry-pick "drivers: btrfs: Allow unprivileged user to delete subvolumes" from containers/storage
831e32b6bd
> In btrfs, subvolume can be deleted by IOC_SNAP_DESTROY ioctl but there
> is one catch: unprivileged IOC_SNAP_DESTROY call is restricted by default.
>
> This is because IOC_SNAP_DESTROY only performs permission checks on
> the top directory(subvolume) and unprivileged user might delete dirs/files
> which cannot be deleted otherwise. This restriction can be relaxed if
> user_subvol_rm_allowed mount option is used.
>
> Although the above ioctl had been the only way to delete a subvolume,
> btrfs now allows deletion of subvolume just like regular directory
> (i.e. rmdir sycall) since kernel 4.18.
>
> So if we fail to cleanup subvolume in subvolDelete(), just fallback to
> system.EnsureRmoveall() to try to cleanup subvolumes again.
> (Note: quota needs privilege, so if quota is enabled we do not fallback)
>
> This fix will allow non-privileged container works with btrfs backend.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
The "userxattr" option is needed for mounting overlayfs inside a user namespace with kernel >= 5.11.
The "userxattr" option is NOT needed for the initial user namespace (aka "the host").
Also, Ubuntu (since circa 2015) and Debian (since 10) with kernel < 5.11 can mount the overlayfs in a user namespace without the "userxattr" option.
The corresponding kernel commit: 2d2f2d7322ff43e0fe92bf8cccdc0b09449bf2e1
> **ovl: user xattr**
>
> Optionally allow using "user.overlay." namespace instead of "trusted.overlay."
> ...
> Disable redirect_dir and metacopy options, because these would allow privilege escalation through direct manipulation of the
> "user.overlay.redirect" or "user.overlay.metacopy" xattrs.
Fix issue 42055
Related to containerd/containerd PR 5076
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Various dirs in /var/lib/docker contain data that needs to be mounted
into a container. For this reason, these dirs are set to be owned by the
remapped root user, otherwise there can be permissions issues.
However, this uneccessarily exposes these dirs to an unprivileged user
on the host.
Instead, set the ownership of these dirs to the real root (or rather the
UID/GID of dockerd) with 0701 permissions, which allows the remapped
root to enter the directories but not read/write to them.
The remapped root needs to enter these dirs so the container's rootfs
can be configured... e.g. to mount /etc/resolve.conf.
This prevents an unprivileged user from having read/write access to
these dirs on the host.
The flip side of this is now any user can enter these directories.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit e908cc3901)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
