Merge branch 'runatlantis:main' into main

.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

.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

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: [email protected]
+
+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`.

go.mod

@@ -22,7 +22,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

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])

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
 }

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("*")

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:
 ```

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
 }

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

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 {

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) {

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 {

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,