From 9caa238bc70244592bc80e34b69a4209c21e9a25 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 10:40:21 +0300 Subject: [PATCH 1/6] Add esti tests for lakectl local with POSIX permissions --- esti/lakectl_local_test.go | 33 +++++++++++++++++++++++++++++++++ esti/lakectl_util.go | 9 +++++++++ 2 files changed, 42 insertions(+) diff --git a/esti/lakectl_local_test.go b/esti/lakectl_local_test.go index 395e5111dd2..950ac855d3b 100644 --- a/esti/lakectl_local_test.go +++ b/esti/lakectl_local_test.go @@ -255,6 +255,39 @@ func TestLakectlLocal_clone(t *testing.T) { require.Contains(t, sanitizedResult, vars["PREFIX"]+"test2.txt") require.NotContains(t, sanitizedResult, vars["PREFIX"]+"nodiff.txt") }) + + t.Run("diff with posix permissions", func(t *testing.T) { + dataDir, err := os.MkdirTemp(tmpDir, "") + require.NoError(t, err) + vars["LOCAL_DIR"] = dataDir + vars["PREFIX"] = "posix" + + lakectl := LakectlWithPosixPerms() + RunCmdAndVerifyContainsText(t, lakectl+" local clone lakefs://"+repoName+"/"+mainBranch+"/"+vars["PREFIX"]+" "+dataDir, false, "Successfully cloned lakefs://${REPO}/${REF}/${PREFIX}/ to ${LOCAL_DIR}.", vars) + localVerifyDirContents(t, dataDir, []string{}) + + // Add new files to path + localCreateTestData(t, vars, []string{vars["PREFIX"] + uri.PathSeparator + "with-diff.txt"}) + localCreateTestData(t, vars, []string{vars["PREFIX"] + uri.PathSeparator + "no-diff.txt"}) + + res := runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) + require.Contains(t, res, "download with-diff.txt") + require.Contains(t, res, "download no-diff.txt") + + //res = runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) + res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) + res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) + //runCmd(t, lakectl+" commit lakefs://"+vars["REPO"]+"/"+vars["BRANCH"]+" --allow-empty-message -m \" \"", false, false, vars) + //res = runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) + + err = os.Chmod(filepath.Join(dataDir, "with-diff.txt"), 0755) + require.NoError(t, err) + + sanitizedResult := runCmd(t, lakectl+" local status "+dataDir, false, false, vars) + + require.Contains(t, sanitizedResult, "with-diff.txt") + require.NotContains(t, sanitizedResult, "no-diff.txt") + }) } func TestLakectlLocal_pull(t *testing.T) { diff --git a/esti/lakectl_util.go b/esti/lakectl_util.go index c03fdc2d0ec..72c8bb03257 100644 --- a/esti/lakectl_util.go +++ b/esti/lakectl_util.go @@ -41,9 +41,14 @@ func lakectlLocation() string { } func LakectlWithParams(accessKeyID, secretAccessKey, endPointURL string) string { + return LakectlWithParamsWithPosixPerms(accessKeyID, secretAccessKey, endPointURL, false) +} + +func LakectlWithParamsWithPosixPerms(accessKeyID, secretAccessKey, endPointURL string, withPosixPerms bool) string { lakectlCmdline := "LAKECTL_CREDENTIALS_ACCESS_KEY_ID=" + accessKeyID + " LAKECTL_CREDENTIALS_SECRET_ACCESS_KEY=" + secretAccessKey + " LAKECTL_SERVER_ENDPOINT_URL=" + endPointURL + + " LAKECTL_EXPERIMENTAL_LOCAL_POSIX_PERMISSIONS_ENABLED=" + strconv.FormatBool(withPosixPerms) + " " + lakectlLocation() return lakectlCmdline @@ -53,6 +58,10 @@ func Lakectl() string { return LakectlWithParams(viper.GetString("access_key_id"), viper.GetString("secret_access_key"), viper.GetString("endpoint_url")) } +func LakectlWithPosixPerms() string { + return LakectlWithParamsWithPosixPerms(viper.GetString("access_key_id"), viper.GetString("secret_access_key"), viper.GetString("endpoint_url"), true) +} + func runShellCommand(t *testing.T, command string, isTerminal bool) ([]byte, error) { t.Helper() t.Logf("Run shell command '%s'", command) From e4c0fb4afc0a6596cc270eb9d31a156841f2e398 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 10:41:54 +0300 Subject: [PATCH 2/6] Fix docs --- docs/experimental/lakectl_local_posix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/experimental/lakectl_local_posix.md b/docs/experimental/lakectl_local_posix.md index 7ae48fd92b3..7fb83452506 100644 --- a/docs/experimental/lakectl_local_posix.md +++ b/docs/experimental/lakectl_local_posix.md @@ -44,7 +44,7 @@ By default, this feature is disabled. Enabling file permission tracking is done ``` It is also possible to enable this feature via the corresponding environment variable: -`LAKECTL_EXPERIMENTAL_LOCAL_UNIX_PERMISSIONS_ENABLED` +`LAKECTL_EXPERIMENTAL_LOCAL_POSIX_PERMISSIONS_ENABLED` ## Usage From fe7b84a9a93f394f8c7bdc108c431ca34a21e08a Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 10:42:38 +0300 Subject: [PATCH 3/6] Remove temp code --- esti/lakectl_local_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/esti/lakectl_local_test.go b/esti/lakectl_local_test.go index 950ac855d3b..3d10d3450e0 100644 --- a/esti/lakectl_local_test.go +++ b/esti/lakectl_local_test.go @@ -274,11 +274,8 @@ func TestLakectlLocal_clone(t *testing.T) { require.Contains(t, res, "download with-diff.txt") require.Contains(t, res, "download no-diff.txt") - //res = runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) - //runCmd(t, lakectl+" commit lakefs://"+vars["REPO"]+"/"+vars["BRANCH"]+" --allow-empty-message -m \" \"", false, false, vars) - //res = runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) err = os.Chmod(filepath.Join(dataDir, "with-diff.txt"), 0755) require.NoError(t, err) From 456239cf245000fab1a9cc73863d2dfdac0af811 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 10:47:19 +0300 Subject: [PATCH 4/6] Revert docs change --- docs/experimental/lakectl_local_posix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/experimental/lakectl_local_posix.md b/docs/experimental/lakectl_local_posix.md index 7fb83452506..7ae48fd92b3 100644 --- a/docs/experimental/lakectl_local_posix.md +++ b/docs/experimental/lakectl_local_posix.md @@ -44,7 +44,7 @@ By default, this feature is disabled. Enabling file permission tracking is done ``` It is also possible to enable this feature via the corresponding environment variable: -`LAKECTL_EXPERIMENTAL_LOCAL_POSIX_PERMISSIONS_ENABLED` +`LAKECTL_EXPERIMENTAL_LOCAL_UNIX_PERMISSIONS_ENABLED` ## Usage From bfc5cc5425ca782ce1ef34abd7ad5e1435ed4133 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 16:53:55 +0300 Subject: [PATCH 5/6] Fix tests --- esti/lakectl_local_test.go | 10 ++++++---- pkg/local/sync.go | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/esti/lakectl_local_test.go b/esti/lakectl_local_test.go index 3d10d3450e0..f15608300f8 100644 --- a/esti/lakectl_local_test.go +++ b/esti/lakectl_local_test.go @@ -267,15 +267,17 @@ func TestLakectlLocal_clone(t *testing.T) { localVerifyDirContents(t, dataDir, []string{}) // Add new files to path - localCreateTestData(t, vars, []string{vars["PREFIX"] + uri.PathSeparator + "with-diff.txt"}) - localCreateTestData(t, vars, []string{vars["PREFIX"] + uri.PathSeparator + "no-diff.txt"}) + localCreateTestData(t, vars, []string{ + vars["PREFIX"] + uri.PathSeparator + "with-diff.txt", + vars["PREFIX"] + uri.PathSeparator + "no-diff.txt", + }) res := runCmd(t, lakectl+" local pull "+dataDir, false, false, vars) require.Contains(t, res, "download with-diff.txt") require.Contains(t, res, "download no-diff.txt") - res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) - res = runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) + // the following commit "initializes" the posix permissions for the remote repo + runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) err = os.Chmod(filepath.Join(dataDir, "with-diff.txt"), 0755) require.NoError(t, err) diff --git a/pkg/local/sync.go b/pkg/local/sync.go index 7361009f7bf..09c0bc6d842 100644 --- a/pkg/local/sync.go +++ b/pkg/local/sync.go @@ -277,7 +277,8 @@ func (s *SyncManager) upload(ctx context.Context, rootPath string, remote *uri.U if err := fileutil.VerifySafeFilename(source); err != nil { return err } - dest := strings.TrimPrefix(filepath.ToSlash(fmt.Sprintf("%s%s%s", remote.GetPath(), uri.PathSeparator, path)), uri.PathSeparator) + remotePath := strings.TrimRight(remote.GetPath(), uri.PathSeparator) + dest := strings.TrimPrefix(filepath.ToSlash(fmt.Sprintf("%s%s%s", remotePath, uri.PathSeparator, path)), uri.PathSeparator) f, err := os.Open(source) if err != nil { From 54771cf05a5e3b0291d6fa62110c005643ab068e Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Sun, 7 Jul 2024 20:29:30 +0300 Subject: [PATCH 6/6] Update tests --- esti/lakectl_local_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/esti/lakectl_local_test.go b/esti/lakectl_local_test.go index f15608300f8..fa491fbdede 100644 --- a/esti/lakectl_local_test.go +++ b/esti/lakectl_local_test.go @@ -276,13 +276,16 @@ func TestLakectlLocal_clone(t *testing.T) { require.Contains(t, res, "download with-diff.txt") require.Contains(t, res, "download no-diff.txt") - // the following commit "initializes" the posix permissions for the remote repo - runCmd(t, lakectl+" local commit "+dataDir+" --allow-empty-message -m \" \"", false, false, vars) + commitMessage := "'initialize' posix permissions for the remote repo" + runCmd(t, lakectl+" local commit "+dataDir+" -m \""+commitMessage+"\"", false, false, vars) + + sanitizedResult := runCmd(t, lakectl+" local status "+dataDir, false, false, vars) + require.Contains(t, sanitizedResult, "No diff found") err = os.Chmod(filepath.Join(dataDir, "with-diff.txt"), 0755) require.NoError(t, err) - sanitizedResult := runCmd(t, lakectl+" local status "+dataDir, false, false, vars) + sanitizedResult = runCmd(t, lakectl+" local status "+dataDir, false, false, vars) require.Contains(t, sanitizedResult, "with-diff.txt") require.NotContains(t, sanitizedResult, "no-diff.txt")