diff --git a/cmd/bb_worker/main.go b/cmd/bb_worker/main.go index 0dd047e9..6b8c37b8 100644 --- a/cmd/bb_worker/main.go +++ b/cmd/bb_worker/main.go @@ -219,16 +219,16 @@ func main() { normalizer = virtual.CaseInsensitiveComponentNormalizer } + defaultAttributesSetter := func(requested virtual.AttributesMask, attributes *virtual.Attributes) { + // No need to set ownership attributes + // on the top-level directory. + } symlinkFactory = virtual.NewHandleAllocatingSymlinkFactory( - virtual.BaseSymlinkFactory, + virtual.NewBaseSymlinkFactory(defaultAttributesSetter), handleAllocator.New()) characterDeviceFactory = virtual.NewHandleAllocatingCharacterDeviceFactory( virtual.BaseCharacterDeviceFactory, handleAllocator.New()) - defaultAttributesSetter := func(requested virtual.AttributesMask, attributes *virtual.Attributes) { - // No need to set ownership attributes - // on the top-level directory. - } virtualBuildDirectory = virtual.NewInMemoryPrepopulatedDirectory( virtual.NewHandleAllocatingFileAllocator( virtual.NewPoolBackedFileAllocator( @@ -343,6 +343,13 @@ func main() { } runnerClient := runner_pb.NewRunnerClient(runnerConnection) + defaultAttributesSetter := func(requested virtual.AttributesMask, attributes *virtual.Attributes) { + attributes.SetOwnerUserID(runnerConfiguration.BuildDirectoryOwnerUserId) + attributes.SetOwnerGroupID(runnerConfiguration.BuildDirectoryOwnerGroupId) + } + symlinkFactory := virtual.NewHandleAllocatingSymlinkFactory( + virtual.NewBaseSymlinkFactory(defaultAttributesSetter), + handleAllocator.New()) for threadID := uint64(0); threadID < runnerConfiguration.Concurrency; threadID++ { // Per-worker separate writer of the Content // Addressable Storage that batches writes after @@ -391,10 +398,7 @@ func main() { symlinkFactory, characterDeviceFactory, handleAllocator, - /* defaultAttributesSetter = */ func(requested virtual.AttributesMask, attributes *virtual.Attributes) { - attributes.SetOwnerUserID(runnerConfiguration.BuildDirectoryOwnerUserId) - attributes.SetOwnerGroupID(runnerConfiguration.BuildDirectoryOwnerGroupId) - }, + defaultAttributesSetter, clock.SystemClock, ) } else { diff --git a/pkg/builder/virtual_build_directory.go b/pkg/builder/virtual_build_directory.go index 5ad71c9b..331ce5f8 100644 --- a/pkg/builder/virtual_build_directory.go +++ b/pkg/builder/virtual_build_directory.go @@ -110,6 +110,7 @@ func (d *virtualBuildDirectory) InstallHooks(filePool pool.FilePool, errorLogger ), do.handleAllocator, ), + do.symlinkFactory, errorLogger, do.defaultAttributesSetter, namedAttributesFactory, diff --git a/pkg/filesystem/virtual/base_symlink_factory.go b/pkg/filesystem/virtual/base_symlink_factory.go index fd030110..5d20c9ec 100644 --- a/pkg/filesystem/virtual/base_symlink_factory.go +++ b/pkg/filesystem/virtual/base_symlink_factory.go @@ -13,19 +13,30 @@ import ( "google.golang.org/grpc/status" ) -type symlinkFactory struct{} +type symlinkFactory struct { + defaultAttributesSetter DefaultAttributesSetter +} -func (symlinkFactory) LookupSymlink(target []byte) LinkableLeaf { - return symlink{target: target} +func (f *symlinkFactory) LookupSymlink(target []byte) LinkableLeaf { + return symlink{ + defaultAttributesSetter: f.defaultAttributesSetter, + target: target, + } } -// BaseSymlinkFactory can be used to create simple immutable symlink nodes. -var BaseSymlinkFactory SymlinkFactory = symlinkFactory{} +// NewBaseSymlinkFactory creates a SymlinkFactory that can be used to create +// simple immutable symlink nodes. +func NewBaseSymlinkFactory(defaultAttributesSetter DefaultAttributesSetter) SymlinkFactory { + return &symlinkFactory{ + defaultAttributesSetter: defaultAttributesSetter, + } +} type symlink struct { placeholderFile - target []byte + defaultAttributesSetter DefaultAttributesSetter + target []byte } func (f symlink) readlinkParser() (path.Parser, error) { @@ -48,6 +59,7 @@ func (f symlink) readlinkString() (string, error) { } func (f symlink) VirtualGetAttributes(ctx context.Context, requested AttributesMask, attributes *Attributes) { + f.defaultAttributesSetter(requested, attributes) attributes.SetChangeID(0) attributes.SetFileType(filesystem.FileTypeSymlink) attributes.SetHasNamedAttributes(false) diff --git a/pkg/filesystem/virtual/in_memory_prepopulated_directory.go b/pkg/filesystem/virtual/in_memory_prepopulated_directory.go index 2c683fd9..4c6257da 100644 --- a/pkg/filesystem/virtual/in_memory_prepopulated_directory.go +++ b/pkg/filesystem/virtual/in_memory_prepopulated_directory.go @@ -25,7 +25,6 @@ type StringMatcher func(s string) bool // inMemoryFilesystem contains state that is shared across all // inMemoryPrepopulatedDirectory objects that form a single hierarchy. type inMemoryFilesystem struct { - symlinkFactory SymlinkFactory statefulHandleAllocator StatefulHandleAllocator initialContentsSorter Sorter hiddenFilesMatcher StringMatcher @@ -44,6 +43,7 @@ type inMemoryFilesystem struct { type inMemorySubtree struct { filesystem *inMemoryFilesystem fileAllocator FileAllocator + symlinkFactory SymlinkFactory errorLogger util.ErrorLogger defaultAttributesSetter DefaultAttributesSetter namedAttributesFactory NamedAttributesFactory @@ -52,7 +52,6 @@ type inMemorySubtree struct { func newInMemorySubtree(fileAllocator FileAllocator, symlinkFactory SymlinkFactory, errorLogger util.ErrorLogger, handleAllocator StatefulHandleAllocator, initialContentsSorter Sorter, hiddenFilesMatcher StringMatcher, clock clock.Clock, normalizer ComponentNormalizer, defaultAttributesSetter DefaultAttributesSetter, namedAttributesFactory NamedAttributesFactory) *inMemorySubtree { return &inMemorySubtree{ filesystem: &inMemoryFilesystem{ - symlinkFactory: symlinkFactory, statefulHandleAllocator: handleAllocator, initialContentsSorter: initialContentsSorter, hiddenFilesMatcher: hiddenFilesMatcher, @@ -60,6 +59,7 @@ func newInMemorySubtree(fileAllocator FileAllocator, symlinkFactory SymlinkFacto clock: clock, }, fileAllocator: fileAllocator, + symlinkFactory: symlinkFactory, errorLogger: errorLogger, defaultAttributesSetter: defaultAttributesSetter, namedAttributesFactory: namedAttributesFactory, @@ -529,13 +529,14 @@ func (i *inMemoryPrepopulatedDirectory) postRemoveChildren(entries *inMemoryDire } } -func (i *inMemoryPrepopulatedDirectory) InstallHooks(fileAllocator FileAllocator, errorLogger util.ErrorLogger, defaultAttributesSetter DefaultAttributesSetter, namedAttributesFactory NamedAttributesFactory) { +func (i *inMemoryPrepopulatedDirectory) InstallHooks(fileAllocator FileAllocator, symlinkFactory SymlinkFactory, errorLogger util.ErrorLogger, defaultAttributesSetter DefaultAttributesSetter, namedAttributesFactory NamedAttributesFactory) { i.lock.Lock() defer i.lock.Unlock() i.subtree = &inMemorySubtree{ filesystem: i.subtree.filesystem, fileAllocator: fileAllocator, + symlinkFactory: symlinkFactory, errorLogger: errorLogger, defaultAttributesSetter: defaultAttributesSetter, namedAttributesFactory: namedAttributesFactory, @@ -1121,7 +1122,7 @@ func (i *inMemoryPrepopulatedDirectory) VirtualSymlink(ctx context.Context, poin if s := contents.virtualMayAttach(normalizedLinkName); s != StatusOK { return nil, ChangeInfo{}, s } - child := i.subtree.filesystem.symlinkFactory.LookupSymlink(pointedTo) + child := i.subtree.symlinkFactory.LookupSymlink(pointedTo) changeIDBefore := contents.changeID contents.attach(i.subtree, linkName, normalizedLinkName, inMemoryDirectoryChild{}.FromLeaf(child)) diff --git a/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go b/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go index 4bb04226..2d1397eb 100644 --- a/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go +++ b/pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go @@ -404,9 +404,10 @@ func TestInMemoryPrepopulatedDirectoryInstallHooks(t *testing.T) { defaultAttributesSetter1 := mock.NewMockDefaultAttributesSetter(ctrl) d := virtual.NewInMemoryPrepopulatedDirectory(fileAllocator1, symlinkFactory1, errorLogger1, handleAllocator, sort.Sort, hiddenFilesPatternForTesting.MatchString, clock.SystemClock, virtual.CaseSensitiveComponentNormalizer, defaultAttributesSetter1.Call, virtual.NoNamedAttributesFactory) fileAllocator2 := mock.NewMockFileAllocator(ctrl) + symlinkFactory2 := mock.NewMockSymlinkFactory(ctrl) errorLogger2 := mock.NewMockErrorLogger(ctrl) defaultAttributesSetter2 := mock.NewMockDefaultAttributesSetter(ctrl) - d.InstallHooks(fileAllocator2, errorLogger2, defaultAttributesSetter2.Call, virtual.NoNamedAttributesFactory) + d.InstallHooks(fileAllocator2, symlinkFactory2, errorLogger2, defaultAttributesSetter2.Call, virtual.NoNamedAttributesFactory) // Validate that the top-level directory uses both the new file // allocator and error logger. @@ -424,6 +425,27 @@ func TestInMemoryPrepopulatedDirectoryInstallHooks(t *testing.T) { &attr) require.Equal(t, virtual.StatusErrIO, s) + // Validate that symlinks uses the new symlink allocator + // and error logger as well. + symlinkLeaf := mock.NewMockLinkableLeaf(ctrl) + symlinkFactory2.EXPECT().LookupSymlink([]byte("target")).Return(symlinkLeaf) + symlinkLeaf.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, []byte("target"), path.MustNewComponent("symlink"), virtual.AttributesMaskInodeNumber, &out) + require.Equal(t, virtual.StatusOK, s) + require.NotNil(t, actualLeaf) + require.Equal(t, virtual.ChangeInfo{ + Before: 0, + After: 1, + }, changeInfo) + require.Equal(t, (&virtual.Attributes{}).SetInodeNumber(3), &out) + // Validate that a subdirectory uses the new file allocator // and error logger as well. inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator) diff --git a/pkg/filesystem/virtual/prepopulated_directory.go b/pkg/filesystem/virtual/prepopulated_directory.go index 0274346e..8686c53b 100644 --- a/pkg/filesystem/virtual/prepopulated_directory.go +++ b/pkg/filesystem/virtual/prepopulated_directory.go @@ -99,7 +99,7 @@ type PrepopulatedDirectory interface { // This function is identical to BuildDirectory.InstallHooks(), // except that it uses the FUSE specific FileAllocator instead // of FilePool. - InstallHooks(fileAllocator FileAllocator, errorLogger util.ErrorLogger, defaultAttributesSetter DefaultAttributesSetter, namedAttributesFactory NamedAttributesFactory) + InstallHooks(fileAllocator FileAllocator, symlinkFactory SymlinkFactory, errorLogger util.ErrorLogger, defaultAttributesSetter DefaultAttributesSetter, namedAttributesFactory NamedAttributesFactory) // FilterChildren() can be used to traverse over all of the // InitialContentsFetcher and LinkableLeaf objects stored in this // directory hierarchy. For each of the objects, a callback is diff --git a/pkg/filesystem/virtual/winfsp/file_system_integration_test.go b/pkg/filesystem/virtual/winfsp/file_system_integration_test.go index 3af7006e..53576ef8 100644 --- a/pkg/filesystem/virtual/winfsp/file_system_integration_test.go +++ b/pkg/filesystem/virtual/winfsp/file_system_integration_test.go @@ -92,7 +92,7 @@ func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, case handleAllocator, ), virtual.NewHandleAllocatingSymlinkFactory( - virtual.BaseSymlinkFactory, + virtual.NewBaseSymlinkFactory(defaultAttributesSetter), handleAllocator.New(), ), util.DefaultErrorLogger,