From 7a9c7a954e394f76d71a0aaa8132e94e8f21f935 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Wed, 28 Feb 2024 21:00:08 +0000 Subject: [PATCH 1/7] Refine the logging in the working dir package --- .../events/events_controller_e2e_test.go | 2 - server/controllers/locks_controller.go | 12 +- server/controllers/locks_controller_test.go | 12 +- server/events/command_requirement_handler.go | 6 +- .../command_requirement_handler_test.go | 13 +- server/events/command_runner_test.go | 85 +++++--- server/events/delete_lock_command.go | 21 +- server/events/delete_lock_command_test.go | 52 ++--- server/events/github_app_working_dir.go | 5 +- server/events/github_app_working_dir_test.go | 14 +- server/events/mock_workingdir_test.go | 181 ++++++++++-------- .../events/mocks/mock_delete_lock_command.go | 53 ++--- server/events/mocks/mock_working_dir.go | 181 ++++++++++-------- .../post_workflow_hooks_command_runner.go | 34 ++-- ...post_workflow_hooks_command_runner_test.go | 84 +++++--- .../pre_workflow_hooks_command_runner.go | 30 ++- .../pre_workflow_hooks_command_runner_test.go | 100 ++++++---- server/events/project_command_builder.go | 37 ++-- .../project_command_builder_internal_test.go | 41 ++-- server/events/project_command_builder_test.go | 102 +++++----- server/events/project_command_runner.go | 6 +- server/events/project_command_runner_test.go | 49 ++--- server/events/pull_closed_executor.go | 7 +- server/events/pull_closed_executor_test.go | 3 +- server/events/unlock_command_runner.go | 2 +- server/events/working_dir.go | 106 +++++----- server/events/working_dir_test.go | 43 ++--- server/server.go | 4 - 28 files changed, 668 insertions(+), 617 deletions(-) diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index e2d0a7115c..15cdd849bb 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1333,7 +1333,6 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers workingDir := &events.FileWorkspace{ DataDir: dataDir, TestingOverrideHeadCloneURL: "override-me", - Logger: logger, } var preWorkflowHooks []*valid.WorkflowHook if !opt.disablePreWorkflowHooks { @@ -1425,7 +1424,6 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers false, "auto", statsScope, - logger, terraformClient, ) diff --git a/server/controllers/locks_controller.go b/server/controllers/locks_controller.go index bab7fad27a..092afe22b8 100644 --- a/server/controllers/locks_controller.go +++ b/server/controllers/locks_controller.go @@ -73,7 +73,7 @@ func (l *LocksController) GetLock(w http.ResponseWriter, r *http.Request) { return } if lock == nil { - l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id %q", idUnencoded) + l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id '%s'", idUnencoded) return } @@ -107,18 +107,18 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) { idUnencoded, err := url.PathUnescape(id) if err != nil { - l.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id %q. Failed with error: %s", id, err) + l.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id '%s'. Failed with error: '%s'", id, err) return } - lock, err := l.DeleteLockCommand.DeleteLock(idUnencoded) + lock, err := l.DeleteLockCommand.DeleteLock(l.Logger, idUnencoded) if err != nil { - l.respond(w, logging.Error, http.StatusInternalServerError, "deleting lock failed with: %s", err) + l.respond(w, logging.Error, http.StatusInternalServerError, "deleting lock failed with: '%s'", err) return } if lock == nil { - l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id %q", idUnencoded) + l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id '%s'", idUnencoded) return } @@ -139,7 +139,7 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) { } else { l.Logger.Debug("skipping commenting on pull request and deleting workspace because BaseRepo field is empty") } - l.respond(w, logging.Info, http.StatusOK, "Deleted lock id %q", id) + l.respond(w, logging.Info, http.StatusOK, "Deleted lock id '%s'", id) } // respond is a helper function to respond and log the response. lvl is the log diff --git a/server/controllers/locks_controller_test.go b/server/controllers/locks_controller_test.go index 0f80e7c1f7..3541f31d48 100644 --- a/server/controllers/locks_controller_test.go +++ b/server/controllers/locks_controller_test.go @@ -222,7 +222,7 @@ func TestDeleteLock_LockerErr(t *testing.T) { t.Log("If there is an error retrieving the lock, a 500 is returned") RegisterMockTestingT(t) dlc := mocks2.NewMockDeleteLockCommand() - When(dlc.DeleteLock("id")).ThenReturn(nil, errors.New("err")) + When(dlc.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(nil, errors.New("err")) lc := controllers.LocksController{ DeleteLockCommand: dlc, Logger: logging.NewNoopLogger(t), @@ -238,7 +238,7 @@ func TestDeleteLock_None(t *testing.T) { t.Log("If there is no lock at that ID we get a 404") RegisterMockTestingT(t) dlc := mocks2.NewMockDeleteLockCommand() - When(dlc.DeleteLock("id")).ThenReturn(nil, nil) + When(dlc.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(nil, nil) lc := controllers.LocksController{ DeleteLockCommand: dlc, Logger: logging.NewNoopLogger(t), @@ -255,7 +255,7 @@ func TestDeleteLock_OldFormat(t *testing.T) { RegisterMockTestingT(t) cp := vcsmocks.NewMockClient() dlc := mocks2.NewMockDeleteLockCommand() - When(dlc.DeleteLock("id")).ThenReturn(&models.ProjectLock{}, nil) + When(dlc.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(&models.ProjectLock{}, nil) lc := controllers.LocksController{ DeleteLockCommand: dlc, Logger: logging.NewNoopLogger(t), @@ -284,7 +284,7 @@ func TestDeleteLock_UpdateProjectStatus(t *testing.T) { pull := models.PullRequest{ BaseRepo: models.Repo{FullName: repoName}, } - When(l.DeleteLock("id")).ThenReturn(&models.ProjectLock{ + When(l.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(&models.ProjectLock{ Pull: pull, Workspace: workspaceName, Project: models.Project{ @@ -338,7 +338,7 @@ func TestDeleteLock_CommentFailed(t *testing.T) { t.Log("If the commenting fails we still return success") RegisterMockTestingT(t) dlc := mocks2.NewMockDeleteLockCommand() - When(dlc.DeleteLock("id")).ThenReturn(&models.ProjectLock{ + When(dlc.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(&models.ProjectLock{ Pull: models.PullRequest{ BaseRepo: models.Repo{FullName: "owner/repo"}, }, @@ -380,7 +380,7 @@ func TestDeleteLock_CommentSuccess(t *testing.T) { pull := models.PullRequest{ BaseRepo: models.Repo{FullName: "owner/repo"}, } - When(dlc.DeleteLock("id")).ThenReturn(&models.ProjectLock{ + When(dlc.DeleteLock(Any[logging.SimpleLogging](), Eq("id"))).ThenReturn(&models.ProjectLock{ Pull: pull, Workspace: "workspace", Project: models.Project{ diff --git a/server/events/command_requirement_handler.go b/server/events/command_requirement_handler.go index 5c7b1c1d54..bf95a255ce 100644 --- a/server/events/command_requirement_handler.go +++ b/server/events/command_requirement_handler.go @@ -33,7 +33,7 @@ func (a *DefaultCommandRequirementHandler) ValidatePlanProject(repoDir string, c return "Pull request must be mergeable before running plan.", nil } case raw.UnDivergedRequirement: - if a.WorkingDir.HasDiverged(repoDir) { + if a.WorkingDir.HasDiverged(ctx.Log, repoDir) { return "Default branch must be rebased onto pull request before running plan.", nil } } @@ -60,7 +60,7 @@ func (a *DefaultCommandRequirementHandler) ValidateApplyProject(repoDir string, return "Pull request must be mergeable before running apply.", nil } case raw.UnDivergedRequirement: - if a.WorkingDir.HasDiverged(repoDir) { + if a.WorkingDir.HasDiverged(ctx.Log, repoDir) { return "Default branch must be rebased onto pull request before running apply.", nil } } @@ -95,7 +95,7 @@ func (a *DefaultCommandRequirementHandler) ValidateImportProject(repoDir string, return "Pull request must be mergeable before running import.", nil } case raw.UnDivergedRequirement: - if a.WorkingDir.HasDiverged(repoDir) { + if a.WorkingDir.HasDiverged(ctx.Log, repoDir) { return "Default branch must be rebased onto pull request before running import.", nil } } diff --git a/server/events/command_requirement_handler_test.go b/server/events/command_requirement_handler_test.go index 1c737f05aa..149e3a608b 100644 --- a/server/events/command_requirement_handler_test.go +++ b/server/events/command_requirement_handler_test.go @@ -9,6 +9,7 @@ import ( "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/mocks" @@ -46,7 +47,7 @@ func TestAggregateApplyRequirements_ValidatePlanProject(t *testing.T) { ProjectPlanStatus: models.PassedPolicyCheckStatus, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(false) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(false) }, wantErr: assert.NoError, }, @@ -76,7 +77,7 @@ func TestAggregateApplyRequirements_ValidatePlanProject(t *testing.T) { PlanRequirements: []string{raw.UnDivergedRequirement}, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(true) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(true) }, wantFailure: "Default branch must be rebased onto pull request before running plan.", wantErr: assert.NoError, @@ -130,7 +131,7 @@ func TestAggregateApplyRequirements_ValidateApplyProject(t *testing.T) { ProjectPlanStatus: models.PassedPolicyCheckStatus, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(false) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(false) }, wantErr: assert.NoError, }, @@ -184,7 +185,7 @@ func TestAggregateApplyRequirements_ValidateApplyProject(t *testing.T) { ApplyRequirements: []string{raw.UnDivergedRequirement}, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(true) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(true) }, wantFailure: "Default branch must be rebased onto pull request before running apply.", wantErr: assert.NoError, @@ -363,7 +364,7 @@ func TestAggregateApplyRequirements_ValidateImportProject(t *testing.T) { ProjectPlanStatus: models.PassedPolicyCheckStatus, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(false) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(false) }, wantErr: assert.NoError, }, @@ -393,7 +394,7 @@ func TestAggregateApplyRequirements_ValidateImportProject(t *testing.T) { ImportRequirements: []string{raw.UnDivergedRequirement}, }, setup: func(workingDir *mocks.MockWorkingDir) { - When(workingDir.HasDiverged(Any[string]())).ThenReturn(true) + When(workingDir.HasDiverged(Any[logging.SimpleLogging](), Any[string]())).ThenReturn(true) }, wantFailure: "Default branch must be rebased onto pull request before running import.", wantErr: assert.NoError, diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index bbc265fb8a..7dc53a9136 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -666,12 +666,16 @@ func TestRunUnlockCommand_VCSComment(t *testing.T) { State: tc.prState, } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) - When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, + testdata.GithubRepo, nil) - ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) + ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, + &events.CommentCommand{Name: command.Unlock}) - deleteLockCommand.VerifyWasCalledOnce().DeleteLocksByPull(testdata.GithubRepo.FullName, testdata.Pull.Num) + deleteLockCommand.VerifyWasCalledOnce().DeleteLocksByPull(Any[logging.SimpleLogging](), + Eq(testdata.GithubRepo.FullName), Eq(testdata.Pull.Num)) vcsClient.VerifyWasCalledOnce().CreateComment( Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num), Eq("All Atlantis locks for this PR have been unlocked and plans discarded"), Eq("unlock")) @@ -688,11 +692,15 @@ func TestRunUnlockCommandFail_VCSComment(t *testing.T) { State: github.String("open"), } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) - When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - When(deleteLockCommand.DeleteLocksByPull(testdata.GithubRepo.FullName, testdata.Pull.Num)).ThenReturn(0, errors.New("err")) + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, + testdata.GithubRepo, nil) + When(deleteLockCommand.DeleteLocksByPull(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo.FullName), + Eq(testdata.Pull.Num))).ThenReturn(0, errors.New("err")) - ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) + ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, + &events.CommentCommand{Name: command.Unlock}) vcsClient.VerifyWasCalledOnce().CreateComment( Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num), Eq("Failed to delete PR locks"), Eq("unlock")) @@ -708,15 +716,20 @@ func TestRunUnlockCommandFail_DisableUnlockLabel(t *testing.T) { State: github.String("open"), } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) - When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - When(deleteLockCommand.DeleteLocksByPull(testdata.GithubRepo.FullName, testdata.Pull.Num)).ThenReturn(0, errors.New("err")) - When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull))).ThenReturn([]string{doNotUnlock, "need-help"}, nil) - - ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) - - vcsClient.VerifyWasCalledOnce().CreateComment( - Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num), Eq("Not allowed to unlock PR with "+doNotUnlock+" label"), Eq("unlock")) + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, + testdata.GithubRepo, nil) + When(deleteLockCommand.DeleteLocksByPull(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo.FullName), + Eq(testdata.Pull.Num))).ThenReturn(0, errors.New("err")) + When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(modelPull))).ThenReturn([]string{doNotUnlock, "need-help"}, nil) + + ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, + &events.CommentCommand{Name: command.Unlock}) + + vcsClient.VerifyWasCalledOnce().CreateComment(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num), Eq("Not allowed to unlock PR with "+doNotUnlock+" label"), Eq("unlock")) } func TestRunUnlockCommandFail_GetLabelsFail(t *testing.T) { @@ -727,15 +740,20 @@ func TestRunUnlockCommandFail_GetLabelsFail(t *testing.T) { State: github.String("open"), } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) - When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - When(deleteLockCommand.DeleteLocksByPull(testdata.GithubRepo.FullName, testdata.Pull.Num)).ThenReturn(0, errors.New("err")) - When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull))).ThenReturn(nil, errors.New("err")) - - ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) - - vcsClient.VerifyWasCalledOnce().CreateComment( - Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num), Eq("Failed to retrieve PR labels... Not unlocking"), Eq("unlock")) + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, + testdata.GithubRepo, nil) + When(deleteLockCommand.DeleteLocksByPull(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo.FullName), + Eq(testdata.Pull.Num))).ThenReturn(0, errors.New("err")) + When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(modelPull))).ThenReturn(nil, errors.New("err")) + + ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, + &events.CommentCommand{Name: command.Unlock}) + + vcsClient.VerifyWasCalledOnce().CreateComment(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num), + Eq("Failed to retrieve PR labels... Not unlocking"), Eq("unlock")) } func TestRunUnlockCommandDoesntRetrieveLabelsIfDisableUnlockLabelNotSet(t *testing.T) { @@ -748,13 +766,18 @@ func TestRunUnlockCommandDoesntRetrieveLabelsIfDisableUnlockLabelNotSet(t *testi State: github.String("open"), } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) - When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - When(deleteLockCommand.DeleteLocksByPull(testdata.GithubRepo.FullName, testdata.Pull.Num)).ThenReturn(0, errors.New("err")) - When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull))).ThenReturn([]string{doNotUnlock, "need-help"}, nil) + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, + testdata.GithubRepo, nil) + When(deleteLockCommand.DeleteLocksByPull(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo.FullName), + Eq(testdata.Pull.Num))).ThenReturn(0, errors.New("err")) + When(ch.VCSClient.GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), + Eq(modelPull))).ThenReturn([]string{doNotUnlock, "need-help"}, nil) unlockCommandRunner.DisableUnlockLabel = "" - ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) + ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, + &events.CommentCommand{Name: command.Unlock}) vcsClient.VerifyWasCalled(Never()).GetPullLabels(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull)) } diff --git a/server/events/delete_lock_command.go b/server/events/delete_lock_command.go index 89016503fb..1c9abcdda0 100644 --- a/server/events/delete_lock_command.go +++ b/server/events/delete_lock_command.go @@ -6,25 +6,24 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) -//go:generate pegomock generate --package mocks -o mocks/mock_delete_lock_command.go DeleteLockCommand +//go:generate pegomock generate github.com/runatlantis/atlantis/server/events --package mocks -o mocks/mock_delete_lock_command.go DeleteLockCommand // DeleteLockCommand is the first step after a command request has been parsed. type DeleteLockCommand interface { - DeleteLock(id string) (*models.ProjectLock, error) - DeleteLocksByPull(repoFullName string, pullNum int) (int, error) + DeleteLock(logger logging.SimpleLogging, id string) (*models.ProjectLock, error) + DeleteLocksByPull(logger logging.SimpleLogging, repoFullName string, pullNum int) (int, error) } // DefaultDeleteLockCommand deletes a specific lock after a request from the LocksController. type DefaultDeleteLockCommand struct { Locker locking.Locker - Logger logging.SimpleLogging WorkingDir WorkingDir WorkingDirLocker WorkingDirLocker Backend locking.Backend } // DeleteLock handles deleting the lock at id -func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, error) { +func (l *DefaultDeleteLockCommand) DeleteLock(logger logging.SimpleLogging, id string) (*models.ProjectLock, error) { lock, err := l.Locker.Unlock(id) if err != nil { return nil, err @@ -33,9 +32,9 @@ func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, e return nil, nil } - removeErr := l.WorkingDir.DeletePlan(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, lock.Project.ProjectName) + removeErr := l.WorkingDir.DeletePlan(logger, lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, lock.Project.ProjectName) if removeErr != nil { - l.Logger.Warn("Failed to delete plan: %s", removeErr) + logger.Warn("Failed to delete plan: %s", removeErr) return nil, removeErr } @@ -43,23 +42,23 @@ func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, e } // DeleteLocksByPull handles deleting all locks for the pull request -func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNum int) (int, error) { +func (l *DefaultDeleteLockCommand) DeleteLocksByPull(logger logging.SimpleLogging, repoFullName string, pullNum int) (int, error) { locks, err := l.Locker.UnlockByPull(repoFullName, pullNum) numLocks := len(locks) if err != nil { return numLocks, err } if numLocks == 0 { - l.Logger.Debug("No locks found for repo '%v', pull request: %v", repoFullName, pullNum) + logger.Debug("No locks found for repo '%v', pull request: %v", repoFullName, pullNum) return numLocks, nil } for i := 0; i < numLocks; i++ { lock := locks[i] - err := l.WorkingDir.DeletePlan(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, lock.Project.ProjectName) + err := l.WorkingDir.DeletePlan(logger, lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path, lock.Project.ProjectName) if err != nil { - l.Logger.Warn("Failed to delete plan: %s", err) + logger.Warn("Failed to delete plan: %s", err) return numLocks, err } } diff --git a/server/events/delete_lock_command_test.go b/server/events/delete_lock_command_test.go index 75ffe0488b..2e652770b9 100644 --- a/server/events/delete_lock_command_test.go +++ b/server/events/delete_lock_command_test.go @@ -15,33 +15,30 @@ import ( func TestDeleteLock_LockerErr(t *testing.T) { t.Log("If there is an error retrieving the lock, we return the error") + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) l := lockmocks.NewMockLocker() When(l.Unlock("id")).ThenReturn(nil, errors.New("err")) - dlc := events.DefaultDeleteLockCommand{ - Locker: l, - Logger: logging.NewNoopLogger(t), - } - _, err := dlc.DeleteLock("id") + dlc := events.DefaultDeleteLockCommand{Locker: l} + _, err := dlc.DeleteLock(logger, "id") ErrEquals(t, "err", err) } func TestDeleteLock_None(t *testing.T) { t.Log("If there is no lock at that ID we return nil") + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) l := lockmocks.NewMockLocker() When(l.Unlock("id")).ThenReturn(nil, nil) - dlc := events.DefaultDeleteLockCommand{ - Locker: l, - Logger: logging.NewNoopLogger(t), - } - lock, err := dlc.DeleteLock("id") + dlc := events.DefaultDeleteLockCommand{Locker: l} + lock, err := dlc.DeleteLock(logger, "id") Ok(t, err) Assert(t, lock == nil, "lock was not nil") } func TestDeleteLock_Success(t *testing.T) { t.Log("Delete lock deletes successfully the plan file") + logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) l := lockmocks.NewMockLocker() When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil) @@ -66,19 +63,20 @@ func TestDeleteLock_Success(t *testing.T) { Ok(t, err) dlc := events.DefaultDeleteLockCommand{ Locker: l, - Logger: logging.NewNoopLogger(t), Backend: db, WorkingDirLocker: workingDirLocker, WorkingDir: workingDir, } - lock, err := dlc.DeleteLock("id") + lock, err := dlc.DeleteLock(logger, "id") Ok(t, err) Assert(t, lock != nil, "lock was nil") - workingDir.VerifyWasCalledOnce().DeletePlan(pull.BaseRepo, pull, workspace, path, projectName) + workingDir.VerifyWasCalledOnce().DeletePlan(Any[logging.SimpleLogging](), Eq(pull.BaseRepo), Eq(pull), Eq(workspace), + Eq(path), Eq(projectName)) } func TestDeleteLocksByPull_LockerErr(t *testing.T) { t.Log("If there is an error retrieving the lock, returned a failed status") + logger := logging.NewNoopLogger(t) repoName := "reponame" pullNum := 2 RegisterMockTestingT(t) @@ -87,16 +85,17 @@ func TestDeleteLocksByPull_LockerErr(t *testing.T) { When(l.UnlockByPull(repoName, pullNum)).ThenReturn(nil, errors.New("err")) dlc := events.DefaultDeleteLockCommand{ Locker: l, - Logger: logging.NewNoopLogger(t), WorkingDir: workingDir, } - _, err := dlc.DeleteLocksByPull(repoName, pullNum) + _, err := dlc.DeleteLocksByPull(logger, repoName, pullNum) ErrEquals(t, "err", err) - workingDir.VerifyWasCalled(Never()).DeletePlan(Any[models.Repo](), Any[models.PullRequest](), Any[string](), Any[string](), Any[string]()) + workingDir.VerifyWasCalled(Never()).DeletePlan(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string](), Any[string](), Any[string]()) } func TestDeleteLocksByPull_None(t *testing.T) { t.Log("If there is no lock at that ID there is no error") + logger := logging.NewNoopLogger(t) repoName := "reponame" pullNum := 2 RegisterMockTestingT(t) @@ -105,16 +104,17 @@ func TestDeleteLocksByPull_None(t *testing.T) { When(l.UnlockByPull(repoName, pullNum)).ThenReturn([]models.ProjectLock{}, nil) dlc := events.DefaultDeleteLockCommand{ Locker: l, - Logger: logging.NewNoopLogger(t), WorkingDir: workingDir, } - _, err := dlc.DeleteLocksByPull(repoName, pullNum) + _, err := dlc.DeleteLocksByPull(logger, repoName, pullNum) Ok(t, err) - workingDir.VerifyWasCalled(Never()).DeletePlan(Any[models.Repo](), Any[models.PullRequest](), Any[string](), Any[string](), Any[string]()) + workingDir.VerifyWasCalled(Never()).DeletePlan(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string](), Any[string](), Any[string]()) } func TestDeleteLocksByPull_SingleSuccess(t *testing.T) { t.Log("If a single lock is successfully deleted") + logger := logging.NewNoopLogger(t) repoName := "reponame" pullNum := 2 path := "." @@ -142,16 +142,17 @@ func TestDeleteLocksByPull_SingleSuccess(t *testing.T) { ) dlc := events.DefaultDeleteLockCommand{ Locker: l, - Logger: logging.NewNoopLogger(t), WorkingDir: workingDir, } - _, err := dlc.DeleteLocksByPull(repoName, pullNum) + _, err := dlc.DeleteLocksByPull(logger, repoName, pullNum) Ok(t, err) - workingDir.VerifyWasCalled(Once()).DeletePlan(pull.BaseRepo, pull, workspace, path, projectName) + workingDir.VerifyWasCalled(Once()).DeletePlan(Any[logging.SimpleLogging](), Eq(pull.BaseRepo), Eq(pull), Eq(workspace), + Eq(path), Eq(projectName)) } func TestDeleteLocksByPull_MultipleSuccess(t *testing.T) { t.Log("If multiple locks are successfully deleted") + logger := logging.NewNoopLogger(t) repoName := "reponame" pullNum := 2 path1 := "path1" @@ -187,11 +188,10 @@ func TestDeleteLocksByPull_MultipleSuccess(t *testing.T) { ) dlc := events.DefaultDeleteLockCommand{ Locker: l, - Logger: logging.NewNoopLogger(t), WorkingDir: workingDir, } - _, err := dlc.DeleteLocksByPull(repoName, pullNum) + _, err := dlc.DeleteLocksByPull(logger, 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) + workingDir.VerifyWasCalled(Once()).DeletePlan(logger, pull.BaseRepo, pull, workspace, path1, projectName) + workingDir.VerifyWasCalled(Once()).DeletePlan(logger, pull.BaseRepo, pull, workspace, path2, projectName) } diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 85435f8590..a06599efe0 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -5,6 +5,7 @@ import ( "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/logging" ) const redactedReplacement = "://:@" @@ -19,7 +20,7 @@ type GithubAppWorkingDir struct { } // Clone writes a fresh token for Github App authentication -func (g *GithubAppWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { +func (g *GithubAppWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { baseRepo := &p.BaseRepo // Realistically, this is a super brittle way of supporting clones using gh app installation tokens @@ -35,5 +36,5 @@ func (g *GithubAppWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:@", replacement, 1) headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, redactedReplacement, replacement, 1) - return g.WorkingDir.Clone(headRepo, p, workspace) + return g.WorkingDir.Clone(logger, headRepo, p, workspace) } diff --git a/server/events/github_app_working_dir_test.go b/server/events/github_app_working_dir_test.go index 28983da870..78e64d4e0b 100644 --- a/server/events/github_app_working_dir_test.go +++ b/server/events/github_app_working_dir_test.go @@ -29,7 +29,6 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { DataDir: dataDir, CheckoutMerge: false, TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), - Logger: logger, } defer disableSSLVerification()() @@ -46,7 +45,7 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { GithubHostname: testServer, } - cloneDir, _, err := gwd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, _, err := gwd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", }, "default") @@ -58,6 +57,8 @@ func TestClone_GithubAppNoneExisting(t *testing.T) { } func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { + logger := logging.NewNoopLogger(t) + RegisterMockTestingT(t) workingDir := eventMocks.NewMockWorkingDir() @@ -88,13 +89,12 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) { modifiedBaseRepo.SanitizedCloneURL = "https://github.com/runatlantis/atlantis.git" When(credentials.GetToken()).ThenReturn("token", nil) - When(workingDir.Clone(modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default")).ThenReturn( - "", true, nil, - ) + When(workingDir.Clone(Any[logging.SimpleLogging](), Eq(modifiedBaseRepo), Eq(models.PullRequest{BaseRepo: modifiedBaseRepo}), + Eq("default"))).ThenReturn("", true, nil) - _, success, _ := ghAppWorkingDir.Clone(headRepo, models.PullRequest{BaseRepo: baseRepo}, "default") + _, success, _ := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default") - workingDir.VerifyWasCalledOnce().Clone(modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default") + workingDir.VerifyWasCalledOnce().Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default") Assert(t, success == true, "clone url mutation error") } diff --git a/server/events/mock_workingdir_test.go b/server/events/mock_workingdir_test.go index 30b344ea3a..c2e070cfed 100644 --- a/server/events/mock_workingdir_test.go +++ b/server/events/mock_workingdir_test.go @@ -6,6 +6,7 @@ package events import ( pegomock "github.com/petergtz/pegomock/v4" models "github.com/runatlantis/atlantis/server/events/models" + logging "github.com/runatlantis/atlantis/server/logging" "reflect" "time" ) @@ -25,11 +26,11 @@ func NewMockWorkingDir(options ...pegomock.Option) *MockWorkingDir { func (mock *MockWorkingDir) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockWorkingDir) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { +func (mock *MockWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{headRepo, p, workspace} + params := []pegomock.Param{logger, headRepo, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("Clone", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 string var ret1 bool @@ -48,11 +49,11 @@ func (mock *MockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, wo return ret0, ret1, ret2 } -func (mock *MockWorkingDir) Delete(r models.Repo, p models.PullRequest) error { +func (mock *MockWorkingDir) Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p} + params := []pegomock.Param{logger, r, p} result := pegomock.GetGenericMockFrom(mock).Invoke("Delete", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -63,11 +64,11 @@ func (mock *MockWorkingDir) Delete(r models.Repo, p models.PullRequest) error { return ret0 } -func (mock *MockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) error { +func (mock *MockWorkingDir) DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p, workspace} + params := []pegomock.Param{logger, r, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("DeleteForWorkspace", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -78,11 +79,11 @@ 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 { +func (mock *MockWorkingDir) DeletePlan(logger logging.SimpleLogging, 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} + params := []pegomock.Param{logger, 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 { @@ -93,11 +94,11 @@ func (mock *MockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, work return ret0 } -func (mock *MockWorkingDir) GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) ([]string, error) { +func (mock *MockWorkingDir) GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) ([]string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p, workspace} + params := []pegomock.Param{logger, r, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("GetGitUntrackedFiles", params, []reflect.Type{reflect.TypeOf((*[]string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 []string var ret1 error @@ -150,11 +151,11 @@ func (mock *MockWorkingDir) GetWorkingDir(r models.Repo, p models.PullRequest, w return ret0, ret1 } -func (mock *MockWorkingDir) HasDiverged(cloneDir string) bool { +func (mock *MockWorkingDir) HasDiverged(logger logging.SimpleLogging, cloneDir string) bool { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{cloneDir} + params := []pegomock.Param{logger, cloneDir} result := pegomock.GetGenericMockFrom(mock).Invoke("HasDiverged", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) var ret0 bool if len(result) != 0 { @@ -210,8 +211,8 @@ type VerifierMockWorkingDir struct { timeout time.Duration } -func (verifier *VerifierMockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_Clone_OngoingVerification { - params := []pegomock.Param{headRepo, p, workspace} +func (verifier *VerifierMockWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_Clone_OngoingVerification { + params := []pegomock.Param{logger, headRepo, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Clone", params, verifier.timeout) return &MockWorkingDir_Clone_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -221,32 +222,36 @@ type MockWorkingDir_Clone_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_Clone_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - headRepo, p, workspace := c.GetAllCapturedArguments() - return headRepo[len(headRepo)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_Clone_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, headRepo, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], headRepo[len(headRepo)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_Clone_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_Clone_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } return } -func (verifier *VerifierMockWorkingDir) Delete(r models.Repo, p models.PullRequest) *MockWorkingDir_Delete_OngoingVerification { - params := []pegomock.Param{r, p} +func (verifier *VerifierMockWorkingDir) Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) *MockWorkingDir_Delete_OngoingVerification { + params := []pegomock.Param{logger, r, p} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Delete", params, verifier.timeout) return &MockWorkingDir_Delete_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -256,28 +261,32 @@ type MockWorkingDir_Delete_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_Delete_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { - r, p := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1] +func (c *MockWorkingDir_Delete_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest) { + logger, r, p := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1] } -func (c *MockWorkingDir_Delete_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { +func (c *MockWorkingDir_Delete_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) + } + _param2 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.PullRequest) } } return } -func (verifier *VerifierMockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_DeleteForWorkspace_OngoingVerification { - params := []pegomock.Param{r, p, workspace} +func (verifier *VerifierMockWorkingDir) DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_DeleteForWorkspace_OngoingVerification { + params := []pegomock.Param{logger, r, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeleteForWorkspace", params, verifier.timeout) return &MockWorkingDir_DeleteForWorkspace_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -287,32 +296,36 @@ type MockWorkingDir_DeleteForWorkspace_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - r, p, workspace := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, r, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } 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} +func (verifier *VerifierMockWorkingDir) DeletePlan(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string, path string, projectName string) *MockWorkingDir_DeletePlan_OngoingVerification { + params := []pegomock.Param{logger, 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} } @@ -322,25 +335,25 @@ type MockWorkingDir_DeletePlan_OngoingVerification struct { 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) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string, string, string) { + logger, r, p, workspace, path, projectName := c.GetAllCapturedArguments() + return logger[len(logger)-1], 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) { +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string, _param4 []string, _param5 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) } _param3 = make([]string, len(c.methodInvocations)) for u, param := range params[3] { @@ -350,12 +363,16 @@ func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments( for u, param := range params[4] { _param4[u] = param.(string) } + _param5 = make([]string, len(c.methodInvocations)) + for u, param := range params[5] { + _param5[u] = param.(string) + } } return } -func (verifier *VerifierMockWorkingDir) GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification { - params := []pegomock.Param{r, p, workspace} +func (verifier *VerifierMockWorkingDir) GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification { + params := []pegomock.Param{logger, r, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetGitUntrackedFiles", params, verifier.timeout) return &MockWorkingDir_GetGitUntrackedFiles_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -365,25 +382,29 @@ type MockWorkingDir_GetGitUntrackedFiles_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - r, p, workspace := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, r, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } return @@ -455,8 +476,8 @@ func (c *MockWorkingDir_GetWorkingDir_OngoingVerification) GetAllCapturedArgumen return } -func (verifier *VerifierMockWorkingDir) HasDiverged(cloneDir string) *MockWorkingDir_HasDiverged_OngoingVerification { - params := []pegomock.Param{cloneDir} +func (verifier *VerifierMockWorkingDir) HasDiverged(logger logging.SimpleLogging, cloneDir string) *MockWorkingDir_HasDiverged_OngoingVerification { + params := []pegomock.Param{logger, cloneDir} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "HasDiverged", params, verifier.timeout) return &MockWorkingDir_HasDiverged_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -466,17 +487,21 @@ type MockWorkingDir_HasDiverged_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetCapturedArguments() string { - cloneDir := c.GetAllCapturedArguments() - return cloneDir[len(cloneDir)-1] +func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string) { + logger, cloneDir := c.GetAllCapturedArguments() + return logger[len(logger)-1], cloneDir[len(cloneDir)-1] } -func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetAllCapturedArguments() (_param0 []string) { +func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(logging.SimpleLogging) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) } } return diff --git a/server/events/mocks/mock_delete_lock_command.go b/server/events/mocks/mock_delete_lock_command.go index ce1afd3b72..a8511f28c8 100644 --- a/server/events/mocks/mock_delete_lock_command.go +++ b/server/events/mocks/mock_delete_lock_command.go @@ -6,6 +6,7 @@ package mocks import ( pegomock "github.com/petergtz/pegomock/v4" models "github.com/runatlantis/atlantis/server/events/models" + logging "github.com/runatlantis/atlantis/server/logging" "reflect" "time" ) @@ -25,11 +26,11 @@ func NewMockDeleteLockCommand(options ...pegomock.Option) *MockDeleteLockCommand func (mock *MockDeleteLockCommand) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockDeleteLockCommand) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, error) { +func (mock *MockDeleteLockCommand) DeleteLock(logger logging.SimpleLogging, id string) (*models.ProjectLock, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockDeleteLockCommand().") } - params := []pegomock.Param{id} + params := []pegomock.Param{logger, id} result := pegomock.GetGenericMockFrom(mock).Invoke("DeleteLock", params, []reflect.Type{reflect.TypeOf((**models.ProjectLock)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 *models.ProjectLock var ret1 error @@ -44,11 +45,11 @@ func (mock *MockDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, e return ret0, ret1 } -func (mock *MockDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNum int) (int, error) { +func (mock *MockDeleteLockCommand) DeleteLocksByPull(logger logging.SimpleLogging, repoFullName string, pullNum int) (int, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockDeleteLockCommand().") } - params := []pegomock.Param{repoFullName, pullNum} + params := []pegomock.Param{logger, repoFullName, pullNum} result := pegomock.GetGenericMockFrom(mock).Invoke("DeleteLocksByPull", params, []reflect.Type{reflect.TypeOf((*int)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 int var ret1 error @@ -100,8 +101,8 @@ type VerifierMockDeleteLockCommand struct { timeout time.Duration } -func (verifier *VerifierMockDeleteLockCommand) DeleteLock(id string) *MockDeleteLockCommand_DeleteLock_OngoingVerification { - params := []pegomock.Param{id} +func (verifier *VerifierMockDeleteLockCommand) DeleteLock(logger logging.SimpleLogging, id string) *MockDeleteLockCommand_DeleteLock_OngoingVerification { + params := []pegomock.Param{logger, id} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeleteLock", params, verifier.timeout) return &MockDeleteLockCommand_DeleteLock_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -111,24 +112,28 @@ type MockDeleteLockCommand_DeleteLock_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockDeleteLockCommand_DeleteLock_OngoingVerification) GetCapturedArguments() string { - id := c.GetAllCapturedArguments() - return id[len(id)-1] +func (c *MockDeleteLockCommand_DeleteLock_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string) { + logger, id := c.GetAllCapturedArguments() + return logger[len(logger)-1], id[len(id)-1] } -func (c *MockDeleteLockCommand_DeleteLock_OngoingVerification) GetAllCapturedArguments() (_param0 []string) { +func (c *MockDeleteLockCommand_DeleteLock_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(logging.SimpleLogging) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) } } return } -func (verifier *VerifierMockDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNum int) *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification { - params := []pegomock.Param{repoFullName, pullNum} +func (verifier *VerifierMockDeleteLockCommand) DeleteLocksByPull(logger logging.SimpleLogging, repoFullName string, pullNum int) *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification { + params := []pegomock.Param{logger, repoFullName, pullNum} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeleteLocksByPull", params, verifier.timeout) return &MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -138,21 +143,25 @@ type MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification) GetCapturedArguments() (string, int) { - repoFullName, pullNum := c.GetAllCapturedArguments() - return repoFullName[len(repoFullName)-1], pullNum[len(pullNum)-1] +func (c *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string, int) { + logger, repoFullName, pullNum := c.GetAllCapturedArguments() + return logger[len(logger)-1], repoFullName[len(repoFullName)-1], pullNum[len(pullNum)-1] } -func (c *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []int) { +func (c *MockDeleteLockCommand_DeleteLocksByPull_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string, _param2 []int) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]int, len(c.methodInvocations)) + _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(int) + _param1[u] = param.(string) + } + _param2 = make([]int, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(int) } } return diff --git a/server/events/mocks/mock_working_dir.go b/server/events/mocks/mock_working_dir.go index 55ecc1ca4c..9c162fc4a2 100644 --- a/server/events/mocks/mock_working_dir.go +++ b/server/events/mocks/mock_working_dir.go @@ -6,6 +6,7 @@ package mocks import ( pegomock "github.com/petergtz/pegomock/v4" models "github.com/runatlantis/atlantis/server/events/models" + logging "github.com/runatlantis/atlantis/server/logging" "reflect" "time" ) @@ -25,11 +26,11 @@ func NewMockWorkingDir(options ...pegomock.Option) *MockWorkingDir { func (mock *MockWorkingDir) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockWorkingDir) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { +func (mock *MockWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{headRepo, p, workspace} + params := []pegomock.Param{logger, headRepo, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("Clone", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 string var ret1 bool @@ -48,11 +49,11 @@ func (mock *MockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, wo return ret0, ret1, ret2 } -func (mock *MockWorkingDir) Delete(r models.Repo, p models.PullRequest) error { +func (mock *MockWorkingDir) Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p} + params := []pegomock.Param{logger, r, p} result := pegomock.GetGenericMockFrom(mock).Invoke("Delete", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -63,11 +64,11 @@ func (mock *MockWorkingDir) Delete(r models.Repo, p models.PullRequest) error { return ret0 } -func (mock *MockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) error { +func (mock *MockWorkingDir) DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p, workspace} + params := []pegomock.Param{logger, r, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("DeleteForWorkspace", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -78,11 +79,11 @@ 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 { +func (mock *MockWorkingDir) DeletePlan(logger logging.SimpleLogging, 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} + params := []pegomock.Param{logger, 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 { @@ -93,11 +94,11 @@ func (mock *MockWorkingDir) DeletePlan(r models.Repo, p models.PullRequest, work return ret0 } -func (mock *MockWorkingDir) GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) ([]string, error) { +func (mock *MockWorkingDir) GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) ([]string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{r, p, workspace} + params := []pegomock.Param{logger, r, p, workspace} result := pegomock.GetGenericMockFrom(mock).Invoke("GetGitUntrackedFiles", params, []reflect.Type{reflect.TypeOf((*[]string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 []string var ret1 error @@ -150,11 +151,11 @@ func (mock *MockWorkingDir) GetWorkingDir(r models.Repo, p models.PullRequest, w return ret0, ret1 } -func (mock *MockWorkingDir) HasDiverged(cloneDir string) bool { +func (mock *MockWorkingDir) HasDiverged(logger logging.SimpleLogging, cloneDir string) bool { if mock == nil { panic("mock must not be nil. Use myMock := NewMockWorkingDir().") } - params := []pegomock.Param{cloneDir} + params := []pegomock.Param{logger, cloneDir} result := pegomock.GetGenericMockFrom(mock).Invoke("HasDiverged", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) var ret0 bool if len(result) != 0 { @@ -210,8 +211,8 @@ type VerifierMockWorkingDir struct { timeout time.Duration } -func (verifier *VerifierMockWorkingDir) Clone(headRepo models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_Clone_OngoingVerification { - params := []pegomock.Param{headRepo, p, workspace} +func (verifier *VerifierMockWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_Clone_OngoingVerification { + params := []pegomock.Param{logger, headRepo, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Clone", params, verifier.timeout) return &MockWorkingDir_Clone_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -221,32 +222,36 @@ type MockWorkingDir_Clone_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_Clone_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - headRepo, p, workspace := c.GetAllCapturedArguments() - return headRepo[len(headRepo)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_Clone_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, headRepo, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], headRepo[len(headRepo)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_Clone_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_Clone_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } return } -func (verifier *VerifierMockWorkingDir) Delete(r models.Repo, p models.PullRequest) *MockWorkingDir_Delete_OngoingVerification { - params := []pegomock.Param{r, p} +func (verifier *VerifierMockWorkingDir) Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) *MockWorkingDir_Delete_OngoingVerification { + params := []pegomock.Param{logger, r, p} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Delete", params, verifier.timeout) return &MockWorkingDir_Delete_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -256,28 +261,32 @@ type MockWorkingDir_Delete_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_Delete_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { - r, p := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1] +func (c *MockWorkingDir_Delete_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest) { + logger, r, p := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1] } -func (c *MockWorkingDir_Delete_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { +func (c *MockWorkingDir_Delete_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) + } + _param2 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.PullRequest) } } return } -func (verifier *VerifierMockWorkingDir) DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_DeleteForWorkspace_OngoingVerification { - params := []pegomock.Param{r, p, workspace} +func (verifier *VerifierMockWorkingDir) DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_DeleteForWorkspace_OngoingVerification { + params := []pegomock.Param{logger, r, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DeleteForWorkspace", params, verifier.timeout) return &MockWorkingDir_DeleteForWorkspace_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -287,32 +296,36 @@ type MockWorkingDir_DeleteForWorkspace_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - r, p, workspace := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, r, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_DeleteForWorkspace_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } 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} +func (verifier *VerifierMockWorkingDir) DeletePlan(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string, path string, projectName string) *MockWorkingDir_DeletePlan_OngoingVerification { + params := []pegomock.Param{logger, 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} } @@ -322,25 +335,25 @@ type MockWorkingDir_DeletePlan_OngoingVerification struct { 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) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string, string, string) { + logger, r, p, workspace, path, projectName := c.GetAllCapturedArguments() + return logger[len(logger)-1], 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) { +func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string, _param4 []string, _param5 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) } _param3 = make([]string, len(c.methodInvocations)) for u, param := range params[3] { @@ -350,12 +363,16 @@ func (c *MockWorkingDir_DeletePlan_OngoingVerification) GetAllCapturedArguments( for u, param := range params[4] { _param4[u] = param.(string) } + _param5 = make([]string, len(c.methodInvocations)) + for u, param := range params[5] { + _param5[u] = param.(string) + } } return } -func (verifier *VerifierMockWorkingDir) GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification { - params := []pegomock.Param{r, p, workspace} +func (verifier *VerifierMockWorkingDir) GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification { + params := []pegomock.Param{logger, r, p, workspace} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetGitUntrackedFiles", params, verifier.timeout) return &MockWorkingDir_GetGitUntrackedFiles_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -365,25 +382,29 @@ type MockWorkingDir_GetGitUntrackedFiles_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string) { - r, p, workspace := c.GetAllCapturedArguments() - return r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] +func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.PullRequest, string) { + logger, r, p, workspace := c.GetAllCapturedArguments() + return logger[len(logger)-1], r[len(r)-1], p[len(p)-1], workspace[len(workspace)-1] } -func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string) { +func (c *MockWorkingDir_GetGitUntrackedFiles_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(models.Repo) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } return @@ -455,8 +476,8 @@ func (c *MockWorkingDir_GetWorkingDir_OngoingVerification) GetAllCapturedArgumen return } -func (verifier *VerifierMockWorkingDir) HasDiverged(cloneDir string) *MockWorkingDir_HasDiverged_OngoingVerification { - params := []pegomock.Param{cloneDir} +func (verifier *VerifierMockWorkingDir) HasDiverged(logger logging.SimpleLogging, cloneDir string) *MockWorkingDir_HasDiverged_OngoingVerification { + params := []pegomock.Param{logger, cloneDir} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "HasDiverged", params, verifier.timeout) return &MockWorkingDir_HasDiverged_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -466,17 +487,21 @@ type MockWorkingDir_HasDiverged_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetCapturedArguments() string { - cloneDir := c.GetAllCapturedArguments() - return cloneDir[len(cloneDir)-1] +func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string) { + logger, cloneDir := c.GetAllCapturedArguments() + return logger[len(logger)-1], cloneDir[len(cloneDir)-1] } -func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetAllCapturedArguments() (_param0 []string) { +func (c *MockWorkingDir_HasDiverged_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(logging.SimpleLogging) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) } } return diff --git a/server/events/post_workflow_hooks_command_runner.go b/server/events/post_workflow_hooks_command_runner.go index 5e36794572..e1f7f10a9d 100644 --- a/server/events/post_workflow_hooks_command_runner.go +++ b/server/events/post_workflow_hooks_command_runner.go @@ -37,18 +37,10 @@ type DefaultPostWorkflowHooksCommandRunner struct { } // RunPostHooks runs post_workflow_hooks after a plan/apply has completed -func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks( - ctx *command.Context, cmd *CommentCommand, -) error { - pull := ctx.Pull - baseRepo := pull.BaseRepo - headRepo := ctx.HeadRepo - user := ctx.User - log := ctx.Log - +func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks(ctx *command.Context, cmd *CommentCommand) error { postWorkflowHooks := make([]*valid.WorkflowHook, 0) for _, repo := range w.GlobalCfg.Repos { - if repo.IDMatches(baseRepo.ID()) && repo.BranchMatches(pull.BaseBranch) && len(repo.PostWorkflowHooks) > 0 { + if repo.IDMatches(ctx.Pull.BaseRepo.ID()) && repo.BranchMatches(ctx.Pull.BaseBranch) && len(repo.PostWorkflowHooks) > 0 { postWorkflowHooks = append(postWorkflowHooks, repo.PostWorkflowHooks...) } } @@ -58,16 +50,16 @@ func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks( return nil } - log.Debug("post-hooks configured, running...") + ctx.Log.Debug("post-hooks configured, running...") - unlockFn, err := w.WorkingDirLocker.TryLock(baseRepo.FullName, pull.Num, DefaultWorkspace, DefaultRepoRelDir) + unlockFn, err := w.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, DefaultWorkspace, DefaultRepoRelDir) if err != nil { return err } - log.Debug("got workspace lock") + ctx.Log.Debug("got workspace lock") defer unlockFn() - repoDir, _, err := w.WorkingDir.Clone(headRepo, pull, DefaultWorkspace) + repoDir, _, err := w.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) if err != nil { return err } @@ -79,11 +71,11 @@ func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks( err = w.runHooks( models.WorkflowHookCommandContext{ - BaseRepo: baseRepo, - HeadRepo: headRepo, - Log: log, - Pull: pull, - User: user, + BaseRepo: ctx.Pull.BaseRepo, + HeadRepo: ctx.HeadRepo, + Log: ctx.Log, + Pull: ctx.Pull, + User: ctx.User, Verbose: false, EscapedCommentArgs: escapedArgs, CommandName: cmd.Name.String(), @@ -123,12 +115,12 @@ func (w *DefaultPostWorkflowHooksCommandRunner) runHooks( ctx.HookID = uuid.NewString() shell := hook.Shell if shell == "" { - ctx.Log.Debug("Setting shell to default: %q", shell) + ctx.Log.Debug("Setting shell to default: '%s'", shell) shell = "sh" } shellArgs := hook.ShellArgs if shellArgs == "" { - ctx.Log.Debug("Setting shellArgs to default: %q", shellArgs) + ctx.Log.Debug("Setting shellArgs to default: '%s'", shellArgs) shellArgs = "-c" } url, err := w.Router.GenerateProjectWorkflowHookURL(ctx.HookID) diff --git a/server/events/post_workflow_hooks_command_runner_test.go b/server/events/post_workflow_hooks_command_runner_test.go index 38cd5ee9ec..29996d8028 100644 --- a/server/events/post_workflow_hooks_command_runner_test.go +++ b/server/events/post_workflow_hooks_command_runner_test.go @@ -140,8 +140,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -180,7 +182,8 @@ func TestRunPostHooks_Clone(t *testing.T) { whPostWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) postWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "path") - postWhWorkingDir.VerifyWasCalled(Never()).Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace) + postWhWorkingDir.VerifyWasCalled(Never()).Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace)) }) t.Run("error locking work dir", func(t *testing.T) { postWorkflowHooksSetup(t) @@ -198,12 +201,14 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(func() {}, errors.New("some error")) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(func() {}, errors.New("some error")) err := postWh.RunPostHooks(ctx, planCmd) Assert(t, err != nil, "error not nil") - postWhWorkingDir.VerifyWasCalled(Never()).Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace) + postWhWorkingDir.VerifyWasCalled(Never()).Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace)) whPostWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) }) @@ -229,8 +234,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, errors.New("some error")) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, errors.New("some error")) err := postWh.RunPostHooks(ctx, planCmd) @@ -262,8 +269,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, errors.New("some error")) @@ -302,8 +311,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -336,8 +347,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithShell.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -370,8 +383,10 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -404,16 +419,19 @@ func TestRunPostHooks_Clone(t *testing.T) { postWh.GlobalCfg = globalCfg - When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(postWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithShellandShellArgs.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(postWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(postWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPostWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithShellandShellArgs.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := postWh.RunPostHooks(ctx, planCmd) Ok(t, err) whPostWorkflowHookRunner.VerifyWasCalledOnce().Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithShellandShellArgs.RunCommand), Eq(testHookWithShellandShellArgs.Shell), Eq(testHookWithShellandShellArgs.ShellArgs), Eq(repoDir)) + Eq(testHookWithShellandShellArgs.RunCommand), Eq(testHookWithShellandShellArgs.Shell), + Eq(testHookWithShellandShellArgs.ShellArgs), Eq(repoDir)) Assert(t, *unlockCalled == true, "unlock function called") }) @@ -438,8 +456,10 @@ func TestRunPostHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanCommand.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -472,10 +492,12 @@ func TestRunPostHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithPlanCommand.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanCommand.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, applyCmd) @@ -506,10 +528,12 @@ func TestRunPostHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithPlanApplyCommands.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanApplyCommands.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, planCmd) diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index 17b9864757..0e81f15ab9 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -38,15 +38,9 @@ type DefaultPreWorkflowHooksCommandRunner struct { // RunPreHooks runs pre_workflow_hooks when PR is opened or updated. func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, cmd *CommentCommand) error { - pull := ctx.Pull - baseRepo := pull.BaseRepo - headRepo := ctx.HeadRepo - user := ctx.User - log := ctx.Log - preWorkflowHooks := make([]*valid.WorkflowHook, 0) for _, repo := range w.GlobalCfg.Repos { - if repo.IDMatches(baseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 { + if repo.IDMatches(ctx.Pull.BaseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 { preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...) } } @@ -56,16 +50,16 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, return nil } - log.Debug("pre-hooks configured, running...") + ctx.Log.Debug("pre-hooks configured, running...") - unlockFn, err := w.WorkingDirLocker.TryLock(baseRepo.FullName, pull.Num, DefaultWorkspace, DefaultRepoRelDir) + unlockFn, err := w.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, DefaultWorkspace, DefaultRepoRelDir) if err != nil { return err } - log.Debug("got workspace lock") + ctx.Log.Debug("got workspace lock") defer unlockFn() - repoDir, _, err := w.WorkingDir.Clone(headRepo, pull, DefaultWorkspace) + repoDir, _, err := w.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) if err != nil { return err } @@ -89,11 +83,11 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, err = w.runHooks( models.WorkflowHookCommandContext{ - BaseRepo: baseRepo, - HeadRepo: headRepo, - Log: log, - Pull: pull, - User: user, + BaseRepo: ctx.Pull.BaseRepo, + HeadRepo: ctx.HeadRepo, + Log: ctx.Log, + Pull: ctx.Pull, + User: ctx.User, Verbose: false, EscapedCommentArgs: escapedArgs, CommandName: cmd.Name.String(), @@ -132,12 +126,12 @@ func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( ctx.HookID = uuid.NewString() shell := hook.Shell if shell == "" { - ctx.Log.Debug("Setting shell to default: %q", shell) + ctx.Log.Debug("Setting shell to default: '%s'", shell) shell = "sh" } shellArgs := hook.ShellArgs if shellArgs == "" { - ctx.Log.Debug("Setting shellArgs to default: %q", shellArgs) + ctx.Log.Debug("Setting shellArgs to default: '%s'", shellArgs) shellArgs = "-c" } url, err := w.Router.GenerateProjectWorkflowHookURL(ctx.HookID) diff --git a/server/events/pre_workflow_hooks_command_runner_test.go b/server/events/pre_workflow_hooks_command_runner_test.go index 3156797f86..191a8c27dc 100644 --- a/server/events/pre_workflow_hooks_command_runner_test.go +++ b/server/events/pre_workflow_hooks_command_runner_test.go @@ -142,8 +142,10 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -180,9 +182,11 @@ func TestRunPreHooks_Clone(t *testing.T) { Ok(t, err) - whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), + Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) preWhWorkingDirLocker.VerifyWasCalled(Never()).TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, "") - preWhWorkingDir.VerifyWasCalled(Never()).Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace) + preWhWorkingDir.VerifyWasCalled(Never()).Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace)) }) t.Run("error locking work dir", func(t *testing.T) { @@ -201,13 +205,16 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(func() {}, errors.New("some error")) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(func() {}, errors.New("some error")) err := preWh.RunPreHooks(ctx, planCmd) Assert(t, err != nil, "error not nil") - preWhWorkingDir.VerifyWasCalled(Never()).Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace) - whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) + preWhWorkingDir.VerifyWasCalled(Never()).Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace)) + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), + Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) }) t.Run("error cloning", func(t *testing.T) { @@ -231,14 +238,17 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, errors.New("some error")) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, errors.New("some error")) err := preWh.RunPreHooks(ctx, planCmd) Assert(t, err != nil, "error not nil") - whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), + Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) Assert(t, *unlockCalled == true, "unlock function called") }) @@ -263,8 +273,10 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, errors.New("some error")) @@ -303,14 +315,18 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), + Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, planCmd) Ok(t, err) - whPreWorkflowHookRunner.VerifyWasCalledOnce().Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) + whPreWorkflowHookRunner.VerifyWasCalledOnce().Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), + Eq(defaultShell), Eq(defaultShellArgs), Eq(repoDir)) Assert(t, *unlockCalled == true, "unlock function called") }) @@ -335,8 +351,10 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithShell.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -369,8 +387,10 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHook.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) @@ -403,10 +423,12 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithShellandShellArgs.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithShellandShellArgs.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, planCmd) @@ -438,10 +460,12 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithPlanCommand.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanCommand.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, planCmd) @@ -472,10 +496,12 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithPlanCommand.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanCommand.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, applyCmd) @@ -506,10 +532,12 @@ func TestRunPreHooks_Clone(t *testing.T) { preWh.GlobalCfg = globalCfg - When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) - When(preWhWorkingDir.Clone(testdata.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) - When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), - Eq(testHookWithPlanApplyCommands.RunCommand), Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) + When(preWhWorkingDirLocker.TryLock(testdata.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace, + events.DefaultRepoRelDir)).ThenReturn(unlockFn, nil) + When(preWhWorkingDir.Clone(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(newPull), + Eq(events.DefaultWorkspace))).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(Any[models.WorkflowHookCommandContext](), Eq(testHookWithPlanApplyCommands.RunCommand), + Any[string](), Any[string](), Eq(repoDir))).ThenReturn(result, runtimeDesc, nil) err := preWh.RunPreHooks(ctx, planCmd) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 98f1ef9997..659cc1da2e 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -11,7 +11,6 @@ import ( "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/core/terraform" - "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/metrics" "github.com/pkg/errors" @@ -57,7 +56,6 @@ func NewInstrumentedProjectCommandBuilder( IncludeGitUntrackedFiles bool, AutoDiscoverMode string, scope tally.Scope, - logger logging.SimpleLogging, terraformClient terraform.Client, ) *InstrumentedProjectCommandBuilder { scope = scope.SubScope("builder") @@ -89,10 +87,8 @@ func NewInstrumentedProjectCommandBuilder( IncludeGitUntrackedFiles, AutoDiscoverMode, scope, - logger, terraformClient, ), - Logger: logger, scope: scope, } } @@ -119,7 +115,6 @@ func NewProjectCommandBuilder( IncludeGitUntrackedFiles bool, AutoDiscoverMode string, scope tally.Scope, - _ logging.SimpleLogging, terraformClient terraform.Client, ) *DefaultProjectCommandBuilder { return &DefaultProjectCommandBuilder{ @@ -262,7 +257,7 @@ func (p *DefaultProjectCommandBuilder) BuildAutoplanCommands(ctx *command.Contex var autoplanEnabled []command.ProjectContext for _, projCtx := range projCtxs { if !projCtx.AutoplanEnabled { - ctx.Log.Debug("ignoring project at dir %q, workspace: %q because autoplan is disabled", projCtx.RepoRelDir, projCtx.Workspace) + ctx.Log.Debug("ignoring project at dir '%s', workspace: '%s' because autoplan is disabled", projCtx.RepoRelDir, projCtx.Workspace) continue } autoplanEnabled = append(autoplanEnabled, projCtx) @@ -334,7 +329,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex if p.IncludeGitUntrackedFiles { ctx.Log.Debug(("'include-git-untracked-files' option is set, getting untracked files")) - untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.HeadRepo, ctx.Pull, DefaultWorkspace) + untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) if err != nil { return nil, err } @@ -402,7 +397,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex ctx.Log.Debug("got workspace lock") defer unlockFn() - repoDir, _, err := p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, workspace) + repoDir, _, err := p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, workspace) if err != nil { return nil, err } @@ -411,7 +406,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) hasRepoCfg, err := p.ParserValidator.HasRepoCfg(repoDir, repoCfgFile) if err != nil { - return nil, errors.Wrapf(err, "looking for %s file in %q", repoCfgFile, repoDir) + return nil, errors.Wrapf(err, "looking for '%s' file in '%s'", repoCfgFile, repoDir) } var projCtxs []command.ProjectContext @@ -440,7 +435,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex if err != nil { ctx.Log.Warn("error(s) loading project module dependencies: %s", err) } - ctx.Log.Debug("moduleInfo for %s (matching %q) = %v", repoDir, p.AutoDetectModuleFiles, moduleInfo) + ctx.Log.Debug("moduleInfo for '%s' (matching '%s') = %v", repoDir, p.AutoDetectModuleFiles, moduleInfo) automerge := p.EnableAutoMerge parallelApply := p.EnableParallelApply @@ -467,7 +462,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex ctx.Log.Info("%d projects are to be planned based on their when_modified config", len(matchingProjects)) for _, mp := range matchingProjects { - ctx.Log.Debug("determining config for project at dir: %q workspace: %q", mp.Dir, mp.Workspace) + ctx.Log.Debug("determining config for project at dir: '%s' workspace: '%s'", mp.Dir, mp.Workspace) mergedCfg := p.GlobalCfg.MergeProjectCfg(ctx.Log, ctx.Pull.BaseRepo.ID(), mp, repoCfg) projCtxs = append(projCtxs, @@ -523,7 +518,7 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex ctx.Log.Info("automatically determined that there were %d additional projects modified in this pull request: %s", len(modifiedProjects), modifiedProjects) for _, mp := range modifiedProjects { - ctx.Log.Debug("determining config for project at dir: %q", mp.Path) + ctx.Log.Debug("determining config for project at dir: '%s'", mp.Path) pWorkspace, err := p.ProjectFinder.DetermineWorkspaceFromHCL(ctx.Log, repoDir) if err != nil { return nil, errors.Wrapf(err, "looking for Terraform Cloud workspace from configuration %s", repoDir) @@ -574,7 +569,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont defer unlockFn() ctx.Log.Debug("cloning repository") - _, _, err = p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, DefaultWorkspace) + _, _, err = p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) if err != nil { return pcc, err } @@ -595,7 +590,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont if p.IncludeGitUntrackedFiles { ctx.Log.Debug(("'include-git-untracked-files' option is set, getting untracked files")) - untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.HeadRepo, ctx.Pull, workspace) + untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.Log, ctx.HeadRepo, ctx.Pull, workspace) if err != nil { return nil, err } @@ -652,7 +647,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont if DefaultWorkspace != workspace { ctx.Log.Debug("cloning repository with workspace %s", workspace) - _, _, err = p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, workspace) + _, _, err = p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, workspace) if err != nil { return pcc, err } @@ -682,7 +677,7 @@ func (p *DefaultProjectCommandBuilder) getCfg(ctx *command.Context, projectName repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) hasRepoCfg, err := p.ParserValidator.HasRepoCfg(repoDir, repoCfgFile) if err != nil { - err = errors.Wrapf(err, "looking for %s file in %q", repoCfgFile, repoDir) + err = errors.Wrapf(err, "looking for '%s' file in '%s'", repoCfgFile, repoDir) return } if !hasRepoCfg { @@ -712,9 +707,9 @@ func (p *DefaultProjectCommandBuilder) getCfg(ctx *command.Context, projectName } if len(projectsCfg) == 0 { if p.SilenceNoProjects && len(repoConfig.Projects) > 0 { - ctx.Log.Debug("no project with name %q found but silencing the error", projectName) + ctx.Log.Debug("no project with name '%s' found but silencing the error", projectName) } else { - err = fmt.Errorf("no project with name %q is defined in %s", projectName, repoCfgFile) + err = fmt.Errorf("no project with name '%s' is defined in '%s'", projectName, repoCfgFile) } return } @@ -726,7 +721,7 @@ func (p *DefaultProjectCommandBuilder) getCfg(ctx *command.Context, projectName return } if len(projCfgs) > 1 { - err = fmt.Errorf("must specify project name: more than one project defined in %s matched dir: %q workspace: %q", repoCfgFile, dir, workspace) + err = fmt.Errorf("must specify project name: more than one project defined in '%s' matched dir: '%s' workspace: '%s'", repoCfgFile, dir, workspace) return } projectsCfg = projCfgs @@ -765,7 +760,7 @@ func (p *DefaultProjectCommandBuilder) buildAllProjectCommandsByPlan(ctx *comman for _, plan := range plans { commentCmds, err := p.buildProjectCommandCtx(ctx, commentCmd.CommandName(), commentCmd.SubName, plan.ProjectName, commentCmd.Flags, defaultRepoDir, plan.RepoRelDir, plan.Workspace, commentCmd.Verbose) if err != nil { - return nil, errors.Wrapf(err, "building command for dir %q", plan.RepoRelDir) + return nil, errors.Wrapf(err, "building command for dir '%s'", plan.RepoRelDir) } cmds = append(cmds, commentCmds...) } @@ -861,7 +856,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommandCtx(ctx *command.Conte repoRelDir = projCfg.RepoRelDir workspace = projCfg.Workspace for _, mp := range matchingProjects { - ctx.Log.Debug("Merging config for project at dir: %q workspace: %q", mp.Dir, mp.Workspace) + ctx.Log.Debug("Merging config for project at dir: '%s' workspace: '%s'", mp.Dir, mp.Workspace) projCfg = p.GlobalCfg.MergeProjectCfg(ctx.Log, ctx.Pull.BaseRepo.ID(), mp, *repoCfgPtr) projCtxs = append(projCtxs, diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index fc7c022073..2d45006959 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -630,10 +630,11 @@ projects: }) workingDir := NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmp, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmp, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) // Write and parse the global config file. globalCfgPath := filepath.Join(tmp, "global.yaml") @@ -671,7 +672,6 @@ projects: false, "auto", statsScope, - logger, terraformClient, ) @@ -845,10 +845,11 @@ projects: }) workingDir := NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmp, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmp, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) // Write and parse the global config file. globalCfgPath := filepath.Join(tmp, "global.yaml") @@ -862,7 +863,6 @@ projects: Ok(t, os.WriteFile(filepath.Join(tmp, "atlantis.yaml"), []byte(c.repoCfg), 0600)) } - logger := logging.NewNoopLogger(t) statsScope, _, _ := metrics.NewLoggingScope(logging.NewNoopLogger(t), "atlantis") terraformClient := mocks.NewMockClient() @@ -889,7 +889,6 @@ projects: false, "auto", statsScope, - logger, terraformClient, ) @@ -1091,10 +1090,11 @@ workflows: }) workingDir := NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmp, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmp, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) // Write and parse the global config file. globalCfgPath := filepath.Join(tmp, "global.yaml") @@ -1136,7 +1136,6 @@ workflows: false, "auto", statsScope, - logger, terraformClient, ) @@ -1246,10 +1245,11 @@ projects: }) workingDir := NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmp, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmp, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"modules/module/main.tf"}, nil) // Write and parse the global config file. globalCfgPath := filepath.Join(tmp, "global.yaml") @@ -1289,7 +1289,6 @@ projects: false, "auto", statsScope, - logger, terraformClient, ) @@ -1386,10 +1385,11 @@ projects: }) workingDir := NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmp, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmp, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.modifiedFiles, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(c.modifiedFiles, nil) // Write and parse the global config file. globalCfgPath := filepath.Join(tmp, "global.yaml") @@ -1431,7 +1431,6 @@ projects: false, "auto", statsScope, - logger, terraformClient, ) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 3bcc0294be..497fb15541 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -161,10 +161,11 @@ projects: }) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -194,7 +195,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -511,11 +511,12 @@ projects: }) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -550,7 +551,6 @@ projects: userConfig.IncludeGitUntrackedFiles, c.AutoDiscoverModeUserCfg, scope, - logger, terraformClient, ) @@ -700,11 +700,12 @@ projects: tmpDir := DirStructure(t, c.DirectoryStructure) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -739,7 +740,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1030,11 +1030,12 @@ projects: tmpDir := DirStructure(t, c.DirStructure) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -1069,7 +1070,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1169,7 +1169,6 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1221,14 +1220,9 @@ projects: err := os.WriteFile(filepath.Join(repoDir, valid.DefaultAtlantisFile), []byte(yamlCfg), 0600) Ok(t, err) - When(workingDir.Clone( - Any[models.Repo](), - Any[models.PullRequest](), + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(repoDir, false, nil) - When(workingDir.GetWorkingDir( - Any[models.Repo](), - Any[models.PullRequest](), - Any[string]())).ThenReturn(repoDir, nil) + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(repoDir, nil) globalCfgArgs := valid.GlobalCfgArgs{ AllowAllRepoSettings: true, @@ -1262,7 +1256,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1316,11 +1309,12 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { }) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) globalCfgArgs := valid.GlobalCfgArgs{ AllowAllRepoSettings: true, @@ -1351,7 +1345,6 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1470,19 +1463,12 @@ projects: tmpDir := DirStructure(t, testCase.DirStructure) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(testCase.ModifiedFiles, nil) - + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(testCase.ModifiedFiles, nil) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone( - Any[models.Repo](), - Any[models.PullRequest](), + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) - - When(workingDir.GetWorkingDir( - Any[models.Repo](), - Any[models.PullRequest](), - Any[string]())).ThenReturn(tmpDir, nil) + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) globalCfgArgs := valid.GlobalCfgArgs{ AllowAllRepoSettings: true, @@ -1521,7 +1507,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1633,7 +1618,6 @@ projects: userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1651,7 +1635,8 @@ projects: }) Ok(t, err) Equals(t, c.ExpectedCtxs, len(actCtxs)) - workingDir.VerifyWasCalled(c.ExpectedClones).Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) + workingDir.VerifyWasCalled(c.ExpectedClones).Clone(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest](), Any[string]()) } } @@ -1666,10 +1651,11 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman userConfig := defaultUserConfig workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"main.tf"}, nil) globalCfgArgs := valid.GlobalCfgArgs{ AllowAllRepoSettings: false, @@ -1702,7 +1688,6 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1792,7 +1777,6 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1886,12 +1870,14 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_Single_With_RestrictFile tmpDir := DirStructure(t, c.DirectoryStructure) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) - When(workingDir.GetGitUntrackedFiles(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(c.UntrackedFiles, nil) + When(workingDir.GetGitUntrackedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(c.UntrackedFiles, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -1922,7 +1908,6 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_Single_With_RestrictFile userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) @@ -1997,12 +1982,14 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_with_IncludeGitUntracked tmpDir := DirStructure(t, c.DirectoryStructure) workingDir := mocks.NewMockWorkingDir() - When(workingDir.Clone(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, false, nil) + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, false, nil) When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) - When(workingDir.GetGitUntrackedFiles(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(c.UntrackedFiles, nil) + When(workingDir.GetGitUntrackedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(c.UntrackedFiles, nil) vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetModifiedFiles( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err) @@ -2033,7 +2020,6 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_with_IncludeGitUntracked userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, - logger, terraformClient, ) diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index cd0130ab6e..2ad783efd1 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -554,7 +554,7 @@ func (p *DefaultProjectCommandRunner) doPlan(ctx command.ProjectContext) (*model p.WorkingDir.SetCheckForUpstreamChanges() // Clone is idempotent so okay to run even if the repo was already cloned. - repoDir, mergedAgain, cloneErr := p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, ctx.Workspace) + repoDir, mergedAgain, cloneErr := p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, ctx.Workspace) if cloneErr != nil { if unlockErr := lockAttempt.UnlockFn(); unlockErr != nil { ctx.Log.Err("error unlocking state after plan error: %v", unlockErr) @@ -667,7 +667,7 @@ func (p *DefaultProjectCommandRunner) doVersion(ctx command.ProjectContext) (ver func (p *DefaultProjectCommandRunner) doImport(ctx command.ProjectContext) (out *models.ImportSuccess, failure string, err error) { // Clone is idempotent so okay to run even if the repo was already cloned. - repoDir, _, cloneErr := p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, ctx.Workspace) + repoDir, _, cloneErr := p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, ctx.Workspace) if cloneErr != nil { return nil, "", cloneErr } @@ -713,7 +713,7 @@ func (p *DefaultProjectCommandRunner) doImport(ctx command.ProjectContext) (out func (p *DefaultProjectCommandRunner) doStateRm(ctx command.ProjectContext) (out *models.StateRmSuccess, failure string, err error) { // Clone is idempotent so okay to run even if the repo was already cloned. - repoDir, _, cloneErr := p.WorkingDir.Clone(ctx.HeadRepo, ctx.Pull, ctx.Workspace) + repoDir, _, cloneErr := p.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, ctx.Workspace) if cloneErr != nil { return nil, "", cloneErr } diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index 786b67088e..103966ef52 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -63,22 +63,10 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { } repoDir := t.TempDir() - When(mockWorkingDir.Clone( - Any[models.Repo](), - Any[models.PullRequest](), - Any[string](), - )).ThenReturn(repoDir, false, nil) - When(mockLocker.TryLock( - Any[logging.SimpleLogging](), - Any[models.PullRequest](), - Any[models.User](), - Any[string](), - Any[models.Project](), - AnyBool(), - )).ThenReturn(&events.TryLockResponse{ - LockAcquired: true, - LockKey: "lock-key", - }, nil) + When(mockWorkingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(repoDir, false, nil) + When(mockLocker.TryLock(Any[logging.SimpleLogging](), Any[models.PullRequest](), Any[models.User](), Any[string](), + Any[models.Project](), AnyBool())).ThenReturn(&events.TryLockResponse{LockAcquired: true, LockKey: "lock-key"}, nil) expEnvs := map[string]string{ "name": "value", @@ -317,7 +305,7 @@ func TestDefaultProjectCommandRunner_ApplyDiverged(t *testing.T) { } tmp := t.TempDir() When(mockWorkingDir.GetWorkingDir(ctx.BaseRepo, ctx.Pull, ctx.Workspace)).ThenReturn(tmp, nil) - When(mockWorkingDir.HasDiverged(tmp)).ThenReturn(true) + When(mockWorkingDir.HasDiverged(ctx.Log, tmp)).ThenReturn(true) res := runner.Apply(ctx) Equals(t, "Default branch must be rebased onto pull request before running apply.", res.Failure) @@ -560,22 +548,10 @@ func TestDefaultProjectCommandRunner_RunEnvSteps(t *testing.T) { } repoDir := t.TempDir() - When(mockWorkingDir.Clone( - Any[models.Repo](), - Any[models.PullRequest](), - Any[string](), - )).ThenReturn(repoDir, false, nil) - When(mockLocker.TryLock( - Any[logging.SimpleLogging](), - Any[models.PullRequest](), - Any[models.User](), - Any[string](), - Any[models.Project](), - AnyBool(), - )).ThenReturn(&events.TryLockResponse{ - LockAcquired: true, - LockKey: "lock-key", - }, nil) + When(mockWorkingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(repoDir, false, nil) + When(mockLocker.TryLock(Any[logging.SimpleLogging](), Any[models.PullRequest](), Any[models.User](), Any[string](), + Any[models.Project](), AnyBool())).ThenReturn(&events.TryLockResponse{LockAcquired: true, LockKey: "lock-key"}, nil) ctx := command.ProjectContext{ Log: logging.NewNoopLogger(t), @@ -714,11 +690,8 @@ func TestDefaultProjectCommandRunner_Import(t *testing.T) { RePlanCmd: "atlantis plan -d . -- addr id", } repoDir := t.TempDir() - When(mockWorkingDir.Clone( - Any[models.Repo](), - Any[models.PullRequest](), - Any[string](), - )).ThenReturn(repoDir, false, nil) + When(mockWorkingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(repoDir, false, nil) if c.setup != nil { c.setup(repoDir, ctx, mockLocker, mockInit, mockImport) } diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 64b929633b..5c005dbc9a 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -51,7 +51,6 @@ type PullClosedExecutor struct { Locker locking.Locker VCSClient vcs.Client WorkingDir WorkingDir - Logger logging.SimpleLogging Backend locking.Backend PullClosedTemplate PullCleanupTemplate LogStreamResourceCleaner ResourceCleaner @@ -82,7 +81,7 @@ func (p *PullClosedExecutor) CleanUpPull(logger logging.SimpleLogging, repo mode pullStatus, err := p.Backend.GetPullStatus(pull) if err != nil { // Log and continue to clean up other resources. - p.Logger.Err("retrieving pull status: %s", err) + logger.Err("retrieving pull status: %s", err) } if pullStatus != nil { @@ -97,7 +96,7 @@ func (p *PullClosedExecutor) CleanUpPull(logger logging.SimpleLogging, repo mode } } - if err := p.WorkingDir.Delete(repo, pull); err != nil { + if err := p.WorkingDir.Delete(logger, repo, pull); err != nil { return errors.Wrap(err, "cleaning workspace") } @@ -111,7 +110,7 @@ func (p *PullClosedExecutor) CleanUpPull(logger logging.SimpleLogging, repo mode // Delete pull from DB. if err := p.Backend.DeletePullStatus(pull); err != nil { - p.Logger.Err("deleting pull from db: %s", err) + logger.Err("deleting pull from db: %s", err) } // If there are no locks then there's no need to comment. diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index 1236060d39..df904a1c6f 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -50,7 +50,7 @@ func TestCleanUpPullWorkspaceErr(t *testing.T) { Backend: db, } err = errors.New("err") - When(w.Delete(testdata.GithubRepo, testdata.Pull)).ThenReturn(err) + When(w.Delete(logger, testdata.GithubRepo, testdata.Pull)).ThenReturn(err) actualErr := pce.CleanUpPull(logger, testdata.GithubRepo, testdata.Pull) Equals(t, "cleaning workspace: err", actualErr.Error()) } @@ -271,7 +271,6 @@ func TestCleanUpLogStreaming(t *testing.T) { VCSClient: client, PullClosedTemplate: &events.PullClosedEventTemplate{}, LogStreamResourceCleaner: prjCmdOutHandler, - Logger: logger, } locks := []models.ProjectLock{ diff --git a/server/events/unlock_command_runner.go b/server/events/unlock_command_runner.go index 470fe26118..af360adf83 100644 --- a/server/events/unlock_command_runner.go +++ b/server/events/unlock_command_runner.go @@ -56,7 +56,7 @@ func (u *UnlockCommandRunner) Run(ctx *command.Context, _ *CommentCommand) { var numLocks int if err == nil && !hasLabel { - numLocks, err = u.deleteLockCommand.DeleteLocksByPull(baseRepo.FullName, pullNum) + numLocks, err = u.deleteLockCommand.DeleteLocksByPull(ctx.Log, baseRepo.FullName, pullNum) if err != nil { vcsMessage = "Failed to delete PR locks" ctx.Log.Err("failed to delete locks by pull %s", err.Error()) diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 886b3c4b40..c3ebe56e80 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -41,23 +41,23 @@ type WorkingDir interface { // absolute path to the root of the cloned repo. It also returns // a boolean indicating if we should warn users that the branch we're // merging into has been updated since we cloned it. - Clone(headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) + Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) // GetWorkingDir returns the path to the workspace for this repo and pull. // If workspace does not exist on disk, error will be of type os.IsNotExist. GetWorkingDir(r models.Repo, p models.PullRequest, workspace string) (string, error) - HasDiverged(cloneDir string) bool + HasDiverged(logger logging.SimpleLogging, cloneDir string) bool GetPullDir(r models.Repo, p models.PullRequest) (string, error) // Delete deletes the workspace for this repo and pull. - Delete(r models.Repo, p models.PullRequest) error - DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) error + Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) error + DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) error // Set a flag in the workingdir so Clone() can know that it is safe to re-clone the workingdir if // the upstream branch has been modified. This is only safe after grabbing the project lock // and before running any plans SetCheckForUpstreamChanges() // 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 + DeletePlan(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string, path string, projectName string) error // GetGitUntrackedFiles returns a list of Git untracked files in the working dir. - GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) ([]string, error) + GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) ([]string, error) } // FileWorkspace implements WorkingDir with the file system. @@ -86,7 +86,6 @@ type FileWorkspace struct { GpgNoSigningEnabled bool // flag indicating if we have to merge with potential new changes upstream (directly after grabbing project lock) CheckForUpstreamChanges bool - Logger logging.SimpleLogging } // Clone git clones headRepo, checks out the branch and then returns the absolute @@ -95,10 +94,7 @@ type FileWorkspace struct { // If the repo already exists and is at // the right commit it does nothing. This is to support running commands in // multiple dirs of the same repo without deleting existing plans. -func (w *FileWorkspace) Clone( - headRepo models.Repo, - p models.PullRequest, - workspace string) (string, bool, error) { +func (w *FileWorkspace) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) { cloneDir := w.cloneDir(p.BaseRepo, p, workspace) defer func() { w.CheckForUpstreamChanges = false }() @@ -106,7 +102,7 @@ func (w *FileWorkspace) Clone( // If the directory already exists, check if it's at the right commit. // If so, then we do nothing. if _, err := os.Stat(cloneDir); err == nil { - w.Logger.Debug("clone directory %q already exists, checking if it's at the right commit", cloneDir) + logger.Debug("clone directory '%s' already exists, checking if it's at the right commit", cloneDir) // We use git rev-parse to see if our repo is at the right commit. // If just checking out the pull request branch, we can use HEAD. @@ -121,28 +117,28 @@ func (w *FileWorkspace) Clone( revParseCmd.Dir = cloneDir outputRevParseCmd, err := revParseCmd.CombinedOutput() if err != nil { - w.Logger.Warn("will re-clone repo, could not determine if was at correct commit: %s: %s: %s", strings.Join(revParseCmd.Args, " "), err, string(outputRevParseCmd)) - return cloneDir, false, w.forceClone(c) + logger.Warn("will re-clone repo, could not determine if was at correct commit: %s: %s: %s", strings.Join(revParseCmd.Args, " "), err, string(outputRevParseCmd)) + return cloneDir, false, w.forceClone(logger, c) } currCommit := strings.Trim(string(outputRevParseCmd), "\n") // We're prefix matching here because BitBucket doesn't give us the full // commit, only a 12 character prefix. if strings.HasPrefix(currCommit, p.HeadCommit) { - if w.CheckForUpstreamChanges && w.CheckoutMerge && w.recheckDiverged(p, headRepo, cloneDir) { - w.Logger.Info("base branch has been updated, using merge strategy and will clone again") - return cloneDir, true, w.mergeAgain(c) + if w.CheckForUpstreamChanges && w.CheckoutMerge && w.recheckDiverged(logger, p, headRepo, cloneDir) { + logger.Info("base branch has been updated, using merge strategy and will clone again") + return cloneDir, true, w.mergeAgain(logger, c) } - w.Logger.Debug("repo is at correct commit %q so will not re-clone", p.HeadCommit) + logger.Debug("repo is at correct commit '%s' so will not re-clone", p.HeadCommit) return cloneDir, false, nil } else { - w.Logger.Debug("repo was already cloned but is not at correct commit, wanted %q got %q", p.HeadCommit, currCommit) + logger.Debug("repo was already cloned but is not at correct commit, wanted '%s' got '%s'", p.HeadCommit, currCommit) } // We'll fall through to re-clone. } // Otherwise we clone the repo. - return cloneDir, false, w.forceClone(c) + return cloneDir, false, w.forceClone(logger, c) } // recheckDiverged returns true if the branch we're merging into has diverged @@ -152,7 +148,7 @@ func (w *FileWorkspace) Clone( // and we have to perform a new merge. // If there are any errors we return false since we prefer things to succeed // vs. stopping the plan/apply. -func (w *FileWorkspace) recheckDiverged(p models.PullRequest, headRepo models.Repo, cloneDir string) bool { +func (w *FileWorkspace) recheckDiverged(logger logging.SimpleLogging, p models.PullRequest, headRepo models.Repo, cloneDir string) bool { if !w.CheckoutMerge { // It only makes sense to warn that main has diverged if we're using // the checkout merge strategy. If we're just checking out the branch, @@ -185,15 +181,15 @@ func (w *FileWorkspace) recheckDiverged(p models.PullRequest, headRepo models.Re output, err := cmd.CombinedOutput() if err != nil { - w.Logger.Warn("getting remote update failed: %s", string(output)) + logger.Warn("getting remote update failed: %s", string(output)) return false } } - return w.HasDiverged(cloneDir) + return w.HasDiverged(logger, cloneDir) } -func (w *FileWorkspace) HasDiverged(cloneDir string) bool { +func (w *FileWorkspace) HasDiverged(logger logging.SimpleLogging, cloneDir string) bool { if !w.CheckoutMerge { // Both the diverged warning and the UnDiverged apply requirement only apply to merge checkout strategy so // we assume false here for 'branch' strategy. @@ -204,7 +200,7 @@ func (w *FileWorkspace) HasDiverged(cloneDir string) bool { statusFetchCmd.Dir = cloneDir outputStatusFetch, err := statusFetchCmd.CombinedOutput() if err != nil { - w.Logger.Warn("fetching repo has failed: %s", string(outputStatusFetch)) + logger.Warn("fetching repo has failed: %s", string(outputStatusFetch)) return false } @@ -213,14 +209,14 @@ func (w *FileWorkspace) HasDiverged(cloneDir string) bool { statusUnoCmd.Dir = cloneDir outputStatusUno, err := statusUnoCmd.CombinedOutput() if err != nil { - w.Logger.Warn("getting repo status has failed: %s", string(outputStatusUno)) + logger.Warn("getting repo status has failed: %s", string(outputStatusUno)) return false } hasDiverged := strings.Contains(string(outputStatusUno), "have diverged") return hasDiverged } -func (w *FileWorkspace) forceClone(c wrappedGitContext) error { +func (w *FileWorkspace) forceClone(logger logging.SimpleLogging, c wrappedGitContext) error { value, _ := cloneLocks.LoadOrStore(c.dir, new(sync.Mutex)) mutex := value.(*sync.Mutex) @@ -232,11 +228,11 @@ func (w *FileWorkspace) forceClone(c wrappedGitContext) error { err := os.RemoveAll(c.dir) if err != nil { - return errors.Wrapf(err, "deleting dir %q before cloning", c.dir) + return errors.Wrapf(err, "deleting dir '%s' before cloning", c.dir) } // Create the directory and parents if necessary. - w.Logger.Info("creating dir %q", c.dir) + logger.Info("creating dir '%s'", c.dir) if err := os.MkdirAll(c.dir, 0700); err != nil { return errors.Wrap(err, "creating new workspace") } @@ -253,37 +249,37 @@ func (w *FileWorkspace) forceClone(c wrappedGitContext) error { // if branch strategy, use depth=1 if !w.CheckoutMerge { - return w.wrappedGit(c, "clone", "--depth=1", "--branch", c.pr.HeadBranch, "--single-branch", headCloneURL, c.dir) + return w.wrappedGit(logger, c, "clone", "--depth=1", "--branch", c.pr.HeadBranch, "--single-branch", headCloneURL, c.dir) } // if merge strategy... // if no checkout depth, omit depth arg if w.CheckoutDepth == 0 { - if err := w.wrappedGit(c, "clone", "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.dir); err != nil { + if err := w.wrappedGit(logger, c, "clone", "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.dir); err != nil { return err } } else { - if err := w.wrappedGit(c, "clone", "--depth", fmt.Sprint(w.CheckoutDepth), "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.dir); err != nil { + if err := w.wrappedGit(logger, c, "clone", "--depth", fmt.Sprint(w.CheckoutDepth), "--branch", c.pr.BaseBranch, "--single-branch", baseCloneURL, c.dir); err != nil { return err } } - if err := w.wrappedGit(c, "remote", "add", "head", headCloneURL); err != nil { + if err := w.wrappedGit(logger, c, "remote", "add", "head", headCloneURL); err != nil { return err } if w.GpgNoSigningEnabled { - if err := w.wrappedGit(c, "config", "--local", "commit.gpgsign", "false"); err != nil { + if err := w.wrappedGit(logger, c, "config", "--local", "commit.gpgsign", "false"); err != nil { return err } } - return w.mergeToBaseBranch(c) + return w.mergeToBaseBranch(logger, c) } // There is a new upstream update that we need, and we want to update to it // without deleting any existing plans -func (w *FileWorkspace) mergeAgain(c wrappedGitContext) error { +func (w *FileWorkspace) mergeAgain(logger logging.SimpleLogging, c wrappedGitContext) error { value, _ := cloneLocks.LoadOrStore(c.dir, new(sync.Mutex)) mutex := value.(*sync.Mutex) @@ -294,11 +290,11 @@ func (w *FileWorkspace) mergeAgain(c wrappedGitContext) error { } // Reset branch as if it was cloned again - if err := w.wrappedGit(c, "reset", "--hard", fmt.Sprintf("refs/remotes/origin/%s", c.pr.BaseBranch)); err != nil { + if err := w.wrappedGit(logger, c, "reset", "--hard", fmt.Sprintf("refs/remotes/origin/%s", c.pr.BaseBranch)); err != nil { return err } - return w.mergeToBaseBranch(c) + return w.mergeToBaseBranch(logger, c) } // wrappedGitContext is the configuration for wrappedGit that is typically unchanged @@ -311,7 +307,7 @@ type wrappedGitContext struct { // wrappedGit runs git with additional environment settings required for git merge, // and with sanitized error logging to avoid leaking git credentials -func (w *FileWorkspace) wrappedGit(c wrappedGitContext, args ...string) error { +func (w *FileWorkspace) wrappedGit(logger logging.SimpleLogging, c wrappedGitContext, args ...string) error { cmd := exec.Command("git", args...) // nolint: gosec cmd.Dir = c.dir // The git merge command requires these env vars are set. @@ -327,12 +323,12 @@ func (w *FileWorkspace) wrappedGit(c wrappedGitContext, args ...string) error { sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), c.pr.BaseRepo, c.head) return fmt.Errorf("running %s: %s: %s", cmdStr, sanitizedOutput, sanitizedErrMsg) } - w.Logger.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(sanitizedOutput, "\n")) + logger.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(sanitizedOutput, "\n")) return nil } // Merge the PR into the base branch. -func (w *FileWorkspace) mergeToBaseBranch(c wrappedGitContext) error { +func (w *FileWorkspace) mergeToBaseBranch(logger logging.SimpleLogging, c wrappedGitContext) error { fetchRef := fmt.Sprintf("+refs/heads/%s:", c.pr.HeadBranch) fetchRemote := "head" if w.GithubAppEnabled { @@ -342,19 +338,19 @@ func (w *FileWorkspace) mergeToBaseBranch(c wrappedGitContext) error { // if no checkout depth, omit depth arg if w.CheckoutDepth == 0 { - if err := w.wrappedGit(c, "fetch", fetchRemote, fetchRef); err != nil { + if err := w.wrappedGit(logger, c, "fetch", fetchRemote, fetchRef); err != nil { return err } } else { - if err := w.wrappedGit(c, "fetch", "--depth", fmt.Sprint(w.CheckoutDepth), fetchRemote, fetchRef); err != nil { + if err := w.wrappedGit(logger, c, "fetch", "--depth", fmt.Sprint(w.CheckoutDepth), fetchRemote, fetchRef); err != nil { return err } } - if err := w.wrappedGit(c, "merge-base", c.pr.BaseBranch, "FETCH_HEAD"); err != nil { + if err := w.wrappedGit(logger, c, "merge-base", c.pr.BaseBranch, "FETCH_HEAD"); err != nil { // git merge-base returning error means that we did not receive enough commits in shallow clone. // Fall back to retrieving full repo history. - if err := w.wrappedGit(c, "fetch", "--unshallow"); err != nil { + if err := w.wrappedGit(logger, c, "fetch", "--unshallow"); err != nil { return err } } @@ -365,7 +361,7 @@ func (w *FileWorkspace) mergeToBaseBranch(c wrappedGitContext) error { // git rev-parse HEAD^2 to get the head commit because it will // always succeed whereas without --no-ff, if the merge was fast // forwarded then git rev-parse HEAD^2 would fail. - return w.wrappedGit(c, "merge", "-q", "--no-ff", "-m", "atlantis-merge", "FETCH_HEAD") + return w.wrappedGit(logger, c, "merge", "-q", "--no-ff", "-m", "atlantis-merge", "FETCH_HEAD") } // GetWorkingDir returns the path to the workspace for this repo and pull. @@ -388,16 +384,16 @@ func (w *FileWorkspace) GetPullDir(r models.Repo, p models.PullRequest) (string, } // Delete deletes the workspace for this repo and pull. -func (w *FileWorkspace) Delete(r models.Repo, p models.PullRequest) error { +func (w *FileWorkspace) Delete(logger logging.SimpleLogging, r models.Repo, p models.PullRequest) error { repoPullDir := w.repoPullDir(r, p) - w.Logger.Info("Deleting repo pull directory: " + repoPullDir) + logger.Info("Deleting repo pull directory: " + repoPullDir) return os.RemoveAll(repoPullDir) } // DeleteForWorkspace deletes the working dir for this workspace. -func (w *FileWorkspace) DeleteForWorkspace(r models.Repo, p models.PullRequest, workspace string) error { +func (w *FileWorkspace) DeleteForWorkspace(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) error { workspaceDir := w.cloneDir(r, p, workspace) - w.Logger.Info("Deleting workspace directory: " + workspaceDir) + logger.Info("Deleting workspace directory: " + workspaceDir) return os.RemoveAll(workspaceDir) } @@ -421,20 +417,20 @@ func (w *FileWorkspace) SetCheckForUpstreamChanges() { w.CheckForUpstreamChanges = true } -func (w *FileWorkspace) DeletePlan(r models.Repo, p models.PullRequest, workspace string, projectPath string, projectName string) error { +func (w *FileWorkspace) DeletePlan(logger logging.SimpleLogging, 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) + logger.Info("Deleting plan: " + planPath) return os.Remove(planPath) } // getGitUntrackedFiles returns a list of Git untracked files in the working dir. -func (w *FileWorkspace) GetGitUntrackedFiles(r models.Repo, p models.PullRequest, workspace string) ([]string, error) { +func (w *FileWorkspace) GetGitUntrackedFiles(logger logging.SimpleLogging, r models.Repo, p models.PullRequest, workspace string) ([]string, error) { workingDir, err := w.GetWorkingDir(r, p, workspace) if err != nil { return nil, err } - w.Logger.Debug("Checking for Git untracked files in directory: '%s'", workingDir) + logger.Debug("Checking for Git untracked files in directory: '%s'", workingDir) cmd := exec.Command("git", "ls-files", "--others", "--exclude-standard") cmd.Dir = workingDir @@ -444,6 +440,6 @@ func (w *FileWorkspace) GetGitUntrackedFiles(r models.Repo, p models.PullRequest } untrackedFiles := strings.Split(string(output), "\n")[:] - w.Logger.Debug("Untracked files: '%s'", strings.Join(untrackedFiles, ",")) + logger.Debug("Untracked files: '%s'", strings.Join(untrackedFiles, ",")) return untrackedFiles, nil } diff --git a/server/events/working_dir_test.go b/server/events/working_dir_test.go index f277c12e6b..de6e067b40 100644 --- a/server/events/working_dir_test.go +++ b/server/events/working_dir_test.go @@ -43,10 +43,9 @@ func TestClone_NoneExisting(t *testing.T) { CheckoutMerge: false, TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, _, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, _, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", }, "default") @@ -96,10 +95,9 @@ func TestClone_CheckoutMergeNoneExisting(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -148,10 +146,9 @@ func TestClone_CheckoutMergeNoReclone(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - _, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + _, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -163,7 +160,7 @@ func TestClone_CheckoutMergeNoReclone(t *testing.T) { runCmd(t, dataDir, "touch", "repos/0/default/proof") // Now run the clone again. - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -201,10 +198,9 @@ func TestClone_CheckoutMergeNoRecloneFastForward(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - _, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + _, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -216,7 +212,7 @@ func TestClone_CheckoutMergeNoRecloneFastForward(t *testing.T) { runCmd(t, dataDir, "touch", "repos/0/default/proof") // Now run the clone again. - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -259,10 +255,9 @@ func TestClone_CheckoutMergeConflict(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - _, _, err := wd.Clone(models.Repo{}, models.PullRequest{ + _, _, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -319,10 +314,9 @@ func TestClone_CheckoutMergeShallow(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -350,10 +344,9 @@ func TestClone_CheckoutMergeShallow(t *testing.T) { TestingOverrideHeadCloneURL: overrideURL, TestingOverrideBaseCloneURL: overrideURL, GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", BaseBranch: "main", @@ -387,9 +380,8 @@ func TestClone_NoReclone(t *testing.T) { CheckoutMerge: false, TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", }, "default") @@ -432,9 +424,8 @@ func TestClone_RecloneWrongCommit(t *testing.T) { CheckoutMerge: false, TestingOverrideHeadCloneURL: fmt.Sprintf("file://%s", repoDir), GpgNoSigningEnabled: true, - Logger: logger, } - cloneDir, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + cloneDir, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "branch", HeadCommit: expCommit, @@ -506,7 +497,6 @@ func TestClone_MasterHasDiverged(t *testing.T) { CheckoutMerge: false, CheckoutDepth: 50, GpgNoSigningEnabled: true, - Logger: logger, } // Pretend terraform has created a plan file, we'll check for it later @@ -518,7 +508,7 @@ func TestClone_MasterHasDiverged(t *testing.T) { // Run the clone without the checkout merge strategy. It should return // false for mergedAgain - _, mergedAgain, err := wd.Clone(models.Repo{}, models.PullRequest{ + _, mergedAgain, err := wd.Clone(logger, models.Repo{}, models.PullRequest{ BaseRepo: models.Repo{}, HeadBranch: "second-pr", BaseBranch: "main", @@ -532,7 +522,7 @@ func TestClone_MasterHasDiverged(t *testing.T) { // Run the clone twice with the merge strategy, the first run should // return true for mergedAgain, subsequent runs should // return false since the first call is supposed to merge. - _, mergedAgain, err = wd.Clone(models.Repo{CloneURL: repoDir}, models.PullRequest{ + _, mergedAgain, err = wd.Clone(logger, models.Repo{CloneURL: repoDir}, models.PullRequest{ BaseRepo: models.Repo{CloneURL: repoDir}, HeadBranch: "second-pr", BaseBranch: "main", @@ -542,7 +532,7 @@ func TestClone_MasterHasDiverged(t *testing.T) { Assert(t, mergedAgain == true, "First clone with CheckoutMerge=true with diverged base should have merged") wd.SetCheckForUpstreamChanges() - _, mergedAgain, err = wd.Clone(models.Repo{CloneURL: repoDir}, models.PullRequest{ + _, mergedAgain, err = wd.Clone(logger, models.Repo{CloneURL: repoDir}, models.PullRequest{ BaseRepo: models.Repo{CloneURL: repoDir}, HeadBranch: "second-pr", BaseBranch: "main", @@ -610,15 +600,14 @@ func TestHasDiverged_MasterHasDiverged(t *testing.T) { CheckoutMerge: true, CheckoutDepth: 50, GpgNoSigningEnabled: true, - Logger: logger, } - hasDiverged := wd.HasDiverged(repoDir + "/repos/0/default") + hasDiverged := wd.HasDiverged(logger, repoDir + "/repos/0/default") Equals(t, hasDiverged, true) // Run it again but without the checkout merge strategy. It should return // false. wd.CheckoutMerge = false - hasDiverged = wd.HasDiverged(repoDir + "/repos/0/default") + hasDiverged = wd.HasDiverged(logger, repoDir + "/repos/0/default") Equals(t, hasDiverged, false) } diff --git a/server/server.go b/server/server.go index eeab9d732e..6f08cb8ac2 100644 --- a/server/server.go +++ b/server/server.go @@ -469,7 +469,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { CheckoutMerge: userConfig.CheckoutStrategy == "merge", CheckoutDepth: userConfig.CheckoutDepth, GithubAppEnabled: githubAppEnabled, - Logger: logger, } scheduledExecutorService := scheduled.NewExecutorService( @@ -503,7 +502,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } deleteLockCommand := &events.DefaultDeleteLockCommand{ Locker: lockingClient, - Logger: logger, WorkingDir: workingDir, WorkingDirLocker: workingDirLocker, Backend: backend, @@ -515,7 +513,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { &events.PullClosedExecutor{ Locker: lockingClient, WorkingDir: workingDir, - Logger: logger, Backend: backend, PullClosedTemplate: &events.PullClosedEventTemplate{}, LogStreamResourceCleaner: projectCmdOutputHandler, @@ -601,7 +598,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.IncludeGitUntrackedFiles, userConfig.AutoDiscoverModeFlag, statsScope, - logger, terraformClient, ) From 26ec5b3d72601eb136f857f447eb2260d5b757a0 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Fri, 1 Mar 2024 17:12:39 +0000 Subject: [PATCH 2/7] Fix tests --- server/controllers/locks_controller_test.go | 14 +++++++------- server/events/project_command_builder_test.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/controllers/locks_controller_test.go b/server/controllers/locks_controller_test.go index 3541f31d48..d899e1ebfa 100644 --- a/server/controllers/locks_controller_test.go +++ b/server/controllers/locks_controller_test.go @@ -159,7 +159,7 @@ func TestGetLock_None(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.GetLock(w, req) - ResponseContains(t, w, http.StatusNotFound, "No lock found at id \"id\"") + ResponseContains(t, w, http.StatusNotFound, "No lock found at id 'id'") } func TestGetLock_Success(t *testing.T) { @@ -215,7 +215,7 @@ func TestDeleteLock_InvalidLockID(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "%A@"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusBadRequest, "Invalid lock id \"%A@\"") + ResponseContains(t, w, http.StatusBadRequest, "Invalid lock id '%A@'") } func TestDeleteLock_LockerErr(t *testing.T) { @@ -247,7 +247,7 @@ func TestDeleteLock_None(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusNotFound, "No lock found at id \"id\"") + ResponseContains(t, w, http.StatusNotFound, "No lock found at id 'id'") } func TestDeleteLock_OldFormat(t *testing.T) { @@ -265,7 +265,7 @@ func TestDeleteLock_OldFormat(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + ResponseContains(t, w, http.StatusOK, "Deleted lock id 'id'") cp.VerifyWasCalled(Never()).CreateComment(Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) } @@ -321,7 +321,7 @@ func TestDeleteLock_UpdateProjectStatus(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + ResponseContains(t, w, http.StatusOK, "Deleted lock id 'id'") status, err := backend.GetPullStatus(pull) Ok(t, err) Assert(t, status.Projects != nil, "status projects was nil") @@ -363,7 +363,7 @@ func TestDeleteLock_CommentFailed(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + ResponseContains(t, w, http.StatusOK, "Deleted lock id 'id'") } func TestDeleteLock_CommentSuccess(t *testing.T) { @@ -400,7 +400,7 @@ func TestDeleteLock_CommentSuccess(t *testing.T) { req = mux.SetURLVars(req, map[string]string{"id": "id"}) w := httptest.NewRecorder() lc.DeleteLock(w, req) - ResponseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + ResponseContains(t, w, http.StatusOK, "Deleted lock id 'id'") cp.VerifyWasCalled(Once()).CreateComment(Any[logging.SimpleLogging](), Eq(pull.BaseRepo), Eq(pull.Num), Eq("**Warning**: The plan for dir: `path` workspace: `workspace` was **discarded** via the Atlantis UI.\n\n"+ "To `apply` this plan you must run `plan` again."), Eq("")) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 497fb15541..5802d550a8 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -384,7 +384,7 @@ projects: dir: . workspace: myworkspace `, - ExpErr: "must specify project name: more than one project defined in atlantis.yaml matched dir: \".\" workspace: \"myworkspace\"", + ExpErr: "must specify project name: more than one project defined in atlantis.yaml matched dir: '.' workspace: 'myworkspace'", }, { Description: "atlantis.yaml with project flag not matching", @@ -399,7 +399,7 @@ version: 3 projects: - dir: . `, - ExpErr: "no project with name \"notconfigured\" is defined in atlantis.yaml", + ExpErr: "no project with name 'notconfigured' is defined in atlantis.yaml", }, { Description: "atlantis.yaml with project flag not matching but silenced", From f3eb7d004f78304514ee627bed8e253b7cfc920b Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Fri, 1 Mar 2024 17:22:15 +0000 Subject: [PATCH 3/7] Fix tests --- server/events/project_command_builder_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 5802d550a8..aba350f3c7 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -384,7 +384,7 @@ projects: dir: . workspace: myworkspace `, - ExpErr: "must specify project name: more than one project defined in atlantis.yaml matched dir: '.' workspace: 'myworkspace'", + ExpErr: "must specify project name: more than one project defined in 'atlantis.yaml' matched dir: '.' workspace: 'myworkspace'", }, { Description: "atlantis.yaml with project flag not matching", @@ -399,7 +399,7 @@ version: 3 projects: - dir: . `, - ExpErr: "no project with name 'notconfigured' is defined in atlantis.yaml", + ExpErr: "no project with name 'notconfigured' is defined in 'atlantis.yaml'", }, { Description: "atlantis.yaml with project flag not matching but silenced", From 4c3d285d9cba396fa332bd0970292d303bdd4ccd Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Fri, 1 Mar 2024 17:32:35 +0000 Subject: [PATCH 4/7] Format working_dir_test --- server/events/working_dir_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/working_dir_test.go b/server/events/working_dir_test.go index de6e067b40..e25c420100 100644 --- a/server/events/working_dir_test.go +++ b/server/events/working_dir_test.go @@ -601,13 +601,13 @@ func TestHasDiverged_MasterHasDiverged(t *testing.T) { CheckoutDepth: 50, GpgNoSigningEnabled: true, } - hasDiverged := wd.HasDiverged(logger, repoDir + "/repos/0/default") + hasDiverged := wd.HasDiverged(logger, repoDir+"/repos/0/default") Equals(t, hasDiverged, true) // Run it again but without the checkout merge strategy. It should return // false. wd.CheckoutMerge = false - hasDiverged = wd.HasDiverged(logger, repoDir + "/repos/0/default") + hasDiverged = wd.HasDiverged(logger, repoDir+"/repos/0/default") Equals(t, hasDiverged, false) } From 3a7ff571d15bf30c0c3837014adf808033e597cb Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Fri, 1 Mar 2024 17:38:08 +0000 Subject: [PATCH 5/7] Remove unused //nolint --- server/events/vcs/github_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 3b5c1ff75d..e29f887a38 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -76,7 +76,7 @@ type GithubPRReviewSummary struct { } // NewGithubClient returns a valid GitHub client. -func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, logger logging.SimpleLogging) (*GithubClient, error) { //nolint:staticcheck +func NewGithubClient(hostname string, credentials GithubCredentials, config GithubConfig, logger logging.SimpleLogging) (*GithubClient, error) { logger.Debug("Creating new GitHub client for host: %s", hostname) transport, err := credentials.Client() if err != nil { From f2561934f100acdbad0b599b5ba71e66bfcae185 Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Sun, 24 Mar 2024 20:08:39 +0000 Subject: [PATCH 6/7] Fix formatting --- server/events/project_command_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index dd0c6d6445..41945ca170 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -89,7 +89,7 @@ func NewInstrumentedProjectCommandBuilder( scope, terraformClient, ), - scope: scope, + scope: scope, } } From 5430ae7ec2cfb2087aa8748857b1cd44787c026b Mon Sep 17 00:00:00 2001 From: X-Guardian Date: Sun, 24 Mar 2024 20:14:23 +0000 Subject: [PATCH 7/7] Fix formatting --- server/events/project_command_builder_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index bc5ee9de45..eb8551f32e 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -244,7 +244,7 @@ terraform { Any[string]())).ThenReturn(tmpDir, false, nil) vcsClient := vcsmocks.NewMockClient() When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), - Any[models.PullRequest]())).ThenReturn(ChangedFiles(c.TestDirStructure, ""), nil) + Any[models.PullRequest]())).ThenReturn(ChangedFiles(c.TestDirStructure, ""), nil) if c.AtlantisYAML != "" { err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(c.AtlantisYAML), 0600) Ok(t, err)