diff --git a/cmd/lakectl/cmd/local.go b/cmd/lakectl/cmd/local.go index 59d1650437e..00dd1198e7c 100644 --- a/cmd/lakectl/cmd/local.go +++ b/cmd/lakectl/cmd/local.go @@ -62,13 +62,12 @@ func withForceFlag(cmd *cobra.Command, usage string) { func localDiff(ctx context.Context, client apigen.ClientWithResponsesInterface, remote *uri.URI, path string) local.Changes { fmt.Printf("\ndiff 'local://%s' <--> '%s'...\n", path, remote) currentRemoteState := make(chan apigen.ObjectStats, maxDiffPageSize) + includePOSIXPermissions := cfg.Experimental.Local.POSIXPerm.Enabled var wg errgroup.Group wg.Go(func() error { - return local.ListRemote(ctx, client, remote, currentRemoteState) + return local.ListRemote(ctx, client, remote, currentRemoteState, includePOSIXPermissions) }) - includePOSIXPermissions := cfg.Experimental.Local.POSIXPerm.Enabled - changes, err := local.DiffLocalWithHead(currentRemoteState, path, includePOSIXPermissions, includePOSIXPermissions) if err != nil { DieErr(err) diff --git a/pkg/local/diff.go b/pkg/local/diff.go index cf3723a7ea8..ab775232ff7 100644 --- a/pkg/local/diff.go +++ b/pkg/local/diff.go @@ -339,7 +339,7 @@ func DiffLocalWithHead(left <-chan apigen.ObjectStats, rightPath string, include } // ListRemote - Lists objects from a remote uri and inserts them into the objects channel -func ListRemote(ctx context.Context, client apigen.ClientWithResponsesInterface, loc *uri.URI, objects chan<- apigen.ObjectStats) error { +func ListRemote(ctx context.Context, client apigen.ClientWithResponsesInterface, loc *uri.URI, objects chan<- apigen.ObjectStats, includeDirs bool) error { hasMore := true var after string defer func() { @@ -362,7 +362,7 @@ func ListRemote(ctx context.Context, client apigen.ClientWithResponsesInterface, for _, o := range listResp.JSON200.Results { p := strings.TrimPrefix(o.Path, loc.GetPath()) // skip directory markers - if p == "" || (strings.HasSuffix(p, uri.PathSeparator) && swag.Int64Value(o.SizeBytes) == 0) { + if !includeDirs && (p == "" || (strings.HasSuffix(p, uri.PathSeparator) && swag.Int64Value(o.SizeBytes) == 0)) { continue } p = strings.TrimPrefix(p, uri.PathSeparator) diff --git a/pkg/local/diff_test.go b/pkg/local/diff_test.go index 699243e4d39..6ad2b068791 100644 --- a/pkg/local/diff_test.go +++ b/pkg/local/diff_test.go @@ -8,6 +8,7 @@ import ( "slices" "sort" "strings" + "syscall" "testing" "time" @@ -17,15 +18,13 @@ import ( "github.com/treeverse/lakefs/pkg/local" ) -const ( - diffTestCorrectTime = 1691570412 - filePermModeFile = 0o100644 - localPermModeFolder = 0o40755 -) +const diffTestCorrectTime = 1691570412 func TestDiffLocal(t *testing.T) { osUid := os.Getuid() osGid := os.Getgid() + umask := syscall.Umask(0) + syscall.Umask(umask) cases := []struct { Name string @@ -63,27 +62,27 @@ func TestDiffLocal(t *testing.T) { Path: ".hidden-file", SizeBytes: swag.Int64(64), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, filePermModeFile), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultFilePermissions-umask), }, { Path: "sub/", SizeBytes: swag.Int64(1), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, localPermModeFolder), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultDirectoryPermissions-umask), }, { Path: "sub/f.txt", SizeBytes: swag.Int64(3), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, filePermModeFile), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultFilePermissions-umask), }, { Path: "sub/folder/", SizeBytes: swag.Int64(1), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, localPermModeFolder), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultDirectoryPermissions-umask), }, { Path: "sub/folder/f.txt", SizeBytes: swag.Int64(6), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, filePermModeFile), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultFilePermissions-umask), }, }, Expected: []*local.Change{}, @@ -110,7 +109,8 @@ func TestDiffLocal(t *testing.T) { { Path: "sub/f.txt", Type: local.ChangeTypeModified, - }, { + }, + { Path: "sub/folder/f.txt", Type: local.ChangeTypeModified, }, @@ -124,7 +124,8 @@ func TestDiffLocal(t *testing.T) { Path: ".hidden-file", SizeBytes: swag.Int64(64), Mtime: diffTestCorrectTime, - }, { + }, + { Path: "sub/folder/f.txt", SizeBytes: swag.Int64(6), Mtime: diffTestCorrectTime, @@ -210,6 +211,18 @@ func TestDiffLocal(t *testing.T) { }, }, Expected: []*local.Change{ + { + Path: ".hidden-file", + Type: local.ChangeTypeModified, + }, + { + Path: "sub/", + Type: local.ChangeTypeModified, + }, + { + Path: "sub/f.txt", + Type: local.ChangeTypeModified, + }, { Path: "sub/folder/", Type: local.ChangeTypeAdded, @@ -234,12 +247,12 @@ func TestDiffLocal(t *testing.T) { Path: "folder/", SizeBytes: swag.Int64(1), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid+1, osGid, localPermModeFolder), + Metadata: getPermissionsMetadata(osUid+1, osGid, local.DefaultDirectoryPermissions-umask), }, { Path: "folder/f.txt", SizeBytes: swag.Int64(6), Mtime: diffTestCorrectTime, - Metadata: getPermissionsMetadata(osUid, osGid, filePermModeFile), + Metadata: getPermissionsMetadata(osUid, osGid, local.DefaultFilePermissions-umask), }, }, Expected: []*local.Change{ @@ -310,7 +323,11 @@ func fixTime(t *testing.T, localPath string, includeDirs bool) { func fixUnixPermissions(t *testing.T, localPath string) { err := filepath.WalkDir(localPath, func(path string, d fs.DirEntry, err error) error { - return os.Chown(path, os.Getuid(), os.Getgid()) + perm := local.GetDefaultPermissions(d.IsDir()) + if err = os.Chown(path, os.Getuid(), os.Getgid()); err != nil { + return err + } + return os.Chmod(path, perm.Mode) }) require.NoError(t, err) } diff --git a/pkg/local/posix_permissions.go b/pkg/local/posix_permissions.go index c2274068d64..90c0c1cf86c 100644 --- a/pkg/local/posix_permissions.go +++ b/pkg/local/posix_permissions.go @@ -68,8 +68,13 @@ func GetDefaultPermissions(isDir bool) POSIXPermissions { } // getPermissionFromStats - Get POSIX mode and ownership from object metadata, fallback to default permissions in case metadata doesn't exist -func getPermissionFromStats(stats apigen.ObjectStats) (*POSIXPermissions, error) { - permissions := GetDefaultPermissions(strings.HasSuffix(stats.Path, uri.PathSeparator)) +// and withDefault is true +func getPermissionFromStats(stats apigen.ObjectStats, withDefault bool) (*POSIXPermissions, error) { + permissions := POSIXPermissions{} + if withDefault { + permissions = GetDefaultPermissions(strings.HasSuffix(stats.Path, uri.PathSeparator)) + } + if stats.Metadata != nil { posixPermissions, ok := stats.Metadata.Get(POSIXPermissionsMetadataKey) if ok { @@ -101,7 +106,7 @@ func isPermissionsChanged(localFileInfo os.FileInfo, remoteFileStats apigen.Obje return true } - remote, err := getPermissionFromStats(remoteFileStats) + remote, err := getPermissionFromStats(remoteFileStats, false) if err != nil { return true } diff --git a/pkg/local/sync.go b/pkg/local/sync.go index ffc604d1478..7361009f7bf 100644 --- a/pkg/local/sync.go +++ b/pkg/local/sync.go @@ -213,7 +213,7 @@ func (s *SyncManager) download(ctx context.Context, rootPath string, remote *uri if err := fileutil.VerifyRelPath(strings.TrimPrefix(path, uri.PathSeparator), rootPath); err != nil { return err } - destination := fmt.Sprintf("%s%c%s", rootPath, os.PathSeparator, path) + destination := filepath.ToSlash(fmt.Sprintf("%s%s%s", rootPath, uri.PathSeparator, path)) destinationDirectory := filepath.Dir(destination) if err := os.MkdirAll(destinationDirectory, os.FileMode(DefaultDirectoryPermissions)); err != nil { @@ -243,7 +243,7 @@ func (s *SyncManager) download(ctx context.Context, rootPath string, remote *uri var perm *POSIXPermissions isDir := strings.HasSuffix(path, uri.PathSeparator) if s.includePerm { // Optimization - fail on to get permissions from metadata before having to download the entire file - if perm, err = getPermissionFromStats(objStat); err != nil { + if perm, err = getPermissionFromStats(objStat, true); err != nil { return err } } else if isDir { @@ -277,7 +277,7 @@ func (s *SyncManager) upload(ctx context.Context, rootPath string, remote *uri.U if err := fileutil.VerifySafeFilename(source); err != nil { return err } - dest := filepath.ToSlash(filepath.Join(remote.GetPath(), path)) + dest := strings.TrimPrefix(filepath.ToSlash(fmt.Sprintf("%s%s%s", remote.GetPath(), uri.PathSeparator, path)), uri.PathSeparator) f, err := os.Open(source) if err != nil {