Get rid of Directory.VirtualSymlink()

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 (

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)
 	}

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)

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.