Skip to content

Commit

Permalink
[local] Refuse to create lakectl locals on case-insensitive filesyste…
Browse files Browse the repository at this point in the history
…ms (#7650)

* Add case-insensitive filesystem checker

* Add case-insensitive check to `lakectl local clone`

Still need to add it to `lakectl local init`.

To check this (on Linux):

```sh
dd if=/dev/zero of=/tmp/my-loopback bs=1k count=10000
sudo losetup -f /tmp/my-loopback
losetup --all
mkfs -t ext4 -O casefold -E encoding_flag=strict /dev/loop11
mkdir /tmp/ifs; sudo mount /dev/loop11 /tmp/ifs/

mkdir /tmp/ifs/dir
chattr +F /tmp/ifs/dir

```

Obviously this won't really work in a CI/CD container, where there are
externalities that we do not control.

* Mention that Git also fails on a case-insensitive filesystem

* [lint] Drop unneeded "_" receiver

* [CR] Rename --force to --allow-case-insensitive

* [CR] *Only* warn on case-insensitive filesystem, never fail

- Don't break existing behaviour
- Stay compatible with Git, which does even less
- Don't tell off Windows and MacOS default filesystems

* [CR] Change order of warning messages for case insensitivity

Warn first if we failed to detect.  This will always produce the same
output, and is easier for readers.

* [CR] Warn when failed to file used to test case insensitivity
  • Loading branch information
arielshaqed authored Apr 17, 2024
1 parent 2bd5a90 commit 173d9de
Show file tree
Hide file tree
Showing 5 changed files with 246 additions and 0 deletions.
14 changes: 14 additions & 0 deletions cmd/lakectl/cmd/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/spf13/cobra"
"github.com/treeverse/lakefs/pkg/api/apigen"
"github.com/treeverse/lakefs/pkg/fileutil"
"github.com/treeverse/lakefs/pkg/local"
"github.com/treeverse/lakefs/pkg/uri"
"golang.org/x/sync/errgroup"
Expand All @@ -28,6 +29,8 @@ const (
pullOperation LocalOperation = "pull"
checkoutOperation LocalOperation = "checkout"
cloneOperation LocalOperation = "clone"

CaseInsensitiveWarningMessageFormat = `Directory '%s' is case-insensitive, versioning tools such as lakectl local and git will work incorrectly.`
)

const localSummaryTemplate = `
Expand Down Expand Up @@ -114,6 +117,17 @@ Use "lakectl local checkout..." to sync with the remote or run "lakectl local cl
}
}

func warnOnCaseInsensitiveDirectory(path string) {
isCaseInsensitive, err := fileutil.IsCaseInsensitiveLocation(fileutil.OSFS{}, path, Warning)
if err != nil {
Warning(fmt.Sprintf("Check whether directory '%s' is case-insensitive: %s", path, err))
Warning("Continuing without this check")
}
if isCaseInsensitive {
Warning(fmt.Sprintf(CaseInsensitiveWarningMessageFormat, path))
}
}

var localCmd = &cobra.Command{
Use: "local",
Short: "Sync local directories with lakeFS paths",
Expand Down
2 changes: 2 additions & 0 deletions cmd/lakectl/cmd/local_clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ var localCloneCmd = &cobra.Command{
DieFmt("directory '%s' exists and is not empty", localPath)
}

warnOnCaseInsensitiveDirectory(localPath)

ctx := cmd.Context()
head, err := localInit(ctx, localPath, remote, false, updateIgnore)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions cmd/lakectl/cmd/local_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ var localInitCmd = &cobra.Command{
remote, localPath := getSyncArgs(args, true, false)
force := Must(cmd.Flags().GetBool(localForceFlagName))
updateIgnore := Must(cmd.Flags().GetBool(localGitIgnoreFlagName))

warnOnCaseInsensitiveDirectory(localPath)

_, err := localInit(cmd.Context(), localPath, remote, force, updateIgnore)
if err != nil {
if errors.Is(err, fs.ErrExist) {
Expand Down
86 changes: 86 additions & 0 deletions pkg/fileutil/fs_case.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package fileutil

import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"

nanoid "github.com/matoous/go-nanoid/v2"
)

// caseSensitiveNamePrefix is a prefix used for testing case sensitivity.
// It contains both lowercase and uppercase characters.
const caseSensitiveNamePrefix = ".cAsE."

// IsCaseInsensitiveLocation returns true if dirPath is a directory on a
// case-insensitive filesystem. dirPath must be writable. If it fails to
// delete the file, it uses warnFunc to emit a warning message.
func IsCaseInsensitiveLocation(fs FS, dirPath string, warnFunc func(string)) (bool, error) {
// Random element in the filename to ensure uniqueness: the filename
// will not be in use from anything else.
id, err := nanoid.New()
if err != nil {
return false, fmt.Errorf("generate random name: %w", err)
}
path := filepath.Join(dirPath, caseSensitiveNamePrefix+id)
err = fs.Touch(path)
if err != nil {
return false, fmt.Errorf("touch %s: %w", path, err)
}
defer func() {
err := fs.Remove(path)
if err != nil {
warnFunc(fmt.Sprintf("Garbage file %s remains: %s; make sure to delete this file", path, err))
}
}()

lowercasePrefix := strings.ToLower(caseSensitiveNamePrefix)
lowercasePath := filepath.Join(dirPath, lowercasePrefix+id)
// If lowercasePath exists, fs is case-insensitive. Random id
// ensures that it can be no other file!
return fs.Exists(lowercasePath)
}

// FS is a tiny filesystem abstraction. The standard io/fs does not support
// any write operations, see https://github.com/golang/go/issues/45757.
type FS interface {
// Touch creates a file at path.
Touch(path string) error
// Exists returns true if there is a file at path. It follows
// symbolic links.
Exists(path string) (bool, error)
// Remove deletes the file at path.
Remove(path string) error
}

// OSFS is the filesystem of the OS.
type OSFS struct{}

func NewOSFS() FS {
return OSFS{}
}

func (OSFS) Touch(path string) error {
file, err := os.Create(path)
if err != nil {
return err
}
return file.Close()
}

func (OSFS) Exists(path string) (bool, error) {
_, err := os.Stat(path)
if errors.Is(err, os.ErrNotExist) {
return false, nil
}
if err != nil {
return false, err
}
return true, nil
}

func (OSFS) Remove(path string) error {
return os.Remove(path)
}
141 changes: 141 additions & 0 deletions pkg/fileutil/fs_case_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package fileutil_test

import (
"errors"
"os"
"strings"
"testing"

"github.com/treeverse/lakefs/pkg/fileutil"
)

func fatal(t testing.TB) func(string) {
return func(message string) {
t.Fatal(message)
}
}

type nothing struct{}

type PathSet map[string]nothing

// PathMapFS fakes a fileutil.FS that maps all pathnames through a mapping.
type PathMapFS struct {
PathMapper func(string) string
Paths PathSet

// If non-nil, return this error for any operation on a path that
// starts with ErrorPathPrefix.
Err error
// Prefix to use for reporting Err.
ErrorPathPrefix string
}

func (f *PathMapFS) Touch(path string) error {
path = f.PathMapper(path)
if f.Err != nil && strings.HasPrefix(path, f.ErrorPathPrefix) {
return f.Err
}

if _, ok := f.Paths[path]; ok {
return os.ErrExist
}
f.Paths[path] = nothing{}
return nil
}

func (f *PathMapFS) Exists(path string) (bool, error) {
path = f.PathMapper(path)
if f.Err != nil && strings.HasPrefix(path, f.ErrorPathPrefix) {
return false, f.Err
}

_, ok := f.Paths[path]
return ok, nil
}

func (f *PathMapFS) Remove(path string) error {
path = f.PathMapper(path)
if f.Err != nil && strings.HasPrefix(path, f.ErrorPathPrefix) {
return f.Err
}

if _, ok := f.Paths[path]; ok {
delete(f.Paths, path)
return nil
}
return os.ErrNotExist
}

// CaseSensitiveFS fakes a case-sensitive fileutil.FS, that returns errors
// for some paths.
func CaseSensitiveFS() *PathMapFS {
return &PathMapFS{
PathMapper: func(p string) string { return p },
Paths: make(PathSet),
}
}

// CaseInsensitiveFS fakes a case-sensitive fileutil.FS.
func CaseInsensitiveFS() *PathMapFS {
return &PathMapFS{
PathMapper: func(p string) string { return strings.ToLower(p) },
Paths: make(PathSet),
}
}

func TestIsCaseInsensitiveLocationFalse(t *testing.T) {
fs := CaseSensitiveFS()

insensitive, err := fileutil.IsCaseInsensitiveLocation(fs, "/home/me/dir", fatal(t))
if insensitive {
t.Error("Expected case-sensitive FS to be reported as such")
}
if err != nil {
t.Errorf("Failed to test case-sensitive FS: %s", err)
}
}

func TestIsCaseInsensitiveLocationTrue(t *testing.T) {
fs := CaseInsensitiveFS()

insensitive, err := fileutil.IsCaseInsensitiveLocation(fs, "/home/me/dir", fatal(t))
if !insensitive {
t.Error("Expected case-insensitive FS to be reported as such")
}
if err != nil {
t.Errorf("Failed to test case-sensitive FS: %s", err)
}
}

func TestIsCaseInsensitiveLocationError(t *testing.T) {
testingErr := errors.New("for testing")

fs := CaseSensitiveFS()
fs.Err = testingErr
fs.ErrorPathPrefix = "/home/me/err/"

_, err := fileutil.IsCaseInsensitiveLocation(fs, "/home/me/err/", fatal(t))
if !errors.Is(err, testingErr) {
t.Errorf("Got error %s when expecting %s", err, testingErr)
}
}

// TestOSIsCaseInsensitiveLocation tests that IsCaseInsensitiveLocation
// works on the OS. It cannot test the result, as it does not know what to
// expect.
func TestOSIsCaseInsensitiveLocation(t *testing.T) {
fs := fileutil.NewOSFS()
tempDir := t.TempDir()
isCaseInsensitive, err := fileutil.IsCaseInsensitiveLocation(fs, tempDir, fatal(t))

if err != nil {
t.Errorf("IsCaseInsensitiveLocation failed: %s", err)
}

if isCaseInsensitive {
t.Logf("Case-insensitive directory: %s", tempDir)
} else {
t.Logf("Case-sensitive directory: %s", tempDir)
}
}

0 comments on commit 173d9de

Please sign in to comment.