diff --git a/server/controllers/locks_controller.go b/server/controllers/locks_controller.go index d32b10dd2f..3fce3f5ad6 100644 --- a/server/controllers/locks_controller.go +++ b/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) } diff --git a/server/events/delete_lock_command.go b/server/events/delete_lock_command.go index cf3eee8afe..ccd7f5d146 100644 --- a/server/events/delete_lock_command.go +++ b/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 } diff --git a/server/events/delete_lock_command_test.go b/server/events/delete_lock_command_test.go index 631ec629d0..ade6e64362 100644 --- a/server/events/delete_lock_command_test.go +++ b/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) { diff --git a/server/events/mock_workingdir_test.go b/server/events/mock_workingdir_test.go index ab2f78714a..78bc560eae 100644 --- a/server/events/mock_workingdir_test.go +++ b/server/events/mock_workingdir_test.go @@ -4,12 +4,11 @@ package events import ( - "reflect" - "time" - pegomock "github.com/petergtz/pegomock/v4" models "github.com/runatlantis/atlantis/server/events/models" logging "github.com/runatlantis/atlantis/server/logging" + "reflect" + "time" ) type MockWorkingDir struct { @@ -80,6 +79,21 @@ func (mock *MockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullReque return ret0 } +func (mock *MockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, workspace string, path string, projectName string) error { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockWorkingDir().") + } + params := []pegomock.Param{r, p, workspace, path, projectName} + result := pegomock.GetGenericMockFrom(mock).Invoke("DeletePlan", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(error) + } + } + return ret0 +} + func (mock *MockWorkingDir) GetPullDir(r models.Repo, p models.PullRequest) (string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") @@ -287,6 +301,49 @@ func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedAr return } +func (verifier *VerifierMockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, workspace string, path string, projectName string) *MockWorkingDir_DeletePlan_OngoingVerification { + params := []pegomock.Param{r, p, workspace, path, projectName} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeletePlan", params, verifier.timeout) + return &MockWorkingDir_DeletePlan_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockWorkingDir_DeletePlan_OngoingVerification struct { + mock *MockWorkingDir + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string, string, string) { + r, p, workspace, path, projectName := c.GetAllCapturedArguments() + return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1], path[len(path)-1], projectName[len(projectName)-1] +} + +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string, _param3 []string, _param4 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.PullRequest) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) + } + _param4 = make([]string, len(c.methodInvocations)) + for u, param := range params[4] { + _param4[u] = param.(string) + } + } + return +} + func (verifier *VerifierMockWorkingDir) GetPullDir(r models.Repo, p models.PullRequest) *MockWorkingDir_GetPullDir_OngoingVerification { params := []pegomock.Param{r, p} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetPullDir", params, verifier.timeout) diff --git a/server/events/mocks/mock_working_dir.go b/server/events/mocks/mock_working_dir.go index d64aed2c55..d923213499 100644 --- a/server/events/mocks/mock_working_dir.go +++ b/server/events/mocks/mock_working_dir.go @@ -79,6 +79,21 @@ func (mock *MockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullReque return ret0 } +func (mock *MockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, workspace string, path string, projectName string) error { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockWorkingDir().") + } + params := []pegomock.Param{r, p, workspace, path, projectName} + result := pegomock.GetGenericMockFrom(mock).Invoke("DeletePlan", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(error) + } + } + return ret0 +} + func (mock *MockWorkingDir) GetPullDir(r models.Repo, p models.PullRequest) (string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") @@ -286,6 +301,49 @@ func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedAr return } +func (verifier *VerifierMockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, workspace string, path string, projectName string) *MockWorkingDir_DeletePlan_OngoingVerification { + params := []pegomock.Param{r, p, workspace, path, projectName} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeletePlan", params, verifier.timeout) + return &MockWorkingDir_DeletePlan_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockWorkingDir_DeletePlan_OngoingVerification struct { + mock *MockWorkingDir + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string, string, string) { + r, p, workspace, path, projectName := c.GetAllCapturedArguments() + return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1], path[len(path)-1], projectName[len(projectName)-1] +} + +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string, _param3 []string, _param4 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.PullRequest) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) + } + _param4 = make([]string, len(c.methodInvocations)) + for u, param := range params[4] { + _param4[u] = param.(string) + } + } + return +} + func (verifier *VerifierMockWorkingDir) GetPullDir(r models.Repo, p models.PullRequest) *MockWorkingDir_GetPullDir_OngoingVerification { params := []pegomock.Param{r, p} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetPullDir", params, verifier.timeout) diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 07678c4b79..1192652aa1 100644 --- a/server/events/working_dir.go +++ b/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) +} diff --git a/server/server.go b/server/server.go index cf3224dda6..3c51d358ed 100644 --- a/server/server.go +++ b/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(