From 34b07d3e2dcc0ecf00efb4fe6cf741ab570fbf68 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Thu, 23 Jul 2020 22:13:08 +1000 Subject: [PATCH 01/30] Implement WCOW parentless active snapshots and view snapshots The WCOW layer support does not support creating sandboxes with no parent. Instead, parentless scratch layers must be layed out as a directory containing only a directory named 'Files', and all data stored inside 'Files'. At commit-time, this will be converted in-place into a read-only layer suitable for use as a parent layer. The WCOW layer support also does not deal with making read-only layers, i.e. layers that are prepared to be parent layers, visible in a read-only manner. A bind-mount or junction point cannot be made read-only, so a view must instead be a small sandbox layer that we can mount via WCOW, and discard later, to protect the layer against accidental or deliberate modification. Signed-off-by: Paul "TBBle" Hampson --- diff/windows/windows.go | 47 +++++++++++++++---- snapshots/windows/windows.go | 90 +++++++++++++++++++++++------------- 2 files changed, 94 insertions(+), 43 deletions(-) diff --git a/diff/windows/windows.go b/diff/windows/windows.go index d59ef1cff..b694b4d67 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -320,10 +320,13 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { return "", nil, fmt.Errorf("number of mounts should always be 1 for Windows layers: %w", errdefs.ErrInvalidArgument) } mnt := mounts[0] - if mnt.Type != "windows-layer" { + + if mnt.Type != "windows-layer" && mnt.Type != "bind" { // This is a special case error. When this is received the diff service // will attempt the next differ in the chain which for Windows is the // lcow differ that we want. + // TODO: Is there any situation where we actually wanted a "bind" mount to + // fall through to the lcow differ? return "", nil, fmt.Errorf("windowsDiff does not support layer type %s: %w", mnt.Type, errdefs.ErrNotImplemented) } @@ -332,6 +335,30 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { return "", nil, err } + isView := false + for _, o := range mnt.Options { + if o == "ro" { + isView = true + break + } + } + + if isView { + if mnt.Type == "bind" && len(parentLayerPaths) != 0 { + return "", nil, fmt.Errorf("unexpected bind-mount View with parents: %w", errdefs.ErrInvalidArgument) + } else if mnt.Type == "bind" { + // rootfs.CreateDiff creates a new, empty View to diff against, + // when diffing something with no parent. + // This makes perfect sense for a walking Diff, but for WCOW, + // we have to recognise this as "diff against nothing" + return "", nil, nil + } else if len(parentLayerPaths) == 0 { + return "", nil, fmt.Errorf("unexpected windows-layer View with no parent: %w", errdefs.ErrInvalidArgument) + } + // Ignore the dummy sandbox. + return parentLayerPaths[0], parentLayerPaths[1:], nil + } + return mnt.Source, parentLayerPaths, nil } @@ -346,8 +373,16 @@ func mountPairToLayerStack(lower, upper []mount.Mount) ([]string, error) { return nil, fmt.Errorf("Upper mount invalid: %w", err) } + lowerLayer, lowerParentLayerPaths, err := mountsToLayerAndParents(lower) + if errdefs.IsNotImplemented(err) { + // Upper was a windows-layer or bind, lower is not. We can't handle that. + return nil, fmt.Errorf("windowsDiff cannot diff a windows-layer against a non-windows-layer: %w", errdefs.ErrInvalidArgument) + } else if err != nil { + return nil, fmt.Errorf("Lower mount invalid: %w", err) + } + // Trivial case, diff-against-nothing - if len(lower) == 0 { + if lowerLayer == "" { if len(upperParentLayerPaths) != 0 { return nil, fmt.Errorf("windowsDiff cannot diff a layer with parents against a null layer: %w", errdefs.ErrInvalidArgument) } @@ -358,14 +393,6 @@ func mountPairToLayerStack(lower, upper []mount.Mount) ([]string, error) { return nil, fmt.Errorf("windowsDiff cannot diff a layer with no parents against another layer: %w", errdefs.ErrInvalidArgument) } - lowerLayer, lowerParentLayerPaths, err := mountsToLayerAndParents(lower) - if errdefs.IsNotImplemented(err) { - // Upper was a windows-layer, lower is not. We can't handle that. - return nil, fmt.Errorf("windowsDiff cannot diff a windows-layer against a non-windows-layer: %w", errdefs.ErrInvalidArgument) - } else if err != nil { - return nil, fmt.Errorf("Lower mount invalid: %w", err) - } - if upperParentLayerPaths[0] != lowerLayer { return nil, fmt.Errorf("windowsDiff cannot diff a layer against a layer other than its own parent: %w", errdefs.ErrInvalidArgument) } diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index 310e4380b..cec364070 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -187,7 +187,7 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (_ []mount.Mount, return nil, err } - return s.mounts(snapshot), nil + return s.mounts(snapshot, key), nil } func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) (retErr error) { @@ -208,7 +208,11 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap // If (windowsDiff).Apply was used to populate this layer, then it's already in the 'committed' state. // See createSnapshot below for more details if !strings.Contains(key, snapshots.UnpackKeyPrefix) { - if err := s.convertScratchToReadOnlyLayer(ctx, snapshot, path); err != nil { + if len(snapshot.ParentIDs) == 0 { + if err = hcsshim.ConvertToBaseLayer(path); err != nil { + return err + } + } else if err := s.convertScratchToReadOnlyLayer(ctx, snapshot, path); err != nil { return err } } @@ -299,11 +303,9 @@ func (s *snapshotter) Close() error { return s.ms.Close() } -func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { +func (s *snapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount { var ( - roFlag string - source string - parentLayerPaths []string + roFlag string ) if sn.Kind == snapshots.KindView { @@ -312,12 +314,19 @@ func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { roFlag = "rw" } - if len(sn.ParentIDs) == 0 || sn.Kind == snapshots.KindActive { - source = s.getSnapshotDir(sn.ID) - parentLayerPaths = s.parentIDsToParentPaths(sn.ParentIDs) - } else { - source = s.getSnapshotDir(sn.ParentIDs[0]) - parentLayerPaths = s.parentIDsToParentPaths(sn.ParentIDs[1:]) + source := s.getSnapshotDir(sn.ID) + parentLayerPaths := s.parentIDsToParentPaths(sn.ParentIDs) + + mountType := "windows-layer" + + if len(sn.ParentIDs) == 0 { + // A mount of a parentless snapshot is a bind-mount. + mountType = "bind" + // If not being extracted into, then the bind-target is the + // "Files" subdirectory. + if !strings.Contains(key, snapshots.UnpackKeyPrefix) { + source = filepath.Join(source, "Files") + } } // error is not checked here, as a string array will never fail to Marshal @@ -327,7 +336,7 @@ func (s *snapshotter) mounts(sn storage.Snapshot) []mount.Mount { var mounts []mount.Mount mounts = append(mounts, mount.Mount{ Source: source, - Type: "windows-layer", + Type: mountType, Options: []string{ roFlag, parentLayersOption, @@ -360,13 +369,22 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return err } - // IO/disk space optimization - // - // We only need one sandbox.vhdx for the container. Skip making one for this - // snapshot if this isn't the snapshot that just houses the final sandbox.vhd - // that will be mounted as the containers scratch. Currently the key for a snapshot - // where a layer will be extracted to will have the string `extract-` in it. - if !strings.Contains(key, snapshots.UnpackKeyPrefix) { + if strings.Contains(key, snapshots.UnpackKeyPrefix) { + // IO/disk space optimization: Do nothing + // + // We only need one sandbox.vhdx for the container. Skip making one for this + // snapshot if this isn't the snapshot that just houses the final sandbox.vhd + // that will be mounted as the containers scratch. Currently the key for a snapshot + // where a layer will be extracted to will have the string `extract-` in it. + } else if len(newSnapshot.ParentIDs) == 0 { + // A parentless snapshot is just a bind-mount to a directory named + // "Files". When committed, there'll be some post-processing to fill in the rest + // of the metadata. + filesDir := filepath.Join(snDir, "Files") + if err := os.MkdirAll(filesDir, 0700); err != nil { + return err + } + } else { parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs) var snapshotInfo snapshots.Info @@ -375,22 +393,28 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k } var sizeInBytes uint64 - if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { - log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) + if kind == snapshots.KindActive { + if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { + log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) - sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) + sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) + } + sizeInBytes = sizeInGB * 1024 * 1024 * 1024 } - sizeInBytes = sizeInGB * 1024 * 1024 * 1024 - } - // Prefer the newer label in bytes over the deprecated Windows specific GB variant. - if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { - sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) + // Prefer the newer label in bytes over the deprecated Windows specific GB variant. + if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { + sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) + } } + } else { + // A view is just a read-write snapshot with a _really_ small sandbox, since we cannot actually + // make a read-only mount or junction point. https://superuser.com/q/881544/112473 + sizeInBytes = 1024 * 1024 * 1024 } var makeUVMScratch bool @@ -415,7 +439,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return nil, err } - return s.mounts(newSnapshot), nil + return s.mounts(newSnapshot, key), nil } func (s *snapshotter) parentIDsToParentPaths(parentIDs []string) []string { From 474a257b16f0aa79421038736126080696605e9d Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Thu, 23 Jul 2020 23:19:55 +1000 Subject: [PATCH 02/30] Implement Windows mounting for bind and windows-layer mounts Using symlinks for bind mounts means we are not protecting an RO-mounted layer against modification. Windows doesn't currently appear to offer a better approach though, as we cannot create arbitrary empty WCOW scratch layers at this time. For windows-layer mounts, Unmount does not have access to the mounts used to create it. So we store the relevant data in an Alternate Data Stream on the mountpoint in order to be able to Unmount later. Based on approach in https://github.com/containerd/containerd/pull/2366, with sign-offs recorded as 'Based-on-work-by' trailers below. This also partially-reverts some changes made in #6034 as they are not needed with this mounting implmentation, which no longer needs to be handled specially by the caller compared to non-Windows mounts. Signed-off-by: Paul "TBBle" Hampson Based-on-work-by: Michael Crosby Based-on-work-by: Darren Stahl --- mount/mount_windows.go | 106 +++++++++++++++++++++++++++--- mount/volumemountutils_windows.go | 75 +++++++++++++++++++++ pkg/cri/opts/container.go | 57 +++++----------- 3 files changed, 191 insertions(+), 47 deletions(-) create mode 100644 mount/volumemountutils_windows.go diff --git a/mount/mount_windows.go b/mount/mount_windows.go index b73fe3646..b3f765129 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -18,6 +18,7 @@ package mount import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -26,8 +27,22 @@ import ( "github.com/Microsoft/hcsshim" ) +const sourceStreamName = "containerd.io-source" + +var ( + // ErrNotImplementOnWindows is returned when an action is not implemented for windows + ErrNotImplementOnWindows = errors.New("not implemented under windows") +) + // Mount to the provided target. func (m *Mount) mount(target string) error { + if m.Type == "bind" { + if err := m.bindMount(target); err != nil { + return fmt.Errorf("failed to bind-mount to %s: %w", target, err) + } + return nil + } + if m.Type != "windows-layer" { return fmt.Errorf("invalid windows mount type: '%s'", m.Type) } @@ -46,22 +61,42 @@ func (m *Mount) mount(target string) error { if err = hcsshim.ActivateLayer(di, layerID); err != nil { return fmt.Errorf("failed to activate layer %s: %w", m.Source, err) } + defer func() { + if err != nil { + hcsshim.DeactivateLayer(di, layerID) + } + }() if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil { return fmt.Errorf("failed to prepare layer %s: %w", m.Source, err) } + defer func() { + if err != nil { + hcsshim.UnprepareLayer(di, layerID) + } + }() - // We can link the layer mount path to the given target. It is an UNC path, and it needs - // a trailing backslash. - mountPath, err := hcsshim.GetLayerMountPath(di, layerID) + volume, err := hcsshim.GetLayerMountPath(di, layerID) if err != nil { - return fmt.Errorf("failed to get layer mount path for %s: %w", m.Source, err) + return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } - mountPath = mountPath + `\` - if err = os.Symlink(mountPath, target); err != nil { - return fmt.Errorf("failed to link mount to target %s: %w", target, err) + if err = setVolumeMountPoint(target, volume); err != nil { + return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } + defer func() { + if err != nil { + deleteVolumeMountPoint(target) + } + }() + + // Add an Alternate Data Stream to record the layer source. + // See https://docs.microsoft.com/en-au/archive/blogs/askcore/alternate-data-streams-in-ntfs + // for details on Alternate Data Streams. + if err = os.WriteFile(filepath.Clean(target)+":"+sourceStreamName, []byte(m.Source), 0666); err != nil { + return fmt.Errorf("failed to record source for layer %s: %w", m.Source, err) + } + return nil } @@ -85,8 +120,37 @@ func (m *Mount) GetParentPaths() ([]string, error) { // Unmount the mount at the provided path func Unmount(mount string, flags int) error { + mount = filepath.Clean(mount) + + // Helpfully, both reparse points and symlinks look like links to Go + // Less-helpfully, ReadLink cannot return \\?\Volume{GUID} for a volume mount, + // and ends up returning the directory we gave it for some reason. + if mountTarget, err := os.Readlink(mount); err != nil { + // Not a mount point. + // This isn't an error, per the EINVAL handling in the Linux version + return nil + } else if mount != filepath.Clean(mountTarget) { + // Directory symlink + if err := bindUnmount(mount); err != nil { + return fmt.Errorf("failed to bind-unmount from %s: %w", mount, err) + } + return nil + } + + layerPathb, err := os.ReadFile(mount + ":" + sourceStreamName) + + if err != nil { + return fmt.Errorf("failed to retrieve source for layer %s: %w", mount, err) + } + + layerPath := string(layerPathb) + + if err := deleteVolumeMountPoint(mount); err != nil { + return fmt.Errorf("failed failed to release volume mount path for layer %s: %w", mount, err) + } + var ( - home, layerID = filepath.Split(mount) + home, layerID = filepath.Split(layerPath) di = hcsshim.DriverInfo{ HomeDir: home, } @@ -95,6 +159,7 @@ func Unmount(mount string, flags int) error { if err := hcsshim.UnprepareLayer(di, layerID); err != nil { return fmt.Errorf("failed to unprepare layer %s: %w", mount, err) } + if err := hcsshim.DeactivateLayer(di, layerID); err != nil { return fmt.Errorf("failed to deactivate layer %s: %w", mount, err) } @@ -104,6 +169,11 @@ func Unmount(mount string, flags int) error { // UnmountAll unmounts from the provided path func UnmountAll(mount string, flags int) error { + if mount == "" { + // This isn't an error, per the EINVAL handling in the Linux version + return nil + } + return Unmount(mount, flags) } @@ -111,3 +181,23 @@ func UnmountAll(mount string, flags int) error { func UnmountRecursive(mount string, flags int) error { return UnmountAll(mount, flags) } + +func (m *Mount) bindMount(target string) error { + for _, option := range m.Options { + if option == "ro" { + return fmt.Errorf("read-only bind mount: %w", ErrNotImplementOnWindows) + } + } + + if err := os.Remove(target); err != nil { + return err + } + + // TODO: We don't honour the Read-Only flag. + // It's possible that Windows simply lacks this. + return os.Symlink(m.Source, target) +} + +func bindUnmount(target string) error { + return os.Remove(target) +} diff --git a/mount/volumemountutils_windows.go b/mount/volumemountutils_windows.go new file mode 100644 index 000000000..1216e6b2b --- /dev/null +++ b/mount/volumemountutils_windows.go @@ -0,0 +1,75 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package mount + +// Simple wrappers around SetVolumeMountPoint and DeleteVolumeMountPoint + +import ( + "fmt" + "path/filepath" + "strings" + "syscall" + + "github.com/containerd/containerd/errdefs" + "golang.org/x/sys/windows" +) + +// Mount volumePath (in format '\\?\Volume{GUID}' at targetPath. +// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setvolumemountpointw +func setVolumeMountPoint(targetPath string, volumePath string) error { + if !strings.HasPrefix(volumePath, "\\\\?\\Volume{") { + return fmt.Errorf("unable to mount non-volume path %s: %w", volumePath, errdefs.ErrInvalidArgument) + } + + // Both must end in a backslash + slashedTarget := filepath.Clean(targetPath) + string(filepath.Separator) + slashedVolume := volumePath + string(filepath.Separator) + + targetP, err := syscall.UTF16PtrFromString(slashedTarget) + if err != nil { + return fmt.Errorf("unable to utf16-ise %s: %w", slashedTarget, err) + } + + volumeP, err := syscall.UTF16PtrFromString(slashedVolume) + if err != nil { + return fmt.Errorf("unable to utf16-ise %s: %w", slashedVolume, err) + } + + if err := windows.SetVolumeMountPoint(targetP, volumeP); err != nil { + return fmt.Errorf("failed calling SetVolumeMount('%s', '%s'): %w", slashedTarget, slashedVolume, err) + } + + return nil +} + +// Remove the volume mount at targetPath +// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-deletevolumemountpointa +func deleteVolumeMountPoint(targetPath string) error { + // Must end in a backslash + slashedTarget := filepath.Clean(targetPath) + string(filepath.Separator) + + targetP, err := syscall.UTF16PtrFromString(slashedTarget) + if err != nil { + return fmt.Errorf("unable to utf16-ise %s: %w", slashedTarget, err) + } + + if err := windows.DeleteVolumeMountPoint(targetP); err != nil { + return fmt.Errorf("failed calling DeleteVolumeMountPoint('%s'): %w", slashedTarget, err) + } + + return nil +} diff --git a/pkg/cri/opts/container.go b/pkg/cri/opts/container.go index 04a3c1777..0f0c0d9b3 100644 --- a/pkg/cri/opts/container.go +++ b/pkg/cri/opts/container.go @@ -21,8 +21,6 @@ import ( "errors" "fmt" "os" - "path/filepath" - goruntime "runtime" "strings" "github.com/containerd/continuity/fs" @@ -86,53 +84,34 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { // https://github.com/containerd/containerd/pull/1785 defer os.Remove(root) - unmounter := func(mountPath string) { - if uerr := mount.Unmount(mountPath, 0); uerr != nil { + if err := mount.All(mounts, root); err != nil { + return fmt.Errorf("failed to mount: %w", err) + } + defer func() { + if uerr := mount.Unmount(root, 0); uerr != nil { log.G(ctx).WithError(uerr).Errorf("Failed to unmount snapshot %q", root) if err == nil { err = uerr } } - } - - var mountPaths []string - if goruntime.GOOS == "windows" { - for _, m := range mounts { - // appending the layerID to the root. - mountPath := filepath.Join(root, filepath.Base(m.Source)) - mountPaths = append(mountPaths, mountPath) - if err := m.Mount(mountPath); err != nil { - return err - } - - defer unmounter(m.Source) - } - } else { - mountPaths = append(mountPaths, root) - if err := mount.All(mounts, root); err != nil { - return fmt.Errorf("failed to mount: %w", err) - } - defer unmounter(root) - } + }() for host, volume := range volumeMounts { // The volume may have been defined with a C: prefix, which we can't use here. volume = strings.TrimPrefix(volume, "C:") - for _, mountPath := range mountPaths { - src, err := fs.RootPath(mountPath, volume) - if err != nil { - return fmt.Errorf("rootpath on mountPath %s, volume %s: %w", mountPath, volume, err) - } - if _, err := os.Stat(src); err != nil { - if os.IsNotExist(err) { - // Skip copying directory if it does not exist. - continue - } - return fmt.Errorf("stat volume in rootfs: %w", err) - } - if err := copyExistingContents(src, host); err != nil { - return fmt.Errorf("taking runtime copy of volume: %w", err) + src, err := fs.RootPath(root, volume) + if err != nil { + return fmt.Errorf("rootpath on mountPath %s, volume %s: %w", root, volume, err) + } + if _, err := os.Stat(src); err != nil { + if os.IsNotExist(err) { + // Skip copying directory if it does not exist. + continue } + return fmt.Errorf("stat volume in rootfs: %w", err) + } + if err := copyExistingContents(src, host); err != nil { + return fmt.Errorf("taking runtime copy of volume: %w", err) } } return nil From 84cc3e496b0d27847da5db1fe5b2d85e9604f865 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Sun, 13 Dec 2020 20:51:13 +1100 Subject: [PATCH 03/30] Unify testutil.Unmount on Windows and Unix Signed-off-by: Paul "TBBle" Hampson --- pkg/testutil/helpers.go | 10 ++++++++++ pkg/testutil/helpers_unix.go | 8 -------- pkg/testutil/helpers_windows.go | 5 ----- pkg/testutil/mount_other.go | 3 ++- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/testutil/helpers.go b/pkg/testutil/helpers.go index 9a9f857e2..850f51f83 100644 --- a/pkg/testutil/helpers.go +++ b/pkg/testutil/helpers.go @@ -23,6 +23,9 @@ import ( "path/filepath" "strconv" "testing" + + "github.com/containerd/containerd/mount" + "github.com/stretchr/testify/assert" ) var rootEnabled bool @@ -79,3 +82,10 @@ func DumpDirOnFailure(t *testing.T, root string) { DumpDir(t, root) } } + +// Unmount unmounts a given mountPoint and sets t.Error if it fails +func Unmount(t testing.TB, mountPoint string) { + t.Log("unmount", mountPoint) + err := mount.UnmountAll(mountPoint, umountflags) + assert.NoError(t, err) +} diff --git a/pkg/testutil/helpers_unix.go b/pkg/testutil/helpers_unix.go index 6e03a07f9..f402e44a3 100644 --- a/pkg/testutil/helpers_unix.go +++ b/pkg/testutil/helpers_unix.go @@ -23,17 +23,9 @@ import ( "os" "testing" - "github.com/containerd/containerd/mount" "github.com/stretchr/testify/assert" ) -// Unmount unmounts a given mountPoint and sets t.Error if it fails -func Unmount(t testing.TB, mountPoint string) { - t.Log("unmount", mountPoint) - err := mount.UnmountAll(mountPoint, umountflags) - assert.NoError(t, err) -} - // RequiresRoot skips tests that require root, unless the test.root flag has // been set func RequiresRoot(t testing.TB) { diff --git a/pkg/testutil/helpers_windows.go b/pkg/testutil/helpers_windows.go index 203d98771..501c3d803 100644 --- a/pkg/testutil/helpers_windows.go +++ b/pkg/testutil/helpers_windows.go @@ -25,8 +25,3 @@ func RequiresRoot(t testing.TB) { // RequiresRootM is similar to RequiresRoot but intended to be called from *testing.M. func RequiresRootM() { } - -// Unmount unmounts a given mountPoint and sets t.Error if it fails -// Does nothing on Windows -func Unmount(t *testing.T, mountPoint string) { -} diff --git a/pkg/testutil/mount_other.go b/pkg/testutil/mount_other.go index 163073bd0..b1aa7d460 100644 --- a/pkg/testutil/mount_other.go +++ b/pkg/testutil/mount_other.go @@ -1,4 +1,5 @@ -//go:build !linux && !windows +//go:build !linux +// +build !linux /* Copyright The containerd Authors. From 469c13997ae92de5504a1fb73bac1486b45fb309 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Sun, 13 Dec 2020 20:53:27 +1100 Subject: [PATCH 04/30] Ensure mounts are unmounted before leaving the test This is necessary on Windows, as it's not possible to delete a snapshot while it is still mounted, even if the mount-point has been deleted. Signed-off-by: Paul "TBBle" Hampson --- snapshots/testsuite/testsuite.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index cd2bfc913..eec4fa82f 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -162,6 +162,7 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh } if err := initialApplier.Apply(preparing); err != nil { + testutil.Unmount(t, preparing) t.Fatalf("failure reason: %+v", err) } // unmount before commit @@ -199,10 +200,12 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh } if err := fstest.CheckDirectoryEqualWithApplier(next, initialApplier); err != nil { + testutil.Unmount(t, next) t.Fatalf("failure reason: %+v", err) } if err := diffApplier.Apply(next); err != nil { + testutil.Unmount(t, next) t.Fatalf("failure reason: %+v", err) } // unmount before commit @@ -386,11 +389,12 @@ func checkSnapshotterTransitivity(ctx context.Context, t *testing.T, snapshotter if err != nil { t.Fatal(err) } - defer testutil.Unmount(t, preparing) if err = os.WriteFile(filepath.Join(preparing, "foo"), []byte("foo\n"), 0777); err != nil { + testutil.Unmount(t, preparing) t.Fatal(err) } + testutil.Unmount(t, preparing) snapA := filepath.Join(work, "snapA") if err = snapshotter.Commit(ctx, snapA, preparing, opt); err != nil { @@ -401,11 +405,12 @@ func checkSnapshotterTransitivity(ctx context.Context, t *testing.T, snapshotter if err != nil { t.Fatal(err) } - defer testutil.Unmount(t, next) if err = os.WriteFile(filepath.Join(next, "foo"), []byte("foo bar\n"), 0777); err != nil { + testutil.Unmount(t, next) t.Fatal(err) } + testutil.Unmount(t, next) snapB := filepath.Join(work, "snapB") if err = snapshotter.Commit(ctx, snapB, next, opt); err != nil { @@ -440,7 +445,7 @@ func checkSnapshotterPrepareView(ctx context.Context, t *testing.T, snapshotter if err != nil { t.Fatal(err) } - defer testutil.Unmount(t, preparing) + testutil.Unmount(t, preparing) snapA := filepath.Join(work, "snapA") if err = snapshotter.Commit(ctx, snapA, preparing, opt); err != nil { @@ -517,6 +522,7 @@ func checkRemoveIntermediateSnapshot(ctx context.Context, t *testing.T, snapshot if err != nil { t.Fatal(err) } + testutil.Unmount(t, base) committedBase := filepath.Join(work, "committed-base") if err = snapshotter.Commit(ctx, committedBase, base, opt); err != nil { @@ -555,7 +561,6 @@ func checkRemoveIntermediateSnapshot(ctx context.Context, t *testing.T, snapshot if err != nil { t.Fatal(err) } - testutil.Unmount(t, base) err = snapshotter.Remove(ctx, committedBase) if err != nil { t.Fatal(err) @@ -817,6 +822,7 @@ func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter if err := os.WriteFile(testfile, []byte("testcontent"), 0777); err != nil { t.Logf("write to %q failed with %v (EROFS is expected but can be other error code)", testfile, err) } else { + testutil.Unmount(t, viewMountPoint) t.Fatalf("write to %q should fail (EROFS) but did not fail", testfile) } testutil.Unmount(t, viewMountPoint) From 909730decb1fe906ec59825adb64f1be59b17fb5 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Sun, 13 Dec 2020 21:49:03 +1100 Subject: [PATCH 05/30] Skip tests that do not apply to WCOW on Windows Filesystem permissions and ownership are not modifiable via an image mount. Signed-off-by: Paul "TBBle" Hampson --- snapshots/testsuite/issues.go | 7 +++++++ snapshots/testsuite/testsuite.go | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/snapshots/testsuite/issues.go b/snapshots/testsuite/issues.go index eea87767e..92a04526e 100644 --- a/snapshots/testsuite/issues.go +++ b/snapshots/testsuite/issues.go @@ -19,6 +19,7 @@ package testsuite import ( "context" "fmt" + "runtime" "strings" "testing" "time" @@ -94,6 +95,9 @@ func checkRemoveDirectoryInLowerLayer(ctx context.Context, t *testing.T, sn snap // See https://github.com/docker/docker/issues/24913 overlay // see https://github.com/docker/docker/issues/28391 overlay2 func checkChown(ctx context.Context, t *testing.T, sn snapshots.Snapshotter, work string) { + if runtime.GOOS == "windows" { + t.Skip("Chown is not supported on WCOW") + } l1Init := fstest.Apply( fstest.CreateDir("/opt", 0700), fstest.CreateDir("/opt/a", 0700), @@ -147,6 +151,9 @@ func checkRename(ss string) func(ctx context.Context, t *testing.T, sn snapshots // checkDirectoryPermissionOnCommit // https://github.com/docker/docker/issues/27298 func checkDirectoryPermissionOnCommit(ctx context.Context, t *testing.T, sn snapshots.Snapshotter, work string) { + if runtime.GOOS == "windows" { + t.Skip("Chown is not supported on WCOW") + } l1Init := fstest.Apply( fstest.CreateDir("/dir1", 0700), fstest.CreateDir("/dir2", 0700), diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index eec4fa82f..ffdbae93e 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -23,6 +23,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "sort" "testing" "time" @@ -795,6 +796,10 @@ func checkRemove(ctx context.Context, t *testing.T, snapshotter snapshots.Snapsh // checkSnapshotterViewReadonly ensures a KindView snapshot to be mounted as a read-only filesystem. // This function is called only when WithTestViewReadonly is true. func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter snapshots.Snapshotter, work string) { + if runtime.GOOS == "windows" { + t.Skip("Read-only protection on views is not supported on WCOW") + } + preparing := filepath.Join(work, "preparing") if _, err := snapshotter.Prepare(ctx, preparing, "", opt); err != nil { t.Fatal(err) @@ -875,6 +880,10 @@ func closeTwice(ctx context.Context, t *testing.T, snapshotter snapshots.Snapsho } func checkRootPermission(ctx context.Context, t *testing.T, snapshotter snapshots.Snapshotter, work string) { + if runtime.GOOS == "windows" { + t.Skip("Filesystem permissions are not supported on WCOW") + } + preparing, err := snapshotterPrepareMount(ctx, snapshotter, "preparing", "", work) if err != nil { t.Fatal(err) From 8395e3a89aaeb3b039c2e71a576fdf50b6ef8146 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Sun, 20 Dec 2020 02:39:16 +1100 Subject: [PATCH 06/30] Don't use all-upper-case filenames in snapshot tests NTFS, when presented with an all-caps filename, assumes you are just being loud for no reason, and instead stores an all-lower-case filename. Signed-off-by: Paul "TBBle" Hampson --- snapshots/testsuite/testsuite.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index ffdbae93e..fc1b31f49 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -903,19 +903,19 @@ func check128LayersMount(name string) func(ctx context.Context, t *testing.T, sn lowestApply := fstest.Apply( fstest.CreateFile("/bottom", []byte("way at the bottom\n"), 0777), fstest.CreateFile("/overwriteme", []byte("FIRST!\n"), 0777), - fstest.CreateDir("/ADDHERE", 0755), - fstest.CreateDir("/ONLYME", 0755), - fstest.CreateFile("/ONLYME/bottom", []byte("bye!\n"), 0777), + fstest.CreateDir("/addhere", 0755), + fstest.CreateDir("/onlyme", 0755), + fstest.CreateFile("/onlyme/bottom", []byte("bye!\n"), 0777), ) appliers := []fstest.Applier{lowestApply} for i := 1; i <= 127; i++ { appliers = append(appliers, fstest.Apply( fstest.CreateFile("/overwriteme", []byte(fmt.Sprintf("%d WAS HERE!\n", i)), 0777), - fstest.CreateFile(fmt.Sprintf("/ADDHERE/file-%d", i), []byte("same\n"), 0755), - fstest.RemoveAll("/ONLYME"), - fstest.CreateDir("/ONLYME", 0755), - fstest.CreateFile(fmt.Sprintf("/ONLYME/file-%d", i), []byte("only me!\n"), 0777), + fstest.CreateFile(fmt.Sprintf("/addhere/file-%d", i), []byte("same\n"), 0755), + fstest.RemoveAll("/onlyme"), + fstest.CreateDir("/onlyme", 0755), + fstest.CreateFile(fmt.Sprintf("/onlyme/file-%d", i), []byte("only me!\n"), 0777), )) } From 639c5799a22376c04ec447822523ada07453cf62 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Wed, 13 Jan 2021 02:52:07 +1100 Subject: [PATCH 07/30] Add paired 'mount' log for 'unmount' Signed-off-by: Paul "TBBle" Hampson --- snapshots/testsuite/testsuite.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index fc1b31f49..cdfa0e7ae 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -941,6 +941,8 @@ func check128LayersMount(name string) func(ctx context.Context, t *testing.T, sn t.Fatalf("[layer %d] failed to mount on the target(%s): %+v", i, preparing, err) } + t.Log("mount", preparing) + if err := fstest.CheckDirectoryEqual(preparing, flat); err != nil { testutil.Unmount(t, preparing) t.Fatalf("[layer %d] preparing doesn't equal to flat before apply: %+v", i, err) From ac30eabbdcd3946155ab449d29e182c5a6290f1c Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Wed, 13 Jan 2021 04:13:45 +1100 Subject: [PATCH 08/30] Fix misspelling of 'Native' as 'Naive' Signed-off-by: Paul "TBBle" Hampson --- snapshots/native/native_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/snapshots/native/native_test.go b/snapshots/native/native_test.go index 79e02637d..4f48e6a76 100644 --- a/snapshots/native/native_test.go +++ b/snapshots/native/native_test.go @@ -35,10 +35,10 @@ func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, fu return snapshotter, func() error { return snapshotter.Close() }, nil } -func TestNaive(t *testing.T) { +func TestNative(t *testing.T) { if runtime.GOOS == "windows" { - t.Skip("snapshotter not implemented on windows") + t.Skip("Native snapshotter not implemented on windows") } testutil.RequiresRoot(t) - testsuite.SnapshotterSuite(t, "Naive", newSnapshotter) + testsuite.SnapshotterSuite(t, "Native", newSnapshotter) } From 7b36becd2dcd071863618dbc06cd815682a6ea2d Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Wed, 13 Jan 2021 04:12:07 +1100 Subject: [PATCH 09/30] Run Windows snapshotter through the test suite Signed-off-by: Paul "TBBle" Hampson --- snapshots/windows/windows_test.go | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 snapshots/windows/windows_test.go diff --git a/snapshots/windows/windows_test.go b/snapshots/windows/windows_test.go new file mode 100644 index 000000000..d8d9eca82 --- /dev/null +++ b/snapshots/windows/windows_test.go @@ -0,0 +1,43 @@ +//go:build windows +// +build windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package windows + +import ( + "context" + "testing" + + "github.com/containerd/containerd/pkg/testutil" + "github.com/containerd/containerd/snapshots" + "github.com/containerd/containerd/snapshots/testsuite" +) + +func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, func() error, error) { + snapshotter, err := NewSnapshotter(root) + if err != nil { + return nil, nil, err + } + + return snapshotter, func() error { return snapshotter.Close() }, nil +} + +func TestWindows(t *testing.T) { + testutil.RequiresRoot(t) + testsuite.SnapshotterSuite(t, "Windows", newSnapshotter) +} From d591bb0421213c1bc427559c64b9657091a7243c Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Sun, 13 Dec 2020 21:03:51 +1100 Subject: [PATCH 10/30] Enable TestSnapshotterClient on Windows Signed-off-by: Paul "TBBle" Hampson --- integration/client/snapshot_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/integration/client/snapshot_test.go b/integration/client/snapshot_test.go index 1aca97147..81505d722 100644 --- a/integration/client/snapshot_test.go +++ b/integration/client/snapshot_test.go @@ -18,7 +18,6 @@ package client import ( "context" - "runtime" "testing" . "github.com/containerd/containerd" @@ -44,8 +43,5 @@ func TestSnapshotterClient(t *testing.T) { if testing.Short() { t.Skip() } - if runtime.GOOS == "windows" { - t.Skip("snapshots not yet supported on Windows") - } - testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) + testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) } From 36dc2782c4b0912f5dfc29cbdd24467b04dc038e Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Wed, 12 Oct 2022 00:03:11 +0300 Subject: [PATCH 11/30] Use bind filer for mounts The bind filter supports bind-like mounts and volume mounts. It also allows us to have read-only mounts. Signed-off-by: Gabriel Adrian Samfira --- mount/bind_filter_windows.go | 327 +++++++++++++++++++++++++++++++++++ mount/mount_windows.go | 110 ++++++------ mount/zsyscall_windows.go | 93 ++++++++++ snapshots/windows/windows.go | 36 ++-- 4 files changed, 493 insertions(+), 73 deletions(-) create mode 100644 mount/bind_filter_windows.go create mode 100644 mount/zsyscall_windows.go diff --git a/mount/bind_filter_windows.go b/mount/bind_filter_windows.go new file mode 100644 index 000000000..b74d928d9 --- /dev/null +++ b/mount/bind_filter_windows.go @@ -0,0 +1,327 @@ +package mount + +import ( + "bytes" + "encoding/binary" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "syscall" + "unicode/utf16" + "unsafe" + + "golang.org/x/sys/windows" +) + +//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go ./*.go +//sys BfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) = bindfltapi.BfSetupFilter? +//sys BfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) = bindfltapi.BfRemoveMapping? +//sys BfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer uintptr) (hr error) = bindfltapi.BfGetMappings? + +// BfSetupFilter flags. See: +// https://github.com/microsoft/BuildXL/blob/a6dce509f0d4f774255e5fbfb75fa6d5290ed163/Public/Src/Utilities/Native/Processes/Windows/NativeContainerUtilities.cs#L193-L240 +const ( + BINDFLT_FLAG_READ_ONLY_MAPPING uint32 = 0x00000001 + // Generates a merged binding, mapping target entries to the virtualization root. + BINDFLT_FLAG_MERGED_BIND_MAPPING uint32 = 0x00000002 + // Use the binding mapping attached to the mapped-in job object (silo) instead of the default global mapping. + BINDFLT_FLAG_USE_CURRENT_SILO_MAPPING uint32 = 0x00000004 + BINDFLT_FLAG_REPARSE_ON_FILES uint32 = 0x00000008 + // Skips checks on file/dir creation inside a non-merged, read-only mapping. + // Only usable when READ_ONLY_MAPPING is set. + BINDFLT_FLAG_SKIP_SHARING_CHECK uint32 = 0x00000010 + BINDFLT_FLAG_CLOUD_FILES_ECPS uint32 = 0x00000020 + // Tells bindflt to fail mapping with STATUS_INVALID_PARAMETER if a mapping produces + // multiple targets. + BINDFLT_FLAG_NO_MULTIPLE_TARGETS uint32 = 0x00000040 + // Turns on caching by asserting that the backing store for name mappings is immutable. + BINDFLT_FLAG_IMMUTABLE_BACKING uint32 = 0x00000080 + BINDFLT_FLAG_PREVENT_CASE_SENSITIVE_BINDING uint32 = 0x00000100 + // Tells bindflt to fail with STATUS_OBJECT_PATH_NOT_FOUND when a mapping is being added + // but its parent paths (ancestors) have not already been added. + BINDFLT_FLAG_EMPTY_VIRT_ROOT uint32 = 0x00000200 + BINDFLT_FLAG_NO_REPARSE_ON_ROOT uint32 = 0x10000000 + BINDFLT_FLAG_BATCHED_REMOVE_MAPPINGS uint32 = 0x20000000 +) + +const ( + BINDFLT_GET_MAPPINGS_FLAG_VOLUME uint32 = 0x00000001 + BINDFLT_GET_MAPPINGS_FLAG_SILO uint32 = 0x00000002 + BINDFLT_GET_MAPPINGS_FLAG_USER uint32 = 0x00000004 +) + +// ApplyFileBinding creates a global mount of the source in root, with an optional +// read only flag. +// The bind filter allows us to create mounts of directories and volumes. By default it allows +// us to mount multiple sources inside a single root, acting as an overlay. Files from the +// second source will superscede the first source that was mounted. +// This function disables this behavior and sets the BINDFLT_FLAG_NO_MULTIPLE_TARGETS flag +// on the mount. +func ApplyFileBinding(root, source string, readOnly bool) error { + // The parent directory needs to exist for the bind to work. MkdirAll stats and + // returns nil if the directory exists internally so we should be fine to mkdirall + // every time. + if err := os.MkdirAll(filepath.Dir(root), 0); err != nil { + return err + } + + if strings.Contains(source, "Volume{") && !strings.HasSuffix(source, "\\") { + // Add trailing slash to volumes, otherwise we get an error when binding it to + // a folder. + source = source + "\\" + } + + rootPtr, err := windows.UTF16PtrFromString(root) + if err != nil { + return err + } + + targetPtr, err := windows.UTF16PtrFromString(source) + if err != nil { + return err + } + flags := BINDFLT_FLAG_NO_MULTIPLE_TARGETS + if readOnly { + flags |= BINDFLT_FLAG_READ_ONLY_MAPPING + } + + // Set the job handle to 0 to create a global mount. + if err := BfSetupFilter( + 0, + flags, + rootPtr, + targetPtr, + nil, + 0, + ); err != nil { + return fmt.Errorf("failed to bind target %q to root %q: %w", source, root, err) + } + return nil +} + +func RemoveFileBinding(root string) error { + rootPtr, err := windows.UTF16PtrFromString(root) + if err != nil { + return fmt.Errorf("converting path to utf-16: %w", err) + } + + if err := BfRemoveMapping(0, rootPtr); err != nil { + return fmt.Errorf("removing file binding: %w", err) + } + return nil +} + +// mappingEntry holds information about where in the response buffer we can +// find information about the virtual root (the mount point) and the targets (sources) +// that get mounted, as well as the flags used to bind the targets to the virtual root. +type mappingEntry struct { + VirtRootLength uint32 + VirtRootOffset uint32 + Flags uint32 + NumberOfTargets uint32 + TargetEntriesOffset uint32 +} + +type mappingTargetEntry struct { + TargetRootLength uint32 + TargetRootOffset uint32 +} + +// getMappingsResponseHeader represents the first 12 bytes of the BfGetMappings() response. +// It gives us the size of the buffer, the status of the call and the number of mappings. +// A response +type getMappingsResponseHeader struct { + Size uint32 + Status uint32 + MappingCount uint32 +} + +type BindMapping struct { + MountPoint string + Flags uint32 + Targets []string +} + +func decodeEntry(buffer []byte) (string, error) { + name := make([]uint16, len(buffer)/2) + err := binary.Read(bytes.NewReader(buffer), binary.LittleEndian, &name) + if err != nil { + return "", fmt.Errorf("decoding name: %w", err) + } + return string(utf16.Decode(name)), nil +} + +func getTargetsFromBuffer(buffer []byte, offset, count int) ([]string, error) { + if len(buffer) < offset+count*6 { + return nil, fmt.Errorf("invalid buffer") + } + + targets := make([]string, count) + for i := 0; i < count; i++ { + entryBuf := buffer[offset+i*8 : offset+i*8+8] + tgt := *(*mappingTargetEntry)(unsafe.Pointer(&entryBuf[0])) + if len(buffer) < int(tgt.TargetRootOffset)+int(tgt.TargetRootLength) { + return nil, fmt.Errorf("invalid buffer") + } + decoded, err := decodeEntry(buffer[tgt.TargetRootOffset : uint32(tgt.TargetRootOffset)+uint32(tgt.TargetRootLength)]) + if err != nil { + return nil, fmt.Errorf("decoding name: %w", err) + } + decoded, err = getFinalPath(decoded) + if err != nil { + return nil, fmt.Errorf("fetching final path: %w", err) + } + + targets[i] = decoded + } + return targets, nil +} + +func getFinalPath(pth string) (string, error) { + // BfGetMappings returns VOLUME_NAME_NT paths like \Device\HarddiskVolume2\ProgramData. + // These can be accessed by prepending \\.\GLOBALROOT to the path. We use this to get the + // DOS paths for these files. + if strings.HasPrefix(pth, `\Device`) { + pth = `\\.\GLOBALROOT` + pth + } + + han, err := getFileHandle(pth) + if err != nil { + return "", fmt.Errorf("fetching file handle: %w", err) + } + + buf := make([]uint16, 100) + var flags uint32 = 0x0 + for { + n, err := windows.GetFinalPathNameByHandle(windows.Handle(han), &buf[0], uint32(len(buf)), flags) + if err != nil { + // if we mounted a volume that does not also have a drive letter assigned, attempting to + // fetch the VOLUME_NAME_DOS will fail with os.ErrNotExist. Attempt to get the VOLUME_NAME_GUID. + if errors.Is(err, os.ErrNotExist) && flags != 0x1 { + flags = 0x1 + continue + } + return "", fmt.Errorf("getting final path name: %w", err) + } + if n < uint32(len(buf)) { + break + } + buf = make([]uint16, n) + } + finalPath := syscall.UTF16ToString(buf) + // We got VOLUME_NAME_DOS, we need to strip away some leading slashes. + // Leave unchanged if we ended up requesting VOLUME_NAME_GUID + if len(finalPath) > 4 && finalPath[:4] == `\\?\` && flags == 0x0 { + finalPath = finalPath[4:] + if len(finalPath) > 3 && finalPath[:3] == `UNC` { + // return path like \\server\share\... + finalPath = `\` + finalPath[3:] + } + } + + return finalPath, nil +} + +func getBindMappingFromBuffer(buffer []byte, entry mappingEntry) (BindMapping, error) { + if len(buffer) < int(entry.VirtRootOffset)+int(entry.VirtRootLength) { + return BindMapping{}, fmt.Errorf("invalid buffer") + } + + src, err := decodeEntry(buffer[entry.VirtRootOffset : entry.VirtRootOffset+uint32(entry.VirtRootLength)]) + if err != nil { + return BindMapping{}, fmt.Errorf("decoding entry: %w", err) + } + targets, err := getTargetsFromBuffer(buffer, int(entry.TargetEntriesOffset), int(entry.NumberOfTargets)) + if err != nil { + return BindMapping{}, fmt.Errorf("fetching targets: %w", err) + } + + src, err = getFinalPath(src) + if err != nil { + return BindMapping{}, fmt.Errorf("fetching final path: %w", err) + } + + return BindMapping{ + Flags: entry.Flags, + Targets: targets, + MountPoint: src, + }, nil +} + +func getFileHandle(pth string) (syscall.Handle, error) { + info, err := os.Lstat(pth) + if err != nil { + return 0, fmt.Errorf("accessing file: %w", err) + } + p, err := syscall.UTF16PtrFromString(pth) + if err != nil { + return 0, err + } + attrs := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS) + if info.Mode()&os.ModeSymlink != 0 { + attrs |= syscall.FILE_FLAG_OPEN_REPARSE_POINT + } + h, err := syscall.CreateFile(p, 0, 0, nil, syscall.OPEN_EXISTING, attrs, 0) + if err != nil { + return 0, err + } + return h, nil +} + +// GetBindMappings returns a list of bind mappings that have their root on a +// particular volume. The volumePath parameter can be any path that exists on +// a volume. For example, if a number of mappings are created in C:\ProgramData\test, +// to get a list of those mappings, the volumePath parameter would have to be set to +// C:\ or the VOLUME_NAME_GUID notation of C:\ (\\?\Volume{GUID}\), or any child +// path that exists. +func GetBindMappings(volumePath string) ([]BindMapping, error) { + rootPtr, err := windows.UTF16PtrFromString(volumePath) + if err != nil { + return nil, err + } + + var flags uint32 = BINDFLT_GET_MAPPINGS_FLAG_VOLUME + // allocate a large buffer for results + var outBuffSize uint32 = 256 * 1024 + buf := make([]byte, outBuffSize) + + if err := BfGetMappings(flags, 0, rootPtr, nil, &outBuffSize, uintptr(unsafe.Pointer(&buf[0]))); err != nil { + return nil, err + } + + if outBuffSize < 12 { + return nil, fmt.Errorf("invalid buffer returned") + } + + result := buf[:outBuffSize] + + // The first 12 bytes are the three uint32 fields in getMappingsResponseHeader{} + headerBuffer := result[:12] + // The alternative to using unsafe and casting it to the above defined structures, is to manually + // parse the fields. Not too terrible, but not sure it'd worth the trouble. + header := *(*getMappingsResponseHeader)(unsafe.Pointer(&headerBuffer[0])) + + if header.MappingCount == 0 { + // no mappings + return []BindMapping{}, nil + } + + mappingsBuffer := result[12 : int(unsafe.Sizeof(mappingEntry{}))*int(header.MappingCount)] + // Get a pointer to the first mapping in the slice + mappingsPointer := (*mappingEntry)(unsafe.Pointer(&mappingsBuffer[0])) + // Get slice of mappings + mappings := unsafe.Slice(mappingsPointer, header.MappingCount) + + mappingEntries := make([]BindMapping, header.MappingCount) + for i := 0; i < int(header.MappingCount); i++ { + bindMapping, err := getBindMappingFromBuffer(result, mappings[i]) + if err != nil { + return nil, fmt.Errorf("fetching bind mappings: %w", err) + } + mappingEntries[i] = bindMapping + } + + return mappingEntries, nil +} diff --git a/mount/mount_windows.go b/mount/mount_windows.go index b3f765129..a00fc2b54 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -36,8 +36,16 @@ var ( // Mount to the provided target. func (m *Mount) mount(target string) error { + readOnly := false + for _, option := range m.Options { + if option == "ro" { + readOnly = true + break + } + } + if m.Type == "bind" { - if err := m.bindMount(target); err != nil { + if err := ApplyFileBinding(target, m.Source, readOnly); err != nil { return fmt.Errorf("failed to bind-mount to %s: %w", target, err) } return nil @@ -81,12 +89,12 @@ func (m *Mount) mount(target string) error { return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } - if err = setVolumeMountPoint(target, volume); err != nil { + if err = ApplyFileBinding(target, volume, readOnly); err != nil { return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } defer func() { if err != nil { - deleteVolumeMountPoint(target) + RemoveFileBinding(target) } }() @@ -120,50 +128,60 @@ func (m *Mount) GetParentPaths() ([]string, error) { // Unmount the mount at the provided path func Unmount(mount string, flags int) error { + var err error mount = filepath.Clean(mount) - - // Helpfully, both reparse points and symlinks look like links to Go - // Less-helpfully, ReadLink cannot return \\?\Volume{GUID} for a volume mount, - // and ends up returning the directory we gave it for some reason. - if mountTarget, err := os.Readlink(mount); err != nil { - // Not a mount point. - // This isn't an error, per the EINVAL handling in the Linux version - return nil - } else if mount != filepath.Clean(mountTarget) { - // Directory symlink - if err := bindUnmount(mount); err != nil { - return fmt.Errorf("failed to bind-unmount from %s: %w", mount, err) - } - return nil - } - - layerPathb, err := os.ReadFile(mount + ":" + sourceStreamName) - + // This should expand paths like ADMINI~1 and PROGRA~1 to long names. + mount, err = getFinalPath(mount) if err != nil { - return fmt.Errorf("failed to retrieve source for layer %s: %w", mount, err) + return fmt.Errorf("fetching real path: %w", err) } - - layerPath := string(layerPathb) - - if err := deleteVolumeMountPoint(mount); err != nil { - return fmt.Errorf("failed failed to release volume mount path for layer %s: %w", mount, err) + mounts, err := GetBindMappings(mount) + if err != nil { + return fmt.Errorf("fetching bind mappings: %w", err) } - - var ( - home, layerID = filepath.Split(layerPath) - di = hcsshim.DriverInfo{ - HomeDir: home, + found := false + for _, mnt := range mounts { + if mnt.MountPoint == mount { + found = true + break } - ) - - if err := hcsshim.UnprepareLayer(di, layerID); err != nil { - return fmt.Errorf("failed to unprepare layer %s: %w", mount, err) + } + if !found { + // not a mount point + return nil } - if err := hcsshim.DeactivateLayer(di, layerID); err != nil { - return fmt.Errorf("failed to deactivate layer %s: %w", mount, err) + adsFile := mount + ":" + sourceStreamName + var layerPath string + + if _, err := os.Lstat(adsFile); err == nil { + layerPathb, err := os.ReadFile(mount + ":" + sourceStreamName) + if err != nil { + return fmt.Errorf("failed to retrieve source for layer %s: %w", mount, err) + } + layerPath = string(layerPathb) } + if err := RemoveFileBinding(mount); err != nil { + return fmt.Errorf("removing mount: %w", err) + } + + if layerPath != "" { + var ( + home, layerID = filepath.Split(layerPath) + di = hcsshim.DriverInfo{ + HomeDir: home, + } + ) + + if err := hcsshim.UnprepareLayer(di, layerID); err != nil { + return fmt.Errorf("failed to unprepare layer %s: %w", mount, err) + } + + if err := hcsshim.DeactivateLayer(di, layerID); err != nil { + return fmt.Errorf("failed to deactivate layer %s: %w", mount, err) + } + } return nil } @@ -183,21 +201,13 @@ func UnmountRecursive(mount string, flags int) error { } func (m *Mount) bindMount(target string) error { + readOnly := false for _, option := range m.Options { if option == "ro" { - return fmt.Errorf("read-only bind mount: %w", ErrNotImplementOnWindows) + readOnly = true + break } } - if err := os.Remove(target); err != nil { - return err - } - - // TODO: We don't honour the Read-Only flag. - // It's possible that Windows simply lacks this. - return os.Symlink(m.Source, target) -} - -func bindUnmount(target string) error { - return os.Remove(target) + return ApplyFileBinding(target, m.Source, readOnly) } diff --git a/mount/zsyscall_windows.go b/mount/zsyscall_windows.go new file mode 100644 index 000000000..4df57b356 --- /dev/null +++ b/mount/zsyscall_windows.go @@ -0,0 +1,93 @@ +//go:build windows + +// Code generated by 'go generate' using "github.com/Microsoft/go-winio/tools/mkwinsyscall"; DO NOT EDIT. + +package mount + +import ( + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +var _ unsafe.Pointer + +// Do the interface allocations only once for common +// Errno values. +const ( + errnoERROR_IO_PENDING = 997 +) + +var ( + errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING) + errERROR_EINVAL error = syscall.EINVAL +) + +// errnoErr returns common boxed Errno values, to prevent +// allocations at runtime. +func errnoErr(e syscall.Errno) error { + switch e { + case 0: + return errERROR_EINVAL + case errnoERROR_IO_PENDING: + return errERROR_IO_PENDING + } + // TODO: add more here, after collecting data on the common + // error values see on Windows. (perhaps when running + // all.bat?) + return e +} + +var ( + modbindfltapi = windows.NewLazySystemDLL("bindfltapi.dll") + + procBfGetMappings = modbindfltapi.NewProc("BfGetMappings") + procBfRemoveMapping = modbindfltapi.NewProc("BfRemoveMapping") + procBfSetupFilter = modbindfltapi.NewProc("BfSetupFilter") +) + +func BfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer uintptr) (hr error) { + hr = procBfGetMappings.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall6(procBfGetMappings.Addr(), 6, uintptr(flags), uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(sid)), uintptr(unsafe.Pointer(bufferSize)), uintptr(outBuffer)) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} + +func BfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) { + hr = procBfRemoveMapping.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall(procBfRemoveMapping.Addr(), 2, uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), 0) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} + +func BfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { + hr = procBfSetupFilter.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall6(procBfSetupFilter.Addr(), 6, uintptr(jobHandle), uintptr(flags), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(virtTargetPath)), uintptr(unsafe.Pointer(virtExceptions)), uintptr(virtExceptionPathCount)) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index cec364070..c20a766b4 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -358,11 +358,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return fmt.Errorf("failed to create snapshot: %w", err) } - if kind != snapshots.KindActive { - return nil - } - - log.G(ctx).Debug("createSnapshot active") + log.G(ctx).Debug("createSnapshot") // Create the new snapshot dir snDir := s.getSnapshotDir(newSnapshot.ID) if err = os.MkdirAll(snDir, 0700); err != nil { @@ -393,28 +389,22 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k } var sizeInBytes uint64 - if kind == snapshots.KindActive { - if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { - log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) + if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { + log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) - sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) - } - sizeInBytes = sizeInGB * 1024 * 1024 * 1024 + sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) } + sizeInBytes = sizeInGB * 1024 * 1024 * 1024 + } - // Prefer the newer label in bytes over the deprecated Windows specific GB variant. - if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { - sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) - } + // Prefer the newer label in bytes over the deprecated Windows specific GB variant. + if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { + sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) } - } else { - // A view is just a read-write snapshot with a _really_ small sandbox, since we cannot actually - // make a read-only mount or junction point. https://superuser.com/q/881544/112473 - sizeInBytes = 1024 * 1024 * 1024 } var makeUVMScratch bool From feb637f92dad76bd7d17c4727244ee67cc4a17f6 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 4 Feb 2023 02:39:30 -0800 Subject: [PATCH 12/30] Fix layer comparison and enable read-only checks fstest.CheckDirectoryEqual checks if any files in the diff matches a list of known metadataFiles. This only happens if we specify the initial layer as the first parameter and the mutated layer as the second. This also enables the read-only view checks, as the bind filter allows us to mount a layer as ro. Signed-off-by: Gabriel Adrian Samfira --- snapshots/testsuite/testsuite.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index cdfa0e7ae..9586ba500 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -796,10 +796,6 @@ func checkRemove(ctx context.Context, t *testing.T, snapshotter snapshots.Snapsh // checkSnapshotterViewReadonly ensures a KindView snapshot to be mounted as a read-only filesystem. // This function is called only when WithTestViewReadonly is true. func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter snapshots.Snapshotter, work string) { - if runtime.GOOS == "windows" { - t.Skip("Read-only protection on views is not supported on WCOW") - } - preparing := filepath.Join(work, "preparing") if _, err := snapshotter.Prepare(ctx, preparing, "", opt); err != nil { t.Fatal(err) @@ -943,7 +939,7 @@ func check128LayersMount(name string) func(ctx context.Context, t *testing.T, sn t.Log("mount", preparing) - if err := fstest.CheckDirectoryEqual(preparing, flat); err != nil { + if err := fstest.CheckDirectoryEqual(flat, preparing); err != nil { testutil.Unmount(t, preparing) t.Fatalf("[layer %d] preparing doesn't equal to flat before apply: %+v", i, err) } @@ -958,7 +954,7 @@ func check128LayersMount(name string) func(ctx context.Context, t *testing.T, sn t.Fatalf("[layer %d] failed to apply on preparing dir: %+v", i, err) } - if err := fstest.CheckDirectoryEqual(preparing, flat); err != nil { + if err := fstest.CheckDirectoryEqual(flat, preparing); err != nil { testutil.Unmount(t, preparing) t.Fatalf("[layer %d] preparing doesn't equal to flat after apply: %+v", i, err) } @@ -987,7 +983,7 @@ func check128LayersMount(name string) func(ctx context.Context, t *testing.T, sn } defer testutil.Unmount(t, view) - if err := fstest.CheckDirectoryEqual(view, flat); err != nil { + if err := fstest.CheckDirectoryEqual(flat, view); err != nil { t.Fatalf("fullview should equal to flat: %+v", err) } } From dc980b14a086600ec30272b79d7e85bd9e41ba1d Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 4 Feb 2023 02:42:45 -0800 Subject: [PATCH 13/30] Grant needed privileges for snapshotter tests Signed-off-by: Gabriel Adrian Samfira --- integration/client/snapshot_test.go | 6 +--- integration/client/snapshot_unix_test.go | 34 ++++++++++++++++++ integration/client/snapshot_windows_test.go | 39 +++++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 integration/client/snapshot_unix_test.go create mode 100644 integration/client/snapshot_windows_test.go diff --git a/integration/client/snapshot_test.go b/integration/client/snapshot_test.go index 81505d722..c30cc6380 100644 --- a/integration/client/snapshot_test.go +++ b/integration/client/snapshot_test.go @@ -22,7 +22,6 @@ import ( . "github.com/containerd/containerd" "github.com/containerd/containerd/snapshots" - "github.com/containerd/containerd/snapshots/testsuite" ) func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, func() error, error) { @@ -40,8 +39,5 @@ func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, fu } func TestSnapshotterClient(t *testing.T) { - if testing.Short() { - t.Skip() - } - testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) + runTestSnapshotterClient(t) } diff --git a/integration/client/snapshot_unix_test.go b/integration/client/snapshot_unix_test.go new file mode 100644 index 000000000..61495e4bc --- /dev/null +++ b/integration/client/snapshot_unix_test.go @@ -0,0 +1,34 @@ +//go:build !windows +// +build !windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package client + +import ( + "testing" + + "github.com/containerd/containerd/snapshots/testsuite" +) + +func runTestSnapshotterClient(t *testing.T) { + if testing.Short() { + t.Skip() + } + + testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) +} diff --git a/integration/client/snapshot_windows_test.go b/integration/client/snapshot_windows_test.go new file mode 100644 index 000000000..b8eaad7fd --- /dev/null +++ b/integration/client/snapshot_windows_test.go @@ -0,0 +1,39 @@ +//go:build windows +// +build windows + +/* +Copyright The containerd Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package client + +import ( + "testing" + + winio "github.com/Microsoft/go-winio" + "github.com/containerd/containerd/snapshots/testsuite" +) + +func runTestSnapshotterClient(t *testing.T) { + if testing.Short() { + t.Skip() + } + // The SeBackupPrivilege and SeRestorePrivilege gives us access to system files inside the container mount points + // (and everywhere on the system), without having to explicitly set DACLs on each location inside the mount point. + if err := winio.EnableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}); err != nil { + t.Error(err) + } + defer winio.DisableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}) + testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) +} From 00efd3e6d8c3befd183ea4545b136572282a8421 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 4 Feb 2023 07:47:48 -0800 Subject: [PATCH 14/30] Remove unused function Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index a00fc2b54..622d5f1ea 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -199,15 +199,3 @@ func UnmountAll(mount string, flags int) error { func UnmountRecursive(mount string, flags int) error { return UnmountAll(mount, flags) } - -func (m *Mount) bindMount(target string) error { - readOnly := false - for _, option := range m.Options { - if option == "ro" { - readOnly = true - break - } - } - - return ApplyFileBinding(target, m.Source, readOnly) -} From db32798592fd6b8378f6d157fac0223e16904c65 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 28 Feb 2023 10:58:50 -0800 Subject: [PATCH 15/30] Update continuity, go-winio and hcsshim Update dependencies and remove the local bindfilter files. Those have been moved to go-winio. Signed-off-by: Gabriel Adrian Samfira --- go.mod | 8 +- go.sum | 16 +- integration/client/go.mod | 7 +- integration/client/go.sum | 13 +- integration/client/snapshot_windows_test.go | 21 +- mount/mount_windows.go | 37 +--- mount/volumemountutils_windows.go | 75 ------- .../Microsoft/go-winio/.golangci.yml | 15 +- .../go-winio/pkg/bindfilter/bind_filter.go | 209 ++++++++---------- .../pkg/bindfilter}/zsyscall_windows.go | 33 ++- .../containerd/continuity/.golangci.yml | 2 - .../containerd/continuity/context.go | 5 +- .../containerd/continuity/driver/utils.go | 2 +- .../containerd/continuity/fs/copy.go | 34 +-- .../containerd/continuity/fs/copy_windows.go | 2 - .../containerd/continuity/fs/diff.go | 13 +- .../containerd/continuity/fs/dtype_linux.go | 3 +- .../containerd/continuity/fs/du_unix.go | 10 +- .../containerd/continuity/fs/du_windows.go | 8 +- .../continuity/fs/fstest/compare_windows.go | 14 +- .../continuity/fs/fstest/continuity_util.go | 1 - .../continuity/fs/fstest/file_windows.go | 16 +- .../continuity/fs/fstest/testsuite.go | 112 +++++----- .../containerd/continuity/fs/path.go | 4 +- .../containerd/continuity/hardlinks.go | 4 +- .../x/tools/go/packages/packages.go | 34 +-- .../x/tools/internal/gcimporter/iexport.go | 128 ++++++++++- .../x/tools/internal/gcimporter/iimport.go | 82 ++++++- .../x/tools/internal/pkgbits/decoder.go | 2 +- .../x/tools/internal/pkgbits/encoder.go | 2 +- .../internal/tokeninternal/tokeninternal.go | 59 +++++ vendor/modules.txt | 10 +- 32 files changed, 581 insertions(+), 400 deletions(-) delete mode 100644 mount/volumemountutils_windows.go rename mount/bind_filter_windows.go => vendor/github.com/Microsoft/go-winio/pkg/bindfilter/bind_filter.go (73%) rename {mount => vendor/github.com/Microsoft/go-winio/pkg/bindfilter}/zsyscall_windows.go (67%) create mode 100644 vendor/golang.org/x/tools/internal/tokeninternal/tokeninternal.go diff --git a/go.mod b/go.mod index bf116f5c5..075827d67 100644 --- a/go.mod +++ b/go.mod @@ -5,13 +5,13 @@ go 1.19 require ( github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1 github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 - github.com/Microsoft/go-winio v0.6.0 + github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 github.com/Microsoft/hcsshim v0.10.0-rc.7 github.com/container-orchestrated-devices/container-device-interface v0.5.4 github.com/containerd/btrfs/v2 v2.0.0 github.com/containerd/cgroups/v3 v3.0.1 github.com/containerd/console v1.0.3 - github.com/containerd/continuity v0.3.0 + github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 github.com/containerd/fifo v1.1.0 github.com/containerd/go-cni v1.1.9 github.com/containerd/go-runc v1.0.0 @@ -125,13 +125,13 @@ require ( go.opentelemetry.io/otel/metric v0.37.0 // indirect go.opentelemetry.io/proto/otlp v0.19.0 // indirect golang.org/x/crypto v0.1.0 // indirect - golang.org/x/mod v0.7.0 // indirect + golang.org/x/mod v0.8.0 // indirect golang.org/x/net v0.7.0 // indirect golang.org/x/oauth2 v0.4.0 // indirect golang.org/x/term v0.5.0 // indirect golang.org/x/text v0.7.0 // indirect golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect - golang.org/x/tools v0.5.0 // indirect + golang.org/x/tools v0.6.0 // indirect google.golang.org/appengine v1.6.7 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/square/go-jose.v2 v2.5.1 // indirect diff --git a/go.sum b/go.sum index 3b88e57b0..890df5279 100644 --- a/go.sum +++ b/go.sum @@ -75,8 +75,8 @@ github.com/Microsoft/go-winio v0.4.17-0.20210211115548-6eac466e5fa3/go.mod h1:JP github.com/Microsoft/go-winio v0.4.17-0.20210324224401-5516f17a5958/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.4.17/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.5.1/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= -github.com/Microsoft/go-winio v0.6.0 h1:slsWYD/zyx7lCXoZVlvQrj0hPTM1HI4+v1sIda2yDvg= -github.com/Microsoft/go-winio v0.6.0/go.mod h1:cTAf44im0RAYeL23bpB+fzCyDH2MJiz2BO69KH/soAE= +github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 h1:PNDnNt0QOfCBd3bmdl9bhAt4+/PRCZpthE3PL0CMMtI= +github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Microsoft/hcsshim v0.8.6/go.mod h1:Op3hHsoHPAvb6lceZHDtd9OkTew38wNoXnJs8iY7rUg= github.com/Microsoft/hcsshim v0.8.7-0.20190325164909-8abdbb8205e4/go.mod h1:Op3hHsoHPAvb6lceZHDtd9OkTew38wNoXnJs8iY7rUg= github.com/Microsoft/hcsshim v0.8.7/go.mod h1:OHd7sQqRFrYd3RmSgbgji+ctCwkbq2wbEYNSzOYtcBQ= @@ -232,8 +232,8 @@ github.com/containerd/continuity v0.0.0-20201208142359-180525291bb7/go.mod h1:kR github.com/containerd/continuity v0.0.0-20210208174643-50096c924a4e/go.mod h1:EXlVlkqNba9rJe3j7w3Xa924itAMLgZH4UD/Q4PExuQ= github.com/containerd/continuity v0.1.0/go.mod h1:ICJu0PwR54nI0yPEnJ6jcS+J7CZAUXrLh8lPo2knzsM= github.com/containerd/continuity v0.2.2/go.mod h1:pWygW9u7LtS1o4N/Tn0FoCFDIXZ7rxcMX7HX1Dmibvk= -github.com/containerd/continuity v0.3.0 h1:nisirsYROK15TAMVukJOUyGJjz4BNQJBVsNvAXZJ/eg= -github.com/containerd/continuity v0.3.0/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= +github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 h1:NKxa3JMWvOvsU7ZgHwd9CZupl/692lPy/MBcumAAHsI= +github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= github.com/containerd/fifo v0.0.0-20180307165137-3d5202aec260/go.mod h1:ODA38xgv3Kuk8dQz2ZQXpnv/UZZUHUCL7pnLehbXgQI= github.com/containerd/fifo v0.0.0-20190226154929-a9fb20d87448/go.mod h1:ODA38xgv3Kuk8dQz2ZQXpnv/UZZUHUCL7pnLehbXgQI= github.com/containerd/fifo v0.0.0-20200410184934-f15a3290365b/go.mod h1:jPQ2IAeZRCYxpS/Cm1495vGFww6ecHmMk1YJH2Q5ln0= @@ -1115,8 +1115,8 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= -golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1405,8 +1405,8 @@ golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.5.0 h1:+bSpV5HIeWkuvgaMfI3UmKRThoTA5ODJTUd8T17NO+4= -golang.org/x/tools v0.5.0/go.mod h1:N+Kgy78s5I24c24dU8OfWNEotWjutIs8SnJvn5IDq+k= +golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/integration/client/go.mod b/integration/client/go.mod index 7075cc60a..b68261e67 100644 --- a/integration/client/go.mod +++ b/integration/client/go.mod @@ -22,9 +22,10 @@ require ( golang.org/x/sys v0.6.0 ) +require github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 + require ( github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 // indirect - github.com/Microsoft/go-winio v0.6.0 // indirect github.com/cilium/ebpf v0.9.1 // indirect github.com/containerd/cgroups v1.1.0 // indirect github.com/containerd/console v1.0.3 // indirect @@ -55,11 +56,11 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/otel/trace v1.14.0 // indirect - golang.org/x/mod v0.7.0 // indirect + golang.org/x/mod v0.8.0 // indirect golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/text v0.7.0 // indirect - golang.org/x/tools v0.5.0 // indirect + golang.org/x/tools v0.6.0 // indirect google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4 // indirect google.golang.org/grpc v1.53.0 // indirect google.golang.org/protobuf v1.28.1 // indirect diff --git a/integration/client/go.sum b/integration/client/go.sum index caec2f416..833ed3466 100644 --- a/integration/client/go.sum +++ b/integration/client/go.sum @@ -538,8 +538,9 @@ github.com/Microsoft/go-winio v0.4.17-0.20210211115548-6eac466e5fa3/go.mod h1:JP github.com/Microsoft/go-winio v0.4.17/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.5.1/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY= -github.com/Microsoft/go-winio v0.6.0 h1:slsWYD/zyx7lCXoZVlvQrj0hPTM1HI4+v1sIda2yDvg= github.com/Microsoft/go-winio v0.6.0/go.mod h1:cTAf44im0RAYeL23bpB+fzCyDH2MJiz2BO69KH/soAE= +github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 h1:PNDnNt0QOfCBd3bmdl9bhAt4+/PRCZpthE3PL0CMMtI= +github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Microsoft/hcsshim v0.8.15/go.mod h1:x38A4YbHbdxJtc0sF6oIz+RG0npwSCAvn69iY6URG00= github.com/Microsoft/hcsshim v0.9.4/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= github.com/Microsoft/hcsshim v0.9.6/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= @@ -652,8 +653,9 @@ github.com/containerd/console v1.0.2/go.mod h1:ytZPjGgY2oeTkAONYafi2kSj0aYggsf8a github.com/containerd/console v1.0.3 h1:lIr7SlA5PxZyMV30bDW0MGbiOPXwc63yRuCP0ARubLw= github.com/containerd/console v1.0.3/go.mod h1:7LqA/THxQ86k76b8c/EMSiaJ3h1eZkMkXar0TQ1gf3U= github.com/containerd/continuity v0.0.0-20210208174643-50096c924a4e/go.mod h1:EXlVlkqNba9rJe3j7w3Xa924itAMLgZH4UD/Q4PExuQ= -github.com/containerd/continuity v0.3.0 h1:nisirsYROK15TAMVukJOUyGJjz4BNQJBVsNvAXZJ/eg= github.com/containerd/continuity v0.3.0/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= +github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 h1:NKxa3JMWvOvsU7ZgHwd9CZupl/692lPy/MBcumAAHsI= +github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= github.com/containerd/fifo v1.0.0/go.mod h1:ocF/ME1SX5b1AOlWi9r677YJmCPSwwWnQ9O123vzpE4= github.com/containerd/fifo v1.1.0 h1:4I2mbh5stb1u6ycIABlBw9zgtlK8viPI9QkQNRQEEmY= github.com/containerd/fifo v1.1.0/go.mod h1:bmC4NWMbXlt2EZ0Hc7Fx7QzTFxgPID13eH0Qu+MAb2o= @@ -1521,8 +1523,9 @@ golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.6.0/go.mod h1:4mET923SAdbXp2ki8ey+zGs1SLqsuM2Y0uvdZR/fUNI= -golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1878,8 +1881,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ= -golang.org/x/tools v0.5.0 h1:+bSpV5HIeWkuvgaMfI3UmKRThoTA5ODJTUd8T17NO+4= -golang.org/x/tools v0.5.0/go.mod h1:N+Kgy78s5I24c24dU8OfWNEotWjutIs8SnJvn5IDq+k= +golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/integration/client/snapshot_windows_test.go b/integration/client/snapshot_windows_test.go index b8eaad7fd..cc5c53fdf 100644 --- a/integration/client/snapshot_windows_test.go +++ b/integration/client/snapshot_windows_test.go @@ -2,20 +2,21 @@ // +build windows /* -Copyright The containerd Authors. + Copyright The containerd Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. */ + package client import ( diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 622d5f1ea..a0ee0543e 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -23,8 +23,11 @@ import ( "os" "path/filepath" "strings" + "syscall" + "github.com/Microsoft/go-winio/pkg/bindfilter" "github.com/Microsoft/hcsshim" + "golang.org/x/sys/windows" ) const sourceStreamName = "containerd.io-source" @@ -45,7 +48,7 @@ func (m *Mount) mount(target string) error { } if m.Type == "bind" { - if err := ApplyFileBinding(target, m.Source, readOnly); err != nil { + if err := bindfilter.ApplyFileBinding(target, m.Source, readOnly); err != nil { return fmt.Errorf("failed to bind-mount to %s: %w", target, err) } return nil @@ -89,12 +92,12 @@ func (m *Mount) mount(target string) error { return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } - if err = ApplyFileBinding(target, volume, readOnly); err != nil { + if err = bindfilter.ApplyFileBinding(target, volume, readOnly); err != nil { return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } defer func() { if err != nil { - RemoveFileBinding(target) + bindfilter.RemoveFileBinding(target) } }() @@ -128,29 +131,7 @@ func (m *Mount) GetParentPaths() ([]string, error) { // Unmount the mount at the provided path func Unmount(mount string, flags int) error { - var err error mount = filepath.Clean(mount) - // This should expand paths like ADMINI~1 and PROGRA~1 to long names. - mount, err = getFinalPath(mount) - if err != nil { - return fmt.Errorf("fetching real path: %w", err) - } - mounts, err := GetBindMappings(mount) - if err != nil { - return fmt.Errorf("fetching bind mappings: %w", err) - } - found := false - for _, mnt := range mounts { - if mnt.MountPoint == mount { - found = true - break - } - } - if !found { - // not a mount point - return nil - } - adsFile := mount + ":" + sourceStreamName var layerPath string @@ -162,7 +143,11 @@ func Unmount(mount string, flags int) error { layerPath = string(layerPathb) } - if err := RemoveFileBinding(mount); err != nil { + if err := bindfilter.RemoveFileBinding(mount); err != nil { + if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER { + // not a mount point + return nil + } return fmt.Errorf("removing mount: %w", err) } diff --git a/mount/volumemountutils_windows.go b/mount/volumemountutils_windows.go deleted file mode 100644 index 1216e6b2b..000000000 --- a/mount/volumemountutils_windows.go +++ /dev/null @@ -1,75 +0,0 @@ -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package mount - -// Simple wrappers around SetVolumeMountPoint and DeleteVolumeMountPoint - -import ( - "fmt" - "path/filepath" - "strings" - "syscall" - - "github.com/containerd/containerd/errdefs" - "golang.org/x/sys/windows" -) - -// Mount volumePath (in format '\\?\Volume{GUID}' at targetPath. -// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setvolumemountpointw -func setVolumeMountPoint(targetPath string, volumePath string) error { - if !strings.HasPrefix(volumePath, "\\\\?\\Volume{") { - return fmt.Errorf("unable to mount non-volume path %s: %w", volumePath, errdefs.ErrInvalidArgument) - } - - // Both must end in a backslash - slashedTarget := filepath.Clean(targetPath) + string(filepath.Separator) - slashedVolume := volumePath + string(filepath.Separator) - - targetP, err := syscall.UTF16PtrFromString(slashedTarget) - if err != nil { - return fmt.Errorf("unable to utf16-ise %s: %w", slashedTarget, err) - } - - volumeP, err := syscall.UTF16PtrFromString(slashedVolume) - if err != nil { - return fmt.Errorf("unable to utf16-ise %s: %w", slashedVolume, err) - } - - if err := windows.SetVolumeMountPoint(targetP, volumeP); err != nil { - return fmt.Errorf("failed calling SetVolumeMount('%s', '%s'): %w", slashedTarget, slashedVolume, err) - } - - return nil -} - -// Remove the volume mount at targetPath -// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-deletevolumemountpointa -func deleteVolumeMountPoint(targetPath string) error { - // Must end in a backslash - slashedTarget := filepath.Clean(targetPath) + string(filepath.Separator) - - targetP, err := syscall.UTF16PtrFromString(slashedTarget) - if err != nil { - return fmt.Errorf("unable to utf16-ise %s: %w", slashedTarget, err) - } - - if err := windows.DeleteVolumeMountPoint(targetP); err != nil { - return fmt.Errorf("failed calling DeleteVolumeMountPoint('%s'): %w", slashedTarget, err) - } - - return nil -} diff --git a/vendor/github.com/Microsoft/go-winio/.golangci.yml b/vendor/github.com/Microsoft/go-winio/.golangci.yml index af403bb13..2bf84e321 100644 --- a/vendor/github.com/Microsoft/go-winio/.golangci.yml +++ b/vendor/github.com/Microsoft/go-winio/.golangci.yml @@ -8,12 +8,8 @@ linters: - containedctx # struct contains a context - dupl # duplicate code - errname # erorrs are named correctly - - goconst # strings that should be constants - - godot # comments end in a period - - misspell - nolintlint # "//nolint" directives are properly explained - revive # golint replacement - - stylecheck # golint replacement, less configurable than revive - unconvert # unnecessary conversions - wastedassign @@ -23,10 +19,7 @@ linters: - exhaustive # check exhaustiveness of enum switch statements - gofmt # files are gofmt'ed - gosec # security - - nestif # deeply nested ifs - nilerr # returns nil even with non-nil error - - prealloc # slices that can be pre-allocated - - structcheck # unused struct fields - unparam # unused function params issues: @@ -56,6 +49,8 @@ issues: linters-settings: + exhaustive: + default-signifies-exhaustive: true govet: enable-all: true disable: @@ -98,6 +93,8 @@ linters-settings: disabled: true - name: flag-parameter # excessive, and a common idiom we use disabled: true + - name: unhandled-error # warns over common fmt.Print* and io.Close; rely on errcheck instead + disabled: true # general config - name: line-length-limit arguments: @@ -138,7 +135,3 @@ linters-settings: - VPCI - WCOW - WIM - stylecheck: - checks: - - "all" - - "-ST1003" # use revive's var naming diff --git a/mount/bind_filter_windows.go b/vendor/github.com/Microsoft/go-winio/pkg/bindfilter/bind_filter.go similarity index 73% rename from mount/bind_filter_windows.go rename to vendor/github.com/Microsoft/go-winio/pkg/bindfilter/bind_filter.go index b74d928d9..7ac377ae4 100644 --- a/mount/bind_filter_windows.go +++ b/vendor/github.com/Microsoft/go-winio/pkg/bindfilter/bind_filter.go @@ -1,4 +1,7 @@ -package mount +//go:build windows +// +build windows + +package bindfilter import ( "bytes" @@ -9,43 +12,28 @@ import ( "path/filepath" "strings" "syscall" - "unicode/utf16" "unsafe" "golang.org/x/sys/windows" ) -//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go ./*.go -//sys BfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) = bindfltapi.BfSetupFilter? -//sys BfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) = bindfltapi.BfRemoveMapping? -//sys BfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer uintptr) (hr error) = bindfltapi.BfGetMappings? +//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go ./bind_filter.go +//sys bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath string, virtTargetPath string, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) = bindfltapi.BfSetupFilter? +//sys bfRemoveMapping(jobHandle windows.Handle, virtRootPath string) (hr error) = bindfltapi.BfRemoveMapping? +//sys bfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer *byte) (hr error) = bindfltapi.BfGetMappings? // BfSetupFilter flags. See: // https://github.com/microsoft/BuildXL/blob/a6dce509f0d4f774255e5fbfb75fa6d5290ed163/Public/Src/Utilities/Native/Processes/Windows/NativeContainerUtilities.cs#L193-L240 +// +//nolint:revive // var-naming: ALL_CAPS const ( BINDFLT_FLAG_READ_ONLY_MAPPING uint32 = 0x00000001 - // Generates a merged binding, mapping target entries to the virtualization root. - BINDFLT_FLAG_MERGED_BIND_MAPPING uint32 = 0x00000002 - // Use the binding mapping attached to the mapped-in job object (silo) instead of the default global mapping. - BINDFLT_FLAG_USE_CURRENT_SILO_MAPPING uint32 = 0x00000004 - BINDFLT_FLAG_REPARSE_ON_FILES uint32 = 0x00000008 - // Skips checks on file/dir creation inside a non-merged, read-only mapping. - // Only usable when READ_ONLY_MAPPING is set. - BINDFLT_FLAG_SKIP_SHARING_CHECK uint32 = 0x00000010 - BINDFLT_FLAG_CLOUD_FILES_ECPS uint32 = 0x00000020 // Tells bindflt to fail mapping with STATUS_INVALID_PARAMETER if a mapping produces // multiple targets. BINDFLT_FLAG_NO_MULTIPLE_TARGETS uint32 = 0x00000040 - // Turns on caching by asserting that the backing store for name mappings is immutable. - BINDFLT_FLAG_IMMUTABLE_BACKING uint32 = 0x00000080 - BINDFLT_FLAG_PREVENT_CASE_SENSITIVE_BINDING uint32 = 0x00000100 - // Tells bindflt to fail with STATUS_OBJECT_PATH_NOT_FOUND when a mapping is being added - // but its parent paths (ancestors) have not already been added. - BINDFLT_FLAG_EMPTY_VIRT_ROOT uint32 = 0x00000200 - BINDFLT_FLAG_NO_REPARSE_ON_ROOT uint32 = 0x10000000 - BINDFLT_FLAG_BATCHED_REMOVE_MAPPINGS uint32 = 0x20000000 ) +//nolint:revive // var-naming: ALL_CAPS const ( BINDFLT_GET_MAPPINGS_FLAG_VOLUME uint32 = 0x00000001 BINDFLT_GET_MAPPINGS_FLAG_SILO uint32 = 0x00000002 @@ -73,26 +61,17 @@ func ApplyFileBinding(root, source string, readOnly bool) error { source = source + "\\" } - rootPtr, err := windows.UTF16PtrFromString(root) - if err != nil { - return err - } - - targetPtr, err := windows.UTF16PtrFromString(source) - if err != nil { - return err - } flags := BINDFLT_FLAG_NO_MULTIPLE_TARGETS if readOnly { flags |= BINDFLT_FLAG_READ_ONLY_MAPPING } // Set the job handle to 0 to create a global mount. - if err := BfSetupFilter( + if err := bfSetupFilter( 0, flags, - rootPtr, - targetPtr, + root, + source, nil, 0, ); err != nil { @@ -101,18 +80,70 @@ func ApplyFileBinding(root, source string, readOnly bool) error { return nil } +// RemoveFileBinding removes a mount from the root path. func RemoveFileBinding(root string) error { - rootPtr, err := windows.UTF16PtrFromString(root) - if err != nil { - return fmt.Errorf("converting path to utf-16: %w", err) - } - - if err := BfRemoveMapping(0, rootPtr); err != nil { + if err := bfRemoveMapping(0, root); err != nil { return fmt.Errorf("removing file binding: %w", err) } return nil } +// GetBindMappings returns a list of bind mappings that have their root on a +// particular volume. The volumePath parameter can be any path that exists on +// a volume. For example, if a number of mappings are created in C:\ProgramData\test, +// to get a list of those mappings, the volumePath parameter would have to be set to +// C:\ or the VOLUME_NAME_GUID notation of C:\ (\\?\Volume{GUID}\), or any child +// path that exists. +func GetBindMappings(volumePath string) ([]BindMapping, error) { + rootPtr, err := windows.UTF16PtrFromString(volumePath) + if err != nil { + return nil, err + } + + flags := BINDFLT_GET_MAPPINGS_FLAG_VOLUME + // allocate a large buffer for results + var outBuffSize uint32 = 256 * 1024 + buf := make([]byte, outBuffSize) + + if err := bfGetMappings(flags, 0, rootPtr, nil, &outBuffSize, &buf[0]); err != nil { + return nil, err + } + + if outBuffSize < 12 { + return nil, fmt.Errorf("invalid buffer returned") + } + + result := buf[:outBuffSize] + + // The first 12 bytes are the three uint32 fields in getMappingsResponseHeader{} + headerBuffer := result[:12] + // The alternative to using unsafe and casting it to the above defined structures, is to manually + // parse the fields. Not too terrible, but not sure it'd worth the trouble. + header := *(*getMappingsResponseHeader)(unsafe.Pointer(&headerBuffer[0])) + + if header.MappingCount == 0 { + // no mappings + return []BindMapping{}, nil + } + + mappingsBuffer := result[12 : int(unsafe.Sizeof(mappingEntry{}))*int(header.MappingCount)] + // Get a pointer to the first mapping in the slice + mappingsPointer := (*mappingEntry)(unsafe.Pointer(&mappingsBuffer[0])) + // Get slice of mappings + mappings := unsafe.Slice(mappingsPointer, header.MappingCount) + + mappingEntries := make([]BindMapping, header.MappingCount) + for i := 0; i < int(header.MappingCount); i++ { + bindMapping, err := getBindMappingFromBuffer(result, mappings[i]) + if err != nil { + return nil, fmt.Errorf("fetching bind mappings: %w", err) + } + mappingEntries[i] = bindMapping + } + + return mappingEntries, nil +} + // mappingEntry holds information about where in the response buffer we can // find information about the virtual root (the mount point) and the targets (sources) // that get mounted, as well as the flags used to bind the targets to the virtual root. @@ -150,7 +181,7 @@ func decodeEntry(buffer []byte) (string, error) { if err != nil { return "", fmt.Errorf("decoding name: %w", err) } - return string(utf16.Decode(name)), nil + return windows.UTF16ToString(name), nil } func getTargetsFromBuffer(buffer []byte, offset, count int) ([]string, error) { @@ -165,7 +196,7 @@ func getTargetsFromBuffer(buffer []byte, offset, count int) ([]string, error) { if len(buffer) < int(tgt.TargetRootOffset)+int(tgt.TargetRootLength) { return nil, fmt.Errorf("invalid buffer") } - decoded, err := decodeEntry(buffer[tgt.TargetRootOffset : uint32(tgt.TargetRootOffset)+uint32(tgt.TargetRootLength)]) + decoded, err := decodeEntry(buffer[tgt.TargetRootOffset : tgt.TargetRootOffset+tgt.TargetRootLength]) if err != nil { return nil, fmt.Errorf("decoding name: %w", err) } @@ -187,15 +218,18 @@ func getFinalPath(pth string) (string, error) { pth = `\\.\GLOBALROOT` + pth } - han, err := getFileHandle(pth) + han, err := openPath(pth) if err != nil { return "", fmt.Errorf("fetching file handle: %w", err) } + defer func() { + _ = windows.CloseHandle(han) + }() buf := make([]uint16, 100) var flags uint32 = 0x0 for { - n, err := windows.GetFinalPathNameByHandle(windows.Handle(han), &buf[0], uint32(len(buf)), flags) + n, err := windows.GetFinalPathNameByHandle(han, &buf[0], uint32(len(buf)), flags) if err != nil { // if we mounted a volume that does not also have a drive letter assigned, attempting to // fetch the VOLUME_NAME_DOS will fail with os.ErrNotExist. Attempt to get the VOLUME_NAME_GUID. @@ -229,7 +263,7 @@ func getBindMappingFromBuffer(buffer []byte, entry mappingEntry) (BindMapping, e return BindMapping{}, fmt.Errorf("invalid buffer") } - src, err := decodeEntry(buffer[entry.VirtRootOffset : entry.VirtRootOffset+uint32(entry.VirtRootLength)]) + src, err := decodeEntry(buffer[entry.VirtRootOffset : entry.VirtRootOffset+entry.VirtRootLength]) if err != nil { return BindMapping{}, fmt.Errorf("decoding entry: %w", err) } @@ -250,78 +284,25 @@ func getBindMappingFromBuffer(buffer []byte, entry mappingEntry) (BindMapping, e }, nil } -func getFileHandle(pth string) (syscall.Handle, error) { - info, err := os.Lstat(pth) - if err != nil { - return 0, fmt.Errorf("accessing file: %w", err) - } - p, err := syscall.UTF16PtrFromString(pth) +func openPath(path string) (windows.Handle, error) { + u16, err := windows.UTF16PtrFromString(path) if err != nil { return 0, err } - attrs := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS) - if info.Mode()&os.ModeSymlink != 0 { - attrs |= syscall.FILE_FLAG_OPEN_REPARSE_POINT - } - h, err := syscall.CreateFile(p, 0, 0, nil, syscall.OPEN_EXISTING, attrs, 0) + h, err := windows.CreateFile( + u16, + 0, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory handle. + 0) if err != nil { - return 0, err + return 0, &os.PathError{ + Op: "CreateFile", + Path: path, + Err: err, + } } return h, nil } - -// GetBindMappings returns a list of bind mappings that have their root on a -// particular volume. The volumePath parameter can be any path that exists on -// a volume. For example, if a number of mappings are created in C:\ProgramData\test, -// to get a list of those mappings, the volumePath parameter would have to be set to -// C:\ or the VOLUME_NAME_GUID notation of C:\ (\\?\Volume{GUID}\), or any child -// path that exists. -func GetBindMappings(volumePath string) ([]BindMapping, error) { - rootPtr, err := windows.UTF16PtrFromString(volumePath) - if err != nil { - return nil, err - } - - var flags uint32 = BINDFLT_GET_MAPPINGS_FLAG_VOLUME - // allocate a large buffer for results - var outBuffSize uint32 = 256 * 1024 - buf := make([]byte, outBuffSize) - - if err := BfGetMappings(flags, 0, rootPtr, nil, &outBuffSize, uintptr(unsafe.Pointer(&buf[0]))); err != nil { - return nil, err - } - - if outBuffSize < 12 { - return nil, fmt.Errorf("invalid buffer returned") - } - - result := buf[:outBuffSize] - - // The first 12 bytes are the three uint32 fields in getMappingsResponseHeader{} - headerBuffer := result[:12] - // The alternative to using unsafe and casting it to the above defined structures, is to manually - // parse the fields. Not too terrible, but not sure it'd worth the trouble. - header := *(*getMappingsResponseHeader)(unsafe.Pointer(&headerBuffer[0])) - - if header.MappingCount == 0 { - // no mappings - return []BindMapping{}, nil - } - - mappingsBuffer := result[12 : int(unsafe.Sizeof(mappingEntry{}))*int(header.MappingCount)] - // Get a pointer to the first mapping in the slice - mappingsPointer := (*mappingEntry)(unsafe.Pointer(&mappingsBuffer[0])) - // Get slice of mappings - mappings := unsafe.Slice(mappingsPointer, header.MappingCount) - - mappingEntries := make([]BindMapping, header.MappingCount) - for i := 0; i < int(header.MappingCount); i++ { - bindMapping, err := getBindMappingFromBuffer(result, mappings[i]) - if err != nil { - return nil, fmt.Errorf("fetching bind mappings: %w", err) - } - mappingEntries[i] = bindMapping - } - - return mappingEntries, nil -} diff --git a/mount/zsyscall_windows.go b/vendor/github.com/Microsoft/go-winio/pkg/bindfilter/zsyscall_windows.go similarity index 67% rename from mount/zsyscall_windows.go rename to vendor/github.com/Microsoft/go-winio/pkg/bindfilter/zsyscall_windows.go index 4df57b356..45c45c96e 100644 --- a/mount/zsyscall_windows.go +++ b/vendor/github.com/Microsoft/go-winio/pkg/bindfilter/zsyscall_windows.go @@ -2,7 +2,7 @@ // Code generated by 'go generate' using "github.com/Microsoft/go-winio/tools/mkwinsyscall"; DO NOT EDIT. -package mount +package bindfilter import ( "syscall" @@ -47,12 +47,12 @@ var ( procBfSetupFilter = modbindfltapi.NewProc("BfSetupFilter") ) -func BfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer uintptr) (hr error) { +func bfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer *byte) (hr error) { hr = procBfGetMappings.Find() if hr != nil { return } - r0, _, _ := syscall.Syscall6(procBfGetMappings.Addr(), 6, uintptr(flags), uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(sid)), uintptr(unsafe.Pointer(bufferSize)), uintptr(outBuffer)) + r0, _, _ := syscall.Syscall6(procBfGetMappings.Addr(), 6, uintptr(flags), uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(sid)), uintptr(unsafe.Pointer(bufferSize)), uintptr(unsafe.Pointer(outBuffer))) if int32(r0) < 0 { if r0&0x1fff0000 == 0x00070000 { r0 &= 0xffff @@ -62,7 +62,16 @@ func BfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, return } -func BfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) { +func bfRemoveMapping(jobHandle windows.Handle, virtRootPath string) (hr error) { + var _p0 *uint16 + _p0, hr = syscall.UTF16PtrFromString(virtRootPath) + if hr != nil { + return + } + return _bfRemoveMapping(jobHandle, _p0) +} + +func _bfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) { hr = procBfRemoveMapping.Find() if hr != nil { return @@ -77,7 +86,21 @@ func BfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) return } -func BfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { +func bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath string, virtTargetPath string, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { + var _p0 *uint16 + _p0, hr = syscall.UTF16PtrFromString(virtRootPath) + if hr != nil { + return + } + var _p1 *uint16 + _p1, hr = syscall.UTF16PtrFromString(virtTargetPath) + if hr != nil { + return + } + return _bfSetupFilter(jobHandle, flags, _p0, _p1, virtExceptions, virtExceptionPathCount) +} + +func _bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { hr = procBfSetupFilter.Find() if hr != nil { return diff --git a/vendor/github.com/containerd/continuity/.golangci.yml b/vendor/github.com/containerd/continuity/.golangci.yml index 2924bc4cf..afebdb4e6 100644 --- a/vendor/github.com/containerd/continuity/.golangci.yml +++ b/vendor/github.com/containerd/continuity/.golangci.yml @@ -1,7 +1,5 @@ linters: enable: - - structcheck - - varcheck - staticcheck - unconvert - gofmt diff --git a/vendor/github.com/containerd/continuity/context.go b/vendor/github.com/containerd/continuity/context.go index f92299c22..f22ea8678 100644 --- a/vendor/github.com/containerd/continuity/context.go +++ b/vendor/github.com/containerd/continuity/context.go @@ -18,6 +18,7 @@ package continuity import ( "bytes" + "errors" "fmt" "io" "os" @@ -151,7 +152,7 @@ func (c *context) Resource(p string, fi os.FileInfo) (Resource, error) { } base.xattrs, err = c.resolveXAttrs(fp, fi, base) - if err != nil && err != ErrNotSupported { + if err != nil && !errors.Is(err, ErrNotSupported) { return nil, err } @@ -410,7 +411,7 @@ func (c *context) Apply(resource Resource) error { return fmt.Errorf("resource %v escapes root", resource) } - var chmod = true + chmod := true fi, err := c.driver.Lstat(fp) if err != nil { if !os.IsNotExist(err) { diff --git a/vendor/github.com/containerd/continuity/driver/utils.go b/vendor/github.com/containerd/continuity/driver/utils.go index d122a3f73..90bfcc3f6 100644 --- a/vendor/github.com/containerd/continuity/driver/utils.go +++ b/vendor/github.com/containerd/continuity/driver/utils.go @@ -56,7 +56,7 @@ func WriteFile(r Driver, filename string, data []byte, perm os.FileMode) error { return nil } -// ReadDir works the same as ioutil.ReadDir with the Driver abstraction +// ReadDir works the same as os.ReadDir with the Driver abstraction func ReadDir(r Driver, dirname string) ([]os.FileInfo, error) { f, err := r.Open(dirname) if err != nil { diff --git a/vendor/github.com/containerd/continuity/fs/copy.go b/vendor/github.com/containerd/continuity/fs/copy.go index 6982a761b..a5c210810 100644 --- a/vendor/github.com/containerd/continuity/fs/copy.go +++ b/vendor/github.com/containerd/continuity/fs/copy.go @@ -18,7 +18,6 @@ package fs import ( "fmt" - "io/ioutil" "os" "path/filepath" "sync" @@ -111,7 +110,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er } } - fis, err := ioutil.ReadDir(src) + entries, err := os.ReadDir(src) if err != nil { return fmt.Errorf("failed to read %s: %w", src, err) } @@ -124,18 +123,23 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er return fmt.Errorf("failed to copy xattrs: %w", err) } - for _, fi := range fis { - source := filepath.Join(src, fi.Name()) - target := filepath.Join(dst, fi.Name()) + for _, entry := range entries { + source := filepath.Join(src, entry.Name()) + target := filepath.Join(dst, entry.Name()) + + fileInfo, err := entry.Info() + if err != nil { + return fmt.Errorf("failed to get file info for %s: %w", entry.Name(), err) + } switch { - case fi.IsDir(): + case entry.IsDir(): if err := copyDirectory(target, source, inodes, o); err != nil { return err } continue - case (fi.Mode() & os.ModeType) == 0: - link, err := getLinkSource(target, fi, inodes) + case (fileInfo.Mode() & os.ModeType) == 0: + link, err := getLinkSource(target, fileInfo, inodes) if err != nil { return fmt.Errorf("failed to get hardlink: %w", err) } @@ -146,7 +150,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er } else if err := CopyFile(target, source); err != nil { return fmt.Errorf("failed to copy files: %w", err) } - case (fi.Mode() & os.ModeSymlink) == os.ModeSymlink: + case (fileInfo.Mode() & os.ModeSymlink) == os.ModeSymlink: link, err := os.Readlink(source) if err != nil { return fmt.Errorf("failed to read link: %s: %w", source, err) @@ -154,18 +158,18 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er if err := os.Symlink(link, target); err != nil { return fmt.Errorf("failed to create symlink: %s: %w", target, err) } - case (fi.Mode() & os.ModeDevice) == os.ModeDevice, - (fi.Mode() & os.ModeNamedPipe) == os.ModeNamedPipe, - (fi.Mode() & os.ModeSocket) == os.ModeSocket: - if err := copyIrregular(target, fi); err != nil { + case (fileInfo.Mode() & os.ModeDevice) == os.ModeDevice, + (fileInfo.Mode() & os.ModeNamedPipe) == os.ModeNamedPipe, + (fileInfo.Mode() & os.ModeSocket) == os.ModeSocket: + if err := copyIrregular(target, fileInfo); err != nil { return fmt.Errorf("failed to create irregular file: %w", err) } default: - logrus.Warnf("unsupported mode: %s: %s", source, fi.Mode()) + logrus.Warnf("unsupported mode: %s: %s", source, fileInfo.Mode()) continue } - if err := copyFileInfo(fi, source, target); err != nil { + if err := copyFileInfo(fileInfo, source, target); err != nil { return fmt.Errorf("failed to copy file info: %w", err) } diff --git a/vendor/github.com/containerd/continuity/fs/copy_windows.go b/vendor/github.com/containerd/continuity/fs/copy_windows.go index 4dad9441d..6ec13b989 100644 --- a/vendor/github.com/containerd/continuity/fs/copy_windows.go +++ b/vendor/github.com/containerd/continuity/fs/copy_windows.go @@ -49,7 +49,6 @@ func copyFileInfo(fi os.FileInfo, src, name string) error { secInfo, err := windows.GetNamedSecurityInfo( src, windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION) - if err != nil { return err } @@ -68,7 +67,6 @@ func copyFileInfo(fi os.FileInfo, src, name string) error { name, windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION, sid, nil, dacl, nil); err != nil { - return err } return nil diff --git a/vendor/github.com/containerd/continuity/fs/diff.go b/vendor/github.com/containerd/continuity/fs/diff.go index 3cd4eee6f..d2c3c568e 100644 --- a/vendor/github.com/containerd/continuity/fs/diff.go +++ b/vendor/github.com/containerd/continuity/fs/diff.go @@ -80,12 +80,13 @@ type ChangeFunc func(ChangeKind, string, os.FileInfo, error) error // // The change callback is called by the order of path names and // should be appliable in that order. -// Due to this apply ordering, the following is true -// - Removed directory trees only create a single change for the root -// directory removed. Remaining changes are implied. -// - A directory which is modified to become a file will not have -// delete entries for sub-path items, their removal is implied -// by the removal of the parent directory. +// +// Due to this apply ordering, the following is true +// - Removed directory trees only create a single change for the root +// directory removed. Remaining changes are implied. +// - A directory which is modified to become a file will not have +// delete entries for sub-path items, their removal is implied +// by the removal of the parent directory. // // Opaque directories will not be treated specially and each file // removed from the base directory will show up as a removal. diff --git a/vendor/github.com/containerd/continuity/fs/dtype_linux.go b/vendor/github.com/containerd/continuity/fs/dtype_linux.go index a8eab1db8..9f55e7980 100644 --- a/vendor/github.com/containerd/continuity/fs/dtype_linux.go +++ b/vendor/github.com/containerd/continuity/fs/dtype_linux.go @@ -21,14 +21,13 @@ package fs import ( "fmt" - "io/ioutil" "os" "syscall" "unsafe" ) func locateDummyIfEmpty(path string) (string, error) { - children, err := ioutil.ReadDir(path) + children, err := os.ReadDir(path) if err != nil { return "", err } diff --git a/vendor/github.com/containerd/continuity/fs/du_unix.go b/vendor/github.com/containerd/continuity/fs/du_unix.go index bf33c42d7..51a08a1d7 100644 --- a/vendor/github.com/containerd/continuity/fs/du_unix.go +++ b/vendor/github.com/containerd/continuity/fs/du_unix.go @@ -28,10 +28,11 @@ import ( // blocksUnitSize is the unit used by `st_blocks` in `stat` in bytes. // See https://man7.org/linux/man-pages/man2/stat.2.html -// st_blocks -// This field indicates the number of blocks allocated to the -// file, in 512-byte units. (This may be smaller than -// st_size/512 when the file has holes.) +// +// st_blocks +// This field indicates the number of blocks allocated to the +// file, in 512-byte units. (This may be smaller than +// st_size/512 when the file has holes.) const blocksUnitSize = 512 type inode struct { @@ -48,7 +49,6 @@ func newInode(stat *syscall.Stat_t) inode { } func diskUsage(ctx context.Context, roots ...string) (Usage, error) { - var ( size int64 inodes = map[inode]struct{}{} // expensive! diff --git a/vendor/github.com/containerd/continuity/fs/du_windows.go b/vendor/github.com/containerd/continuity/fs/du_windows.go index 08fb28333..ea721f826 100644 --- a/vendor/github.com/containerd/continuity/fs/du_windows.go +++ b/vendor/github.com/containerd/continuity/fs/du_windows.go @@ -26,9 +26,7 @@ import ( ) func diskUsage(ctx context.Context, roots ...string) (Usage, error) { - var ( - size int64 - ) + var size int64 // TODO(stevvooe): Support inodes (or equivalent) for windows. @@ -57,9 +55,7 @@ func diskUsage(ctx context.Context, roots ...string) (Usage, error) { } func diffUsage(ctx context.Context, a, b string) (Usage, error) { - var ( - size int64 - ) + var size int64 if err := Changes(ctx, a, b, func(kind ChangeKind, _ string, fi os.FileInfo, err error) error { if err != nil { diff --git a/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go b/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go index a35781999..f1f378410 100644 --- a/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go +++ b/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go @@ -16,9 +16,15 @@ package fstest -// TODO: Any more metadata files generated by Windows layers? -// TODO: Also skip Recycle Bin contents in Windows layers which is used to store deleted files in some cases var metadataFiles = map[string]bool{ - "\\System Volume Information": true, - "\\WcSandboxState": true, + "\\System Volume Information": true, + "\\WcSandboxState": true, + "\\Windows": true, + "\\Windows\\System32": true, + "\\Windows\\System32\\config": true, + "\\Windows\\System32\\config\\DEFAULT": true, + "\\Windows\\System32\\config\\SAM": true, + "\\Windows\\System32\\config\\SECURITY": true, + "\\Windows\\System32\\config\\SOFTWARE": true, + "\\Windows\\System32\\config\\SYSTEM": true, } diff --git a/vendor/github.com/containerd/continuity/fs/fstest/continuity_util.go b/vendor/github.com/containerd/continuity/fs/fstest/continuity_util.go index 3b687a64a..45dd57413 100644 --- a/vendor/github.com/containerd/continuity/fs/fstest/continuity_util.go +++ b/vendor/github.com/containerd/continuity/fs/fstest/continuity_util.go @@ -129,7 +129,6 @@ func compareResource(r1, r2 continuity.Resource) bool { // TODO(dmcgowan): Check if is XAttrer return compareResourceTypes(r1, r2) - } func compareResourceTypes(r1, r2 continuity.Resource) bool { diff --git a/vendor/github.com/containerd/continuity/fs/fstest/file_windows.go b/vendor/github.com/containerd/continuity/fs/fstest/file_windows.go index 45fd9f611..6fd9ee8d9 100644 --- a/vendor/github.com/containerd/continuity/fs/fstest/file_windows.go +++ b/vendor/github.com/containerd/continuity/fs/fstest/file_windows.go @@ -32,13 +32,13 @@ func Lchtimes(name string, atime, mtime time.Time) Applier { // that the filter will mount. It is used for testing the snapshotter func Base() Applier { return Apply( - CreateDir("Windows", 0755), - CreateDir("Windows/System32", 0755), - CreateDir("Windows/System32/Config", 0755), - CreateFile("Windows/System32/Config/SYSTEM", []byte("foo\n"), 0777), - CreateFile("Windows/System32/Config/SOFTWARE", []byte("foo\n"), 0777), - CreateFile("Windows/System32/Config/SAM", []byte("foo\n"), 0777), - CreateFile("Windows/System32/Config/SECURITY", []byte("foo\n"), 0777), - CreateFile("Windows/System32/Config/DEFAULT", []byte("foo\n"), 0777), + CreateDir("Windows", 0o755), + CreateDir("Windows/System32", 0o755), + CreateDir("Windows/System32/Config", 0o755), + CreateFile("Windows/System32/Config/SYSTEM", []byte("foo\n"), 0o777), + CreateFile("Windows/System32/Config/SOFTWARE", []byte("foo\n"), 0o777), + CreateFile("Windows/System32/Config/SAM", []byte("foo\n"), 0o777), + CreateFile("Windows/System32/Config/SECURITY", []byte("foo\n"), 0o777), + CreateFile("Windows/System32/Config/DEFAULT", []byte("foo\n"), 0o777), ) } diff --git a/vendor/github.com/containerd/continuity/fs/fstest/testsuite.go b/vendor/github.com/containerd/continuity/fs/fstest/testsuite.go index 420126e6c..cda8a0fb0 100644 --- a/vendor/github.com/containerd/continuity/fs/fstest/testsuite.go +++ b/vendor/github.com/containerd/continuity/fs/fstest/testsuite.go @@ -81,14 +81,14 @@ var ( // baseApplier creates a basic filesystem layout // with multiple types of files for basic tests. baseApplier = Apply( - CreateDir("/etc/", 0755), - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost"), 0644), + CreateDir("/etc/", 0o755), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost"), 0o644), Link("/etc/hosts", "/etc/hosts.allow"), - CreateDir("/usr/local/lib", 0755), - CreateFile("/usr/local/lib/libnothing.so", []byte{0x00, 0x00}, 0755), + CreateDir("/usr/local/lib", 0o755), + CreateFile("/usr/local/lib/libnothing.so", []byte{0x00, 0x00}, 0o755), Symlink("libnothing.so", "/usr/local/lib/libnothing.so.2"), - CreateDir("/home", 0755), - CreateDir("/home/derek", 0700), + CreateDir("/home", 0o755), + CreateDir("/home/derek", 0o700), // TODO: CreateSocket: how should Sockets be handled in continuity? ) @@ -96,10 +96,10 @@ var ( basicTest = []Applier{ baseApplier, Apply( - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0644), - CreateFile("/etc/fstab", []byte("/dev/sda1\t/\text4\tdefaults 1 1\n"), 0600), - CreateFile("/etc/badfile", []byte(""), 0666), - CreateFile("/home/derek/.zshrc", []byte("#ZSH is just better\n"), 0640), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0o644), + CreateFile("/etc/fstab", []byte("/dev/sda1\t/\text4\tdefaults 1 1\n"), 0o600), + CreateFile("/etc/badfile", []byte(""), 0o666), + CreateFile("/home/derek/.zshrc", []byte("#ZSH is just better\n"), 0o640), ), Apply( Remove("/etc/badfile"), @@ -111,8 +111,8 @@ var ( ), Apply( RemoveAll("/home"), - CreateDir("/home/derek", 0700), - CreateFile("/home/derek/.bashrc", []byte("#not going away\n"), 0640), + CreateDir("/home/derek", 0o700), + CreateFile("/home/derek/.bashrc", []byte("#not going away\n"), 0o640), Link("/etc/hosts", "/etc/hosts.allow"), ), } @@ -121,58 +121,58 @@ var ( // deletions are properly picked up and applied deletionTest = []Applier{ Apply( - CreateDir("/test/somedir", 0755), - CreateDir("/lib", 0700), - CreateFile("/lib/hidden", []byte{}, 0644), + CreateDir("/test/somedir", 0o755), + CreateDir("/lib", 0o700), + CreateFile("/lib/hidden", []byte{}, 0o644), ), Apply( - CreateFile("/test/a", []byte{}, 0644), - CreateFile("/test/b", []byte{}, 0644), - CreateDir("/test/otherdir", 0755), - CreateFile("/test/otherdir/.empty", []byte{}, 0644), + CreateFile("/test/a", []byte{}, 0o644), + CreateFile("/test/b", []byte{}, 0o644), + CreateDir("/test/otherdir", 0o755), + CreateFile("/test/otherdir/.empty", []byte{}, 0o644), RemoveAll("/lib"), - CreateDir("/lib", 0700), - CreateFile("/lib/not-hidden", []byte{}, 0644), + CreateDir("/lib", 0o700), + CreateFile("/lib/not-hidden", []byte{}, 0o644), ), Apply( Remove("/test/a"), Remove("/test/b"), RemoveAll("/test/otherdir"), - CreateFile("/lib/newfile", []byte{}, 0644), + CreateFile("/lib/newfile", []byte{}, 0o644), ), } // updateTest covers file updates for content and permission updateTest = []Applier{ Apply( - CreateDir("/d1", 0755), - CreateDir("/d2", 0700), - CreateFile("/d1/f1", []byte("something..."), 0644), - CreateFile("/d1/f2", []byte("else..."), 0644), - CreateFile("/d1/f3", []byte("entirely..."), 0644), + CreateDir("/d1", 0o755), + CreateDir("/d2", 0o700), + CreateFile("/d1/f1", []byte("something..."), 0o644), + CreateFile("/d1/f2", []byte("else..."), 0o644), + CreateFile("/d1/f3", []byte("entirely..."), 0o644), ), Apply( - CreateFile("/d1/f1", []byte("file content of a different length"), 0664), + CreateFile("/d1/f1", []byte("file content of a different length"), 0o664), Remove("/d1/f3"), - CreateFile("/d1/f3", []byte("updated content"), 0664), - Chmod("/d1/f2", 0766), - Chmod("/d2", 0777), + CreateFile("/d1/f3", []byte("updated content"), 0o664), + Chmod("/d1/f2", 0o766), + Chmod("/d2", 0o777), ), } // directoryPermissionsTest covers directory permissions on update directoryPermissionsTest = []Applier{ Apply( - CreateDir("/d1", 0700), - CreateDir("/d2", 0751), - CreateDir("/d3", 0777), + CreateDir("/d1", 0o700), + CreateDir("/d2", 0o751), + CreateDir("/d3", 0o777), ), Apply( - CreateFile("/d1/f", []byte("irrelevant"), 0644), - CreateDir("/d1/d", 0700), - CreateFile("/d1/d/f", []byte("irrelevant"), 0644), - CreateFile("/d2/f", []byte("irrelevant"), 0644), - CreateFile("/d3/f", []byte("irrelevant"), 0644), + CreateFile("/d1/f", []byte("irrelevant"), 0o644), + CreateDir("/d1/d", 0o700), + CreateFile("/d1/d/f", []byte("irrelevant"), 0o644), + CreateFile("/d2/f", []byte("irrelevant"), 0o644), + CreateFile("/d3/f", []byte("irrelevant"), 0o644), ), } @@ -180,28 +180,28 @@ var ( // files parentDirectoryPermissionsTest = []Applier{ Apply( - CreateDir("/d1", 0700), - CreateDir("/d1/a", 0700), - CreateDir("/d1/a/b", 0700), - CreateDir("/d1/a/b/c", 0700), - CreateFile("/d1/a/b/f", []byte("content1"), 0644), - CreateDir("/d2", 0751), - CreateDir("/d2/a/b", 0751), - CreateDir("/d2/a/b/c", 0751), - CreateFile("/d2/a/b/f", []byte("content1"), 0644), + CreateDir("/d1", 0o700), + CreateDir("/d1/a", 0o700), + CreateDir("/d1/a/b", 0o700), + CreateDir("/d1/a/b/c", 0o700), + CreateFile("/d1/a/b/f", []byte("content1"), 0o644), + CreateDir("/d2", 0o751), + CreateDir("/d2/a/b", 0o751), + CreateDir("/d2/a/b/c", 0o751), + CreateFile("/d2/a/b/f", []byte("content1"), 0o644), ), Apply( - CreateFile("/d1/a/b/f", []byte("content1"), 0644), - Chmod("/d1/a/b/c", 0700), - CreateFile("/d2/a/b/f", []byte("content2"), 0644), - Chmod("/d2/a/b/c", 0751), + CreateFile("/d1/a/b/f", []byte("content1"), 0o644), + Chmod("/d1/a/b/c", 0o700), + CreateFile("/d2/a/b/f", []byte("content2"), 0o644), + Chmod("/d2/a/b/c", 0o751), ), } hardlinkUnmodified = []Applier{ baseApplier, Apply( - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0644), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0o644), ), Apply( Link("/etc/hosts", "/etc/hosts.deny"), @@ -213,7 +213,7 @@ var ( hardlinkBeforeUnmodified = []Applier{ baseApplier, Apply( - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0644), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0o644), ), Apply( Link("/etc/hosts", "/etc/before-hosts"), @@ -225,11 +225,11 @@ var ( hardlinkBeforeModified = []Applier{ baseApplier, Apply( - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0644), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost.localdomain"), 0o644), ), Apply( Remove("/etc/hosts"), - CreateFile("/etc/hosts", []byte("127.0.0.1 localhost"), 0644), + CreateFile("/etc/hosts", []byte("127.0.0.1 localhost"), 0o644), Link("/etc/hosts", "/etc/before-hosts"), ), } diff --git a/vendor/github.com/containerd/continuity/fs/path.go b/vendor/github.com/containerd/continuity/fs/path.go index 97313e2b8..ec6e6a2fa 100644 --- a/vendor/github.com/containerd/continuity/fs/path.go +++ b/vendor/github.com/containerd/continuity/fs/path.go @@ -25,9 +25,7 @@ import ( "path/filepath" ) -var ( - errTooManyLinks = errors.New("too many links") -) +var errTooManyLinks = errors.New("too many links") type currentPath struct { path string diff --git a/vendor/github.com/containerd/continuity/hardlinks.go b/vendor/github.com/containerd/continuity/hardlinks.go index 1df07f54a..9cff20b86 100644 --- a/vendor/github.com/containerd/continuity/hardlinks.go +++ b/vendor/github.com/containerd/continuity/hardlinks.go @@ -21,9 +21,7 @@ import ( "os" ) -var ( - errNotAHardLink = fmt.Errorf("invalid hardlink") -) +var errNotAHardLink = fmt.Errorf("invalid hardlink") type hardlinkManager struct { hardlinks map[hardlinkKey][]Resource diff --git a/vendor/golang.org/x/tools/go/packages/packages.go b/vendor/golang.org/x/tools/go/packages/packages.go index 1d5f0e45b..0f1505b80 100644 --- a/vendor/golang.org/x/tools/go/packages/packages.go +++ b/vendor/golang.org/x/tools/go/packages/packages.go @@ -878,12 +878,19 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { // never has to create a types.Package for an indirect dependency, // which would then require that such created packages be explicitly // inserted back into the Import graph as a final step after export data loading. + // (Hence this return is after the Types assignment.) // The Diamond test exercises this case. if !lpkg.needtypes && !lpkg.needsrc { return } if !lpkg.needsrc { - ld.loadFromExportData(lpkg) + if err := ld.loadFromExportData(lpkg); err != nil { + lpkg.Errors = append(lpkg.Errors, Error{ + Pos: "-", + Msg: err.Error(), + Kind: UnknownError, // e.g. can't find/open/parse export data + }) + } return // not a source package, don't get syntax trees } @@ -970,7 +977,8 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { // The config requested loading sources and types, but sources are missing. // Add an error to the package and fall back to loading from export data. appendError(Error{"-", fmt.Sprintf("sources missing for package %s", lpkg.ID), ParseError}) - ld.loadFromExportData(lpkg) + _ = ld.loadFromExportData(lpkg) // ignore any secondary errors + return // can't get syntax trees for this package } @@ -1194,9 +1202,10 @@ func sameFile(x, y string) bool { return false } -// loadFromExportData returns type information for the specified +// loadFromExportData ensures that type information is present for the specified // package, loading it from an export data file on the first request. -func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error) { +// On success it sets lpkg.Types to a new Package. +func (ld *loader) loadFromExportData(lpkg *loaderPackage) error { if lpkg.PkgPath == "" { log.Fatalf("internal error: Package %s has no PkgPath", lpkg) } @@ -1207,8 +1216,8 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error // must be sequential. (Finer-grained locking would require // changes to the gcexportdata API.) // - // The exportMu lock guards the Package.Pkg field and the - // types.Package it points to, for each Package in the graph. + // The exportMu lock guards the lpkg.Types field and the + // types.Package it points to, for each loaderPackage in the graph. // // Not all accesses to Package.Pkg need to be protected by exportMu: // graph ordering ensures that direct dependencies of source @@ -1217,18 +1226,18 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error defer ld.exportMu.Unlock() if tpkg := lpkg.Types; tpkg != nil && tpkg.Complete() { - return tpkg, nil // cache hit + return nil // cache hit } lpkg.IllTyped = true // fail safe if lpkg.ExportFile == "" { // Errors while building export data will have been printed to stderr. - return nil, fmt.Errorf("no export data file") + return fmt.Errorf("no export data file") } f, err := os.Open(lpkg.ExportFile) if err != nil { - return nil, err + return err } defer f.Close() @@ -1240,7 +1249,7 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error // queries.) r, err := gcexportdata.NewReader(f) if err != nil { - return nil, fmt.Errorf("reading %s: %v", lpkg.ExportFile, err) + return fmt.Errorf("reading %s: %v", lpkg.ExportFile, err) } // Build the view. @@ -1284,7 +1293,7 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error // (May modify incomplete packages in view but not create new ones.) tpkg, err := gcexportdata.Read(r, ld.Fset, view, lpkg.PkgPath) if err != nil { - return nil, fmt.Errorf("reading %s: %v", lpkg.ExportFile, err) + return fmt.Errorf("reading %s: %v", lpkg.ExportFile, err) } if _, ok := view["go.shape"]; ok { // Account for the pseudopackage "go.shape" that gets @@ -1297,8 +1306,7 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error lpkg.Types = tpkg lpkg.IllTyped = false - - return tpkg, nil + return nil } // impliedLoadMode returns loadMode with its dependencies. diff --git a/vendor/golang.org/x/tools/internal/gcimporter/iexport.go b/vendor/golang.org/x/tools/internal/gcimporter/iexport.go index 7d90f00f3..ba53cdcdd 100644 --- a/vendor/golang.org/x/tools/internal/gcimporter/iexport.go +++ b/vendor/golang.org/x/tools/internal/gcimporter/iexport.go @@ -22,6 +22,7 @@ import ( "strconv" "strings" + "golang.org/x/tools/internal/tokeninternal" "golang.org/x/tools/internal/typeparams" ) @@ -138,6 +139,17 @@ func iexportCommon(out io.Writer, fset *token.FileSet, bundle, shallow bool, ver p.doDecl(p.declTodo.popHead()) } + // Produce index of offset of each file record in files. + var files intWriter + var fileOffset []uint64 // fileOffset[i] is offset in files of file encoded as i + if p.shallow { + fileOffset = make([]uint64, len(p.fileInfos)) + for i, info := range p.fileInfos { + fileOffset[i] = uint64(files.Len()) + p.encodeFile(&files, info.file, info.needed) + } + } + // Append indices to data0 section. dataLen := uint64(p.data0.Len()) w := p.newWriter() @@ -163,16 +175,75 @@ func iexportCommon(out io.Writer, fset *token.FileSet, bundle, shallow bool, ver } hdr.uint64(uint64(p.version)) hdr.uint64(uint64(p.strings.Len())) + if p.shallow { + hdr.uint64(uint64(files.Len())) + hdr.uint64(uint64(len(fileOffset))) + for _, offset := range fileOffset { + hdr.uint64(offset) + } + } hdr.uint64(dataLen) // Flush output. io.Copy(out, &hdr) io.Copy(out, &p.strings) + if p.shallow { + io.Copy(out, &files) + } io.Copy(out, &p.data0) return nil } +// encodeFile writes to w a representation of the file sufficient to +// faithfully restore position information about all needed offsets. +// Mutates the needed array. +func (p *iexporter) encodeFile(w *intWriter, file *token.File, needed []uint64) { + _ = needed[0] // precondition: needed is non-empty + + w.uint64(p.stringOff(file.Name())) + + size := uint64(file.Size()) + w.uint64(size) + + // Sort the set of needed offsets. Duplicates are harmless. + sort.Slice(needed, func(i, j int) bool { return needed[i] < needed[j] }) + + lines := tokeninternal.GetLines(file) // byte offset of each line start + w.uint64(uint64(len(lines))) + + // Rather than record the entire array of line start offsets, + // we save only a sparse list of (index, offset) pairs for + // the start of each line that contains a needed position. + var sparse [][2]int // (index, offset) pairs +outer: + for i, lineStart := range lines { + lineEnd := size + if i < len(lines)-1 { + lineEnd = uint64(lines[i+1]) + } + // Does this line contains a needed offset? + if needed[0] < lineEnd { + sparse = append(sparse, [2]int{i, lineStart}) + for needed[0] < lineEnd { + needed = needed[1:] + if len(needed) == 0 { + break outer + } + } + } + } + + // Delta-encode the columns. + w.uint64(uint64(len(sparse))) + var prev [2]int + for _, pair := range sparse { + w.uint64(uint64(pair[0] - prev[0])) + w.uint64(uint64(pair[1] - prev[1])) + prev = pair + } +} + // writeIndex writes out an object index. mainIndex indicates whether // we're writing out the main index, which is also read by // non-compiler tools and includes a complete package description @@ -255,6 +326,12 @@ type iexporter struct { strings intWriter stringIndex map[string]uint64 + // In shallow mode, object positions are encoded as (file, offset). + // Each file is recorded as a line-number table. + // Only the lines of needed positions are saved faithfully. + fileInfo map[*token.File]uint64 // value is index in fileInfos + fileInfos []*filePositions + data0 intWriter declIndex map[types.Object]uint64 tparamNames map[types.Object]string // typeparam->exported name @@ -263,6 +340,11 @@ type iexporter struct { indent int // for tracing support } +type filePositions struct { + file *token.File + needed []uint64 // unordered list of needed file offsets +} + func (p *iexporter) trace(format string, args ...interface{}) { if !trace { // Call sites should also be guarded, but having this check here allows @@ -286,6 +368,25 @@ func (p *iexporter) stringOff(s string) uint64 { return off } +// fileIndexAndOffset returns the index of the token.File and the byte offset of pos within it. +func (p *iexporter) fileIndexAndOffset(file *token.File, pos token.Pos) (uint64, uint64) { + index, ok := p.fileInfo[file] + if !ok { + index = uint64(len(p.fileInfo)) + p.fileInfos = append(p.fileInfos, &filePositions{file: file}) + if p.fileInfo == nil { + p.fileInfo = make(map[*token.File]uint64) + } + p.fileInfo[file] = index + } + // Record each needed offset. + info := p.fileInfos[index] + offset := uint64(file.Offset(pos)) + info.needed = append(info.needed, offset) + + return index, offset +} + // pushDecl adds n to the declaration work queue, if not already present. func (p *iexporter) pushDecl(obj types.Object) { // Package unsafe is known to the compiler and predeclared. @@ -346,7 +447,13 @@ func (p *iexporter) doDecl(obj types.Object) { case *types.Func: sig, _ := obj.Type().(*types.Signature) if sig.Recv() != nil { - panic(internalErrorf("unexpected method: %v", sig)) + // We shouldn't see methods in the package scope, + // but the type checker may repair "func () F() {}" + // to "func (Invalid) F()" and then treat it like "func F()", + // so allow that. See golang/go#57729. + if sig.Recv().Type() != types.Typ[types.Invalid] { + panic(internalErrorf("unexpected method: %v", sig)) + } } // Function. @@ -458,13 +565,30 @@ func (w *exportWriter) tag(tag byte) { } func (w *exportWriter) pos(pos token.Pos) { - if w.p.version >= iexportVersionPosCol { + if w.p.shallow { + w.posV2(pos) + } else if w.p.version >= iexportVersionPosCol { w.posV1(pos) } else { w.posV0(pos) } } +// posV2 encoding (used only in shallow mode) records positions as +// (file, offset), where file is the index in the token.File table +// (which records the file name and newline offsets) and offset is a +// byte offset. It effectively ignores //line directives. +func (w *exportWriter) posV2(pos token.Pos) { + if pos == token.NoPos { + w.uint64(0) + return + } + file := w.p.fset.File(pos) // fset must be non-nil + index, offset := w.p.fileIndexAndOffset(file, pos) + w.uint64(1 + index) + w.uint64(offset) +} + func (w *exportWriter) posV1(pos token.Pos) { if w.p.fset == nil { w.int64(0) diff --git a/vendor/golang.org/x/tools/internal/gcimporter/iimport.go b/vendor/golang.org/x/tools/internal/gcimporter/iimport.go index a1c469653..448f903e8 100644 --- a/vendor/golang.org/x/tools/internal/gcimporter/iimport.go +++ b/vendor/golang.org/x/tools/internal/gcimporter/iimport.go @@ -137,12 +137,23 @@ func iimportCommon(fset *token.FileSet, imports map[string]*types.Package, data } sLen := int64(r.uint64()) + var fLen int64 + var fileOffset []uint64 + if insert != nil { + // Shallow mode uses a different position encoding. + fLen = int64(r.uint64()) + fileOffset = make([]uint64, r.uint64()) + for i := range fileOffset { + fileOffset[i] = r.uint64() + } + } dLen := int64(r.uint64()) whence, _ := r.Seek(0, io.SeekCurrent) stringData := data[whence : whence+sLen] - declData := data[whence+sLen : whence+sLen+dLen] - r.Seek(sLen+dLen, io.SeekCurrent) + fileData := data[whence+sLen : whence+sLen+fLen] + declData := data[whence+sLen+fLen : whence+sLen+fLen+dLen] + r.Seek(sLen+fLen+dLen, io.SeekCurrent) p := iimporter{ version: int(version), @@ -151,6 +162,9 @@ func iimportCommon(fset *token.FileSet, imports map[string]*types.Package, data stringData: stringData, stringCache: make(map[uint64]string), + fileOffset: fileOffset, + fileData: fileData, + fileCache: make([]*token.File, len(fileOffset)), pkgCache: make(map[uint64]*types.Package), declData: declData, @@ -280,6 +294,9 @@ type iimporter struct { stringData []byte stringCache map[uint64]string + fileOffset []uint64 // fileOffset[i] is offset in fileData for info about file encoded as i + fileData []byte + fileCache []*token.File // memoized decoding of file encoded as i pkgCache map[uint64]*types.Package declData []byte @@ -352,6 +369,55 @@ func (p *iimporter) stringAt(off uint64) string { return s } +func (p *iimporter) fileAt(index uint64) *token.File { + file := p.fileCache[index] + if file == nil { + off := p.fileOffset[index] + file = p.decodeFile(intReader{bytes.NewReader(p.fileData[off:]), p.ipath}) + p.fileCache[index] = file + } + return file +} + +func (p *iimporter) decodeFile(rd intReader) *token.File { + filename := p.stringAt(rd.uint64()) + size := int(rd.uint64()) + file := p.fake.fset.AddFile(filename, -1, size) + + // SetLines requires a nondecreasing sequence. + // Because it is common for clients to derive the interval + // [start, start+len(name)] from a start position, and we + // want to ensure that the end offset is on the same line, + // we fill in the gaps of the sparse encoding with values + // that strictly increase by the largest possible amount. + // This allows us to avoid having to record the actual end + // offset of each needed line. + + lines := make([]int, int(rd.uint64())) + var index, offset int + for i, n := 0, int(rd.uint64()); i < n; i++ { + index += int(rd.uint64()) + offset += int(rd.uint64()) + lines[index] = offset + + // Ensure monotonicity between points. + for j := index - 1; j > 0 && lines[j] == 0; j-- { + lines[j] = lines[j+1] - 1 + } + } + + // Ensure monotonicity after last point. + for j := len(lines) - 1; j > 0 && lines[j] == 0; j-- { + size-- + lines[j] = size + } + + if !file.SetLines(lines) { + errorf("SetLines failed: %d", lines) // can't happen + } + return file +} + func (p *iimporter) pkgAt(off uint64) *types.Package { if pkg, ok := p.pkgCache[off]; ok { return pkg @@ -645,6 +711,9 @@ func (r *importReader) qualifiedIdent() (*types.Package, string) { } func (r *importReader) pos() token.Pos { + if r.p.insert != nil { // shallow mode + return r.posv2() + } if r.p.version >= iexportVersionPosCol { r.posv1() } else { @@ -681,6 +750,15 @@ func (r *importReader) posv1() { } } +func (r *importReader) posv2() token.Pos { + file := r.uint64() + if file == 0 { + return token.NoPos + } + tf := r.p.fileAt(file - 1) + return tf.Pos(int(r.uint64())) +} + func (r *importReader) typ() types.Type { return r.p.typAt(r.uint64(), nil) } diff --git a/vendor/golang.org/x/tools/internal/pkgbits/decoder.go b/vendor/golang.org/x/tools/internal/pkgbits/decoder.go index 3205c9a16..b92e8e6eb 100644 --- a/vendor/golang.org/x/tools/internal/pkgbits/decoder.go +++ b/vendor/golang.org/x/tools/internal/pkgbits/decoder.go @@ -373,7 +373,7 @@ func (r *Decoder) Int64() int64 { return r.rawVarint() } -// Int64 decodes and returns a uint64 value from the element bitstream. +// Uint64 decodes and returns a uint64 value from the element bitstream. func (r *Decoder) Uint64() uint64 { r.Sync(SyncUint64) return r.rawUvarint() diff --git a/vendor/golang.org/x/tools/internal/pkgbits/encoder.go b/vendor/golang.org/x/tools/internal/pkgbits/encoder.go index e98e41171..6482617a4 100644 --- a/vendor/golang.org/x/tools/internal/pkgbits/encoder.go +++ b/vendor/golang.org/x/tools/internal/pkgbits/encoder.go @@ -293,7 +293,7 @@ func (w *Encoder) Len(x int) { assert(x >= 0); w.Uint64(uint64(x)) } // Int encodes and writes an int value into the element bitstream. func (w *Encoder) Int(x int) { w.Int64(int64(x)) } -// Len encodes and writes a uint value into the element bitstream. +// Uint encodes and writes a uint value into the element bitstream. func (w *Encoder) Uint(x uint) { w.Uint64(uint64(x)) } // Reloc encodes and writes a relocation for the given (section, diff --git a/vendor/golang.org/x/tools/internal/tokeninternal/tokeninternal.go b/vendor/golang.org/x/tools/internal/tokeninternal/tokeninternal.go new file mode 100644 index 000000000..a3fb2d4f2 --- /dev/null +++ b/vendor/golang.org/x/tools/internal/tokeninternal/tokeninternal.go @@ -0,0 +1,59 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// package tokeninternal provides access to some internal features of the token +// package. +package tokeninternal + +import ( + "go/token" + "sync" + "unsafe" +) + +// GetLines returns the table of line-start offsets from a token.File. +func GetLines(file *token.File) []int { + // token.File has a Lines method on Go 1.21 and later. + if file, ok := (interface{})(file).(interface{ Lines() []int }); ok { + return file.Lines() + } + + // This declaration must match that of token.File. + // This creates a risk of dependency skew. + // For now we check that the size of the two + // declarations is the same, on the (fragile) assumption + // that future changes would add fields. + type tokenFile119 struct { + _ string + _ int + _ int + mu sync.Mutex // we're not complete monsters + lines []int + _ []struct{} + } + type tokenFile118 struct { + _ *token.FileSet // deleted in go1.19 + tokenFile119 + } + + type uP = unsafe.Pointer + switch unsafe.Sizeof(*file) { + case unsafe.Sizeof(tokenFile118{}): + var ptr *tokenFile118 + *(*uP)(uP(&ptr)) = uP(file) + ptr.mu.Lock() + defer ptr.mu.Unlock() + return ptr.lines + + case unsafe.Sizeof(tokenFile119{}): + var ptr *tokenFile119 + *(*uP)(uP(&ptr)) = uP(file) + ptr.mu.Lock() + defer ptr.mu.Unlock() + return ptr.lines + + default: + panic("unexpected token.File size") + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 413c80ed5..9cb5ccc21 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -4,11 +4,12 @@ github.com/AdaLogics/go-fuzz-headers # github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 ## explicit; go 1.18 github.com/AdamKorcz/go-118-fuzz-build/testing -# github.com/Microsoft/go-winio v0.6.0 +# github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 ## explicit; go 1.17 github.com/Microsoft/go-winio github.com/Microsoft/go-winio/backuptar github.com/Microsoft/go-winio/internal/socket +github.com/Microsoft/go-winio/pkg/bindfilter github.com/Microsoft/go-winio/pkg/etw github.com/Microsoft/go-winio/pkg/etwlogrus github.com/Microsoft/go-winio/pkg/fs @@ -97,7 +98,7 @@ github.com/containerd/cgroups/v3/cgroup2/stats # github.com/containerd/console v1.0.3 ## explicit; go 1.13 github.com/containerd/console -# github.com/containerd/continuity v0.3.0 +# github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 ## explicit; go 1.17 github.com/containerd/continuity github.com/containerd/continuity/devices @@ -496,7 +497,7 @@ golang.org/x/crypto/openpgp/errors golang.org/x/crypto/openpgp/packet golang.org/x/crypto/openpgp/s2k golang.org/x/crypto/pbkdf2 -# golang.org/x/mod v0.7.0 +# golang.org/x/mod v0.8.0 ## explicit; go 1.17 golang.org/x/mod/semver # golang.org/x/net v0.7.0 @@ -544,7 +545,7 @@ golang.org/x/text/unicode/norm # golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 ## explicit golang.org/x/time/rate -# golang.org/x/tools v0.5.0 +# golang.org/x/tools v0.6.0 ## explicit; go 1.18 golang.org/x/tools/cmd/stringer golang.org/x/tools/go/gcexportdata @@ -558,6 +559,7 @@ golang.org/x/tools/internal/gcimporter golang.org/x/tools/internal/gocommand golang.org/x/tools/internal/packagesinternal golang.org/x/tools/internal/pkgbits +golang.org/x/tools/internal/tokeninternal golang.org/x/tools/internal/typeparams golang.org/x/tools/internal/typesinternal # google.golang.org/appengine v1.6.7 From 7a36efd75e8b767511e63b0a22818726d902ceec Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 28 Feb 2023 14:31:26 -0800 Subject: [PATCH 16/30] Ignore ERROR_NOT_FOUND error when removing mount Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index a0ee0543e..fd4a2a60a 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -144,7 +144,7 @@ func Unmount(mount string, flags int) error { } if err := bindfilter.RemoveFileBinding(mount); err != nil { - if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER { + if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER || errno == windows.ERROR_NOT_FOUND { // not a mount point return nil } From 95687a9324821d33c28dfdd55274390cc2044598 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 2 Mar 2023 10:32:15 -0800 Subject: [PATCH 17/30] Fix go.mod, simplify boolean logic, add logging Signed-off-by: Gabriel Adrian Samfira --- integration/client/go.mod | 3 +- mount/mount_windows.go | 18 +++++--- snapshots/windows/windows.go | 82 ++++++++++++++++++------------------ 3 files changed, 56 insertions(+), 47 deletions(-) diff --git a/integration/client/go.mod b/integration/client/go.mod index b68261e67..d8b4c97f4 100644 --- a/integration/client/go.mod +++ b/integration/client/go.mod @@ -4,6 +4,7 @@ go 1.19 require ( github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1 + github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 github.com/Microsoft/hcsshim v0.10.0-rc.7 github.com/Microsoft/hcsshim/test v0.0.0-20210408205431-da33ecd607e1 github.com/containerd/cgroups/v3 v3.0.1 @@ -22,8 +23,6 @@ require ( golang.org/x/sys v0.6.0 ) -require github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 - require ( github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 // indirect github.com/cilium/ebpf v0.9.1 // indirect diff --git a/mount/mount_windows.go b/mount/mount_windows.go index fd4a2a60a..3f29d1b4e 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -17,16 +17,17 @@ package mount import ( + "context" "encoding/json" "errors" "fmt" "os" "path/filepath" "strings" - "syscall" "github.com/Microsoft/go-winio/pkg/bindfilter" "github.com/Microsoft/hcsshim" + "github.com/containerd/containerd/log" "golang.org/x/sys/windows" ) @@ -74,16 +75,21 @@ func (m *Mount) mount(target string) error { } defer func() { if err != nil { - hcsshim.DeactivateLayer(di, layerID) + if layerErr := hcsshim.DeactivateLayer(di, layerID); layerErr != nil { + log.G(context.TODO()).WithError(layerErr).Error("failed to deactivate layer during mount failure cleanup") + } } }() if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil { return fmt.Errorf("failed to prepare layer %s: %w", m.Source, err) } + defer func() { if err != nil { - hcsshim.UnprepareLayer(di, layerID) + if layerErr := hcsshim.UnprepareLayer(di, layerID); layerErr != nil { + log.G(context.TODO()).WithError(layerErr).Error("failed to unprepare layer during mount failure cleanup") + } } }() @@ -97,7 +103,9 @@ func (m *Mount) mount(target string) error { } defer func() { if err != nil { - bindfilter.RemoveFileBinding(target) + if bindErr := bindfilter.RemoveFileBinding(target); bindErr != nil { + log.G(context.TODO()).WithError(bindErr).Error("failed to remove binding during mount failure cleanup") + } } }() @@ -144,7 +152,7 @@ func Unmount(mount string, flags int) error { } if err := bindfilter.RemoveFileBinding(mount); err != nil { - if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER || errno == windows.ERROR_NOT_FOUND { + if errors.Is(err, windows.ERROR_INVALID_PARAMETER) || errors.Is(err, windows.ERROR_NOT_FOUND) { // not a mount point return nil } diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index c20a766b4..25b87300a 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -362,7 +362,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k // Create the new snapshot dir snDir := s.getSnapshotDir(newSnapshot.ID) if err = os.MkdirAll(snDir, 0700); err != nil { - return err + return fmt.Errorf("creating snapshot dir: %w", err) } if strings.Contains(key, snapshots.UnpackKeyPrefix) { @@ -372,57 +372,59 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k // snapshot if this isn't the snapshot that just houses the final sandbox.vhd // that will be mounted as the containers scratch. Currently the key for a snapshot // where a layer will be extracted to will have the string `extract-` in it. - } else if len(newSnapshot.ParentIDs) == 0 { + return nil + } + + if len(newSnapshot.ParentIDs) == 0 { // A parentless snapshot is just a bind-mount to a directory named // "Files". When committed, there'll be some post-processing to fill in the rest // of the metadata. filesDir := filepath.Join(snDir, "Files") if err := os.MkdirAll(filesDir, 0700); err != nil { - return err + return fmt.Errorf("creating Files dir: %w", err) } - } else { - parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs) + return nil + } - var snapshotInfo snapshots.Info - for _, o := range opts { - o(&snapshotInfo) + parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs) + var snapshotInfo snapshots.Info + for _, o := range opts { + o(&snapshotInfo) + } + + var sizeInBytes uint64 + if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { + log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) + + sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) } + sizeInBytes = sizeInGB * 1024 * 1024 * 1024 + } - var sizeInBytes uint64 - if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok { - log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel) - - sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err) - } - sizeInBytes = sizeInGB * 1024 * 1024 * 1024 - } - - // Prefer the newer label in bytes over the deprecated Windows specific GB variant. - if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { - sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) - if err != nil { - return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) - } - } - - var makeUVMScratch bool - if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok { - makeUVMScratch = true - } - - // This has to be run first to avoid clashing with the containers sandbox.vhdx. - if makeUVMScratch { - if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil { - return fmt.Errorf("failed to make UVM's scratch layer: %w", err) - } - } - if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil { - return fmt.Errorf("failed to create scratch layer: %w", err) + // Prefer the newer label in bytes over the deprecated Windows specific GB variant. + if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok { + sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64) + if err != nil { + return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err) } } + var makeUVMScratch bool + if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok { + makeUVMScratch = true + } + + // This has to be run first to avoid clashing with the containers sandbox.vhdx. + if makeUVMScratch { + if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil { + return fmt.Errorf("failed to make UVM's scratch layer: %w", err) + } + } + if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil { + return fmt.Errorf("failed to create scratch layer: %w", err) + } return nil }) if err != nil { From e31bef15fa7d333b837f6ad0ab3af6645a8cde49 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 6 Mar 2023 22:58:39 -0800 Subject: [PATCH 18/30] Update continuity Signed-off-by: Gabriel Adrian Samfira --- go.mod | 2 +- go.sum | 4 +-- integration/client/go.mod | 2 +- integration/client/go.sum | 4 +-- .../continuity/fs/fstest/compare_windows.go | 27 ++++++++++++------- .../containerd/continuity/ioutils.go | 16 +++++++---- vendor/modules.txt | 4 +-- 7 files changed, 36 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index 075827d67..b2587e3ff 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/containerd/btrfs/v2 v2.0.0 github.com/containerd/cgroups/v3 v3.0.1 github.com/containerd/console v1.0.3 - github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 + github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081 github.com/containerd/fifo v1.1.0 github.com/containerd/go-cni v1.1.9 github.com/containerd/go-runc v1.0.0 diff --git a/go.sum b/go.sum index 890df5279..89c6fcb11 100644 --- a/go.sum +++ b/go.sum @@ -232,8 +232,8 @@ github.com/containerd/continuity v0.0.0-20201208142359-180525291bb7/go.mod h1:kR github.com/containerd/continuity v0.0.0-20210208174643-50096c924a4e/go.mod h1:EXlVlkqNba9rJe3j7w3Xa924itAMLgZH4UD/Q4PExuQ= github.com/containerd/continuity v0.1.0/go.mod h1:ICJu0PwR54nI0yPEnJ6jcS+J7CZAUXrLh8lPo2knzsM= github.com/containerd/continuity v0.2.2/go.mod h1:pWygW9u7LtS1o4N/Tn0FoCFDIXZ7rxcMX7HX1Dmibvk= -github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 h1:NKxa3JMWvOvsU7ZgHwd9CZupl/692lPy/MBcumAAHsI= -github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= +github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081 h1:1/vHscvA9Q2uXev+au1072As9m7pa9C4OaFDs6NgEBk= +github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081/go.mod h1:X1lYRPhc8/FifyvZvm13dkXxT6X+MhWvZykoSnMPyWU= github.com/containerd/fifo v0.0.0-20180307165137-3d5202aec260/go.mod h1:ODA38xgv3Kuk8dQz2ZQXpnv/UZZUHUCL7pnLehbXgQI= github.com/containerd/fifo v0.0.0-20190226154929-a9fb20d87448/go.mod h1:ODA38xgv3Kuk8dQz2ZQXpnv/UZZUHUCL7pnLehbXgQI= github.com/containerd/fifo v0.0.0-20200410184934-f15a3290365b/go.mod h1:jPQ2IAeZRCYxpS/Cm1495vGFww6ecHmMk1YJH2Q5ln0= diff --git a/integration/client/go.mod b/integration/client/go.mod index d8b4c97f4..af081c755 100644 --- a/integration/client/go.mod +++ b/integration/client/go.mod @@ -9,7 +9,7 @@ require ( github.com/Microsoft/hcsshim/test v0.0.0-20210408205431-da33ecd607e1 github.com/containerd/cgroups/v3 v3.0.1 github.com/containerd/containerd v1.7.0-beta.0 // see replace; the actual version of containerd is replaced with the code at the root of this repository - github.com/containerd/continuity v0.3.0 + github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081 github.com/containerd/go-runc v1.0.0 github.com/containerd/ttrpc v1.2.1 github.com/containerd/typeurl/v2 v2.1.0 diff --git a/integration/client/go.sum b/integration/client/go.sum index 833ed3466..38c5dbd5c 100644 --- a/integration/client/go.sum +++ b/integration/client/go.sum @@ -654,8 +654,8 @@ github.com/containerd/console v1.0.3 h1:lIr7SlA5PxZyMV30bDW0MGbiOPXwc63yRuCP0ARu github.com/containerd/console v1.0.3/go.mod h1:7LqA/THxQ86k76b8c/EMSiaJ3h1eZkMkXar0TQ1gf3U= github.com/containerd/continuity v0.0.0-20210208174643-50096c924a4e/go.mod h1:EXlVlkqNba9rJe3j7w3Xa924itAMLgZH4UD/Q4PExuQ= github.com/containerd/continuity v0.3.0/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= -github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 h1:NKxa3JMWvOvsU7ZgHwd9CZupl/692lPy/MBcumAAHsI= -github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1Ag8espWhkykbPM= +github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081 h1:1/vHscvA9Q2uXev+au1072As9m7pa9C4OaFDs6NgEBk= +github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081/go.mod h1:X1lYRPhc8/FifyvZvm13dkXxT6X+MhWvZykoSnMPyWU= github.com/containerd/fifo v1.0.0/go.mod h1:ocF/ME1SX5b1AOlWi9r677YJmCPSwwWnQ9O123vzpE4= github.com/containerd/fifo v1.1.0 h1:4I2mbh5stb1u6ycIABlBw9zgtlK8viPI9QkQNRQEEmY= github.com/containerd/fifo v1.1.0/go.mod h1:bmC4NWMbXlt2EZ0Hc7Fx7QzTFxgPID13eH0Qu+MAb2o= diff --git a/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go b/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go index f1f378410..0cf3dbe95 100644 --- a/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go +++ b/vendor/github.com/containerd/continuity/fs/fstest/compare_windows.go @@ -17,14 +17,21 @@ package fstest var metadataFiles = map[string]bool{ - "\\System Volume Information": true, - "\\WcSandboxState": true, - "\\Windows": true, - "\\Windows\\System32": true, - "\\Windows\\System32\\config": true, - "\\Windows\\System32\\config\\DEFAULT": true, - "\\Windows\\System32\\config\\SAM": true, - "\\Windows\\System32\\config\\SECURITY": true, - "\\Windows\\System32\\config\\SOFTWARE": true, - "\\Windows\\System32\\config\\SYSTEM": true, + "\\System Volume Information": true, + "\\WcSandboxState": true, + "\\WcSandboxState\\Hives": true, + "\\WcSandboxState\\Hives\\DefaultUser_Delta": true, + "\\WcSandboxState\\Hives\\Sam_Delta": true, + "\\WcSandboxState\\Hives\\Security_Delta": true, + "\\WcSandboxState\\Hives\\Software_Delta": true, + "\\WcSandboxState\\Hives\\System_Delta": true, + "\\WcSandboxState\\initialized": true, + "\\Windows": true, + "\\Windows\\System32": true, + "\\Windows\\System32\\config": true, + "\\Windows\\System32\\config\\DEFAULT": true, + "\\Windows\\System32\\config\\SAM": true, + "\\Windows\\System32\\config\\SECURITY": true, + "\\Windows\\System32\\config\\SOFTWARE": true, + "\\Windows\\System32\\config\\SYSTEM": true, } diff --git a/vendor/github.com/containerd/continuity/ioutils.go b/vendor/github.com/containerd/continuity/ioutils.go index 392c407fa..8b0e72013 100644 --- a/vendor/github.com/containerd/continuity/ioutils.go +++ b/vendor/github.com/containerd/continuity/ioutils.go @@ -37,26 +37,32 @@ func atomicWriteFile(filename string, r io.Reader, dataSize int64, perm os.FileM if err != nil { return err } + needClose := true + defer func() { + if needClose { + f.Close() + } + }() + err = os.Chmod(f.Name(), perm) if err != nil { - f.Close() return err } n, err := io.Copy(f, r) if err == nil && n < dataSize { - f.Close() return io.ErrShortWrite } if err != nil { - f.Close() return err } - if err := f.Sync(); err != nil { - f.Close() + if err = f.Sync(); err != nil { return err } + + needClose = false if err := f.Close(); err != nil { return err } + return os.Rename(f.Name(), filename) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 9cb5ccc21..9fd9fef03 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -98,8 +98,8 @@ github.com/containerd/cgroups/v3/cgroup2/stats # github.com/containerd/console v1.0.3 ## explicit; go 1.13 github.com/containerd/console -# github.com/containerd/continuity v0.3.1-0.20230206102841-68f7b34f5e11 -## explicit; go 1.17 +# github.com/containerd/continuity v0.3.1-0.20230307035957-72c70feb3081 +## explicit; go 1.19 github.com/containerd/continuity github.com/containerd/continuity/devices github.com/containerd/continuity/driver From 47dd3dcffbfbf146e93712eb4980879a1f523c33 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 13 Mar 2023 01:15:55 -0700 Subject: [PATCH 19/30] use t.Fatal if we cannot enable process privileges Signed-off-by: Gabriel Adrian Samfira --- integration/client/snapshot_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/client/snapshot_windows_test.go b/integration/client/snapshot_windows_test.go index cc5c53fdf..ea1109e98 100644 --- a/integration/client/snapshot_windows_test.go +++ b/integration/client/snapshot_windows_test.go @@ -33,7 +33,7 @@ func runTestSnapshotterClient(t *testing.T) { // The SeBackupPrivilege and SeRestorePrivilege gives us access to system files inside the container mount points // (and everywhere on the system), without having to explicitly set DACLs on each location inside the mount point. if err := winio.EnableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}); err != nil { - t.Error(err) + t.Fatal(err) } defer winio.DisableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}) testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) From 54f8abe553ed4d97b2ad523f084296ef1e2c5e09 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 13 Mar 2023 08:05:40 -0700 Subject: [PATCH 20/30] Use DefaultSnapshotter Signed-off-by: Gabriel Adrian Samfira --- integration/client/snapshot_unix_test.go | 3 ++- integration/client/snapshot_windows_test.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/integration/client/snapshot_unix_test.go b/integration/client/snapshot_unix_test.go index 61495e4bc..5aac3cb00 100644 --- a/integration/client/snapshot_unix_test.go +++ b/integration/client/snapshot_unix_test.go @@ -22,6 +22,7 @@ package client import ( "testing" + . "github.com/containerd/containerd" "github.com/containerd/containerd/snapshots/testsuite" ) @@ -30,5 +31,5 @@ func runTestSnapshotterClient(t *testing.T) { t.Skip() } - testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) + testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) } diff --git a/integration/client/snapshot_windows_test.go b/integration/client/snapshot_windows_test.go index ea1109e98..fc0cf38ca 100644 --- a/integration/client/snapshot_windows_test.go +++ b/integration/client/snapshot_windows_test.go @@ -23,6 +23,8 @@ import ( "testing" winio "github.com/Microsoft/go-winio" + + . "github.com/containerd/containerd" "github.com/containerd/containerd/snapshots/testsuite" ) @@ -36,5 +38,5 @@ func runTestSnapshotterClient(t *testing.T) { t.Fatal(err) } defer winio.DisableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}) - testsuite.SnapshotterSuite(t, "SnapshotterClient", newSnapshotter) + testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) } From 4012c1b853adf637667ad62c74c3fa61b3f5e22e Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 30 Mar 2023 06:07:28 -0700 Subject: [PATCH 21/30] Remove escalated privileges Signed-off-by: Gabriel Adrian Samfira --- integration/client/go.mod | 2 +- integration/client/snapshot_test.go | 7 +++- integration/client/snapshot_unix_test.go | 35 ----------------- integration/client/snapshot_windows_test.go | 42 --------------------- mount/mount_windows.go | 16 ++++---- snapshots/testsuite/testsuite.go | 6 +-- 6 files changed, 18 insertions(+), 90 deletions(-) delete mode 100644 integration/client/snapshot_unix_test.go delete mode 100644 integration/client/snapshot_windows_test.go diff --git a/integration/client/go.mod b/integration/client/go.mod index af081c755..db78ee998 100644 --- a/integration/client/go.mod +++ b/integration/client/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1 - github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 + github.com/Microsoft/go-winio v0.6.1-0.20230228163719-dd5de6900b62 // indirect github.com/Microsoft/hcsshim v0.10.0-rc.7 github.com/Microsoft/hcsshim/test v0.0.0-20210408205431-da33ecd607e1 github.com/containerd/cgroups/v3 v3.0.1 diff --git a/integration/client/snapshot_test.go b/integration/client/snapshot_test.go index c30cc6380..4086bd42d 100644 --- a/integration/client/snapshot_test.go +++ b/integration/client/snapshot_test.go @@ -22,6 +22,7 @@ import ( . "github.com/containerd/containerd" "github.com/containerd/containerd/snapshots" + "github.com/containerd/containerd/snapshots/testsuite" ) func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, func() error, error) { @@ -39,5 +40,9 @@ func newSnapshotter(ctx context.Context, root string) (snapshots.Snapshotter, fu } func TestSnapshotterClient(t *testing.T) { - runTestSnapshotterClient(t) + if testing.Short() { + t.Skip() + } + + testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) } diff --git a/integration/client/snapshot_unix_test.go b/integration/client/snapshot_unix_test.go deleted file mode 100644 index 5aac3cb00..000000000 --- a/integration/client/snapshot_unix_test.go +++ /dev/null @@ -1,35 +0,0 @@ -//go:build !windows -// +build !windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package client - -import ( - "testing" - - . "github.com/containerd/containerd" - "github.com/containerd/containerd/snapshots/testsuite" -) - -func runTestSnapshotterClient(t *testing.T) { - if testing.Short() { - t.Skip() - } - - testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) -} diff --git a/integration/client/snapshot_windows_test.go b/integration/client/snapshot_windows_test.go deleted file mode 100644 index fc0cf38ca..000000000 --- a/integration/client/snapshot_windows_test.go +++ /dev/null @@ -1,42 +0,0 @@ -//go:build windows -// +build windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package client - -import ( - "testing" - - winio "github.com/Microsoft/go-winio" - - . "github.com/containerd/containerd" - "github.com/containerd/containerd/snapshots/testsuite" -) - -func runTestSnapshotterClient(t *testing.T) { - if testing.Short() { - t.Skip() - } - // The SeBackupPrivilege and SeRestorePrivilege gives us access to system files inside the container mount points - // (and everywhere on the system), without having to explicitly set DACLs on each location inside the mount point. - if err := winio.EnableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}); err != nil { - t.Fatal(err) - } - defer winio.DisableProcessPrivileges([]string{winio.SeBackupPrivilege, winio.SeRestorePrivilege}) - testsuite.SnapshotterSuite(t, DefaultSnapshotter, newSnapshotter) -} diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 3f29d1b4e..9b7707f21 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -39,7 +39,7 @@ var ( ) // Mount to the provided target. -func (m *Mount) mount(target string) error { +func (m *Mount) mount(target string) (retErr error) { readOnly := false for _, option := range m.Options { if option == "ro" { @@ -70,23 +70,23 @@ func (m *Mount) mount(target string) error { HomeDir: home, } - if err = hcsshim.ActivateLayer(di, layerID); err != nil { + if err := hcsshim.ActivateLayer(di, layerID); err != nil { return fmt.Errorf("failed to activate layer %s: %w", m.Source, err) } defer func() { - if err != nil { + if retErr != nil { if layerErr := hcsshim.DeactivateLayer(di, layerID); layerErr != nil { log.G(context.TODO()).WithError(layerErr).Error("failed to deactivate layer during mount failure cleanup") } } }() - if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil { + if err := hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil { return fmt.Errorf("failed to prepare layer %s: %w", m.Source, err) } defer func() { - if err != nil { + if retErr != nil { if layerErr := hcsshim.UnprepareLayer(di, layerID); layerErr != nil { log.G(context.TODO()).WithError(layerErr).Error("failed to unprepare layer during mount failure cleanup") } @@ -98,11 +98,11 @@ func (m *Mount) mount(target string) error { return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } - if err = bindfilter.ApplyFileBinding(target, volume, readOnly); err != nil { + if err := bindfilter.ApplyFileBinding(target, volume, readOnly); err != nil { return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } defer func() { - if err != nil { + if retErr != nil { if bindErr := bindfilter.RemoveFileBinding(target); bindErr != nil { log.G(context.TODO()).WithError(bindErr).Error("failed to remove binding during mount failure cleanup") } @@ -112,7 +112,7 @@ func (m *Mount) mount(target string) error { // Add an Alternate Data Stream to record the layer source. // See https://docs.microsoft.com/en-au/archive/blogs/askcore/alternate-data-streams-in-ntfs // for details on Alternate Data Streams. - if err = os.WriteFile(filepath.Clean(target)+":"+sourceStreamName, []byte(m.Source), 0666); err != nil { + if err := os.WriteFile(filepath.Clean(target)+":"+sourceStreamName, []byte(m.Source), 0666); err != nil { return fmt.Errorf("failed to record source for layer %s: %w", m.Source, err) } diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index 9586ba500..0eee9938d 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -820,13 +820,13 @@ func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter } testfile := filepath.Join(viewMountPoint, "testfile") - if err := os.WriteFile(testfile, []byte("testcontent"), 0777); err != nil { + err = os.WriteFile(testfile, []byte("testcontent"), 0777) + testutil.Unmount(t, viewMountPoint) + if err != nil { t.Logf("write to %q failed with %v (EROFS is expected but can be other error code)", testfile, err) } else { - testutil.Unmount(t, viewMountPoint) t.Fatalf("write to %q should fail (EROFS) but did not fail", testfile) } - testutil.Unmount(t, viewMountPoint) assert.Nil(t, snapshotter.Remove(ctx, view)) assert.Nil(t, snapshotter.Remove(ctx, committed)) } From 7f82dd91f4cb20078e587b0e4cae38dc2c3f90c6 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 1 Apr 2023 08:43:14 -0700 Subject: [PATCH 22/30] Add ReadOnly() function Signed-off-by: Gabriel Adrian Samfira --- diff/windows/windows.go | 10 +--------- mount/mount_windows.go | 15 ++++++++------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/diff/windows/windows.go b/diff/windows/windows.go index b694b4d67..0778ccf82 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -335,15 +335,7 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { return "", nil, err } - isView := false - for _, o := range mnt.Options { - if o == "ro" { - isView = true - break - } - } - - if isView { + if mnt.ReadOnly() { if mnt.Type == "bind" && len(parentLayerPaths) != 0 { return "", nil, fmt.Errorf("unexpected bind-mount View with parents: %w", errdefs.ErrInvalidArgument) } else if mnt.Type == "bind" { diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 9b7707f21..e964c9bfb 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -38,18 +38,19 @@ var ( ErrNotImplementOnWindows = errors.New("not implemented under windows") ) -// Mount to the provided target. -func (m *Mount) mount(target string) (retErr error) { - readOnly := false +func (m *Mount) ReadOnly() bool { for _, option := range m.Options { if option == "ro" { - readOnly = true - break + return true } } + return false +} +// Mount to the provided target. +func (m *Mount) mount(target string) (retErr error) { if m.Type == "bind" { - if err := bindfilter.ApplyFileBinding(target, m.Source, readOnly); err != nil { + if err := bindfilter.ApplyFileBinding(target, m.Source, m.ReadOnly()); err != nil { return fmt.Errorf("failed to bind-mount to %s: %w", target, err) } return nil @@ -98,7 +99,7 @@ func (m *Mount) mount(target string) (retErr error) { return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } - if err := bindfilter.ApplyFileBinding(target, volume, readOnly); err != nil { + if err := bindfilter.ApplyFileBinding(target, volume, m.ReadOnly()); err != nil { return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } defer func() { From ca5605b4a208d9d5c809813adbec6b2227ab6c23 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 1 Apr 2023 08:49:24 -0700 Subject: [PATCH 23/30] Skip parent layer options on bind mounts Signed-off-by: Gabriel Adrian Samfira --- snapshots/windows/windows.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index 25b87300a..0f35000a7 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -333,15 +333,19 @@ func (s *snapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount { parentLayersJSON, _ := json.Marshal(parentLayerPaths) parentLayersOption := mount.ParentLayerPathsFlag + string(parentLayersJSON) - var mounts []mount.Mount - mounts = append(mounts, mount.Mount{ - Source: source, - Type: mountType, - Options: []string{ - roFlag, - parentLayersOption, + options := []string{ + roFlag, + } + if mountType != "bind" { + options = append(options, parentLayersOption) + } + mounts := []mount.Mount{ + { + Source: source, + Type: mountType, + Options: options, }, - }) + } return mounts } From d373ebc4de9a514af7d42c2edc980ec1d0868598 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sun, 2 Apr 2023 08:35:34 -0700 Subject: [PATCH 24/30] Properly mount base layers As opposed to a writable layer derived from a base layer, the volume path of a base layer, once activated and prepared will not be a WCIFS volume, but the actual path on disk to the snapshot. We cannot directly mount this folder, as that would mean a client may gain access and potentially damage important metadata files that would render the layer unusabble. For base layers we need to mount the Files folder which must exist in any valid base windows-layer. Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 8 ++++++++ snapshots/windows/windows.go | 12 +----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index e964c9bfb..3ef7a3872 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -99,6 +99,14 @@ func (m *Mount) mount(target string) (retErr error) { return fmt.Errorf("failed to get volume path for layer %s: %w", m.Source, err) } + if len(parentLayerPaths) == 0 { + // this is a base layer. It gets mounted without going through WCIFS. We need to mount the Files + // folder, not the actual source, or the client may inadvertently remove metadata files. + volume = filepath.Join(volume, "Files") + if _, err := os.Stat(volume); err != nil { + return fmt.Errorf("no Files folder in layer %s", layerID) + } + } if err := bindfilter.ApplyFileBinding(target, volume, m.ReadOnly()); err != nil { return fmt.Errorf("failed to set volume mount path for layer %s: %w", m.Source, err) } diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index 0f35000a7..eca46dfe8 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -319,16 +319,6 @@ func (s *snapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount { mountType := "windows-layer" - if len(sn.ParentIDs) == 0 { - // A mount of a parentless snapshot is a bind-mount. - mountType = "bind" - // If not being extracted into, then the bind-target is the - // "Files" subdirectory. - if !strings.Contains(key, snapshots.UnpackKeyPrefix) { - source = filepath.Join(source, "Files") - } - } - // error is not checked here, as a string array will never fail to Marshal parentLayersJSON, _ := json.Marshal(parentLayerPaths) parentLayersOption := mount.ParentLayerPathsFlag + string(parentLayersJSON) @@ -336,7 +326,7 @@ func (s *snapshotter) mounts(sn storage.Snapshot, key string) []mount.Mount { options := []string{ roFlag, } - if mountType != "bind" { + if len(sn.ParentIDs) != 0 { options = append(options, parentLayersOption) } mounts := []mount.Mount{ From 6a5b4c9c242a0278033b20f7f5eca4fdbbea231a Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 3 Apr 2023 08:11:35 -0700 Subject: [PATCH 25/30] Remove "bind" code path from diff Signed-off-by: Gabriel Adrian Samfira --- diff/windows/windows.go | 13 +++---------- mount/mount_windows.go | 8 ++++++++ snapshots/windows/windows.go | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/diff/windows/windows.go b/diff/windows/windows.go index 0778ccf82..2cfdfd71a 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -321,12 +321,10 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { } mnt := mounts[0] - if mnt.Type != "windows-layer" && mnt.Type != "bind" { + if mnt.Type != "windows-layer" { // This is a special case error. When this is received the diff service // will attempt the next differ in the chain which for Windows is the // lcow differ that we want. - // TODO: Is there any situation where we actually wanted a "bind" mount to - // fall through to the lcow differ? return "", nil, fmt.Errorf("windowsDiff does not support layer type %s: %w", mnt.Type, errdefs.ErrNotImplemented) } @@ -336,21 +334,16 @@ func mountsToLayerAndParents(mounts []mount.Mount) (string, []string, error) { } if mnt.ReadOnly() { - if mnt.Type == "bind" && len(parentLayerPaths) != 0 { - return "", nil, fmt.Errorf("unexpected bind-mount View with parents: %w", errdefs.ErrInvalidArgument) - } else if mnt.Type == "bind" { + if len(parentLayerPaths) == 0 { // rootfs.CreateDiff creates a new, empty View to diff against, // when diffing something with no parent. // This makes perfect sense for a walking Diff, but for WCOW, // we have to recognise this as "diff against nothing" return "", nil, nil - } else if len(parentLayerPaths) == 0 { - return "", nil, fmt.Errorf("unexpected windows-layer View with no parent: %w", errdefs.ErrInvalidArgument) } // Ignore the dummy sandbox. return parentLayerPaths[0], parentLayerPaths[1:], nil } - return mnt.Source, parentLayerPaths, nil } @@ -367,7 +360,7 @@ func mountPairToLayerStack(lower, upper []mount.Mount) ([]string, error) { lowerLayer, lowerParentLayerPaths, err := mountsToLayerAndParents(lower) if errdefs.IsNotImplemented(err) { - // Upper was a windows-layer or bind, lower is not. We can't handle that. + // Upper was a windows-layer, lower is not. We can't handle that. return nil, fmt.Errorf("windowsDiff cannot diff a windows-layer against a non-windows-layer: %w", errdefs.ErrInvalidArgument) } else if err != nil { return nil, fmt.Errorf("Lower mount invalid: %w", err) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 3ef7a3872..7e527c8df 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -148,6 +148,14 @@ func (m *Mount) GetParentPaths() ([]string, error) { // Unmount the mount at the provided path func Unmount(mount string, flags int) error { + if _, err := os.Stat(mount); err != nil { + if os.IsNotExist(err) { + return nil + } + + return fmt.Errorf("failed to access mount point %s: %w", mount, err) + } + mount = filepath.Clean(mount) adsFile := mount + ":" + sourceStreamName var layerPath string diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index eca46dfe8..4b89afe44 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -370,8 +370,8 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k } if len(newSnapshot.ParentIDs) == 0 { - // A parentless snapshot is just a bind-mount to a directory named - // "Files". When committed, there'll be some post-processing to fill in the rest + // A parentless snapshot a new base layer. Valid base layers must have a "Files" folder. + // When committed, there'll be some post-processing to fill in the rest // of the metadata. filesDir := filepath.Join(snDir, "Files") if err := os.MkdirAll(filesDir, 0700); err != nil { From 1279ad880cd6d99f66fa916177362a37a4e8b8fe Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 3 Apr 2023 09:15:24 -0700 Subject: [PATCH 26/30] Remove bind code path in mount() Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 7e527c8df..e7d5a5113 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -49,13 +49,6 @@ func (m *Mount) ReadOnly() bool { // Mount to the provided target. func (m *Mount) mount(target string) (retErr error) { - if m.Type == "bind" { - if err := bindfilter.ApplyFileBinding(target, m.Source, m.ReadOnly()); err != nil { - return fmt.Errorf("failed to bind-mount to %s: %w", target, err) - } - return nil - } - if m.Type != "windows-layer" { return fmt.Errorf("invalid windows mount type: '%s'", m.Type) } From 7bb2756bc49d176d4e08e068ff3f78face1f8a13 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 4 Apr 2023 00:29:14 -0700 Subject: [PATCH 27/30] Increase integration test tmieout to 20m Signed-off-by: Gabriel Adrian Samfira --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 392d3af50..a7a67baf5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -322,6 +322,7 @@ jobs: ENABLE_CRI_SANDBOXES: ${{ matrix.enable_cri_sandboxes }} GOTESTSUM_JUNITFILE: ${{github.workspace}}/test-integration-serial-junit.xml GOTESTSUM_JSONFILE: ${{github.workspace}}/test-integration-serial-gotest.json + EXTRA_TESTFLAGS: "-timeout=20m" run: mingw32-make.exe integration - run: if [ -f *-gotest.json ]; then echo '# Integration 1' >> $GITHUB_STEP_SUMMARY; teststat -markdown *-gotest.json >> $GITHUB_STEP_SUMMARY; fi if: always() From ba74cdf1503de9a5a79227ff0eda15f38ad3eeab Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 4 Apr 2023 02:04:56 -0700 Subject: [PATCH 28/30] Make ReadOnly() available on all platforms Signed-off-by: Gabriel Adrian Samfira --- mount/mount.go | 11 +++++++++++ mount/mount_windows.go | 9 --------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/mount/mount.go b/mount/mount.go index 24bfc7d01..ae7520f98 100644 --- a/mount/mount.go +++ b/mount/mount.go @@ -68,6 +68,17 @@ func UnmountMounts(mounts []Mount, target string, flags int) error { return nil } +// ReadOnly returns a boolean value indicating whether this mount has the "ro" +// option set. +func (m *Mount) ReadOnly() bool { + for _, option := range m.Options { + if option == "ro" { + return true + } + } + return false +} + // Mount to the provided target path. func (m *Mount) Mount(target string) error { target, err := fs.RootPath(target, m.Target) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index e7d5a5113..b86ba0d7a 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -38,15 +38,6 @@ var ( ErrNotImplementOnWindows = errors.New("not implemented under windows") ) -func (m *Mount) ReadOnly() bool { - for _, option := range m.Options { - if option == "ro" { - return true - } - } - return false -} - // Mount to the provided target. func (m *Mount) mount(target string) (retErr error) { if m.Type != "windows-layer" { From 8538e7a2ac700cfe788e5eea709fd31521bad8d9 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 4 Apr 2023 12:07:34 -0700 Subject: [PATCH 29/30] Improve error messages and remove check * Improve error messages * remove a check for the existance of unmount target. We probably should not mask that the target was missing. Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 8 -------- snapshots/testsuite/issues.go | 2 +- snapshots/testsuite/testsuite.go | 2 +- snapshots/windows/windows.go | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index b86ba0d7a..855d78c02 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -132,14 +132,6 @@ func (m *Mount) GetParentPaths() ([]string, error) { // Unmount the mount at the provided path func Unmount(mount string, flags int) error { - if _, err := os.Stat(mount); err != nil { - if os.IsNotExist(err) { - return nil - } - - return fmt.Errorf("failed to access mount point %s: %w", mount, err) - } - mount = filepath.Clean(mount) adsFile := mount + ":" + sourceStreamName var layerPath string diff --git a/snapshots/testsuite/issues.go b/snapshots/testsuite/issues.go index 92a04526e..df6eb4848 100644 --- a/snapshots/testsuite/issues.go +++ b/snapshots/testsuite/issues.go @@ -96,7 +96,7 @@ func checkRemoveDirectoryInLowerLayer(ctx context.Context, t *testing.T, sn snap // see https://github.com/docker/docker/issues/28391 overlay2 func checkChown(ctx context.Context, t *testing.T, sn snapshots.Snapshotter, work string) { if runtime.GOOS == "windows" { - t.Skip("Chown is not supported on WCOW") + t.Skip("Chown is not supported on Windows") } l1Init := fstest.Apply( fstest.CreateDir("/opt", 0700), diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go index 0eee9938d..36982adab 100644 --- a/snapshots/testsuite/testsuite.go +++ b/snapshots/testsuite/testsuite.go @@ -877,7 +877,7 @@ func closeTwice(ctx context.Context, t *testing.T, snapshotter snapshots.Snapsho func checkRootPermission(ctx context.Context, t *testing.T, snapshotter snapshots.Snapshotter, work string) { if runtime.GOOS == "windows" { - t.Skip("Filesystem permissions are not supported on WCOW") + t.Skip("Filesystem permissions are not supported on Windows") } preparing, err := snapshotterPrepareMount(ctx, snapshotter, "preparing", "", work) diff --git a/snapshots/windows/windows.go b/snapshots/windows/windows.go index 4b89afe44..8e79f10e7 100644 --- a/snapshots/windows/windows.go +++ b/snapshots/windows/windows.go @@ -356,7 +356,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k // Create the new snapshot dir snDir := s.getSnapshotDir(newSnapshot.ID) if err = os.MkdirAll(snDir, 0700); err != nil { - return fmt.Errorf("creating snapshot dir: %w", err) + return fmt.Errorf("failed to create snapshot dir %s: %w", snDir, err) } if strings.Contains(key, snapshots.UnpackKeyPrefix) { From c9e5c33a18acffe583592828a5a6696cbc779c1a Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 4 Apr 2023 12:59:52 -0700 Subject: [PATCH 30/30] UnmountAll is a no-op for missing mount points Signed-off-by: Gabriel Adrian Samfira --- mount/mount_windows.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mount/mount_windows.go b/mount/mount_windows.go index 855d78c02..7c24fa600 100644 --- a/mount/mount_windows.go +++ b/mount/mount_windows.go @@ -177,6 +177,9 @@ func UnmountAll(mount string, flags int) error { // This isn't an error, per the EINVAL handling in the Linux version return nil } + if _, err := os.Stat(mount); os.IsNotExist(err) { + return nil + } return Unmount(mount, flags) }