fix: Discarding a Plan Causes the Whole Working Directory to be Delet…

…ed (runatlantis#3553)

server/controllers/locks_controller.go

@@ -126,16 +126,6 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) {
 	// installations of Atlantis will have locks in their DB that do not have
 	// this field on PullRequest. We skip commenting in this case.
 	if lock.Pull.BaseRepo != (models.Repo{}) {
-		unlock, err := l.WorkingDirLocker.TryLock(lock.Pull.BaseRepo.FullName, lock.Pull.Num, lock.Workspace, lock.Project.Path)
-		if err != nil {
-			l.Logger.Err("unable to obtain working dir lock when trying to delete old plans: %s", err)
-		} else {
-			defer unlock()
-			// nolint: vetshadow
-			if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace); err != nil {
-				l.Logger.Err("unable to delete workspace: %s", err)
-			}
-		}
 		if err := l.Backend.UpdateProjectStatus(lock.Pull, lock.Workspace, lock.Project.Path, models.DiscardedPlanStatus); err != nil {
 			l.Logger.Err("unable to update project status: %s", err)
 		}

server/events/delete_lock_command.go

@@ -33,7 +33,15 @@ func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, e
 		return nil, nil
 	}
 
-	l.deleteWorkingDir(*lock)
+	// The locks controller currently has no implementation of Atlantis project names, so this is hardcoded to an empty string.
+	projectName := ""
+
+	removeErr := l.WorkingDir.DeletePlan(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, projectName)
+	if removeErr != nil {
+		l.Logger.Warn("Failed to delete plan: %s", removeErr)
+		return nil, removeErr
+	}
+
 	return lock, nil
 }
 

server/events/delete_lock_command_test.go

@@ -40,36 +40,25 @@ func TestDeleteLock_None(t *testing.T) {
 	Assert(t, lock == nil, "lock was not nil")
 }
 
-func TestDeleteLock_OldFormat(t *testing.T) {
-	t.Log("If the lock doesn't have BaseRepo set it is deleted successfully")
-	RegisterMockTestingT(t)
-	l := lockmocks.NewMockLocker()
-	When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil)
-	dlc := events.DefaultDeleteLockCommand{
-		Locker: l,
-		Logger: logging.NewNoopLogger(t),
-	}
-	lock, err := dlc.DeleteLock("id")
-	Ok(t, err)
-	Assert(t, lock != nil, "lock was nil")
-}
-
 func TestDeleteLock_Success(t *testing.T) {
-	t.Log("Delete lock deletes successfully the working dir")
+	t.Log("Delete lock deletes successfully the plan file")
 	RegisterMockTestingT(t)
 	l := lockmocks.NewMockLocker()
 	When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil)
 	workingDir := events.NewMockWorkingDir()
 	workingDirLocker := events.NewDefaultWorkingDirLocker()
+	workspace := "workspace"
+	path := "path"
+	projectName := ""
 	pull := models.PullRequest{
 		BaseRepo: models.Repo{FullName: "owner/repo"},
 	}
 	When(l.Unlock("id")).ThenReturn(&models.ProjectLock{
 		Pull:      pull,
-		Workspace: "workspace",
+		Workspace: workspace,
 		Project: models.Project{
-			Path:         "path",
-			RepoFullName: "owner/repo",
+			Path:         path,
+			RepoFullName: pull.BaseRepo.FullName,
 		},
 	}, nil)
 	tmp := t.TempDir()
@@ -85,7 +74,7 @@ func TestDeleteLock_Success(t *testing.T) {
 	lock, err := dlc.DeleteLock("id")
 	Ok(t, err)
 	Assert(t, lock != nil, "lock was nil")
-	workingDir.VerifyWasCalledOnce().DeleteForWorkspace(pull.BaseRepo, pull, "workspace")
+	workingDir.VerifyWasCalledOnce().DeletePlan(pull.BaseRepo, pull, workspace, path, projectName)
 }
 
 func TestDeleteLocksByPull_LockerErr(t *testing.T) {
@@ -94,13 +83,16 @@ func TestDeleteLocksByPull_LockerErr(t *testing.T) {
 	pullNum := 2
 	RegisterMockTestingT(t)
 	l := lockmocks.NewMockLocker()
+	workingDir := events.NewMockWorkingDir()
 	When(l.UnlockByPull(repoName, pullNum)).ThenReturn(nil, errors.New("err"))
 	dlc := events.DefaultDeleteLockCommand{
-		Locker: l,
-		Logger: logging.NewNoopLogger(t),
+		Locker:     l,
+		Logger:     logging.NewNoopLogger(t),
+		WorkingDir: workingDir,
 	}
 	_, err := dlc.DeleteLocksByPull(repoName, pullNum)
 	ErrEquals(t, "err", err)
+	workingDir.VerifyWasCalled(Never()).DeletePlan(Any[models.Repo](), Any[models.PullRequest](), Any[string](), Any[string](), Any[string]())
 }
 
 func TestDeleteLocksByPull_None(t *testing.T) {
@@ -109,13 +101,16 @@ func TestDeleteLocksByPull_None(t *testing.T) {
 	pullNum := 2
 	RegisterMockTestingT(t)
 	l := lockmocks.NewMockLocker()
+	workingDir := events.NewMockWorkingDir()
 	When(l.UnlockByPull(repoName, pullNum)).ThenReturn([]models.ProjectLock{}, nil)
 	dlc := events.DefaultDeleteLockCommand{
-		Locker: l,
-		Logger: logging.NewNoopLogger(t),
+		Locker:     l,
+		Logger:     logging.NewNoopLogger(t),
+		WorkingDir: workingDir,
 	}
 	_, err := dlc.DeleteLocksByPull(repoName, pullNum)
 	Ok(t, err)
+	workingDir.VerifyWasCalled(Never()).DeletePlan(Any[models.Repo](), Any[models.PullRequest](), Any[string](), Any[string](), Any[string]())
 }
 
 func TestDeleteLocksByPull_OldFormat(t *testing.T) {

server/events/working_dir.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 
 	"github.com/pkg/errors"
+	"github.com/runatlantis/atlantis/server/core/runtime"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/logging"
 )
@@ -53,6 +54,8 @@ type WorkingDir interface {
 	// the upstream branch has been modified. This is only safe after grabbing the project lock
 	// and before running any plans
 	SetSafeToReClone()
+	// DeletePlan deletes the plan for this repo, pull, workspace path and project name
+	DeletePlan(r models.Repo, p models.PullRequest, workspace string, path string, projectName string) error
 }
 
 // FileWorkspace implements WorkingDir with the file system.
@@ -81,6 +84,7 @@ type FileWorkspace struct {
 	GpgNoSigningEnabled bool
 	// flag indicating if a re-clone will be safe (project lock held, about to run plan)
 	SafeToReClone bool
+	Logger        logging.SimpleLogging
 }
 
 // Clone git clones headRepo, checks out the branch and then returns the absolute
@@ -367,3 +371,9 @@ func (w *FileWorkspace) sanitizeGitCredentials(s string, base models.Repo, head
 func (w *FileWorkspace) SetSafeToReClone() {
 	w.SafeToReClone = true
 }
+
+func (w *FileWorkspace) DeletePlan(r models.Repo, p models.PullRequest, workspace string, projectPath string, projectName string) error {
+	planPath := filepath.Join(w.cloneDir(r, p, workspace), projectPath, runtime.GetPlanFilename(workspace, projectName))
+	w.Logger.Info("Deleting plan: " + planPath)
+	return os.Remove(planPath)
+}

server/server.go

@@ -463,6 +463,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		CheckoutMerge:    userConfig.CheckoutStrategy == "merge",
 		CheckoutDepth:    userConfig.CheckoutDepth,
 		GithubAppEnabled: githubAppEnabled,
+		Logger:           logger,
 	}
 
 	scheduledExecutorService := scheduled.NewExecutorService(