From cd35fc9b47b58490dbe0574398d9613e5bbfbb1c Mon Sep 17 00:00:00 2001 From: Ed Schouten Date: Mon, 1 Jun 2026 17:31:28 +0200 Subject: [PATCH] Get rid of Directory.VirtualSymlink() In PR #226 we changed VirtualMknod() to accept a virtual.Attributes instead of just the file type. With that change merged, we can argue that VirtualSymlink() has become superfluous. The signature of VirtualMknod() is complete enough to also support the creation of symlinks. Change InMemoryPrepopulatedDirectory.VirtualMknod() to either create special files or symlinks depending on the provided file type. With his change made, we can patch up all of the callers of VirtualSymlink() to call VirtualMknod() instead. --- pkg/filesystem/virtual/directory.go | 8 +- .../virtual/fuse/simple_raw_file_system.go | 5 +- .../fuse/simple_raw_file_system_test.go | 19 +- .../in_memory_prepopulated_directory.go | 74 ++-- .../in_memory_prepopulated_directory_test.go | 380 +++++++++--------- pkg/filesystem/virtual/nfsv4/nfs40_program.go | 10 +- .../virtual/nfsv4/nfs40_program_test.go | 15 +- pkg/filesystem/virtual/nfsv4/nfs41_program.go | 10 +- pkg/filesystem/virtual/read_only_directory.go | 6 - pkg/filesystem/virtual/winfsp/file_system.go | 10 +- .../virtual/winfsp/file_system_test.go | 18 +- 11 files changed, 274 insertions(+), 281 deletions(-) diff --git a/pkg/filesystem/virtual/directory.go b/pkg/filesystem/virtual/directory.go index 63d06385..958a573a 100644 --- a/pkg/filesystem/virtual/directory.go +++ b/pkg/filesystem/virtual/directory.go @@ -55,8 +55,9 @@ type Directory interface { // VirtualMkdir creates an empty directory within the current // directory. VirtualMkdir(ctx context.Context, name path.Component, createAttributes *Attributes, requested AttributesMask, createdDirectoryAttributes *Attributes) (Directory, ChangeInfo, Status) - // VirtualMknod creates a special file (FIFO, UNIX domain socket, - // block device or character device) within the current directory. + // VirtualMknod creates a special file (FIFO, UNIX domain + // socket, block device or character device) or symbolic link + // within the current directory. VirtualMknod(ctx context.Context, name path.Component, createAttributes *Attributes, requested AttributesMask, createdFileAttributes *Attributes) (Leaf, ChangeInfo, Status) // VirtualReadDir reports files and directories stored within // the directory. @@ -69,9 +70,6 @@ type Directory interface { // this method behaves like rmdir(), unlink() or a mixture of // the two. The latter is needed by NFSv4. VirtualRemove(ctx context.Context, name path.Component, removeDirectory, removeLeaf bool) (ChangeInfo, Status) - // VirtualSymlink creates a symbolic link within the current - // directory. - VirtualSymlink(ctx context.Context, pointedTo path.Parser, linkName path.Component, requested AttributesMask, attributes *Attributes) (Leaf, ChangeInfo, Status) } const ( diff --git a/pkg/filesystem/virtual/fuse/simple_raw_file_system.go b/pkg/filesystem/virtual/fuse/simple_raw_file_system.go index 30819714..e17e51f2 100644 --- a/pkg/filesystem/virtual/fuse/simple_raw_file_system.go +++ b/pkg/filesystem/virtual/fuse/simple_raw_file_system.go @@ -532,8 +532,11 @@ func (rfs *simpleRawFileSystem) Symlink(cancel <-chan struct{}, header *fuse.InH i := rfs.getDirectoryLocked(header.NodeId) rfs.nodeLock.RUnlock() + var createAttributes virtual.Attributes + createAttributes.SetFileType(filesystem.FileTypeSymlink) + createAttributes.SetSymlinkTarget(path.UNIXFormat.NewParser(pointedTo)) var attributes virtual.Attributes - child, _, vs := i.VirtualSymlink(ctx, path.UNIXFormat.NewParser(pointedTo), path.MustNewComponent(linkName), AttributesMaskForFUSEAttr, &attributes) + child, _, vs := i.VirtualMknod(ctx, path.MustNewComponent(linkName), &createAttributes, AttributesMaskForFUSEAttr, &attributes) if vs != virtual.StatusOK { return toFUSEStatus(vs) } diff --git a/pkg/filesystem/virtual/fuse/simple_raw_file_system_test.go b/pkg/filesystem/virtual/fuse/simple_raw_file_system_test.go index 48f5c5ec..6217b155 100644 --- a/pkg/filesystem/virtual/fuse/simple_raw_file_system_test.go +++ b/pkg/filesystem/virtual/fuse/simple_raw_file_system_test.go @@ -620,10 +620,10 @@ func TestSimpleRawFileSystemSymlink(t *testing.T) { rfs := fuse.NewSimpleRawFileSystem(rootDirectory, removalNotifierRegistrar.Call, fuse.AllowAuthenticator) t.Run("Failure", func(t *testing.T) { - rootDirectory.EXPECT().VirtualSymlink( - gomock.Any(), + rootDirectory.EXPECT().VirtualMknod( gomock.Any(), path.MustNewComponent("symlink"), + gomock.Any(), fuse.AttributesMaskForFUSEAttr, gomock.Any(), ).Return(nil, virtual.ChangeInfo{}, virtual.StatusErrExist) @@ -637,16 +637,19 @@ func TestSimpleRawFileSystemSymlink(t *testing.T) { t.Run("Success", func(t *testing.T) { // Create a symbolic link. symlink := mock.NewMockVirtualLeaf(ctrl) - rootDirectory.EXPECT().VirtualSymlink( - gomock.Any(), + rootDirectory.EXPECT().VirtualMknod( gomock.Any(), path.MustNewComponent("symlink"), + gomock.Any(), fuse.AttributesMaskForFUSEAttr, gomock.Any(), - ).DoAndReturn(func(ctx context.Context, pointedTo path.Parser, linkName path.Component, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { - pointedToBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) - require.NoError(t, path.Resolve(pointedTo, scopeWalker)) - require.Equal(t, "target", pointedToBuilder.GetUNIXString()) + ).DoAndReturn(func(ctx context.Context, linkName path.Component, createAttributes *virtual.Attributes, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + require.Equal(t, filesystem.FileTypeSymlink, createAttributes.GetFileType()) + symlinkTarget, ok := createAttributes.GetSymlinkTarget() + require.True(t, ok) + symlinkTargetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(symlinkTarget, scopeWalker)) + require.Equal(t, "target", symlinkTargetBuilder.GetUNIXString()) out.SetFileType(filesystem.FileTypeSymlink) out.SetInodeNumber(123) diff --git a/pkg/filesystem/virtual/in_memory_prepopulated_directory.go b/pkg/filesystem/virtual/in_memory_prepopulated_directory.go index 15fa3dad..a77c926c 100644 --- a/pkg/filesystem/virtual/in_memory_prepopulated_directory.go +++ b/pkg/filesystem/virtual/in_memory_prepopulated_directory.go @@ -880,16 +880,6 @@ func (i *inMemoryPrepopulatedDirectory) VirtualMkdir(ctx context.Context, name p } func (i *inMemoryPrepopulatedDirectory) VirtualMknod(ctx context.Context, name path.Component, createAttributes *Attributes, requested AttributesMask, out *Attributes) (Leaf, ChangeInfo, Status) { - // Block and character devices have no backing in an in-memory - // directory; reject rather than create an inert inode. - fileType := createAttributes.GetFileType() - switch fileType { - case filesystem.FileTypeFIFO, filesystem.FileTypeSocket: - // allowed - default: - return nil, ChangeInfo{}, StatusErrPerm - } - i.lock.Lock() defer i.lock.Unlock() @@ -902,12 +892,36 @@ func (i *inMemoryPrepopulatedDirectory) VirtualMknod(ctx context.Context, name p if s := contents.virtualMayAttach(normalizedName); s != StatusOK { return nil, ChangeInfo{}, s } - // Every FIFO or UNIX domain socket needs to have its own inode - // number, as the kernel uses that to tell instances apart. We - // therefore consider it to be stateful, like a writable file. - child := i.subtree.filesystem.statefulHandleAllocator. - New(). - AsLinkableLeaf(NewSpecialFile(fileType, nil)) + + var child LinkableLeaf + switch fileType := createAttributes.GetFileType(); fileType { + case filesystem.FileTypeFIFO, filesystem.FileTypeSocket: + // Every FIFO or UNIX domain socket needs to have its + // own inode number, as the kernel uses that to tell + // instances apart. We therefore consider it to be + // stateful, like a writable file. + child = i.subtree.filesystem.statefulHandleAllocator. + New(). + AsLinkableLeaf(NewSpecialFile(fileType, nil)) + case filesystem.FileTypeSymlink: + symlinkTarget, ok := createAttributes.GetSymlinkTarget() + if !ok { + panic("Symbolic links should have a target") + } + var err error + child, err = i.subtree.symlinkFactory.LookupSymlink(symlinkTarget) + if err != nil { + i.subtree.errorLogger.Log(util.StatusWrapf(err, "Failed to create new symlink")) + return nil, ChangeInfo{}, StatusErrIO + } + default: + // Don't permit the creation of other types of files, + // such as block devices and character devices. It + // wouldn't be safe if build actions could create those + // arbitrarily. + return nil, ChangeInfo{}, StatusErrPerm + } + changeIDBefore := contents.changeID contents.attach(i.subtree, name, normalizedName, inMemoryDirectoryChild{}.FromLeaf(child)) @@ -1127,34 +1141,6 @@ func (i *inMemoryPrepopulatedDirectory) VirtualSetAttributes(ctx context.Context return StatusOK } -func (i *inMemoryPrepopulatedDirectory) VirtualSymlink(ctx context.Context, pointedTo path.Parser, linkName path.Component, requested AttributesMask, out *Attributes) (Leaf, ChangeInfo, Status) { - i.lock.Lock() - defer i.lock.Unlock() - - contents, s := i.virtualGetContents() - if s != StatusOK { - return nil, ChangeInfo{}, s - } - - normalizedLinkName := i.subtree.filesystem.normalizer.Normalize(linkName) - if s := contents.virtualMayAttach(normalizedLinkName); s != StatusOK { - return nil, ChangeInfo{}, s - } - child, err := i.subtree.symlinkFactory.LookupSymlink(pointedTo) - if err != nil { - i.subtree.errorLogger.Log(util.StatusWrapf(err, "Failed to create new symlink")) - return nil, ChangeInfo{}, StatusErrIO - } - changeIDBefore := contents.changeID - contents.attach(i.subtree, linkName, normalizedLinkName, inMemoryDirectoryChild{}.FromLeaf(child)) - - child.VirtualGetAttributes(ctx, requested, out) - return child, ChangeInfo{ - Before: changeIDBefore, - After: contents.changeID, - }, StatusOK -} - // directoryPrepopulatedDirEntryList is a list of DirectoryDirEntry // objects returned by LookupAllChildren(). This type may be used to // sort elements in the list by name. diff --git a/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go b/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go index 7e4a2ef8..bb7959ab 100644 --- a/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go +++ b/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go @@ -438,7 +438,15 @@ func TestInMemoryPrepopulatedDirectoryInstallHooks(t *testing.T) { attributes.SetInodeNumber(3) }) var out virtual.Attributes - actualLeaf, changeInfo, s := d.VirtualSymlink(ctx, path.UNIXFormat.NewParser("target"), path.MustNewComponent("symlink"), virtual.AttributesMaskInodeNumber, &out) + actualLeaf, changeInfo, s := d.VirtualMknod( + ctx, + path.MustNewComponent("symlink"), + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(path.UNIXFormat.NewParser("target")), + virtual.AttributesMaskInodeNumber, + &out, + ) require.Equal(t, virtual.StatusOK, s) require.NotNil(t, actualLeaf) require.Equal(t, virtual.ChangeInfo{ @@ -1083,7 +1091,7 @@ func TestInMemoryPrepopulatedDirectoryVirtualMkdir(t *testing.T) { }) } -func TestInMemoryPrepopulatedDirectoryVirtualMknodExists(t *testing.T) { +func TestInMemoryPrepopulatedDirectoryVirtualMknod(t *testing.T) { ctrl, ctx := gomock.WithContext(context.Background(), t) fileAllocator := mock.NewMockFileAllocator(ctrl) @@ -1094,121 +1102,195 @@ func TestInMemoryPrepopulatedDirectoryVirtualMknodExists(t *testing.T) { defaultAttributesSetter := mock.NewMockDefaultAttributesSetter(ctrl) d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator, symlinkFactory, errorLogger, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter.Call, virtual.NoNamedAttributesFactory) - // Files may not be overwritten by mknod(). - inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) - require.NoError(t, d.CreateChildren(map[path.Component]virtual.InitialChild{ - path.MustNewComponent("dir"): virtual.InitialChild{}.FromDirectory(virtual.EmptyInitialContentsFetcher), - }, false)) - var createAttr virtual.Attributes - createAttr.SetFileType(filesystem.FileTypeFIFO) - var attr virtual.Attributes - _, _, s := d.VirtualMknod(ctx, path.MustNewComponent("dir"), &createAttr, virtual.AttributesMask(0), &attr) - require.Equal(t, virtual.StatusErrExist, s) -} + t.Run("FailureInitialContentsFetcher", func(t *testing.T) { + // Create a subdirectory that has an initial contents fetcher. + inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) + initialContentsFetcher := mock.NewMockInitialContentsFetcher(ctrl) + require.NoError(t, d.CreateChildren(map[path.Component]virtual.InitialChild{ + path.MustNewComponent("subdir"): virtual.InitialChild{}.FromDirectory(initialContentsFetcher), + }, false)) -func TestInMemoryPrepopulatedDirectoryVirtualMknodSuccess(t *testing.T) { - ctrl, ctx := gomock.WithContext(context.Background(), t) + child, err := d.LookupChild(path.MustNewComponent("subdir")) + require.NoError(t, err) - fileAllocator := mock.NewMockFileAllocator(ctrl) - symlinkFactory := mock.NewMockSymlinkFactory(ctrl) - errorLogger := mock.NewMockErrorLogger(ctrl) - handleAllocator := mock.NewMockStatefulHandleAllocator(ctrl) - inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) - defaultAttributesSetter := mock.NewMockDefaultAttributesSetter(ctrl) - d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator, symlinkFactory, errorLogger, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter.Call, virtual.NoNamedAttributesFactory) + childDirectory, childLeaf := child.GetPair() + require.NotNil(t, childDirectory) + require.Nil(t, childLeaf) - // Create a FIFO and a UNIX domain socket. - fifoHandleAllocation := mock.NewMockStatefulHandleAllocation(ctrl) - handleAllocator.EXPECT().New().Return(fifoHandleAllocation) - fifoHandleAllocation.EXPECT().AsLinkableLeaf(gomock.Any()). - DoAndReturn(func(leaf virtual.LinkableLeaf) virtual.LinkableLeaf { return leaf }) - var fifoCreateAttr virtual.Attributes - fifoCreateAttr.SetFileType(filesystem.FileTypeFIFO) - var fifoAttr virtual.Attributes - fifoNode, changeInfo, s := d.VirtualMknod(ctx, path.MustNewComponent("fifo"), &fifoCreateAttr, specialFileAttributesMask, &fifoAttr) - require.Equal(t, virtual.StatusOK, s) - require.NotNil(t, fifoNode) - require.Equal(t, virtual.ChangeInfo{ - Before: 0, - After: 1, - }, changeInfo) - require.Equal( - t, - *(&virtual.Attributes{}). - SetChangeID(0). - SetPermissions(virtual.PermissionsRead | virtual.PermissionsWrite). - SetFileType(filesystem.FileTypeFIFO). - SetHasNamedAttributes(false). - SetSizeBytes(0), - fifoAttr, - ) + // Creating a symlink in a directory whose initial + // contents cannot be fetched, should fail. The reason + // being that we can't accurately determine whether a + // file under that name is already present. + initialContentsFetcher.EXPECT().FetchContents(gomock.Any()). + Return(nil, status.Error(codes.Internal, "Network error")) + errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Internal, "Failed to initialize directory: Network error"))) - socketHandleAllocation := mock.NewMockStatefulHandleAllocation(ctrl) - handleAllocator.EXPECT().New().Return(socketHandleAllocation) - socketHandleAllocation.EXPECT().AsLinkableLeaf(gomock.Any()). - DoAndReturn(func(leaf virtual.LinkableLeaf) virtual.LinkableLeaf { return leaf }) - var socketCreateAttr virtual.Attributes - socketCreateAttr.SetFileType(filesystem.FileTypeSocket) - var socketAttr virtual.Attributes - socketNode, changeInfo, s := d.VirtualMknod(ctx, path.MustNewComponent("socket"), &socketCreateAttr, specialFileAttributesMask, &socketAttr) - require.Equal(t, virtual.StatusOK, s) - require.NotNil(t, socketNode) - require.Equal(t, virtual.ChangeInfo{ - Before: 1, - After: 2, - }, changeInfo) - require.Equal( - t, - *(&virtual.Attributes{}). - SetChangeID(0). - SetPermissions(virtual.PermissionsRead | virtual.PermissionsWrite). - SetFileType(filesystem.FileTypeSocket). - SetHasNamedAttributes(false). - SetSizeBytes(0), - socketAttr, - ) + _, _, s := childDirectory.VirtualMknod( + ctx, + path.MustNewComponent("symlink"), + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(path.UNIXFormat.NewParser("target")), + 0, + &virtual.Attributes{}, + ) + require.Equal(t, virtual.StatusErrIO, s) + }) - // Check whether the devices are reported properly using the - // native ReadDir() method. - entries, err := d.ReadDir() - require.NoError(t, err) - require.Equal(t, - []filesystem.FileInfo{ - filesystem.NewFileInfo(path.MustNewComponent("fifo"), filesystem.FileTypeFIFO, false), - filesystem.NewFileInfo(path.MustNewComponent("socket"), filesystem.FileTypeSocket, false), - }, entries) + t.Run("FailureExist", func(t *testing.T) { + // The operation should fail if a file or directory + // already exists under the provided name. + existingFile := mock.NewMockLinkableLeaf(ctrl) + require.NoError(t, d.CreateChildren(map[path.Component]virtual.InitialChild{ + path.MustNewComponent("existing_file"): virtual.InitialChild{}.FromLeaf(existingFile), + }, false)) - // Check whether the devices are reported properly using the - // VirtualReadDir() method. - reporter := mock.NewMockDirectoryEntryReporter(ctrl) - reporter.EXPECT().ReportEntry(uint64(1), path.MustNewComponent("fifo"), virtual.DirectoryChild{}.FromLeaf(fifoNode), &fifoAttr).Return(true) - reporter.EXPECT().ReportEntry(uint64(2), path.MustNewComponent("socket"), virtual.DirectoryChild{}.FromLeaf(socketNode), &socketAttr).Return(true) - require.Equal(t, virtual.StatusOK, d.VirtualReadDir(ctx, 0, specialFileAttributesMask, reporter)) -} + _, _, s := d.VirtualMknod( + ctx, + path.MustNewComponent("existing_file"), + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(path.UNIXFormat.NewParser("target")), + 0, + &virtual.Attributes{}, + ) + require.Equal(t, virtual.StatusErrExist, s) + }) -func TestInMemoryPrepopulatedDirectoryVirtualMknodDeviceRejected(t *testing.T) { - ctrl, ctx := gomock.WithContext(context.Background(), t) + t.Run("FailingSymlinkFactory", func(t *testing.T) { + targetPathParser := path.UNIXFormat.NewParser("target") + symlinkFactory.EXPECT().LookupSymlink(targetPathParser).Return(nil, status.Error(codes.Internal, "Not allowed")) + errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Internal, "Failed to create new symlink: Not allowed"))) + _, _, s := d.VirtualMknod( + ctx, + path.MustNewComponent("symlink"), + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(targetPathParser), + 0, + &virtual.Attributes{}, + ) + require.Equal(t, virtual.StatusErrIO, s) + }) - fileAllocator := mock.NewMockFileAllocator(ctrl) - symlinkFactory := mock.NewMockSymlinkFactory(ctrl) - errorLogger := mock.NewMockErrorLogger(ctrl) - handleAllocator := mock.NewMockStatefulHandleAllocator(ctrl) - inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) - defaultAttributesSetter := mock.NewMockDefaultAttributesSetter(ctrl) - d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator, symlinkFactory, errorLogger, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter.Call, virtual.NoNamedAttributesFactory) + t.Run("SuccessSymlink", func(t *testing.T) { + leaf := mock.NewMockLinkableLeaf(ctrl) + targetPathParser := path.UNIXFormat.NewParser("target") + symlinkFactory.EXPECT().LookupSymlink(targetPathParser).Return(leaf, nil) + leaf.EXPECT().VirtualGetAttributes( + ctx, + virtual.AttributesMaskInodeNumber, + gomock.Any(), + ).Do(func(ctx context.Context, requested virtual.AttributesMask, attributes *virtual.Attributes) { + attributes.SetInodeNumber(3) + }) - // Block and character device creation is not supported on - // loopback in-memory directories. - for _, ft := range []filesystem.FileType{ - filesystem.FileTypeBlockDevice, - filesystem.FileTypeCharacterDevice, - } { - var createAttr virtual.Attributes - createAttr.SetFileType(ft) - var attr virtual.Attributes - _, _, s := d.VirtualMknod(ctx, path.MustNewComponent("dev"), &createAttr, virtual.AttributesMask(0), &attr) - require.Equal(t, virtual.StatusErrPerm, s) - } + var out virtual.Attributes + actualLeaf, changeInfo, s := d.VirtualMknod( + ctx, + path.MustNewComponent("symlink"), + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(targetPathParser), + virtual.AttributesMaskInodeNumber, + &out, + ) + require.Equal(t, virtual.StatusOK, s) + require.NotNil(t, actualLeaf) + require.Equal(t, virtual.ChangeInfo{ + Before: 2, + After: 3, + }, changeInfo) + require.Equal(t, (&virtual.Attributes{}).SetInodeNumber(3), &out) + }) + + t.Run("FailureBlockCharacterDevice", func(t *testing.T) { + // Block and character device creation is not supported on + // loopback in-memory directories. + for _, ft := range []filesystem.FileType{ + filesystem.FileTypeBlockDevice, + filesystem.FileTypeCharacterDevice, + } { + var createAttr virtual.Attributes + createAttr.SetFileType(ft) + var attr virtual.Attributes + _, _, s := d.VirtualMknod(ctx, path.MustNewComponent("dev"), &createAttr, virtual.AttributesMask(0), &attr) + require.Equal(t, virtual.StatusErrPerm, s) + } + }) + + t.Run("SuccessFIFOAndSocket", func(t *testing.T) { + inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) + d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator, symlinkFactory, errorLogger, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter.Call, virtual.NoNamedAttributesFactory) + + // Create a FIFO and a UNIX domain socket. + fifoHandleAllocation := mock.NewMockStatefulHandleAllocation(ctrl) + handleAllocator.EXPECT().New().Return(fifoHandleAllocation) + fifoHandleAllocation.EXPECT().AsLinkableLeaf(gomock.Any()). + DoAndReturn(func(leaf virtual.LinkableLeaf) virtual.LinkableLeaf { return leaf }) + var fifoCreateAttr virtual.Attributes + fifoCreateAttr.SetFileType(filesystem.FileTypeFIFO) + var fifoAttr virtual.Attributes + fifoNode, changeInfo, s := d.VirtualMknod(ctx, path.MustNewComponent("fifo"), &fifoCreateAttr, specialFileAttributesMask, &fifoAttr) + require.Equal(t, virtual.StatusOK, s) + require.NotNil(t, fifoNode) + require.Equal(t, virtual.ChangeInfo{ + Before: 0, + After: 1, + }, changeInfo) + require.Equal( + t, + *(&virtual.Attributes{}). + SetChangeID(0). + SetPermissions(virtual.PermissionsRead | virtual.PermissionsWrite). + SetFileType(filesystem.FileTypeFIFO). + SetHasNamedAttributes(false). + SetSizeBytes(0), + fifoAttr, + ) + + socketHandleAllocation := mock.NewMockStatefulHandleAllocation(ctrl) + handleAllocator.EXPECT().New().Return(socketHandleAllocation) + socketHandleAllocation.EXPECT().AsLinkableLeaf(gomock.Any()). + DoAndReturn(func(leaf virtual.LinkableLeaf) virtual.LinkableLeaf { return leaf }) + var socketCreateAttr virtual.Attributes + socketCreateAttr.SetFileType(filesystem.FileTypeSocket) + var socketAttr virtual.Attributes + socketNode, changeInfo, s := d.VirtualMknod(ctx, path.MustNewComponent("socket"), &socketCreateAttr, specialFileAttributesMask, &socketAttr) + require.Equal(t, virtual.StatusOK, s) + require.NotNil(t, socketNode) + require.Equal(t, virtual.ChangeInfo{ + Before: 1, + After: 2, + }, changeInfo) + require.Equal( + t, + *(&virtual.Attributes{}). + SetChangeID(0). + SetPermissions(virtual.PermissionsRead | virtual.PermissionsWrite). + SetFileType(filesystem.FileTypeSocket). + SetHasNamedAttributes(false). + SetSizeBytes(0), + socketAttr, + ) + + // Check whether the devices are reported properly using the + // native ReadDir() method. + entries, err := d.ReadDir() + require.NoError(t, err) + require.Equal(t, + []filesystem.FileInfo{ + filesystem.NewFileInfo(path.MustNewComponent("fifo"), filesystem.FileTypeFIFO, false), + filesystem.NewFileInfo(path.MustNewComponent("socket"), filesystem.FileTypeSocket, false), + }, entries) + + // Check whether the devices are reported properly using the + // VirtualReadDir() method. + reporter := mock.NewMockDirectoryEntryReporter(ctrl) + reporter.EXPECT().ReportEntry(uint64(1), path.MustNewComponent("fifo"), virtual.DirectoryChild{}.FromLeaf(fifoNode), &fifoAttr).Return(true) + reporter.EXPECT().ReportEntry(uint64(2), path.MustNewComponent("socket"), virtual.DirectoryChild{}.FromLeaf(socketNode), &socketAttr).Return(true) + require.Equal(t, virtual.StatusOK, d.VirtualReadDir(ctx, 0, specialFileAttributesMask, reporter)) + }) } func TestInMemoryPrepopulatedDirectoryVirtualReadDir(t *testing.T) { @@ -1719,85 +1801,3 @@ func TestInMemoryPrepopulatedDirectoryVirtualRemove(t *testing.T) { }, changeInfo) }) } - -func TestInMemoryPrepopulatedDirectoryVirtualSymlink(t *testing.T) { - ctrl, ctx := gomock.WithContext(context.Background(), t) - - fileAllocator := mock.NewMockFileAllocator(ctrl) - symlinkFactory := mock.NewMockSymlinkFactory(ctrl) - errorLogger := mock.NewMockErrorLogger(ctrl) - handleAllocator := mock.NewMockStatefulHandleAllocator(ctrl) - inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) - defaultAttributesSetter := mock.NewMockDefaultAttributesSetter(ctrl) - d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator, symlinkFactory, errorLogger, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter.Call, virtual.NoNamedAttributesFactory) - - t.Run("FailureInitialContentsFetcher", func(t *testing.T) { - // Create a subdirectory that has an initial contents fetcher. - inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) - initialContentsFetcher := mock.NewMockInitialContentsFetcher(ctrl) - require.NoError(t, d.CreateChildren(map[path.Component]virtual.InitialChild{ - path.MustNewComponent("subdir"): virtual.InitialChild{}.FromDirectory(initialContentsFetcher), - }, false)) - - child, err := d.LookupChild(path.MustNewComponent("subdir")) - require.NoError(t, err) - - childDirectory, childLeaf := child.GetPair() - require.NotNil(t, childDirectory) - require.Nil(t, childLeaf) - - // Creating a symlink in a directory whose initial - // contents cannot be fetched, should fail. The reason - // being that we can't accurately determine whether a - // file under that name is already present. - initialContentsFetcher.EXPECT().FetchContents(gomock.Any()). - Return(nil, status.Error(codes.Internal, "Network error")) - errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Internal, "Failed to initialize directory: Network error"))) - - _, _, s := childDirectory.VirtualSymlink(ctx, path.UNIXFormat.NewParser("target"), path.MustNewComponent("symlink"), 0, &virtual.Attributes{}) - require.Equal(t, virtual.StatusErrIO, s) - }) - - t.Run("FailureExist", func(t *testing.T) { - // The operation should fail if a file or directory - // already exists under the provided name. - existingFile := mock.NewMockLinkableLeaf(ctrl) - require.NoError(t, d.CreateChildren(map[path.Component]virtual.InitialChild{ - path.MustNewComponent("existing_file"): virtual.InitialChild{}.FromLeaf(existingFile), - }, false)) - - _, _, s := d.VirtualSymlink(ctx, path.UNIXFormat.NewParser("target"), path.MustNewComponent("existing_file"), 0, &virtual.Attributes{}) - require.Equal(t, virtual.StatusErrExist, s) - }) - - t.Run("FailingSymlinkFactory", func(t *testing.T) { - targetPathParser := path.UNIXFormat.NewParser("target") - symlinkFactory.EXPECT().LookupSymlink(targetPathParser).Return(nil, status.Error(codes.Internal, "Not allowed")) - errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Internal, "Failed to create new symlink: Not allowed"))) - _, _, s := d.VirtualSymlink(ctx, targetPathParser, path.MustNewComponent("symlink"), 0, &virtual.Attributes{}) - require.Equal(t, virtual.StatusErrIO, s) - }) - - t.Run("Success", func(t *testing.T) { - leaf := mock.NewMockLinkableLeaf(ctrl) - targetPathParser := path.UNIXFormat.NewParser("target") - symlinkFactory.EXPECT().LookupSymlink(targetPathParser).Return(leaf, nil) - leaf.EXPECT().VirtualGetAttributes( - ctx, - virtual.AttributesMaskInodeNumber, - gomock.Any(), - ).Do(func(ctx context.Context, requested virtual.AttributesMask, attributes *virtual.Attributes) { - attributes.SetInodeNumber(3) - }) - - var out virtual.Attributes - actualLeaf, changeInfo, s := d.VirtualSymlink(ctx, targetPathParser, path.MustNewComponent("symlink"), virtual.AttributesMaskInodeNumber, &out) - require.Equal(t, virtual.StatusOK, s) - require.NotNil(t, actualLeaf) - require.Equal(t, virtual.ChangeInfo{ - Before: 2, - After: 3, - }, changeInfo) - require.Equal(t, (&virtual.Attributes{}).SetInodeNumber(3), &out) - }) -} diff --git a/pkg/filesystem/virtual/nfsv4/nfs40_program.go b/pkg/filesystem/virtual/nfsv4/nfs40_program.go index cd9cad0e..f4cc1d9e 100644 --- a/pkg/filesystem/virtual/nfsv4/nfs40_program.go +++ b/pkg/filesystem/virtual/nfsv4/nfs40_program.go @@ -1098,17 +1098,13 @@ func (s *compoundState) opCreate(ctx context.Context, args *nfsv4.Create4args) n leaf, changeInfo, vs = currentDirectory.VirtualMknod(ctx, name, &createAttributes, virtual.AttributesMaskFileHandle, &actualAttributes) fileHandle.node = virtual.DirectoryChild{}.FromLeaf(leaf) case *nfsv4.Createtype4_NF4LNK: + createAttributes.SetFileType(filesystem.FileTypeSymlink) if !utf8.Valid(objectType.Linkdata) { return &nfsv4.Create4res_default{Status: nfsv4.NFS4ERR_BADCHAR} } + createAttributes.SetSymlinkTarget(path.UNIXFormat.NewParser(string(objectType.Linkdata))) var leaf virtual.Leaf - leaf, changeInfo, vs = currentDirectory.VirtualSymlink( - ctx, - path.UNIXFormat.NewParser(string(objectType.Linkdata)), - name, - virtual.AttributesMaskFileHandle, - &actualAttributes, - ) + leaf, changeInfo, vs = currentDirectory.VirtualMknod(ctx, name, &createAttributes, virtual.AttributesMaskFileHandle, &actualAttributes) fileHandle.node = virtual.DirectoryChild{}.FromLeaf(leaf) case *nfsv4.Createtype4_NF4SOCK: createAttributes.SetFileType(filesystem.FileTypeSocket) diff --git a/pkg/filesystem/virtual/nfsv4/nfs40_program_test.go b/pkg/filesystem/virtual/nfsv4/nfs40_program_test.go index 477bbe9e..8182ca42 100644 --- a/pkg/filesystem/virtual/nfsv4/nfs40_program_test.go +++ b/pkg/filesystem/virtual/nfsv4/nfs40_program_test.go @@ -1184,10 +1184,10 @@ func TestNFS40ProgramCompound_OP_CREATE(t *testing.T) { }) t.Run("SymlinkFailure", func(t *testing.T) { - rootDirectory.EXPECT().VirtualSymlink( + rootDirectory.EXPECT().VirtualMknod( ctx, - gomock.Any(), path.MustNewComponent("symlink"), + gomock.Any(), virtual.AttributesMaskFileHandle, gomock.Any(), ).Return(nil, virtual.ChangeInfo{}, virtual.StatusErrAccess) @@ -1227,15 +1227,18 @@ func TestNFS40ProgramCompound_OP_CREATE(t *testing.T) { t.Run("SymlinkSuccess", func(t *testing.T) { leaf := mock.NewMockVirtualLeaf(ctrl) - rootDirectory.EXPECT().VirtualSymlink( + rootDirectory.EXPECT().VirtualMknod( ctx, - gomock.Any(), path.MustNewComponent("symlink"), + gomock.Any(), virtual.AttributesMaskFileHandle, gomock.Any(), - ).DoAndReturn(func(ctx context.Context, target path.Parser, name path.Component, requested virtual.AttributesMask, attributes *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + ).DoAndReturn(func(ctx context.Context, name path.Component, createAttributes *virtual.Attributes, requested virtual.AttributesMask, attributes *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + require.Equal(t, filesystem.FileTypeSymlink, createAttributes.GetFileType()) + symlinkTarget, ok := createAttributes.GetSymlinkTarget() + require.True(t, ok) targetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) - require.NoError(t, path.Resolve(target, scopeWalker)) + require.NoError(t, path.Resolve(symlinkTarget, scopeWalker)) require.Equal(t, "target", targetBuilder.GetUNIXString()) attributes.SetFileHandle([]byte{0xbe, 0xb7, 0xe9, 0xb1, 0xbb, 0x21, 0x9a, 0xa8}) diff --git a/pkg/filesystem/virtual/nfsv4/nfs41_program.go b/pkg/filesystem/virtual/nfsv4/nfs41_program.go index 62a24afb..ee60012f 100644 --- a/pkg/filesystem/virtual/nfsv4/nfs41_program.go +++ b/pkg/filesystem/virtual/nfsv4/nfs41_program.go @@ -1918,17 +1918,13 @@ func (s *sequenceState) opCreate(ctx context.Context, args *nfsv4.Create4args) n leaf, changeInfo, vs = currentDirectory.VirtualMknod(ctx, name, &createAttributes, virtual.AttributesMaskFileHandle, &actualAttributes) fileHandle.node = virtual.DirectoryChild{}.FromLeaf(leaf) case *nfsv4.Createtype4_NF4LNK: + createAttributes.SetFileType(filesystem.FileTypeSymlink) if !utf8.Valid(objectType.Linkdata) { return &nfsv4.Create4res_default{Status: nfsv4.NFS4ERR_BADCHAR} } + createAttributes.SetSymlinkTarget(path.UNIXFormat.NewParser(string(objectType.Linkdata))) var leaf virtual.Leaf - leaf, changeInfo, vs = currentDirectory.VirtualSymlink( - ctx, - s.program.pathFormat.NewParser(string(objectType.Linkdata)), - name, - virtual.AttributesMaskFileHandle, - &actualAttributes, - ) + leaf, changeInfo, vs = currentDirectory.VirtualMknod(ctx, name, &createAttributes, virtual.AttributesMaskFileHandle, &actualAttributes) fileHandle.node = virtual.DirectoryChild{}.FromLeaf(leaf) case *nfsv4.Createtype4_NF4SOCK: createAttributes.SetFileType(filesystem.FileTypeSocket) diff --git a/pkg/filesystem/virtual/read_only_directory.go b/pkg/filesystem/virtual/read_only_directory.go index a6059657..e5b52306 100644 --- a/pkg/filesystem/virtual/read_only_directory.go +++ b/pkg/filesystem/virtual/read_only_directory.go @@ -57,12 +57,6 @@ func (ReadOnlyDirectory) VirtualSetAttributes(ctx context.Context, in *Attribute return StatusErrROFS } -// VirtualSymlink is an implementation of the symlink() system call that -// treats the target directory as being read-only. -func (ReadOnlyDirectory) VirtualSymlink(ctx context.Context, pointedTo path.Parser, linkName path.Component, requested AttributesMask, out *Attributes) (Leaf, ChangeInfo, Status) { - return nil, ChangeInfo{}, StatusErrROFS -} - // ReadOnlyDirectoryOpenChildWrongFileType is a helper function for // implementing Directory.VirtualOpenChild() for read-only directories. // It can be used to obtain return values in case the directory already diff --git a/pkg/filesystem/virtual/winfsp/file_system.go b/pkg/filesystem/virtual/winfsp/file_system.go index f6445711..6a52b1da 100644 --- a/pkg/filesystem/virtual/winfsp/file_system.go +++ b/pkg/filesystem/virtual/winfsp/file_system.go @@ -1455,7 +1455,15 @@ func (fs *FileSystem) SetReparsePoint(ref *ffi.FileSystemRef, handle uintptr, na return toNTStatus(s) } var outAttributes virtual.Attributes - if _, _, s := node.parent.VirtualSymlink(ctx, path.LocalFormat.NewParser(targetPath), node.name, virtual.AttributesMaskFileType, &outAttributes); s != virtual.StatusOK { + if _, _, s := node.parent.VirtualMknod( + ctx, + node.name, + (&virtual.Attributes{}). + SetFileType(filesystem.FileTypeSymlink). + SetSymlinkTarget(path.LocalFormat.NewParser(targetPath)), + virtual.AttributesMaskFileType, + &outAttributes, + ); s != virtual.StatusOK { return toNTStatus(s) } diff --git a/pkg/filesystem/virtual/winfsp/file_system_test.go b/pkg/filesystem/virtual/winfsp/file_system_test.go index aa817f62..bb97a1e6 100644 --- a/pkg/filesystem/virtual/winfsp/file_system_test.go +++ b/pkg/filesystem/virtual/winfsp/file_system_test.go @@ -1985,13 +1985,16 @@ func TestWinFSPFileSystemSymlinkCreation(t *testing.T) { true, ).Return(virtual.ChangeInfo{}, virtual.StatusOK) - rootDirectory.EXPECT().VirtualSymlink( - gomock.Any(), + rootDirectory.EXPECT().VirtualMknod( gomock.Any(), path.MustNewComponent("symlink.txt"), + gomock.Any(), virtual.AttributesMaskFileType, gomock.Any(), - ).DoAndReturn(func(ctx context.Context, target path.Parser, name path.Component, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + ).DoAndReturn(func(ctx context.Context, name path.Component, createAttributes *virtual.Attributes, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + require.Equal(t, filesystem.FileTypeSymlink, createAttributes.GetFileType()) + target, ok := createAttributes.GetSymlinkTarget() + require.True(t, ok) targetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) require.NoError(t, path.Resolve(target, scopeWalker)) targetStr, err := path.LocalFormat.GetString(targetBuilder) @@ -2050,13 +2053,16 @@ func TestWinFSPFileSystemSymlinkCreation(t *testing.T) { true, ).Return(virtual.ChangeInfo{}, virtual.StatusOK) - rootDirectory.EXPECT().VirtualSymlink( - gomock.Any(), + rootDirectory.EXPECT().VirtualMknod( gomock.Any(), path.MustNewComponent("abs_symlink.txt"), + gomock.Any(), virtual.AttributesMaskFileType, gomock.Any(), - ).DoAndReturn(func(ctx context.Context, target path.Parser, name path.Component, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + ).DoAndReturn(func(ctx context.Context, name path.Component, createAttributes *virtual.Attributes, requested virtual.AttributesMask, out *virtual.Attributes) (virtual.Leaf, virtual.ChangeInfo, virtual.Status) { + require.Equal(t, filesystem.FileTypeSymlink, createAttributes.GetFileType()) + target, ok := createAttributes.GetSymlinkTarget() + require.True(t, ok) targetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) require.NoError(t, path.Resolve(target, scopeWalker)) targetStr, err := path.LocalFormat.GetString(targetBuilder)