From 4fe60817a0d41710fab4a97c33e0c74c2ce2ebce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20L=C3=B3pez?= <1953782+julio-lopez@users.noreply.github.com> Date: Thu, 19 Mar 2026 12:24:38 -0700 Subject: [PATCH] fix(snapshots): inaccessible entry causes parent directory to be skipped (#5217) Revert "feat(snapshots): localfs support for passing options (#5044)" commit c8c461559568e5781471d96222864cac1c3f9d4f. Fix: return `ErrorEntry` for permission denied instead of aborting When iterating a directory, if `lstat` fails with permission denied on an entry, return an `ErrorEntry` instead of an error that stops the entire directory iteration. Previously, a single inaccessible entry, such as, a FUSE/sshfs mount owned by another user, would cause the entire containing directory to fail and be omitted from the snapshot, resulting in data loss. Now, the inaccessible entry is returned as an ErrorEntry which is handled according to the configured error handling policy, allowing iteration to continue and the rest of the directory to be backed up. - Fixes: #5045 Differentiate entry type when ignoring failed entries Fix tests for new behavior, including handling timing-dependent behavior when snapshots --fail-fast --------- Co-authored-by: Geoffrey D. Bennett --- fs/localfs/local_fs.go | 29 +- fs/localfs/local_fs_os.go | 74 ++--- fs/localfs/local_fs_test.go | 309 +++----------------- snapshot/upload/upload.go | 10 +- tests/end_to_end_test/snapshot_fail_test.go | 42 ++- 5 files changed, 129 insertions(+), 335 deletions(-) diff --git a/fs/localfs/local_fs.go b/fs/localfs/local_fs.go index d0ce32376..ed89abdbe 100644 --- a/fs/localfs/local_fs.go +++ b/fs/localfs/local_fs.go @@ -13,16 +13,6 @@ const numEntriesToRead = 100 // number of directory entries to read in one shot -// Options contains configuration options for localfs operations. -type Options struct { - // IgnoreUnreadableDirEntries, when true, causes unreadable directory entries - // to be silently skipped during directory iteration instead of causing errors. - IgnoreUnreadableDirEntries bool -} - -// DefaultOptions stores the default options used by localfs functions. -var DefaultOptions = &Options{} - type filesystemEntry struct { name string size int64 @@ -31,8 +21,7 @@ type filesystemEntry struct { owner fs.OwnerInfo device fs.DeviceInfo - prefix string - options *Options + prefix string } func (e *filesystemEntry) Name() string { @@ -103,7 +92,6 @@ func (fsd *filesystemDirectory) Size() int64 { type fileWithMetadata struct { *os.File - options *Options } func (f *fileWithMetadata) Entry() (fs.Entry, error) { @@ -114,7 +102,7 @@ func (f *fileWithMetadata) Entry() (fs.Entry, error) { basename, prefix := splitDirPrefix(f.Name()) - return newFilesystemFile(newEntry(basename, fi, prefix, f.options)), nil + return newFilesystemFile(newEntry(basename, fi, prefix)), nil } func (fsf *filesystemFile) Open(_ context.Context) (fs.Reader, error) { @@ -123,7 +111,7 @@ func (fsf *filesystemFile) Open(_ context.Context) (fs.Reader, error) { return nil, errors.Wrap(err, "unable to open local file") } - return &fileWithMetadata{File: f, options: fsf.options}, nil + return &fileWithMetadata{f}, nil } func (fsl *filesystemSymlink) Readlink(_ context.Context) (string, error) { @@ -137,7 +125,7 @@ func (fsl *filesystemSymlink) Resolve(_ context.Context) (fs.Entry, error) { return nil, errors.Wrapf(err, "cannot resolve symlink for '%q'", fsl.fullPath()) } - return NewEntryWithOptions(target, fsl.options) + return NewEntry(target) } func (e *filesystemErrorEntry) ErrorInfo() error { @@ -157,15 +145,8 @@ func splitDirPrefix(s string) (basename, prefix string) { } // Directory returns fs.Directory for the specified path. -// It uses DefaultOptions for configuration. func Directory(path string) (fs.Directory, error) { - return DirectoryWithOptions(path, DefaultOptions) -} - -// DirectoryWithOptions returns fs.Directory for the specified path. -// It uses the provided Options for configuration. -func DirectoryWithOptions(path string, options *Options) (fs.Directory, error) { - e, err := NewEntryWithOptions(path, options) + e, err := NewEntry(path) if err != nil { return nil, err } diff --git a/fs/localfs/local_fs_os.go b/fs/localfs/local_fs_os.go index cfc410d1d..97b13ea65 100644 --- a/fs/localfs/local_fs_os.go +++ b/fs/localfs/local_fs_os.go @@ -18,7 +18,6 @@ type filesystemDirectoryIterator struct { dirHandle *os.File childPrefix string - options *Options currentIndex int currentBatch []os.DirEntry @@ -46,7 +45,7 @@ func (it *filesystemDirectoryIterator) Next(_ context.Context) (fs.Entry, error) n := it.currentIndex it.currentIndex++ - e, err := toDirEntryOrNil(it.currentBatch[n], it.childPrefix, it.options) + e, err := toDirEntryOrNil(it.currentBatch[n], it.childPrefix) if err != nil { // stop iteration return nil, err @@ -75,7 +74,7 @@ func (fsd *filesystemDirectory) Iterate(_ context.Context) (fs.DirectoryIterator childPrefix := fullPath + separatorStr - return &filesystemDirectoryIterator{dirHandle: d, childPrefix: childPrefix, options: fsd.options}, nil + return &filesystemDirectoryIterator{dirHandle: d, childPrefix: childPrefix}, nil } func (fsd *filesystemDirectory) Child(_ context.Context, name string) (fs.Entry, error) { @@ -90,39 +89,47 @@ func (fsd *filesystemDirectory) Child(_ context.Context, name string) (fs.Entry, return nil, errors.Wrap(err, "unable to get child") } - return entryFromDirEntry(name, st, fullPath+separatorStr, fsd.options), nil + return entryFromDirEntry(name, st, fullPath+separatorStr), nil } -func toDirEntryOrNil(dirEntry os.DirEntry, prefix string, options *Options) (fs.Entry, error) { +func toDirEntryOrNil(dirEntry os.DirEntry, prefix string) (fs.Entry, error) { n := dirEntry.Name() - fi, err := os.Lstat(prefix + n) - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - - if options != nil && options.IgnoreUnreadableDirEntries { - return nil, nil + switch fi, err := os.Lstat(prefix + n); { + case err == nil: + return entryFromDirEntry(n, fi, prefix), nil + case os.IsNotExist(err): + return nil, nil + case os.IsPermission(err): + // For permission denied errors, return an ErrorEntry instead of failing + // the entire directory iteration. This allows the upload process to + // handle the error according to the configured error handling policy + // and continue processing other entries in the directory. + // + // This is particularly important for inaccessible mount points such as + // FUSE/sshfs mounts owned by another user. If an error is returned here + // then a single inaccessible entry causes the entire containing directory + // to fail and be omitted from the snapshot, which results in omitting + // other accessible entries in the same directory. + e := filesystemEntry{ + name: TrimShallowSuffix(n), + size: 0, + mtimeNanos: 0, + mode: dirEntry.Type(), + owner: fs.OwnerInfo{}, + device: fs.DeviceInfo{}, + prefix: prefix, } + return newFilesystemErrorEntry(e, err), nil + default: return nil, errors.Wrap(err, "error reading directory") } - - return entryFromDirEntry(n, fi, prefix, options), nil } // NewEntry returns fs.Entry for the specified path, the result will be one of supported entry types: fs.File, fs.Directory, fs.Symlink // or fs.UnsupportedEntry. -// It uses DefaultOptions for configuration. func NewEntry(path string) (fs.Entry, error) { - return NewEntryWithOptions(path, DefaultOptions) -} - -// NewEntryWithOptions returns fs.Entry for the specified path, the result will be one of supported entry types: fs.File, fs.Directory, fs.Symlink -// or fs.UnsupportedEntry. -// It uses the provided Options for configuration. -func NewEntryWithOptions(path string, options *Options) (fs.Entry, error) { path = filepath.Clean(path) fi, err := os.Lstat(path) @@ -143,42 +150,42 @@ func NewEntryWithOptions(path string, options *Options) (fs.Entry, error) { } if path == "/" { - return entryFromDirEntry("/", fi, "", options), nil + return entryFromDirEntry("/", fi, ""), nil } basename, prefix := splitDirPrefix(path) - return entryFromDirEntry(basename, fi, prefix, options), nil + return entryFromDirEntry(basename, fi, prefix), nil } -func entryFromDirEntry(basename string, fi os.FileInfo, prefix string, options *Options) fs.Entry { +func entryFromDirEntry(basename string, fi os.FileInfo, prefix string) fs.Entry { isplaceholder := strings.HasSuffix(basename, ShallowEntrySuffix) maskedmode := fi.Mode() & os.ModeType switch { case maskedmode == os.ModeDir && !isplaceholder: - return newFilesystemDirectory(newEntry(basename, fi, prefix, options)) + return newFilesystemDirectory(newEntry(basename, fi, prefix)) case maskedmode == os.ModeDir && isplaceholder: - return newShallowFilesystemDirectory(newEntry(basename, fi, prefix, options)) + return newShallowFilesystemDirectory(newEntry(basename, fi, prefix)) case maskedmode == os.ModeSymlink && !isplaceholder: - return newFilesystemSymlink(newEntry(basename, fi, prefix, options)) + return newFilesystemSymlink(newEntry(basename, fi, prefix)) case maskedmode == 0 && !isplaceholder: - return newFilesystemFile(newEntry(basename, fi, prefix, options)) + return newFilesystemFile(newEntry(basename, fi, prefix)) case maskedmode == 0 && isplaceholder: - return newShallowFilesystemFile(newEntry(basename, fi, prefix, options)) + return newShallowFilesystemFile(newEntry(basename, fi, prefix)) default: - return newFilesystemErrorEntry(newEntry(basename, fi, prefix, options), fs.ErrUnknown) + return newFilesystemErrorEntry(newEntry(basename, fi, prefix), fs.ErrUnknown) } } var _ os.FileInfo = (*filesystemEntry)(nil) -func newEntry(basename string, fi os.FileInfo, prefix string, options *Options) filesystemEntry { +func newEntry(basename string, fi os.FileInfo, prefix string) filesystemEntry { return filesystemEntry{ TrimShallowSuffix(basename), fi.Size(), @@ -187,6 +194,5 @@ func newEntry(basename string, fi os.FileInfo, prefix string, options *Options) platformSpecificOwnerInfo(fi), platformSpecificDeviceInfo(fi), prefix, - options, } } diff --git a/fs/localfs/local_fs_test.go b/fs/localfs/local_fs_test.go index f44c340d5..4f412753a 100644 --- a/fs/localfs/local_fs_test.go +++ b/fs/localfs/local_fs_test.go @@ -307,276 +307,45 @@ type pair struct { } } -// getOptionsFromEntry extracts the options pointer from an fs.Entry by type assertion. -// Returns nil if the entry doesn't have options or if type assertion fails. -func getOptionsFromEntry(entry fs.Entry) *Options { - switch e := entry.(type) { - case *filesystemDirectory: - return e.options - case *filesystemFile: - return e.options - case *filesystemSymlink: - return e.options - case *filesystemErrorEntry: - return e.options - default: +func TestIteratePermissionDenied(t *testing.T) { + if isWindows { + t.Skip("test not applicable on Windows") + } + + if os.Getuid() == 0 { + t.Skip("test cannot run as root") + } + + tmp := testutil.TempDirectory(t) + + // Create a directory with files, then remove execute permission. + // Without execute permission, the directory can be listed (read) + // but lstat on children will fail with permission denied. + require.NoError(t, os.WriteFile(filepath.Join(tmp, "a"), []byte{1}, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(tmp, "b"), []byte{2}, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(tmp, "c"), []byte{3}, 0o644)) + + require.NoError(t, os.Chmod(tmp, 0o644)) + t.Cleanup(func() { os.Chmod(tmp, 0o755) }) + + dir, err := Directory(tmp) + require.NoError(t, err) + + ctx := testlogging.Context(t) + + var entries []fs.Entry + + err = fs.IterateEntries(ctx, dir, func(ctx context.Context, e fs.Entry) error { + entries = append(entries, e) return nil + }) + + require.NoError(t, err, "iteration should complete without error") + require.Len(t, entries, 3, "should have 3 entries") + + for _, e := range entries { + ee, ok := e.(fs.ErrorEntry) + require.True(t, ok, "entry should be ErrorEntry") + require.True(t, os.IsPermission(ee.ErrorInfo()), "error should be permission denied") } } - -func TestOptionsPassedToChildEntries(t *testing.T) { - ctx := testlogging.Context(t) - tmp := testutil.TempDirectory(t) - - // Create a test directory structure - require.NoError(t, os.WriteFile(filepath.Join(tmp, "file1.txt"), []byte{1, 2, 3}, 0o777)) - require.NoError(t, os.WriteFile(filepath.Join(tmp, "file2.txt"), []byte{4, 5, 6}, 0o777)) - subdir := filepath.Join(tmp, "subdir") - require.NoError(t, os.Mkdir(subdir, 0o777)) - require.NoError(t, os.WriteFile(filepath.Join(subdir, "subfile.txt"), []byte{7, 8, 9}, 0o777)) - - // Create custom options - customOptions := &Options{ - IgnoreUnreadableDirEntries: true, - } - - // Create directory with custom options - dir, err := DirectoryWithOptions(tmp, customOptions) - require.NoError(t, err) - - // Verify the directory itself has the correct options - dirOptions := getOptionsFromEntry(dir) - require.NotNil(t, dirOptions, "directory should have options") - require.Equal(t, customOptions, dirOptions, "directory should have the same options pointer") - require.True(t, dirOptions.IgnoreUnreadableDirEntries, "directory options should match") - - // Test that options are passed to children via Child() - childFile, err := dir.Child(ctx, "file1.txt") - require.NoError(t, err) - - childOptions := getOptionsFromEntry(childFile) - require.NotNil(t, childOptions, "child file should have options") - require.Equal(t, customOptions, childOptions, "child file should have the same options pointer") - - // Test that options are passed to subdirectories - childDir, err := dir.Child(ctx, "subdir") - require.NoError(t, err) - - subdirOptions := getOptionsFromEntry(childDir) - require.NotNil(t, subdirOptions, "subdirectory should have options") - require.Equal(t, customOptions, subdirOptions, "subdirectory should have the same options pointer") - - // Test that options are passed to nested children - subdirEntry, ok := childDir.(fs.Directory) - require.True(t, ok, "child directory should be a directory") - - nestedFile, err := subdirEntry.Child(ctx, "subfile.txt") - require.NoError(t, err) - - nestedOptions := getOptionsFromEntry(nestedFile) - require.NotNil(t, nestedOptions, "nested file should have options") - require.Equal(t, customOptions, nestedOptions, "nested file should have the same options pointer") -} - -func TestOptionsPassedThroughIteration(t *testing.T) { - ctx := testlogging.Context(t) - tmp := testutil.TempDirectory(t) - - // Create a test directory structure - require.NoError(t, os.WriteFile(filepath.Join(tmp, "file1.txt"), []byte{1, 2, 3}, 0o777)) - require.NoError(t, os.WriteFile(filepath.Join(tmp, "file2.txt"), []byte{4, 5, 6}, 0o777)) - require.NoError(t, os.Mkdir(filepath.Join(tmp, "subdir"), 0o777)) - - // Create custom options - customOptions := &Options{ - IgnoreUnreadableDirEntries: true, - } - - // Create directory with custom options - dir, err := DirectoryWithOptions(tmp, customOptions) - require.NoError(t, err) - - // Iterate through entries and verify all have the same options pointer - iter, err := dir.Iterate(ctx) - require.NoError(t, err) - - defer iter.Close() - - entryCount := 0 - for { - entry, err := iter.Next(ctx) - if err != nil { - t.Fatalf("iteration error: %v", err) - } - - if entry == nil { - break - } - - entryCount++ - entryOptions := getOptionsFromEntry(entry) - require.NotNil(t, entryOptions, "entry %s should have options", entry.Name()) - require.Equal(t, customOptions, entryOptions, "entry %s should have the same options pointer", entry.Name()) - } - - require.Equal(t, 3, entryCount, "should have found 3 entries") -} - -func TestOptionsPassedThroughSymlinkResolution(t *testing.T) { - ctx := testlogging.Context(t) - tmp := testutil.TempDirectory(t) - - // Create a target file - targetFile := filepath.Join(tmp, "target.txt") - require.NoError(t, os.WriteFile(targetFile, []byte{1, 2, 3}, 0o777)) - - // Create a symlink - symlinkPath := filepath.Join(tmp, "link") - require.NoError(t, os.Symlink(targetFile, symlinkPath)) - - // Create custom options - customOptions := &Options{ - IgnoreUnreadableDirEntries: true, - } - - // Create symlink entry with custom options - symlinkEntry, err := NewEntryWithOptions(symlinkPath, customOptions) - require.NoError(t, err) - - // Verify the symlink has the correct options - symlinkOptions := getOptionsFromEntry(symlinkEntry) - require.NotNil(t, symlinkOptions, "symlink should have options") - require.Equal(t, customOptions, symlinkOptions, "symlink should have the same options pointer") - - // Resolve the symlink and verify the resolved entry has the same options - symlink, ok := symlinkEntry.(fs.Symlink) - require.True(t, ok, "entry should be a symlink") - - resolved, err := symlink.Resolve(ctx) - require.NoError(t, err) - - resolvedOptions := getOptionsFromEntry(resolved) - require.NotNil(t, resolvedOptions, "resolved entry should have options") - require.Equal(t, customOptions, resolvedOptions, "resolved entry should have the same options pointer") -} - -func TestOptionsPassedToNewEntry(t *testing.T) { - tmp := testutil.TempDirectory(t) - - // Create a file - filePath := filepath.Join(tmp, "testfile.txt") - require.NoError(t, os.WriteFile(filePath, []byte{1, 2, 3}, 0o777)) - - // Create custom options - customOptions := &Options{ - IgnoreUnreadableDirEntries: true, - } - - // Create entry with custom options - entry, err := NewEntryWithOptions(filePath, customOptions) - require.NoError(t, err) - - // Verify the entry has the correct options - entryOptions := getOptionsFromEntry(entry) - require.NotNil(t, entryOptions, "entry should have options") - require.Equal(t, customOptions, entryOptions, "entry should have the same options pointer") -} - -func TestOptionsPassedToNestedDirectories(t *testing.T) { - ctx := testlogging.Context(t) - tmp := testutil.TempDirectory(t) - - // Create nested directory structure - level1 := filepath.Join(tmp, "level1") - level2 := filepath.Join(level1, "level2") - level3 := filepath.Join(level2, "level3") - - require.NoError(t, os.MkdirAll(level3, 0o777)) - require.NoError(t, os.WriteFile(filepath.Join(level3, "file.txt"), []byte{1, 2, 3}, 0o777)) - - // Create custom options - customOptions := &Options{ - IgnoreUnreadableDirEntries: true, - } - - // Create root directory with custom options - rootDir, err := DirectoryWithOptions(tmp, customOptions) - require.NoError(t, err) - - // Navigate through nested directories and verify options are passed - currentDir := rootDir - levels := []string{"level1", "level2", "level3"} - - for _, level := range levels { - child, err := currentDir.Child(ctx, level) - require.NoError(t, err) - - childOptions := getOptionsFromEntry(child) - require.NotNil(t, childOptions, "directory %s should have options", level) - require.Equal(t, customOptions, childOptions, "directory %s should have the same options pointer", level) - - var ok bool - - currentDir, ok = child.(fs.Directory) - require.True(t, ok, "child should be a directory") - } - - // Verify the file in the deepest directory has the same options - file, err := currentDir.Child(ctx, "file.txt") - require.NoError(t, err) - - fileOptions := getOptionsFromEntry(file) - require.NotNil(t, fileOptions, "file should have options") - require.Equal(t, customOptions, fileOptions, "file should have the same options pointer") -} - -func TestDefaultOptionsUsedByDefault(t *testing.T) { - tmp := testutil.TempDirectory(t) - - // Create a file - filePath := filepath.Join(tmp, "testfile.txt") - require.NoError(t, os.WriteFile(filePath, []byte{1, 2, 3}, 0o777)) - - // Use default NewEntry (should use DefaultOptions) - entry, err := NewEntry(filePath) - require.NoError(t, err) - - // Verify the entry has DefaultOptions - entryOptions := getOptionsFromEntry(entry) - require.NotNil(t, entryOptions, "entry should have options") - require.Equal(t, DefaultOptions, entryOptions, "entry should have DefaultOptions pointer") -} - -func TestDifferentOptionsInstances(t *testing.T) { - tmp := testutil.TempDirectory(t) - - // Create two different files - filePath1 := filepath.Join(tmp, "testfile1.txt") - filePath2 := filepath.Join(tmp, "testfile2.txt") - - require.NoError(t, os.WriteFile(filePath1, []byte{1, 2, 3}, 0o777)) - require.NoError(t, os.WriteFile(filePath2, []byte{4, 5, 6}, 0o777)) - - // Create two different options instances with same values - options1 := &Options{IgnoreUnreadableDirEntries: true} - options2 := &Options{IgnoreUnreadableDirEntries: false} - - // Create entries with different options instances - entry1, err := NewEntryWithOptions(filePath1, options1) - require.NoError(t, err) - - entry2, err := NewEntryWithOptions(filePath2, options2) - require.NoError(t, err) - - // Verify they have the correct options pointers - entry1Options := getOptionsFromEntry(entry1) - entry2Options := getOptionsFromEntry(entry2) - - require.NotNil(t, entry1Options) - require.NotNil(t, entry2Options) - require.Equal(t, options1, entry1Options, "entry1 should have options1 pointer") - require.Equal(t, options2, entry2Options, "entry2 should have options2 pointer") - require.NotEqual(t, entry1Options, entry2Options, "entries should have different options pointers") - require.True(t, entry1Options.IgnoreUnreadableDirEntries, "entry1 options should have IgnoreUnreadableDirEntries=true") - require.False(t, entry2Options.IgnoreUnreadableDirEntries, "entry2 options should have IgnoreUnreadableDirEntries=false") -} diff --git a/snapshot/upload/upload.go b/snapshot/upload/upload.go index 63b8d0315..a9be13533 100644 --- a/snapshot/upload/upload.go +++ b/snapshot/upload/upload.go @@ -831,7 +831,7 @@ func (u *Uploader) processDirectoryEntries( return nil } -//nolint:funlen +//nolint:funlen,gocyclo func (u *Uploader) processSingle( ctx context.Context, entry fs.Entry, @@ -938,8 +938,14 @@ func (u *Uploader) processSingle( prefix = "unknown entry" } else { - isIgnoredError = policyTree.EffectivePolicy().ErrorHandlingPolicy.IgnoreFileErrors.OrDefault(false) prefix = "error" + ehp := policyTree.EffectivePolicy().ErrorHandlingPolicy + + if entry.IsDir() { + isIgnoredError = ehp.IgnoreDirectoryErrors.OrDefault(false) + } else { + isIgnoredError = ehp.IgnoreFileErrors.OrDefault(false) + } } return u.processEntryUploadResult(ctx, nil, entry.ErrorInfo(), entryRelativePath, parentDirBuilder, diff --git a/tests/end_to_end_test/snapshot_fail_test.go b/tests/end_to_end_test/snapshot_fail_test.go index d2a1bd537..2c2fbd47f 100644 --- a/tests/end_to_end_test/snapshot_fail_test.go +++ b/tests/end_to_end_test/snapshot_fail_test.go @@ -157,6 +157,13 @@ func testSnapshotFailCases( wantIgnoredErrors: cond(ignoringDirs, 1, 0), wantPartial: !ignoringDirs && isFailFast, } + + expectedWhenUnreadableDirEntries = expectedSnapshotResult{ + success: ignoringFiles && ignoringDirs, + wantErrors: fatalErrorCount(ignoringDirs, ignoringFiles), + wantIgnoredErrors: ignoredErrorCount(ignoringDirs, ignoringFiles), + wantPartial: !(ignoringFiles && ignoringDirs) && isFailFast, //nolint:staticcheck + } ) // Test the root dir permissions @@ -213,7 +220,7 @@ func testSnapshotFailCases( expectSuccess: map[os.FileMode]expectedSnapshotResult{ 0o000: expectEarlyFailure, 0o100: expectEarlyFailure, - 0o400: expectEarlyFailure, + 0o400: expectedWhenUnreadableDirEntries, }, }, { @@ -243,7 +250,7 @@ func testSnapshotFailCases( expectSuccess: map[os.FileMode]expectedSnapshotResult{ 0o000: expectedWhenIgnoringDirs, 0o100: expectedWhenIgnoringDirs, - 0o400: expectedWhenIgnoringDirs, + 0o400: expectedWhenUnreadableDirEntries, }, }, { @@ -273,7 +280,7 @@ func testSnapshotFailCases( expectSuccess: map[os.FileMode]expectedSnapshotResult{ 0o000: expectedWhenIgnoringDirs, 0o100: expectedWhenIgnoringDirs, - 0o400: expectedWhenIgnoringDirs, + 0o400: expectedWhenUnreadableDirEntries, }, }, { @@ -295,6 +302,24 @@ func testSnapshotFailCases( } } +func ignoredErrorCount(ignoringDirErrs, ignoringFileErrs bool) int { + var errCount int + + if ignoringDirErrs { + errCount += 2 + } + + if ignoringFileErrs { + errCount += 1 + } + + return errCount +} + +func fatalErrorCount(ignoringDirErrs, ignoringFileErrs bool) int { + return 3 - ignoredErrorCount(ignoringDirErrs, ignoringFileErrs) +} + func createSimplestFileTree(t *testing.T, maxDirDepth, currDepth int, currPath string) { t.Helper() @@ -397,9 +422,16 @@ func testPermissions( e.RunAndExpectSuccess(t, "snapshot", "restore", parsed.manifestID, target) } - require.Equal(t, expected.wantErrors, parsed.errorCount, "unexpected number of errors") - require.Equal(t, expected.wantIgnoredErrors, parsed.ignoredErrorCount, "unexpected number of ignored errors") require.Equal(t, expected.wantPartial, parsed.partial, "unexpected partial") + + if expected.wantPartial { + // for partial snapshots, only check that at least one fatal error was recorded + require.Positive(t, parsed.errorCount, "expected at least one fatal error") + } else { + // the total number of errors can only be validated for non-partial snapshots + require.Equal(t, expected.wantErrors, parsed.errorCount, "unexpected number of errors") + require.Equal(t, expected.wantIgnoredErrors, parsed.ignoredErrorCount, "unexpected number of ignored errors") + } }) } }