diff --git a/api/filesystem/filesystem.go b/api/filesystem/filesystem.go index 85c519844..778c979d8 100644 --- a/api/filesystem/filesystem.go +++ b/api/filesystem/filesystem.go @@ -515,3 +515,20 @@ func FileExists(filePath string) (bool, error) { } return true, nil } + +func MoveDirectory(originalPath, newPath string) error { + if _, err := os.Stat(originalPath); err != nil { + return err + } + + alreadyExists, err := FileExists(newPath) + if err != nil { + return err + } + + if alreadyExists { + return errors.New("Target path already exists") + } + + return os.Rename(originalPath, newPath) +} diff --git a/api/filesystem/filesystem_fileexists_test.go b/api/filesystem/filesystem_fileexists_test.go new file mode 100644 index 000000000..f0f203151 --- /dev/null +++ b/api/filesystem/filesystem_fileexists_test.go @@ -0,0 +1,55 @@ +package filesystem + +import ( + "fmt" + "math/rand" + "os" + "path" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_fileSystemService_FileExists_whenFileExistsShouldReturnTrue(t *testing.T) { + service := createService(t) + testHelperFileExists_fileExists(t, service.FileExists) +} + +func Test_fileSystemService_FileExists_whenFileNotExistsShouldReturnFalse(t *testing.T) { + service := createService(t) + testHelperFileExists_fileNotExists(t, service.FileExists) +} + +func Test_FileExists_whenFileExistsShouldReturnTrue(t *testing.T) { + testHelperFileExists_fileExists(t, FileExists) +} + +func Test_FileExists_whenFileNotExistsShouldReturnFalse(t *testing.T) { + testHelperFileExists_fileNotExists(t, FileExists) +} + +func testHelperFileExists_fileExists(t *testing.T, checker func(path string) (bool, error)) { + file, err := os.CreateTemp("", t.Name()) + assert.NoError(t, err, "CreateTemp should not fail") + + t.Cleanup(func() { + os.RemoveAll(file.Name()) + }) + + exists, err := checker(file.Name()) + assert.NoError(t, err, "FileExists should not fail") + + assert.True(t, exists) +} + +func testHelperFileExists_fileNotExists(t *testing.T, checker func(path string) (bool, error)) { + filePath := path.Join(os.TempDir(), fmt.Sprintf("%s%d", t.Name(), rand.Int())) + + err := os.RemoveAll(filePath) + assert.NoError(t, err, "RemoveAll should not fail") + + exists, err := checker(filePath) + assert.NoError(t, err, "FileExists should not fail") + + assert.False(t, exists) +} diff --git a/api/filesystem/filesystem_move_test.go b/api/filesystem/filesystem_move_test.go new file mode 100644 index 000000000..863075399 --- /dev/null +++ b/api/filesystem/filesystem_move_test.go @@ -0,0 +1,49 @@ +package filesystem + +import ( + "fmt" + "os" + "path" + "testing" + + "github.com/stretchr/testify/assert" +) + +// temporary function until upgrade to 1.16 +func tempDir(t *testing.T) string { + tmpDir, err := os.MkdirTemp("", "dir") + assert.NoError(t, err, "MkdirTemp should not fail") + + return tmpDir +} + +func Test_movePath_shouldFailIfOriginalPathDoesntExist(t *testing.T) { + tmpDir := tempDir(t) + missingPath := path.Join(tmpDir, "missing") + targetPath := path.Join(tmpDir, "target") + + defer os.RemoveAll(tmpDir) + + err := MoveDirectory(missingPath, targetPath) + assert.Error(t, err, "move directory should fail when target path exists") +} + +func Test_movePath_shouldFailIfTargetPathDoesExist(t *testing.T) { + originalPath := tempDir(t) + missingPath := tempDir(t) + + defer os.RemoveAll(originalPath) + defer os.RemoveAll(missingPath) + + err := MoveDirectory(originalPath, missingPath) + assert.Error(t, err, "move directory should fail when target path exists") +} + +func Test_movePath_success(t *testing.T) { + originalPath := tempDir(t) + + defer os.RemoveAll(originalPath) + + err := MoveDirectory(originalPath, fmt.Sprintf("%s-old", originalPath)) + assert.NoError(t, err) +} diff --git a/api/filesystem/filesystem_test.go b/api/filesystem/filesystem_test.go index 9688dacc8..f2b46d8a3 100644 --- a/api/filesystem/filesystem_test.go +++ b/api/filesystem/filesystem_test.go @@ -1,8 +1,6 @@ package filesystem import ( - "fmt" - "math/rand" "os" "path" "testing" @@ -22,47 +20,3 @@ func createService(t *testing.T) *Service { return service } - -func Test_fileSystemService_FileExists_whenFileExistsShouldReturnTrue(t *testing.T) { - service := createService(t) - testHelperFileExists_fileExists(t, service.FileExists) -} - -func Test_fileSystemService_FileExists_whenFileNotExistsShouldReturnFalse(t *testing.T) { - service := createService(t) - testHelperFileExists_fileNotExists(t, service.FileExists) -} - -func Test_FileExists_whenFileExistsShouldReturnTrue(t *testing.T) { - testHelperFileExists_fileExists(t, FileExists) -} - -func Test_FileExists_whenFileNotExistsShouldReturnFalse(t *testing.T) { - testHelperFileExists_fileNotExists(t, FileExists) -} - -func testHelperFileExists_fileExists(t *testing.T, checker func(path string) (bool, error)) { - file, err := os.CreateTemp("", t.Name()) - assert.NoError(t, err, "CreateTemp should not fail") - - t.Cleanup(func() { - os.RemoveAll(file.Name()) - }) - - exists, err := checker(file.Name()) - assert.NoError(t, err, "FileExists should not fail") - - assert.True(t, exists) -} - -func testHelperFileExists_fileNotExists(t *testing.T, checker func(path string) (bool, error)) { - filePath := path.Join(os.TempDir(), fmt.Sprintf("%s%d", t.Name(), rand.Int())) - - err := os.RemoveAll(filePath) - assert.NoError(t, err, "RemoveAll should not fail") - - exists, err := checker(filePath) - assert.NoError(t, err, "FileExists should not fail") - - assert.False(t, exists) -} diff --git a/api/http/handler/stacks/stack_update_git.go b/api/http/handler/stacks/stack_update_git.go index 00f3b6cf2..43d5eae73 100644 --- a/api/http/handler/stacks/stack_update_git.go +++ b/api/http/handler/stacks/stack_update_git.go @@ -2,6 +2,8 @@ package stacks import ( "errors" + "fmt" + "log" "net/http" "time" @@ -11,6 +13,7 @@ import ( "github.com/portainer/libhttp/response" portainer "github.com/portainer/portainer/api" bolterrors "github.com/portainer/portainer/api/bolt/errors" + "github.com/portainer/portainer/api/filesystem" httperrors "github.com/portainer/portainer/api/http/errors" "github.com/portainer/portainer/api/http/security" "github.com/portainer/portainer/api/internal/stackutils" @@ -101,17 +104,23 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * stack.Env = payload.Env stack.GitConfig.ReferenceName = payload.RepositoryReferenceName - err = handler.FileService.RemoveDirectory(stack.ProjectPath) + backupProjectPath := fmt.Sprintf("%s-old", stack.ProjectPath) + err = filesystem.MoveDirectory(stack.ProjectPath, backupProjectPath) if err != nil { - return &httperror.HandlerError{http.StatusInternalServerError, "Unable to remove git repository directory", err} + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to move git repository directory", err} } err = handler.GitService.CloneRepository(stack.ProjectPath, stack.GitConfig.URL, payload.RepositoryReferenceName, payload.RepositoryUsername, payload.RepositoryPassword) if err != nil { + restoreError := filesystem.MoveDirectory(backupProjectPath, stack.ProjectPath) + if restoreError != nil { + log.Printf("[WARN] [http,stacks,git] [error: %s] [message: failed restoring backup folder]", restoreError) + } + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to clone git repository", err} } - httpErr := handler.deployStack(r, stack, endpoint, payload.Prune) + httpErr := handler.deployStack(r, stack, endpoint) if httpErr != nil { return httpErr } @@ -121,6 +130,11 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * return &httperror.HandlerError{http.StatusInternalServerError, "Unable to persist the stack changes inside the database", err} } + err = handler.FileService.RemoveDirectory(backupProjectPath) + if err != nil { + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to remove git repository directory", err} + } + return response.JSON(w, stack) }