containerd / continuity

A transport-agnostic, filesystem metadata manifest system
https://containerd.io
Apache License 2.0
137 stars 66 forks source link

Allow Walk to visit volume mounts on Windows #208

Closed gabriel-samfira closed 1 year ago

gabriel-samfira commented 1 year ago

This change adds a fork of the filepath.Walk function from the standard library, with changes that allows it to visit reparse points that are in fact volume mounts.

Currently go makes no distinction between a symlink, directory junction, volume mount or Unix domain socket. All reparse points are regarded as symlinks. This means that FileMode does not have ModeDir set for any path that also has ModeSymlink. This includes volume mounts. See: https://go-review.googlesource.com/c/go/+/41830/

This implementation also attempts to avoid recursions. I am on the fence about the added ErrPathRecursion. The idea is to raise an error if a recursion is detected and allow the walkFn function to handle that specific error. But I am not sure what the idiomatic way to handle this, is. Any suggestions are welcome.

The current test is a copy of the test from the standard library. I am looking into adding an additional test to check the volume mount specific code and recursion prevention, but that requires mounting a volume somewhere and attempting to walk it. I am unsure how this scenario would easily be set up. Locally, I created a VHDx, attached it to the system, created a partition, formatted it and then mounted it. But this requires some powershell commandlets to be available, or at least the windows provider to handle VHDx disks.

This change should remove the need to resort to workarounds like these: https://github.com/containerd/containerd/pull/4419/commits/100624ba59f76fd771a81dd38b088b1ed66d44b8 on Windows. See https://github.com/containerd/containerd/pull/4419 for details regarding the need for this.

Signed-off-by: Gabriel Adrian Samfira gsamfira@cloudbasesolutions.com

gabriel-samfira commented 1 year ago

A sample application to test this:

package main

import (
    "errors"
    "flag"
    "fmt"
    "log"
    "os"
    "path/filepath"
    "syscall"

    "github.com/containerd/continuity/pathdriver"
    "golang.org/x/sys/windows"
)

func isSystemOrHidden(path string) (bool, error) {
    attr, err := windows.GetFileAttributes(syscall.StringToUTF16Ptr(path))
    if err != nil {
        return false, err
    }
    if attr&syscall.FILE_ATTRIBUTE_HIDDEN != 0 && attr&syscall.FILE_ATTRIBUTE_SYSTEM != 0 {
        return true, nil
    }

    return false, nil
}

func main() {
    pth := flag.String("target", "", "path to walk")
    flag.Parse()

    final, err := filepath.Abs(*pth)
    if err != nil {
        log.Fatal(err)
    }

    fmt.Println(final)
    err = pathdriver.LocalPathDriver.Walk(*pth, func(path string, info os.FileInfo, err error) error {
        if err != nil {
            isSystemOrHidden, errHidden := isSystemOrHidden(path)
            if errHidden != nil {
                return errHidden
            }
            if isSystemOrHidden {
                log.Printf("Skipping hidden system file/folder: %s", path)
                return nil
            }
            if errors.Is(err, pathdriver.ErrPathRecursion) {
                log.Printf("Not visiting previously seen path %s", path)
                return nil
            }
            log.Fatalf("%+v", err)
        }

        fmt.Printf("visiting %v\n", path)
        return nil
    })

    if err != nil {
        log.Fatalf("error: %+v", err)
    }
}

The folder structure and volume mount set up for testing. Volume is mounted twice. Once in C:\var\vhd and in C:\var\vhd\recurse:

Screenshot from 2022-09-26 19-12-44

Output of that sample app:

Screenshot from 2022-09-26 19-10-41

TBBle commented 1 year ago

I had written some tests that create and mount VHDs, but they end up pulling in hcsshim... (for access to computestorage API) and that seems to have pulled in a lot of stuff as a consequence.

It's possible that hcsshim has been fixed to not end up pulling in containerd when used as a library (it was only needed in the test code) but I realise now I had forgotten about that PR for more than a year. >_<

gabriel-samfira commented 1 year ago

Closing this PR. We've replaced the need for junctions with the bind filter (https://github.com/containerd/containerd/pull/8043#issuecomment-1416718655), which does not use reparse points. If anyone still needs this work, feel free to grab the branch and open a new PR.