From be06063668c49a1c90127cbe8a4b5a68b3576e74 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Sat, 1 Feb 2025 19:12:54 +0000 Subject: [PATCH 1/8] fix: Pre Workflow Hook VCS Combined Status Check Set to Pending Twice (#5242) Signed-off-by: X-Guardian <32168619+X-Guardian@users.noreply.github.com> --- server/controllers/api_controller.go | 11 +++++++ server/controllers/api_controller_test.go | 5 +++ .../events/events_controller_e2e_test.go | 1 + server/events/apply_command_runner.go | 4 --- server/events/command_runner.go | 18 +++++++++++ server/events/command_runner_test.go | 31 +++++++------------ server/events/plan_command_runner.go | 9 ------ .../pre_workflow_hooks_command_runner.go | 12 ------- 8 files changed, 47 insertions(+), 44 deletions(-) diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index ff85371469..29037ec9a0 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -33,6 +33,7 @@ type APIController struct { RepoAllowlistChecker *events.RepoAllowlistChecker Scope tally.Scope VCSClient vcs.Client + CommitStatusUpdater events.CommitStatusUpdater } type APIRequest struct { @@ -150,6 +151,11 @@ func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*com return nil, err } + // Update the combined plan commit status to pending + if err := a.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + var projectResults []command.ProjectResult for i, cmd := range cmds { err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) @@ -173,6 +179,11 @@ func (a *APIController) apiApply(request *APIRequest, ctx *command.Context) (*co return nil, err } + // Update the combined apply commit status to pending + if err := a.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { + ctx.Log.Warn("unable to update apply commit status: %s", err) + } + var projectResults []command.ProjectResult for i, cmd := range cmds { err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index 3b3aa520aa..778c41bee2 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -94,6 +94,10 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, When(postWorkflowHooksCommandRunner.RunPostHooks(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(nil) + commitStatusUpdater := NewMockCommitStatusUpdater() + + When(commitStatusUpdater.UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]())).ThenReturn(nil) + ac := controllers.APIController{ APISecret: []byte(atlantisToken), Locker: locker, @@ -107,6 +111,7 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, VCSClient: vcsClient, RepoAllowlistChecker: repoAllowlistChecker, + CommitStatusUpdater: commitStatusUpdater, } return ac, projectCommandBuilder, projectCommandRunner } diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 4588b04127..d06b237b9f 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1647,6 +1647,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, PullStatusFetcher: backend, DisableAutoplan: opt.disableAutoplan, + CommitStatusUpdater: commitStatusUpdater, } repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 6c69032910..c68110e518 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -94,10 +94,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { return } - if err = a.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - // Get the mergeable status before we set any build statuses of our own. // We do this here because when we set a "Pending" status, if users have // required the Atlantis status checks to pass, then we've now changed diff --git a/server/events/command_runner.go b/server/events/command_runner.go index daa66356d2..30a82105a9 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -202,6 +202,12 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo cmd := &CommentCommand{ Name: command.Autoplan, } + + // Update the combined plan commit status to pending + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if err != nil { @@ -354,6 +360,18 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } + // Update the combined plan or apply commit status to pending + switch cmd.Name { + case command.Plan: + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("unable to update plan commit status: %s", err) + } + case command.Apply: + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { + ctx.Log.Warn("unable to update apply commit status: %s", err) + } + } + err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if err != nil { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index b3b88c40d0..4e32994824 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -253,6 +253,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, PullStatusFetcher: testConfig.backend, + CommitStatusUpdater: commitUpdater, } return vcsClient @@ -440,15 +441,8 @@ func TestRunCommentCommandApply_NoProjects_SilenceEnabled(t *testing.T) { ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Apply}) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalledOnce().UpdateCombinedCount( - Any[logging.SimpleLogging](), - Any[models.Repo](), - Any[models.PullRequest](), - Eq[models.CommitStatus](models.SuccessCommitStatus), - Eq[command.Name](command.Apply), - Eq(0), - Eq(0), - ) + commitUpdater.VerifyWasCalledOnce().UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq(models.PendingCommitStatus), Eq(command.Apply)) } func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { @@ -463,15 +457,6 @@ func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.ApprovePolicies}) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalledOnce().UpdateCombinedCount( - Any[logging.SimpleLogging](), - Any[models.Repo](), - Any[models.PullRequest](), - Eq[models.CommitStatus](models.SuccessCommitStatus), - Eq[command.Name](command.PolicyCheck), - Eq(0), - Eq(0), - ) } func TestRunCommentCommandUnlock_NoProjects_SilenceEnabled(t *testing.T) { @@ -485,6 +470,8 @@ func TestRunCommentCommandUnlock_NoProjects_SilenceEnabled(t *testing.T) { ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Unlock}) vcsClient.VerifyWasCalled(Never()).CreateComment(Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq(models.PendingCommitStatus), Any[command.Name]()) } func TestRunCommentCommandImport_NoProjects_SilenceEnabled(t *testing.T) { @@ -535,7 +522,7 @@ func TestRunCommentCommand_DisableAutoplan(t *testing.T) { CommandName: command.Plan, }, }, nil) - + When(commitUpdater.UpdateCombinedCount(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[int](), Any[int]())).ThenReturn(nil) ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, modelPull, testdata.User) projectCommandBuilder.VerifyWasCalled(Never()).BuildAutoplanCommands(Any[*command.Context]()) } @@ -831,6 +818,10 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Fal ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User) pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp) lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num) + commitUpdater.VerifyWasCalledOnce().UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.PendingCommitStatus), Eq(command.Plan)) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.FailedCommitStatus), Any[command.Name]()) } func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_True(t *testing.T) { @@ -853,6 +844,8 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Tru ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User) pendingPlanFinder.VerifyWasCalled(Never()).DeletePlans(Any[string]()) lockingLocker.VerifyWasCalled(Never()).UnlockByPull(Any[string](), Any[int]()) + commitUpdater.VerifyWasCalledOnce().UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Eq(models.PendingCommitStatus), Eq(command.Plan)) } func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_False(t *testing.T) { diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index c1cc3e81e0..a9652b0cab 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -114,11 +114,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { return } - // At this point we are sure Atlantis has work to do, so set commit status to pending - if err := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - // discard previous plans that might not be relevant anymore ctx.Log.Debug("deleting previous plans and locks") p.deletePlans(ctx) @@ -188,10 +183,6 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { } } - if err = p.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - projectCmds, err := p.prjCmdBuilder.BuildPlanCommands(ctx, cmd) if err != nil { if statusErr := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil { diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index 7d152c7328..be175d0b7d 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -69,18 +69,6 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, escapedArgs = escapeArgs(cmd.Flags) } - // Update the plan or apply commit status to pending whilst the pre workflow hook is running - switch cmd.Name { - case command.Plan: - if err := w.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - case command.Apply: - if err := w.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { - ctx.Log.Warn("unable to update apply commit status: %s", err) - } - } - err = w.runHooks( models.WorkflowHookCommandContext{ BaseRepo: ctx.Pull.BaseRepo, From 8157a8db24f81c5204fd0e53728d12b802b46938 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Sat, 1 Feb 2025 19:17:35 +0000 Subject: [PATCH 2/8] fix: Workspace Error when include-git-untracked-files is true (#5288) Signed-off-by: X-Guardian <32168619+X-Guardian@users.noreply.github.com> --- server/events/project_command_builder.go | 31 +++++++------ server/events/project_command_builder_test.go | 44 ++++++++++++------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 2e42cfc8a4..f626d4b605 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -455,23 +455,17 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex return nil, err } - if p.IncludeGitUntrackedFiles { - ctx.Log.Debug(("'include-git-untracked-files' option is set, getting untracked files")) - untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) + ctx.Log.Debug("%d files were modified in this pull request. Modified files: %v", len(modifiedFiles), modifiedFiles) + + // If we're not including git untracked files, we can skip the clone if there are no modified files. + if !p.IncludeGitUntrackedFiles { + shouldSkipClone, err := p.shouldSkipClone(ctx, modifiedFiles) if err != nil { return nil, err } - modifiedFiles = append(modifiedFiles, untrackedFiles...) - } - - ctx.Log.Debug("%d files were modified in this pull request. Modified files: %v", len(modifiedFiles), modifiedFiles) - - shouldSkipClone, err := p.shouldSkipClone(ctx, modifiedFiles) - if err != nil { - return nil, err - } - if shouldSkipClone { - return []command.ProjectContext{}, nil + if shouldSkipClone { + return []command.ProjectContext{}, nil + } } // Need to lock the workspace we're about to clone to. @@ -490,6 +484,15 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex return nil, err } + if p.IncludeGitUntrackedFiles { + ctx.Log.Debug(("'include-git-untracked-files' option is set, getting untracked files")) + untrackedFiles, err := p.WorkingDir.GetGitUntrackedFiles(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace) + if err != nil { + return nil, err + } + modifiedFiles = append(modifiedFiles, untrackedFiles...) + } + // Parse config file if it exists. repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) hasRepoCfg, err := p.ParserValidator.HasRepoCfg(repoDir, repoCfgFile) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index bb16148893..e74c563ed7 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -45,7 +45,7 @@ var defaultUserConfig = struct { AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl,**/.terraform.lock.hcl", RestrictFileList: false, SilenceNoProjects: false, - IncludeGitUntrackedFiles: true, + IncludeGitUntrackedFiles: false, AutoDiscoverMode: "auto", } @@ -1695,27 +1695,40 @@ projects: // Test that we don't clone the repo if there were no changes based on the atlantis.yaml file. func TestDefaultProjectCommandBuilder_SkipCloneNoChanges(t *testing.T) { cases := []struct { - AtlantisYAML string - ExpectedCtxs int - ExpectedClones InvocationCountMatcher - ModifiedFiles []string + AtlantisYAML string + ExpectedCtxs int + ExpectedClones InvocationCountMatcher + ModifiedFiles []string + IncludeGitUntrackedFiles bool }{ { AtlantisYAML: ` version: 3 projects: - dir: dir1`, - ExpectedCtxs: 0, - ExpectedClones: Never(), - ModifiedFiles: []string{"dir2/main.tf"}, + ExpectedCtxs: 0, + ExpectedClones: Never(), + ModifiedFiles: []string{"dir2/main.tf"}, + IncludeGitUntrackedFiles: false, + }, + { + AtlantisYAML: ` +version: 3 +projects: +- dir: dir1`, + ExpectedCtxs: 0, + ExpectedClones: Once(), + ModifiedFiles: []string{"dir2/main.tf"}, + IncludeGitUntrackedFiles: true, }, { AtlantisYAML: ` version: 3 parallel_plan: true`, - ExpectedCtxs: 0, - ExpectedClones: Once(), - ModifiedFiles: []string{"README.md"}, + ExpectedCtxs: 0, + ExpectedClones: Once(), + ModifiedFiles: []string{"README.md"}, + IncludeGitUntrackedFiles: false, }, { AtlantisYAML: ` @@ -1724,9 +1737,10 @@ autodiscover: mode: enabled projects: - dir: dir1`, - ExpectedCtxs: 0, - ExpectedClones: Once(), - ModifiedFiles: []string{"dir2/main.tf"}, + ExpectedCtxs: 0, + ExpectedClones: Once(), + ModifiedFiles: []string{"dir2/main.tf"}, + IncludeGitUntrackedFiles: false, }, } @@ -1770,7 +1784,7 @@ projects: userConfig.AutoplanFileList, userConfig.RestrictFileList, userConfig.SilenceNoProjects, - userConfig.IncludeGitUntrackedFiles, + c.IncludeGitUntrackedFiles, userConfig.AutoDiscoverMode, scope, terraformClient, From a470d428636c3eaf2928db5b229369ee993f0f5c Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Sat, 1 Feb 2025 17:11:26 -0500 Subject: [PATCH 3/8] chore: Add doc for how to rotate gitlab token (#5290) Signed-off-by: Luke Massa --- e2e/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 e2e/README.md diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 0000000000..fcaec9ad74 --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,15 @@ +# End to end tests + +Tests run against actual repos in various VCS providers + +## Configuration + +### Gitlab + +User: https://gitlab.com/atlantis-tests +Email: maintainers@runatlantis.io + +To rotate token: +1. Login to account +2. Select avatar -> Edit Profile -> Access tokens -> Add new token +3. Create a new token, and upload it to Github Action as environment secret `ATLANTIS_GITLAB_TOKEN`. From 8018a2f3a12e1716a19c4c64c135f61e814b27bb Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sun, 2 Feb 2025 01:27:38 +0000 Subject: [PATCH 4/8] chore(deps): update dependency raviqqe/muffet to v2.10.7 in .github/workflows/website.yml (main) (#5292) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/website.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/website.yml b/.github/workflows/website.yml index ee65114b8b..f67514bd8e 100644 --- a/.github/workflows/website.yml +++ b/.github/workflows/website.yml @@ -73,7 +73,7 @@ jobs: - name: run http-server env: # renovate: datasource=github-releases depName=raviqqe/muffet - MUFFET_VERSION: 2.10.6 + MUFFET_VERSION: 2.10.7 run: | # install raviqqe/muffet to check for broken links. curl -Ls https://github.com/raviqqe/muffet/releases/download/v${MUFFET_VERSION}/muffet_linux_amd64.tar.gz | tar -xz From 27007e88884461edd610ee637bf1e1c6844c50f9 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Sun, 2 Feb 2025 12:29:18 -0500 Subject: [PATCH 5/8] chore: Remove usage of errors.Cause (#5291) Signed-off-by: Luke Massa --- 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 f626d4b605..f66d2f659f 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -825,7 +825,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, // use the default repository workspace because it is the only one guaranteed to have an atlantis.yaml, // other workspaces will not have the file if they are using pre_workflow_hooks to generate it dynamically repoDir, err := p.WorkingDir.GetWorkingDir(ctx.Pull.BaseRepo, ctx.Pull, DefaultWorkspace) - if os.IsNotExist(errors.Cause(err)) { + if errors.Is(err, os.ErrNotExist) { return projCtx, errors.New("no working directory found–did you run plan?") } else if err != nil { return projCtx, err From aa4a98032eb43f562d325466ff89b310de8dc8b7 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Sun, 2 Feb 2025 12:44:39 -0500 Subject: [PATCH 6/8] chore: Remove dependency on multierror (#5275) Signed-off-by: Luke Massa --- go.mod | 2 +- .../exp-output-approve-policies.txt | 5 +---- server/core/runtime/policy/conftest_client.go | 21 +++++++------------ server/events/project_command_runner.go | 21 +++++++++---------- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/go.mod b/go.mod index 03730a8f60..d17aafafc9 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/gorilla/mux v1.8.1 github.com/gorilla/websocket v1.5.3 github.com/hashicorp/go-getter/v2 v2.2.3 - github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-version v1.7.0 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/hashicorp/hc-install v0.9.0 @@ -96,6 +95,7 @@ require ( github.com/gorilla/css v1.0.1 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-retryablehttp v0.7.7 // indirect github.com/hashicorp/go-safetemp v1.0.0 // indirect github.com/hashicorp/hcl v1.0.0 // indirect diff --git a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt index b842f99682..017f43b8b8 100644 --- a/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt +++ b/server/controllers/events/testdata/test-repos/policy-checks-diff-owner/exp-output-approve-policies.txt @@ -6,10 +6,7 @@ Ran Approve Policies for 1 projects: ### 1. dir: `.` workspace: `default` **Approve Policies Error** ``` -1 error occurred: - * policy set: test_policy user runatlantis is not a policy owner - please contact policy owners to approve failing policies - - +policy set: test_policy user runatlantis is not a policy owner - please contact policy owners to approve failing policies ``` #### Policy Approval Status: ``` diff --git a/server/core/runtime/policy/conftest_client.go b/server/core/runtime/policy/conftest_client.go index dd69bba4cd..88dc3bb173 100644 --- a/server/core/runtime/policy/conftest_client.go +++ b/server/core/runtime/policy/conftest_client.go @@ -2,6 +2,7 @@ package policy import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -12,9 +13,9 @@ import ( "regexp" "github.com/hashicorp/go-getter/v2" - "github.com/hashicorp/go-multierror" + version "github.com/hashicorp/go-version" - "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/core/runtime/cache" runtime_models "github.com/runatlantis/atlantis/server/core/runtime/models" @@ -139,7 +140,7 @@ func (c ConfTestVersionDownloader) downloadConfTestVersion(v *version.Version, d fullSrcURL := fmt.Sprintf("%s?checksum=file:%s", binURL, checksumURL) if err := c.downloader.GetAny(destPath, fullSrcURL); err != nil { - return runtime_models.LocalFilePath(""), errors.Wrapf(err, "downloading conftest version %s at %q", v.String(), fullSrcURL) + return runtime_models.LocalFilePath(""), fmt.Errorf("downloading conftest version %s at %q: %w", v.String(), fullSrcURL, err) } binPath := filepath.Join(destPath, "conftest") @@ -212,9 +213,9 @@ func (c *ConfTestExecutorWorkflow) Run(ctx command.ProjectContext, executablePat if cmdErr != nil { // Since we're running conftest for each policyset, individual command errors should be concatenated. if isValidConftestOutput(cmdOutput) { - combinedErr = multierror.Append(combinedErr, fmt.Errorf("policy_set: %s: conftest: some policies failed", policySet.Name)) + combinedErr = errors.Join(combinedErr, fmt.Errorf("policy_set: %s: conftest: some policies failed", policySet.Name)) } else { - combinedErr = multierror.Append(combinedErr, fmt.Errorf("policy_set: %s: conftest: %s", policySet.Name, cmdOutput)) + combinedErr = errors.Join(combinedErr, fmt.Errorf("policy_set: %s: conftest: %s", policySet.Name, cmdOutput)) } } @@ -247,13 +248,7 @@ func (c *ConfTestExecutorWorkflow) Run(ctx command.ProjectContext, executablePat policyCheckResultFile := filepath.Join(workdir, ctx.GetPolicyCheckResultFileName()) err = os.WriteFile(policyCheckResultFile, marshaledStatus, 0600) - combinedErr = multierror.Append(combinedErr, err) - - // Multierror will wrap combined errors in a way that the upstream functions won't be able to read it as nil. - // Let's pass nil back if there are no wrapped errors. - if errors.Unwrap(combinedErr) == nil { - combinedErr = nil - } + combinedErr = errors.Join(combinedErr, err) output := string(marshaledStatus) @@ -306,7 +301,7 @@ func getDefaultVersion() (*version.Version, error) { wrappedVersion, err := version.NewVersion(defaultVersion) if err != nil { - return nil, errors.Wrapf(err, "wrapping version %s", defaultVersion) + return nil, fmt.Errorf("wrapping version %s: %w", defaultVersion, err) } return wrappedVersion, nil } diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index 0558fe35aa..523dbd87ac 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -15,13 +15,12 @@ package events import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" "strings" - "github.com/hashicorp/go-multierror" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/core/runtime" "github.com/runatlantis/atlantis/server/events/command" @@ -347,7 +346,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte // Acquire Atlantis lock for this repo/dir/workspace. lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode == valid.RepoLocksOnPlanMode) if err != nil { - return nil, "", errors.Wrap(err, "acquiring lock") + return nil, "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return nil, lockAttempt.LockFailureReason, nil @@ -408,10 +407,10 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte } // User matches the author and prevent self approve is set to true } else if isOwner && !ignorePolicy && ctx.User.Username == ctx.Pull.Author && policySet.PreventSelfApprove { - prjErr = multierror.Append(prjErr, fmt.Errorf("policy set: %s the author of pr %s matches the command commenter user %s - please contact another policy owners to approve failing policies", policySet.Name, ctx.Pull.Author, ctx.User.Username)) + prjErr = errors.Join(prjErr, fmt.Errorf("policy set: %s the author of pr %s matches the command commenter user %s - please contact another policy owners to approve failing policies", policySet.Name, ctx.Pull.Author, ctx.User.Username)) // User is not authorized to approve policy set. } else if !ignorePolicy { - prjErr = multierror.Append(prjErr, fmt.Errorf("policy set: %s user %s is not a policy owner - please contact policy owners to approve failing policies", policySet.Name, ctx.User.Username)) + prjErr = errors.Join(prjErr, fmt.Errorf("policy set: %s user %s is not a policy owner - please contact policy owners to approve failing policies", policySet.Name, ctx.User.Username)) } // Still bubble up this failure, even if policy set is not targeted. if !policyStatus.Passed && (prjPolicyStatus[i].Approvals != policySet.ApproveCount) { @@ -449,7 +448,7 @@ func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode == valid.RepoLocksOnPlanMode) if err != nil { - return nil, "", errors.Wrap(err, "acquiring lock") + return nil, "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return nil, lockAttempt.LockFailureReason, nil @@ -502,7 +501,7 @@ func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) } // Exclude errors for failed policies if !strings.Contains(err.Error(), "some policies failed") { - errs = multierror.Append(errs, err) + errs = errors.Join(errs, err) } } @@ -567,7 +566,7 @@ func (p *DefaultProjectCommandRunner) doPlan(ctx command.ProjectContext) (*model // Acquire Atlantis lock for this repo/dir/workspace. lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode == valid.RepoLocksOnPlanMode) if err != nil { - return nil, "", errors.Wrap(err, "acquiring lock") + return nil, "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return nil, lockAttempt.LockFailureReason, nil @@ -644,7 +643,7 @@ func (p *DefaultProjectCommandRunner) doApply(ctx command.ProjectContext) (apply // Acquire Atlantis lock for this repo/dir/workspace. lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode == valid.RepoLocksOnApplyMode) if err != nil { - return "", "", errors.Wrap(err, "acquiring lock") + return "", "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return "", lockAttempt.LockFailureReason, nil @@ -724,7 +723,7 @@ func (p *DefaultProjectCommandRunner) doImport(ctx command.ProjectContext) (out // Acquire Atlantis lock for this repo/dir/workspace. lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode != valid.RepoLocksDisabledMode) if err != nil { - return nil, "", errors.Wrap(err, "acquiring lock") + return nil, "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return nil, lockAttempt.LockFailureReason, nil @@ -765,7 +764,7 @@ func (p *DefaultProjectCommandRunner) doStateRm(ctx command.ProjectContext) (out // Acquire Atlantis lock for this repo/dir/workspace. lockAttempt, err := p.Locker.TryLock(ctx.Log, ctx.Pull, ctx.User, ctx.Workspace, models.NewProject(ctx.Pull.BaseRepo.FullName, ctx.RepoRelDir, ctx.ProjectName), ctx.RepoLocksMode != valid.RepoLocksDisabledMode) if err != nil { - return nil, "", errors.Wrap(err, "acquiring lock") + return nil, "", fmt.Errorf("acquiring lock: %w", err) } if !lockAttempt.LockAcquired { return nil, lockAttempt.LockFailureReason, nil From c2c25aeef2af1dbd3bb6798aed5633166aa0b3c3 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Sun, 2 Feb 2025 22:32:50 -0500 Subject: [PATCH 7/8] fix: Minor bug in determining teams for gitlab (#5294) Signed-off-by: Luke Massa --- server/events/vcs/gitlab_client.go | 4 +- server/events/vcs/gitlab_client_test.go | 106 ++++++++++++------ .../vcs/testdata/gitlab-user-multiple.json | 20 ++++ .../events/vcs/testdata/gitlab-user-none.json | 1 + 4 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 server/events/vcs/testdata/gitlab-user-multiple.json create mode 100644 server/events/vcs/testdata/gitlab-user-none.json diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 2ac61a403f..68f78a3829 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -637,10 +637,10 @@ func (g *GitlabClient) GetTeamNamesForUser(logger logging.SimpleLogging, _ model if err != nil { return nil, errors.Wrapf(err, "GET /users returned: %d", resp.StatusCode) } else if len(users) == 0 { - return nil, errors.Wrap(err, "GET /users returned no user") + return nil, errors.New("GET /users returned no user") } else if len(users) > 1 { // Theoretically impossible, just being extra safe - return nil, errors.Wrap(err, "GET /users returned more than 1 user") + return nil, errors.New("GET /users returned more than 1 user") } userID := users[0].ID for _, groupName := range g.ConfiguredGroups { diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 49c9a0f8f0..aaa85b0e5b 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -1121,40 +1121,82 @@ func TestGitlabClient_GetTeamNamesForUser(t *testing.T) { userSuccess, err := os.ReadFile("testdata/gitlab-user-success.json") Ok(t, err) - configuredGroups := []string{"someorg/group1", "someorg/group2", "someorg/group3", "someorg/group4"} - testServer := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.RequestURI { - case "/api/v4/users?username=testuser": - w.WriteHeader(http.StatusOK) - w.Write(userSuccess) // nolint: errcheck - case "/api/v4/groups/someorg%2Fgroup1/members/123", "/api/v4/groups/someorg%2Fgroup2/members/123": - w.WriteHeader(http.StatusOK) - w.Write(groupMembershipSuccess) // nolint: errcheck - case "/api/v4/groups/someorg%2Fgroup3/members/123": - http.Error(w, "forbidden", http.StatusForbidden) - case "/api/v4/groups/someorg%2Fgroup4/members/123": - http.Error(w, "not found", http.StatusNotFound) - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - } - })) - internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + userEmpty, err := os.ReadFile("testdata/gitlab-user-none.json") Ok(t, err) - client := &GitlabClient{ - Client: internalClient, - Version: nil, - ConfiguredGroups: configuredGroups, + + multipleUsers, err := os.ReadFile("testdata/gitlab-user-multiple.json") + Ok(t, err) + + configuredGroups := []string{"someorg/group1", "someorg/group2", "someorg/group3", "someorg/group4"} + + cases := []struct { + userName string + expErr string + expTeams []string + }{ + { + userName: "testuser", + expTeams: []string{"someorg/group1", "someorg/group2"}, + }, + { + userName: "none", + expErr: "GET /users returned no user", + }, + { + userName: "multiuser", + expErr: "GET /users returned more than 1 user", + }, } + for _, c := range cases { + t.Run(c.userName, func(t *testing.T) { + + testServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v4/users?username=testuser": + w.WriteHeader(http.StatusOK) + w.Write(userSuccess) // nolint: errcheck + case "/api/v4/users?username=none": + w.WriteHeader(http.StatusOK) + w.Write(userEmpty) // nolint: errcheck + case "/api/v4/users?username=multiuser": + w.WriteHeader(http.StatusOK) + w.Write(multipleUsers) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup1/members/123", "/api/v4/groups/someorg%2Fgroup2/members/123": + w.WriteHeader(http.StatusOK) + w.Write(groupMembershipSuccess) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup3/members/123": + http.Error(w, "forbidden", http.StatusForbidden) + case "/api/v4/groups/someorg%2Fgroup4/members/123": + http.Error(w, "not found", http.StatusNotFound) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + } + })) + internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + Ok(t, err) + client := &GitlabClient{ + Client: internalClient, + Version: nil, + ConfiguredGroups: configuredGroups, + } + + teams, err := client.GetTeamNamesForUser( + logger, + models.Repo{ + Owner: "someorg", + }, models.User{ + Username: c.userName, + }) + if c.expErr == "" { + Ok(t, err) + Equals(t, c.expTeams, teams) + } else { + ErrContains(t, c.expErr, err) + + } - teams, err := client.GetTeamNamesForUser( - logger, - models.Repo{ - Owner: "someorg", - }, models.User{ - Username: "testuser", }) - Ok(t, err) - Equals(t, []string{"someorg/group1", "someorg/group2"}, teams) + } } diff --git a/server/events/vcs/testdata/gitlab-user-multiple.json b/server/events/vcs/testdata/gitlab-user-multiple.json new file mode 100644 index 0000000000..dfd5373067 --- /dev/null +++ b/server/events/vcs/testdata/gitlab-user-multiple.json @@ -0,0 +1,20 @@ +[ + { + "id": 123, + "username": "multiuser", + "name": "Multiple User 1", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/123/avatar.png", + "web_url": "https://gitlab.com/multiuser" + }, + { + "id": 124, + "username": "multiuser", + "name": "Multiple User 2", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/124/avatar.png", + "web_url": "https://gitlab.com/multiuser" + } +] diff --git a/server/events/vcs/testdata/gitlab-user-none.json b/server/events/vcs/testdata/gitlab-user-none.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/server/events/vcs/testdata/gitlab-user-none.json @@ -0,0 +1 @@ +[] From 618d5acd4545cf3d0f8c773ca4d5e146b982466e Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 3 Feb 2025 03:34:41 +0000 Subject: [PATCH 8/8] chore(deps): update github/codeql-action action to v3.28.6 in .github/workflows/scorecard.yml (main) (#5295) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/scorecard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index df1fdaed38..e63e974909 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -56,6 +56,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: 'Upload to code-scanning' - uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0 + uses: github/codeql-action/upload-sarif@17a820bf2e43b47be2c72b39cc905417bc1ab6d0 # v3.28.6 with: sarif_file: results.sarif