From a755e6be157ce9bd178ff90b712cc027ffb761f8 Mon Sep 17 00:00:00 2001 From: Oscar Zhou <100548325+oscarzhou-portainer@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:34:38 +1200 Subject: [PATCH] fix(stack/git): option to overwrite target path during dir move [EE-6871] (#11627) --- api/filesystem/filesystem.go | 14 ++++++++++---- api/filesystem/filesystem_move_test.go | 22 +++++++++++++++++++--- api/git/backup.go | 4 ++-- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/api/filesystem/filesystem.go b/api/filesystem/filesystem.go index 8eed27fa7..bddfc0f61 100644 --- a/api/filesystem/filesystem.go +++ b/api/filesystem/filesystem.go @@ -934,7 +934,7 @@ func FileExists(filePath string) (bool, error) { func (service *Service) SafeMoveDirectory(originalPath, newPath string) error { // 1. Backup the source directory to a different folder backupDir := fmt.Sprintf("%s-%s", filepath.Dir(originalPath), "backup") - err := MoveDirectory(originalPath, backupDir) + err := MoveDirectory(originalPath, backupDir, false) if err != nil { return fmt.Errorf("failed to backup source directory: %w", err) } @@ -973,14 +973,14 @@ func restoreBackup(src, backupDir string) error { return fmt.Errorf("failed to delete destination directory: %w", err) } - err = MoveDirectory(backupDir, src) + err = MoveDirectory(backupDir, src, false) if err != nil { return fmt.Errorf("failed to restore backup directory: %w", err) } return nil } -func MoveDirectory(originalPath, newPath string) error { +func MoveDirectory(originalPath, newPath string, overwriteTargetPath bool) error { if _, err := os.Stat(originalPath); err != nil { return err } @@ -991,7 +991,13 @@ func MoveDirectory(originalPath, newPath string) error { } if alreadyExists { - return errors.New("Target path already exists") + if !overwriteTargetPath { + return fmt.Errorf("Target path already exists") + } + + if err = os.RemoveAll(newPath); err != nil { + return fmt.Errorf("failed to overwrite path %s: %s", newPath, err.Error()) + } } return os.Rename(originalPath, newPath) diff --git a/api/filesystem/filesystem_move_test.go b/api/filesystem/filesystem_move_test.go index de268371c..e2965c953 100644 --- a/api/filesystem/filesystem_move_test.go +++ b/api/filesystem/filesystem_move_test.go @@ -16,7 +16,7 @@ func Test_movePath_shouldFailIfSourceDirDoesNotExist(t *testing.T) { file1 := addFile(destinationDir, "dir", "file") file2 := addFile(destinationDir, "file") - err := MoveDirectory(sourceDir, destinationDir) + err := MoveDirectory(sourceDir, destinationDir, false) assert.Error(t, err, "move directory should fail when source path is missing") assert.FileExists(t, file1, "destination dir contents should remain") assert.FileExists(t, file2, "destination dir contents should remain") @@ -30,7 +30,7 @@ func Test_movePath_shouldFailIfDestinationDirExists(t *testing.T) { file3 := addFile(destinationDir, "dir", "file") file4 := addFile(destinationDir, "file") - err := MoveDirectory(sourceDir, destinationDir) + err := MoveDirectory(sourceDir, destinationDir, false) assert.Error(t, err, "move directory should fail when destination directory already exists") assert.FileExists(t, file1, "source dir contents should remain") assert.FileExists(t, file2, "source dir contents should remain") @@ -38,6 +38,22 @@ func Test_movePath_shouldFailIfDestinationDirExists(t *testing.T) { assert.FileExists(t, file4, "destination dir contents should remain") } +func Test_movePath_succesIfOverwriteSetWhenDestinationDirExists(t *testing.T) { + sourceDir := t.TempDir() + file1 := addFile(sourceDir, "dir", "file") + file2 := addFile(sourceDir, "file") + destinationDir := t.TempDir() + file3 := addFile(destinationDir, "dir", "file") + file4 := addFile(destinationDir, "file") + + err := MoveDirectory(sourceDir, destinationDir, true) + assert.NoError(t, err) + assert.NoFileExists(t, file1, "source dir contents should be moved") + assert.NoFileExists(t, file2, "source dir contents should be moved") + assert.FileExists(t, file3, "destination dir contents should remain") + assert.FileExists(t, file4, "destination dir contents should remain") +} + func Test_movePath_successWhenSourceExistsAndDestinationIsMissing(t *testing.T) { tmp := t.TempDir() sourceDir := path.Join(tmp, "source") @@ -46,7 +62,7 @@ func Test_movePath_successWhenSourceExistsAndDestinationIsMissing(t *testing.T) file2 := addFile(sourceDir, "file") destinationDir := path.Join(tmp, "destination") - err := MoveDirectory(sourceDir, destinationDir) + err := MoveDirectory(sourceDir, destinationDir, false) assert.NoError(t, err) assert.NoFileExists(t, file1, "source dir contents should be moved") assert.NoFileExists(t, file2, "source dir contents should be moved") diff --git a/api/git/backup.go b/api/git/backup.go index 45c45cdf1..465a05af1 100644 --- a/api/git/backup.go +++ b/api/git/backup.go @@ -38,7 +38,7 @@ func CloneWithBackup(gitService portainer.GitService, fileService portainer.File } } - err = filesystem.MoveDirectory(options.ProjectPath, backupProjectPath) + err = filesystem.MoveDirectory(options.ProjectPath, backupProjectPath, true) if err != nil { return cleanFn, errors.WithMessage(err, "Unable to move git repository directory") } @@ -48,7 +48,7 @@ func CloneWithBackup(gitService portainer.GitService, fileService portainer.File err = gitService.CloneRepository(options.ProjectPath, options.URL, options.ReferenceName, options.Username, options.Password, options.TLSSkipVerify) if err != nil { cleanUp = false - restoreError := filesystem.MoveDirectory(backupProjectPath, options.ProjectPath) + restoreError := filesystem.MoveDirectory(backupProjectPath, options.ProjectPath, false) if restoreError != nil { log.Warn().Err(restoreError).Msg("failed restoring backup folder") }