fix: Running 'atlantis unlock' on a PR Causes The Whole Working Direc…

…tory to be Deleted (runatlantis#3751)

* Running 'atlantis unlock' on a PR Causes The Whole Working Directory to be Deleted

* Update events controller tests

* Fix events controller tests

---------

Co-authored-by: PePe Amengual <[email protected]>

server/controllers/events/events_controller.go

@@ -587,7 +587,8 @@ func (e *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b
 		}
 	}
 
-	logger.Debug("executing command")
+	logger.Info("Running comment command '%v' on repo '%v', pull request: %v for user '%v'.",
+		parseResult.Command.Name, baseRepo.FullName, pullNum, user.Username)
 	if !e.TestingMode {
 		// Respond with success and then actually execute the command asynchronously.
 		// We use a goroutine so that this function returns and the connection is

server/controllers/events/events_controller_test.go

@@ -31,6 +31,7 @@ import (
 	events_controllers "github.com/runatlantis/atlantis/server/controllers/events"
 	"github.com/runatlantis/atlantis/server/controllers/events/mocks"
 	"github.com/runatlantis/atlantis/server/events"
+	"github.com/runatlantis/atlantis/server/events/command"
 	emocks "github.com/runatlantis/atlantis/server/events/mocks"
 	"github.com/runatlantis/atlantis/server/events/models"
 	vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks"
@@ -347,15 +348,17 @@ func TestPost_GithubCommentResponse(t *testing.T) {
 
 func TestPost_GitlabCommentSuccess(t *testing.T) {
 	t.Log("when the event is a gitlab comment with a valid command we call the command handler")
-	e, _, gl, _, _, cr, _, _, _ := setup(t)
+	e, _, gl, _, _, cr, _, _, cp := setup(t)
 	req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
 	req.Header.Set(gitlabHeader, "value")
+	cmd := events.CommentCommand{}
 	When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
+	When(cp.Parse(Any[string](), Eq(models.Gitlab))).ThenReturn(events.CommentParseResult{Command: &cmd})
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 	ResponseContains(t, w, http.StatusOK, "Processing...")
 
-	cr.VerifyWasCalledOnce().RunCommentCommand(models.Repo{}, &models.Repo{}, nil, models.User{}, 0, nil)
+	cr.VerifyWasCalledOnce().RunCommentCommand(models.Repo{}, &models.Repo{}, nil, models.User{}, 0, &cmd)
 }
 
 func TestPost_GithubCommentSuccess(t *testing.T) {
@@ -378,17 +381,18 @@ func TestPost_GithubCommentSuccess(t *testing.T) {
 }
 
 func TestPost_GithubCommentReaction(t *testing.T) {
-	t.Log("when the event is a github comment with a valid command we call the command handler")
+	t.Log("when the event is a github comment with a valid command we call the ReactToComment handler")
 	e, v, _, _, p, _, _, vcsClient, cp := setup(t)
 	req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
 	req.Header.Set(githubHeader, "issue_comment")
-	event := `{"action": "created", "comment": {"body": "@atlantis-bot help", "id": 1}}`
+	testComment := "atlantis plan"
+	event := fmt.Sprintf(`{"action": "created", "comment": {"body": "%v", "id": 1}}`, testComment)
 	When(v.Validate(req, secret)).ThenReturn([]byte(event), nil)
 	baseRepo := models.Repo{}
 	user := models.User{}
-	cmd := events.CommentCommand{}
+	cmd := events.CommentCommand{Name: command.Plan}
 	When(p.ParseGithubIssueCommentEvent(Any[*github.IssueCommentEvent]())).ThenReturn(baseRepo, user, 1, nil)
-	When(cp.Parse("", models.Github)).ThenReturn(events.CommentParseResult{Command: &cmd})
+	When(cp.Parse(testComment, models.Github)).ThenReturn(events.CommentParseResult{Command: &cmd})
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 	ResponseContains(t, w, http.StatusOK, "Processing...")
@@ -398,10 +402,12 @@ func TestPost_GithubCommentReaction(t *testing.T) {
 
 func TestPost_GilabCommentReaction(t *testing.T) {
 	t.Log("when the event is a gitlab comment with a valid command we call the ReactToComment handler")
-	e, _, gl, _, _, _, _, vcsClient, _ := setup(t)
+	e, _, gl, _, _, _, _, vcsClient, cp := setup(t)
 	req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
 	req.Header.Set(gitlabHeader, "value")
+	cmd := events.CommentCommand{}
 	When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
+	When(cp.Parse(Any[string](), Eq(models.Gitlab))).ThenReturn(events.CommentParseResult{Command: &cmd})
 	w := httptest.NewRecorder()
 	e.Post(w, req)
 	ResponseContains(t, w, http.StatusOK, "Processing...")
@@ -685,12 +691,14 @@ func TestPost_AzureDevopsPullRequestWebhookTestIgnoreEvent(t *testing.T) {
 
 func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) {
 	t.Log("when the event should not be ignored it should pass through all ignore statements without error")
-	e, _, _, ado, _, _, _, _, _ := setup(t)
+	e, _, _, ado, _, _, _, _, cp := setup(t)
 
+	testComment := "atlantis plan"
 	repo := models.Repo{}
+	cmd := events.CommentCommand{Name: command.Plan}
 	When(e.Parser.ParseAzureDevopsRepo(Any[*azuredevops.GitRepository]())).ThenReturn(repo, nil)
-
-	payload := `{
+	When(cp.Parse(testComment, models.AzureDevops)).ThenReturn(events.CommentParseResult{Command: &cmd})
+	payload := fmt.Sprintf(`{
 		"subscriptionId": "11111111-1111-1111-1111-111111111111",
 		"notificationId": 1,
 		"id": "22222222-2222-2222-2222-222222222222",
@@ -703,14 +711,14 @@ func TestPost_AzureDevopsPullRequestCommentPassingIgnores(t *testing.T) {
 			"comment": {
 				"id": 1,
 				"commentType": "text",
-				"content": "test"
+				"content": "%v"
 			},
 			"pullRequest": {
 				"pullRequestId": 1,
 				"repository": {}
 			}
 		}
-	}`
+	}`, testComment)
 
 	t.Run("Testing to see if comment passes ignore conditions", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "", strings.NewReader(payload))

server/events/delete_lock_command.go

@@ -53,37 +53,22 @@ func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNu
 		return numLocks, err
 	}
 	if numLocks == 0 {
-		l.Logger.Debug("No locks found for pull")
+		l.Logger.Debug("No locks found for repo '%v', pull request: %v", repoFullName, pullNum)
 		return numLocks, nil
 	}
 
+	// The locks controller currently has no implementation of Atlantis project names, so this is hardcoded to an empty string.
+	projectName := ""
+
 	for i := 0; i < numLocks; i++ {
 		lock := locks[i]
-		l.deleteWorkingDir(lock)
-	}
-
-	return numLocks, nil
-}
 
-func (l *DefaultDeleteLockCommand) deleteWorkingDir(lock models.ProjectLock) {
-	// NOTE: Because BaseRepo was added to the PullRequest model later, previous
-	// installations of Atlantis will have locks in their DB that do not have
-	// this field on PullRequest. We skip deleting the working dir in this case.
-	if lock.Pull.BaseRepo == (models.Repo{}) {
-		l.Logger.Debug("Not deleting the working dir.")
-		return
-	}
-	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)
+		err := l.WorkingDir.DeletePlan(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, projectName)
+		if err != nil {
+			l.Logger.Warn("Failed to delete plan: %s", err)
+			return numLocks, err
 		}
 	}
-	if err := l.Backend.UpdateProjectStatus(lock.Pull, lock.Workspace, lock.Project.Path, models.DiscardedPlanStatus); err != nil {
-		l.Logger.Err("unable to delete project status: %s", err)
-	}
+
+	return numLocks, nil
 }

server/events/delete_lock_command_test.go

@@ -113,17 +113,84 @@ func TestDeleteLocksByPull_None(t *testing.T) {
 	workingDir.VerifyWasCalled(Never()).DeletePlan(Any[models.Repo](), Any[models.PullRequest](), Any[string](), Any[string](), Any[string]())
 }
 
-func TestDeleteLocksByPull_OldFormat(t *testing.T) {
-	t.Log("If the lock doesn't have BaseRepo set it is deleted successfully")
+func TestDeleteLocksByPull_SingleSuccess(t *testing.T) {
+	t.Log("If a single lock is successfully deleted")
 	repoName := "reponame"
 	pullNum := 2
+	path := "."
+	workspace := "default"
+	projectName := ""
+
 	RegisterMockTestingT(t)
 	l := lockmocks.NewMockLocker()
-	When(l.UnlockByPull(repoName, pullNum)).ThenReturn([]models.ProjectLock{{}}, nil)
+	workingDir := events.NewMockWorkingDir()
+	pull := models.PullRequest{
+		BaseRepo: models.Repo{FullName: repoName},
+		Num:      pullNum,
+	}
+	When(l.UnlockByPull(repoName, pullNum)).ThenReturn([]models.ProjectLock{
+		{
+			Pull:      pull,
+			Workspace: workspace,
+			Project: models.Project{
+				Path:         path,
+				RepoFullName: pull.BaseRepo.FullName,
+			},
+		},
+	}, 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(Once()).DeletePlan(pull.BaseRepo, pull, workspace, path, projectName)
+}
+
+func TestDeleteLocksByPull_MultipleSuccess(t *testing.T) {
+	t.Log("If multiple locks are successfully deleted")
+	repoName := "reponame"
+	pullNum := 2
+	path1 := "path1"
+	path2 := "path2"
+	workspace := "default"
+	projectName := ""
+
+	RegisterMockTestingT(t)
+	l := lockmocks.NewMockLocker()
+	workingDir := events.NewMockWorkingDir()
+	pull := models.PullRequest{
+		BaseRepo: models.Repo{FullName: repoName},
+		Num:      pullNum,
+	}
+	When(l.UnlockByPull(repoName, pullNum)).ThenReturn([]models.ProjectLock{
+		{
+			Pull:      pull,
+			Workspace: workspace,
+			Project: models.Project{
+				Path:         path1,
+				RepoFullName: pull.BaseRepo.FullName,
+			},
+		},
+		{
+			Pull:      pull,
+			Workspace: workspace,
+			Project: models.Project{
+				Path:         path2,
+				RepoFullName: pull.BaseRepo.FullName,
+			},
+		},
+	}, nil,
+	)
+	dlc := events.DefaultDeleteLockCommand{
+		Locker:     l,
+		Logger:     logging.NewNoopLogger(t),
+		WorkingDir: workingDir,
 	}
 	_, err := dlc.DeleteLocksByPull(repoName, pullNum)
 	Ok(t, err)
+	workingDir.VerifyWasCalled(Once()).DeletePlan(pull.BaseRepo, pull, workspace, path1, projectName)
+	workingDir.VerifyWasCalled(Once()).DeletePlan(pull.BaseRepo, pull, workspace, path2, projectName)
 }

server/events/unlock_command_runner.go

@@ -32,6 +32,7 @@ func (u *UnlockCommandRunner) Run(
 	baseRepo := ctx.Pull.BaseRepo
 	pullNum := ctx.Pull.Num
 
+	ctx.Log.Info("Unlocking all locks")
 	vcsMessage := "All Atlantis locks for this PR have been unlocked and plans discarded"
 	numLocks, err := u.deleteLockCommand.DeleteLocksByPull(baseRepo.FullName, pullNum)
 	if err != nil {
@@ -40,8 +41,11 @@ func (u *UnlockCommandRunner) Run(
 	}
 
 	// if there are no locks to delete, no errors, and SilenceNoProjects is enabled, don't comment
-	if err == nil && numLocks == 0 && u.SilenceNoProjects {
-		return
+	if err == nil && numLocks == 0 {
+		ctx.Log.Info("No locks to delete")
+		if u.SilenceNoProjects {
+			return
+		}
 	}
 
 	if commentErr := u.vcsClient.CreateComment(baseRepo, pullNum, vcsMessage, command.Unlock.String()); commentErr != nil {