From 5ce6b8d2bde9ce4c0091f1b53eb11805d4c11edf Mon Sep 17 00:00:00 2001 From: Mario Camou Date: Tue, 19 Nov 2024 07:45:24 +0100 Subject: [PATCH] feat(snapshots): Fix for #2037 Add symlink support for .kopiaignore (#4190) * Add symlink support for .kopiaignore * Address PR comments * Fix linting --- fs/entry.go | 8 +++-- fs/ignorefs/ignorefs.go | 42 ++++++++++++++++++++--- fs/ignorefs/ignorefs_test.go | 64 ++++++++++++++++++++++++++++++++++- fs/localfs/local_fs.go | 11 ++++++ fs/localfs/local_fs_test.go | 44 ++++++++++++++++++++++++ internal/mockfs/mockfs.go | 42 +++++++++++++++++++++-- snapshot/snapshotfs/repofs.go | 4 +++ 7 files changed, 205 insertions(+), 10 deletions(-) diff --git a/fs/entry.go b/fs/entry.go index 30def5417..f208166a7 100644 --- a/fs/entry.go +++ b/fs/entry.go @@ -72,16 +72,19 @@ type Directory interface { func IterateEntries(ctx context.Context, dir Directory, cb func(context.Context, Entry) error) error { iter, err := dir.Iterate(ctx) if err != nil { - return err //nolint:wrapcheck + return errors.Wrapf(err, "in fs.IterateEntries, creating iterator for directory %s", dir.Name()) } defer iter.Close() cur, err := iter.Next(ctx) + if err != nil { + err = errors.Wrapf(err, "in fs.IterateEntries, on first iteration") + } for cur != nil { if err2 := cb(ctx, cur); err2 != nil { - return err2 + return errors.Wrapf(err2, "in fs.IterateEntries, while calling callback on file %s", cur.Name()) } cur, err = iter.Next(ctx) @@ -209,6 +212,7 @@ func (s *DirectorySummary) Clone() DirectorySummary { type Symlink interface { Entry Readlink(ctx context.Context) (string, error) + Resolve(ctx context.Context) (Entry, error) } // FindByName returns an entry with a given name, or nil if not found. Assumes diff --git a/fs/ignorefs/ignorefs.go b/fs/ignorefs/ignorefs.go index b86c98cbb..6c7277913 100644 --- a/fs/ignorefs/ignorefs.go +++ b/fs/ignorefs/ignorefs.go @@ -17,7 +17,10 @@ import ( "github.com/kopia/kopia/snapshot/policy" ) -var log = logging.Module("ignorefs") +var ( + log = logging.Module("ignorefs") + errSymlinkNotAFile = errors.New("Symlink does not link to a file") +) // IgnoreCallback is a function called by ignorefs to report whenever a file or directory is being ignored while listing its parent. type IgnoreCallback func(ctx context.Context, path string, metadata fs.Entry, pol *policy.Tree) @@ -185,12 +188,12 @@ func (d *ignoreDirectory) Iterate(ctx context.Context) (fs.DirectoryIterator, er thisContext, err := d.buildContext(ctx) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "in ignoreDirectory.Iterate, when building context") } inner, err := d.Directory.Iterate(ctx) if err != nil { - return nil, err //nolint:wrapcheck + return nil, errors.Wrapf(err, "in ignoreDirectory.Iterate, when creating iterator") } it := ignoreDirIteratorPool.Get().(*ignoreDirIterator) //nolint:forcetypeassert @@ -271,6 +274,26 @@ func (d *ignoreDirectory) Child(ctx context.Context, name string) (fs.Entry, err return nil, fs.ErrEntryNotFound } +func resolveSymlink(ctx context.Context, entry fs.Symlink) (fs.File, error) { + for { + target, err := entry.Resolve(ctx) + if err != nil { + link, _ := entry.Readlink(ctx) + return nil, errors.Wrapf(err, "when resolving symlink %s of type %T, which points to %s", entry.Name(), entry, link) + } + + switch t := target.(type) { + case fs.File: + return t, nil + case fs.Symlink: + entry = t + continue + default: + return nil, errors.Wrapf(errSymlinkNotAFile, "%s does not eventually link to a file", entry.Name()) + } + } +} + func (d *ignoreDirectory) buildContext(ctx context.Context) (*ignoreContext, error) { effectiveDotIgnoreFiles := d.parentContext.dotIgnoreFiles @@ -283,8 +306,17 @@ func (d *ignoreDirectory) buildContext(ctx context.Context) (*ignoreContext, err for _, dotfile := range effectiveDotIgnoreFiles { if e, err := d.Directory.Child(ctx, dotfile); err == nil { - if f, ok := e.(fs.File); ok { - dotIgnoreFiles = append(dotIgnoreFiles, f) + switch entry := e.(type) { + case fs.File: + dotIgnoreFiles = append(dotIgnoreFiles, entry) + + case fs.Symlink: + target, err := resolveSymlink(ctx, entry) + if err != nil { + return nil, err + } + + dotIgnoreFiles = append(dotIgnoreFiles, target) } } } diff --git a/fs/ignorefs/ignorefs_test.go b/fs/ignorefs/ignorefs_test.go index 6dec83aa5..1afd138f8 100644 --- a/fs/ignorefs/ignorefs_test.go +++ b/fs/ignorefs/ignorefs_test.go @@ -190,6 +190,68 @@ var cases = []struct { "./src/some-src/f1", }, }, + { + desc: "default policy, have dotignore relative symlink", + policyTree: defaultPolicy, + setup: func(root *mockfs.Directory) { + dir := root.AddDir("ignoredir", 0) + dir.AddFileLines("kopiaignore", []string{"file[12]"}, 0) + root.AddSymlink(".kopiaignore", "./ignoredir/kopiaignore", 0) + }, + addedFiles: []string{ + "./.kopiaignore", + "./ignoredir/", + "./ignoredir/kopiaignore", + }, + ignoredFiles: []string{ + "./ignored-by-rule", + "./largefile1", + "./file1", + "./file2", + }, + }, + { + desc: "default policy, have dotignore absolute symlink", + policyTree: defaultPolicy, + setup: func(root *mockfs.Directory) { + dir := root.AddDir("ignoredir", 0) + dir.AddFileLines("kopiaignore", []string{"file[12]"}, 0) + root.AddSymlink(".kopiaignore", "/ignoredir/kopiaignore", 0) + }, + addedFiles: []string{ + "./.kopiaignore", + "./ignoredir/", + "./ignoredir/kopiaignore", + }, + ignoredFiles: []string{ + "./ignored-by-rule", + "./largefile1", + "./file1", + "./file2", + }, + }, + { + desc: "default policy, have dotignore recursive symlink", + policyTree: defaultPolicy, + setup: func(root *mockfs.Directory) { + dir := root.AddDir("ignoredir", 0) + dir.AddFileLines("kopiaignore", []string{"file[12]"}, 0) + root.AddSymlink(".ignorelink", "/ignoredir/kopiaignore", 0) + root.AddSymlink(".kopiaignore", "/.ignorelink", 0) + }, + addedFiles: []string{ + "./.kopiaignore", + "./.ignorelink", + "./ignoredir/", + "./ignoredir/kopiaignore", + }, + ignoredFiles: []string{ + "./ignored-by-rule", + "./largefile1", + "./file1", + "./file2", + }, + }, { desc: "two policies, nested policy excludes files", policyTree: rootAndSrcPolicy, @@ -553,7 +615,7 @@ func walkTree(t *testing.T, dir fs.Directory) []string { if subdir, ok := e.(fs.Directory); ok { if err := walk(relPath, subdir); err != nil { - return err + t.Fatalf("%s not found in %s", relPath, subdir.Name()) } } else { output = append(output, relPath) diff --git a/fs/localfs/local_fs.go b/fs/localfs/local_fs.go index 44097746a..b21ec972c 100644 --- a/fs/localfs/local_fs.go +++ b/fs/localfs/local_fs.go @@ -117,6 +117,17 @@ func (fsl *filesystemSymlink) Readlink(ctx context.Context) (string, error) { return os.Readlink(fsl.fullPath()) } +func (fsl *filesystemSymlink) Resolve(ctx context.Context) (fs.Entry, error) { + target, err := filepath.EvalSymlinks(fsl.fullPath()) + if err != nil { + return nil, errors.Wrapf(err, "while reading symlink %s", fsl.fullPath()) + } + + entry, err := NewEntry(target) + + return entry, err +} + func (e *filesystemErrorEntry) ErrorInfo() error { return e.err } diff --git a/fs/localfs/local_fs_test.go b/fs/localfs/local_fs_test.go index 458488191..6564c0e91 100644 --- a/fs/localfs/local_fs_test.go +++ b/fs/localfs/local_fs_test.go @@ -22,6 +22,50 @@ type fileEnt struct { isFile bool } +func TestSymlink(t *testing.T) { + tmp := testutil.TempDirectory(t) + + fn := filepath.Join(tmp, "target") + absLink := filepath.Join(tmp, "abslink") + relLink := filepath.Join(tmp, "rellink") + + assertNoError(t, os.WriteFile(fn, []byte{1, 2, 3}, 0o777)) + assertNoError(t, os.Symlink(fn, absLink)) + assertNoError(t, os.Symlink("./target", relLink)) + + verifyLink(t, absLink, fn) + verifyLink(t, relLink, fn) +} + +func verifyLink(t *testing.T, path, expected string) { + t.Helper() + + ctx := testlogging.Context(t) + + entry, err := NewEntry(path) + require.NoError(t, err) + + if link, ok := entry.(fs.Symlink); !ok { + t.Errorf("entry is not a symlink: %s", path) + } else { + target, err := link.Resolve(ctx) + require.NoError(t, err) + + if f, ok := target.(fs.File); !ok { + t.Errorf("Link does not resolve to a file: %s", path) + } else { + // Canonicalize paths (for example, on MacOS /var points to /private/var) + actual, _ := filepath.EvalSymlinks(f.LocalFilesystemPath()) + expected, _ := filepath.EvalSymlinks(expected) + + actual = filepath.Clean(actual) + expected = filepath.Clean(expected) + + require.Equal(t, expected, actual) + } + } +} + //nolint:gocyclo func TestFiles(t *testing.T) { ctx := testlogging.Context(t) diff --git a/internal/mockfs/mockfs.go b/internal/mockfs/mockfs.go index 0ef689190..7936978ed 100644 --- a/internal/mockfs/mockfs.go +++ b/internal/mockfs/mockfs.go @@ -10,6 +10,8 @@ import ( "strings" "time" + "github.com/pkg/errors" + "github.com/kopia/kopia/fs" ) @@ -85,6 +87,7 @@ func (e *entry) Close() { type Directory struct { entry + parent *Directory children []fs.Entry readdirError error onReaddir func() @@ -134,6 +137,15 @@ func (imd *Directory) AddFileWithSource(name string, permissions os.FileMode, so return file } +func (imd *Directory) getRoot() *Directory { + root := imd + for root.parent != nil { + root = root.parent + } + + return root +} + // AddSymlink adds a mock symlink with the specified name, target and permissions. func (imd *Directory) AddSymlink(name, target string, permissions os.FileMode) *Symlink { imd, name = imd.resolveSubdir(name) @@ -144,6 +156,7 @@ func (imd *Directory) AddSymlink(name, target string, permissions os.FileMode) * size: int64(len(target)), modTime: DefaultModTime, }, + parent: imd, target: target, } @@ -183,6 +196,7 @@ func (imd *Directory) AddDir(name string, permissions os.FileMode) *Directory { mode: permissions | os.ModeDir, modTime: DefaultModTime, }, + parent: imd, } imd.addChild(subdir) @@ -238,7 +252,14 @@ func (imd *Directory) addChild(e fs.Entry) { func (imd *Directory) resolveSubdir(name string) (parent *Directory, leaf string) { parts := strings.Split(name, "/") for _, n := range parts[0 : len(parts)-1] { - imd = imd.Subdir(n) + switch n { + case ".", "": + continue + case "..": + imd = imd.parent + default: + imd = imd.Subdir(n) + } } return imd, parts[len(parts)-1] @@ -306,7 +327,7 @@ func (imd *Directory) Child(ctx context.Context, name string) (fs.Entry, error) // Iterate returns directory iterator. func (imd *Directory) Iterate(ctx context.Context) (fs.DirectoryIterator, error) { if imd.readdirError != nil { - return nil, imd.readdirError + return nil, errors.Wrapf(imd.readdirError, "in mockfs Directory.Iterate on directory %s", imd.name) } if imd.onReaddir != nil { @@ -356,9 +377,26 @@ func (imf *File) Open(ctx context.Context) (fs.Reader, error) { type Symlink struct { entry + parent *Directory target string } +// Resolve implements fs.Symlink interface. +func (imsl *Symlink) Resolve(ctx context.Context) (fs.Entry, error) { + dir := imsl.parent + + // Mockfs uses Unix path separators + if imsl.target[0] == '/' { + // Absolute link + dir = dir.getRoot() + } + + dir, name := dir.resolveSubdir(imsl.target) + target, err := dir.Child(ctx, name) + + return target, err +} + // Readlink implements fs.Symlink interface. func (imsl *Symlink) Readlink(ctx context.Context) (string, error) { return imsl.target, nil diff --git a/snapshot/snapshotfs/repofs.go b/snapshot/snapshotfs/repofs.go index 225b081ea..cb99eaa65 100644 --- a/snapshot/snapshotfs/repofs.go +++ b/snapshot/snapshotfs/repofs.go @@ -230,6 +230,10 @@ func (rsl *repositorySymlink) Readlink(ctx context.Context) (string, error) { return string(b), nil } +func (rsl *repositorySymlink) Resolve(ctx context.Context) (fs.Entry, error) { + return nil, errors.New("Symlink.Resolve not implemented in Repofs") +} + func (ee *repositoryEntryError) ErrorInfo() error { return ee.err }