diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 7975a168b6..8c125bdd77 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -726,6 +726,7 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) { userConfig.EnablePolicyChecksFlag = true ctrl, vcsClient, githubGetter, atlantisWorkspace := setupE2E(t, c.RepoDir) + // Set the repo to be cloned through the testing backdoor. repoDir, headSHA, cleanup := initializeRepo(t, c.RepoDir) defer cleanup() @@ -937,8 +938,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl Webhooks: &mockWebhookSender{}, WorkingDirLocker: locker, AggregateApplyRequirements: &events.AggregateApplyRequirements{ - PullApprovedChecker: e2eVCSClient, - WorkingDir: workingDir, + WorkingDir: workingDir, }, } @@ -984,6 +984,8 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl boltdb, ) + e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient) + applyCommandRunner := events.NewApplyCommandRunner( e2eVCSClient, false, @@ -998,6 +1000,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl parallelPoolSize, silenceNoProjects, false, + e2ePullReqStatusFetcher, ) approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner( diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 6cd23c213a..e0c1ac2f87 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -21,6 +21,7 @@ func NewApplyCommandRunner( parallelPoolSize int, SilenceNoProjects bool, silenceVCSStatusNoProjects bool, + pullReqStatusFetcher vcs.PullReqStatusFetcher, ) *ApplyCommandRunner { return &ApplyCommandRunner{ vcsClient: vcsClient, @@ -36,21 +37,23 @@ func NewApplyCommandRunner( parallelPoolSize: parallelPoolSize, SilenceNoProjects: SilenceNoProjects, silenceVCSStatusNoProjects: silenceVCSStatusNoProjects, + pullReqStatusFetcher: pullReqStatusFetcher, } } type ApplyCommandRunner struct { - DisableApplyAll bool - DB *db.BoltDB - locker locking.ApplyLockChecker - vcsClient vcs.Client - commitStatusUpdater CommitStatusUpdater - prjCmdBuilder ProjectApplyCommandBuilder - prjCmdRunner ProjectApplyCommandRunner - autoMerger *AutoMerger - pullUpdater *PullUpdater - dbUpdater *DBUpdater - parallelPoolSize int + DisableApplyAll bool + DB *db.BoltDB + locker locking.ApplyLockChecker + vcsClient vcs.Client + commitStatusUpdater CommitStatusUpdater + prjCmdBuilder ProjectApplyCommandBuilder + prjCmdRunner ProjectApplyCommandRunner + autoMerger *AutoMerger + pullUpdater *PullUpdater + dbUpdater *DBUpdater + parallelPoolSize int + pullReqStatusFetcher vcs.PullReqStatusFetcher // SilenceNoProjects is whether Atlantis should respond to PRs if no projects // are found SilenceNoProjects bool @@ -98,17 +101,16 @@ func (a *ApplyCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) { // 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 // the mergeability status of the pull request. - ctx.PullMergeable, err = a.vcsClient.PullIsMergeable(baseRepo, pull) + // This sets the approved, mergeable, and sqlocked status in the context. + ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull) if err != nil { // On error we continue the request with mergeable assumed false. // We want to continue because not all apply's will need this status, // only if they rely on the mergeability requirement. - ctx.PullMergeable = false - ctx.Log.Warn("unable to get mergeable status: %s. Continuing with mergeable assumed false", err) + // All PullRequestStatus fields are set to false by default when error. + ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) } - ctx.Log.Info("pull request mergeable status: %t", ctx.PullMergeable) - var projectCmds []models.ProjectCommandContext projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) diff --git a/server/events/apply_requirement_handler.go b/server/events/apply_requirement_handler.go index cec45ca4d7..8ca844a88d 100644 --- a/server/events/apply_requirement_handler.go +++ b/server/events/apply_requirement_handler.go @@ -1,8 +1,6 @@ package events import ( - "github.com/pkg/errors" - "github.com/runatlantis/atlantis/server/core/runtime" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/raw" "github.com/runatlantis/atlantis/server/events/yaml/valid" @@ -14,20 +12,14 @@ type ApplyRequirement interface { } type AggregateApplyRequirements struct { - PullApprovedChecker runtime.PullApprovedChecker - WorkingDir WorkingDir + WorkingDir WorkingDir } func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models.ProjectCommandContext) (failure string, err error) { - for _, req := range ctx.ApplyRequirements { switch req { case raw.ApprovedApplyRequirement: - approvalStatus, err := a.PullApprovedChecker.PullIsApproved(ctx.Pull.BaseRepo, ctx.Pull) // nolint: vetshadow - if err != nil { - return "", errors.Wrap(err, "checking if pull request was approved") - } - if !approvalStatus.IsApproved { + if !ctx.PullReqStatus.ApprovalStatus.IsApproved { return "Pull request must be approved by at least one person other than the author before running apply.", nil } // this should come before mergeability check since mergeability is a superset of this check. @@ -36,7 +28,7 @@ func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models. return "All policies must pass for project before running apply", nil } case raw.MergeableApplyRequirement: - if !ctx.PullMergeable { + if !ctx.PullReqStatus.Mergeable { return "Pull request must be mergeable before running apply.", nil } case raw.UnDivergedApplyRequirement: diff --git a/server/events/command_context.go b/server/events/command_context.go index 6e9228b969..fcea22dfed 100644 --- a/server/events/command_context.go +++ b/server/events/command_context.go @@ -40,11 +40,6 @@ type CommandContext struct { // User is the user that triggered this command. User models.User Log logging.SimpleLogging - // PullMergeable is true if Pull is able to be merged. This is available in - // the CommandContext because we want to collect this information before we - // set our own build statuses which can affect mergeability if users have - // required the Atlantis status to be successful prior to merging. - PullMergeable bool // Current PR state PullRequestStatus models.PullReqStatus diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 0184d226e2..b6fb657ff3 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/runatlantis/atlantis/server/core/db" + "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" @@ -131,6 +132,8 @@ func setup(t *testing.T) *vcsmocks.MockClient { defaultBoltDB, ) + pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient) + applyCommandRunner = events.NewApplyCommandRunner( vcsClient, false, @@ -145,6 +148,7 @@ func setup(t *testing.T) *vcsmocks.MockClient { parallelPoolSize, SilenceNoProjects, false, + pullReqStatusFetcher, ) approvePoliciesCommandRunner = events.NewApprovePoliciesCommandRunner( diff --git a/server/events/models/models.go b/server/events/models/models.go index edc389bfd8..93af30539d 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -35,6 +35,11 @@ const ( planfileSlashReplace = "::" ) +type PullReqStatus struct { + ApprovalStatus ApprovalStatus + Mergeable bool +} + // Repo is a VCS repository. type Repo struct { // FullName is the owner and repo name separated @@ -142,10 +147,6 @@ func NewRepo(vcsHostType VCSHostType, repoFullName string, cloneURL string, vcsU }, nil } -type PullReqStatus struct { - Approved ApprovalStatus -} - type ApprovalStatus struct { IsApproved bool ApprovedBy string @@ -374,9 +375,7 @@ type ProjectCommandContext struct { HeadRepo Repo // Log is a logger that's been set up for this context. Log logging.SimpleLogging - // PullMergeable is true if the pull request for this project is able to be merged. - PullMergeable bool - // CurrentProjectPlanStatus is the status of the current project prior to this command. + // PullReqStatus holds state about the PR that requires additional computation outside models.PullRequest PullReqStatus PullReqStatus // CurrentProjectPlanStatus is the status of the current project prior to this command. ProjectPlanStatus ProjectPlanStatus diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 11dd743b1f..087aaf78ad 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -66,16 +66,18 @@ workflows: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"init", "plan"}, expApplySteps: []string{"apply"}, @@ -116,18 +118,20 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"init", "plan"}, expApplySteps: []string{"apply"}, @@ -168,18 +172,20 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{"approved", "mergeable"}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{"approved", "mergeable"}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"init", "plan"}, expApplySteps: []string{"apply"}, @@ -228,18 +234,20 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{"approved"}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{"approved"}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"plan"}, expApplySteps: []string{}, @@ -375,18 +383,20 @@ workflows: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"plan"}, expApplySteps: []string{"apply"}, @@ -431,18 +441,20 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"plan"}, expApplySteps: []string{"apply"}, @@ -490,18 +502,20 @@ workflows: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{}, expApplySteps: []string{}, @@ -533,17 +547,19 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{"approved"}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{"approved"}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"plan"}, expApplySteps: []string{"apply"}, @@ -609,7 +625,9 @@ projects: Pull: models.PullRequest{ BaseRepo: baseRepo, }, - PullMergeable: true, + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, }, cmd, "", []string{"flag"}, tmp, "project1", "myworkspace", true) if c.expErr != "" { @@ -725,18 +743,20 @@ projects: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logging.NewNoopLogger(t), - PullMergeable: true, - Pull: pull, - ProjectName: "myproject_1", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -p myproject_1 -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "myproject_1", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -p myproject_1 -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPlanSteps: []string{"init", "plan"}, expApplySteps: []string{"apply"}, @@ -795,8 +815,10 @@ projects: Pull: models.PullRequest{ BaseRepo: baseRepo, }, - Log: logging.NewNoopLogger(t), - PullMergeable: true, + Log: logging.NewNoopLogger(t), + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, }, cmd, "myproject_[1-2]", []string{"flag"}, tmp, "project1", "myworkspace", true) if c.expErr != "" { @@ -877,16 +899,18 @@ repos: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPolicyCheckSteps: []string{"show", "policy_check"}, }, @@ -932,18 +956,20 @@ workflows: AutoplanEnabled: true, HeadRepo: models.Repo{}, Log: logger, - PullMergeable: true, - Pull: pull, - ProjectName: "", - ApplyRequirements: []string{}, - RepoConfigVersion: 3, - RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", - RepoRelDir: "project1", - TerraformVersion: mustVersion("10.0"), - User: models.User{}, - Verbose: true, - Workspace: "myworkspace", - PolicySets: emptyPolicySets, + PullReqStatus: models.PullReqStatus{ + Mergeable: true, + }, + Pull: pull, + ProjectName: "", + ApplyRequirements: []string{}, + RepoConfigVersion: 3, + RePlanCmd: "atlantis plan -d project1 -w myworkspace -- flag", + RepoRelDir: "project1", + TerraformVersion: mustVersion("10.0"), + User: models.User{}, + Verbose: true, + Workspace: "myworkspace", + PolicySets: emptyPolicySets, }, expPolicyCheckSteps: []string{"policy_check"}, }, @@ -1008,7 +1034,9 @@ workflows: Pull: models.PullRequest{ BaseRepo: baseRepo, }, - PullMergeable: true, + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, }, models.PlanCommand, "", []string{"flag"}, tmp, "project1", "myworkspace", true) if c.expErr != "" { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index b7f32c88fc..8545164d94 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -159,8 +159,10 @@ projects: ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ - PullMergeable: true, - Log: logger, + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, + Log: logger, }) Ok(t, err) Equals(t, len(c.exp), len(ctxs)) @@ -1058,11 +1060,13 @@ projects: var actCtxs []models.ProjectCommandContext var err error actCtxs, err = builder.BuildAutoplanCommands(&events.CommandContext{ - HeadRepo: models.Repo{}, - Pull: models.PullRequest{}, - User: models.User{}, - Log: logger, - PullMergeable: true, + HeadRepo: models.Repo{}, + Pull: models.PullRequest{}, + User: models.User{}, + Log: logger, + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, }) Ok(t, err) Equals(t, 0, len(actCtxs)) @@ -1108,8 +1112,10 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ - PullMergeable: true, - Log: logger, + PullRequestStatus: models.PullReqStatus{ + Mergeable: true, + }, + Log: logger, }) Ok(t, err) diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 1f2cd3e119..7f72ec755c 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -70,7 +70,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( prjCfg.TerraformVersion = getTfVersion(ctx, filepath.Join(repoDir, prjCfg.RepoRelDir)) } - projectCmds = append(projectCmds, newProjectCommandContext( + projectCmdContext := newProjectCommandContext( ctx, cmdName, cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled), @@ -85,7 +85,10 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( parallelApply, parallelPlan, verbose, - )) + ctx.PullRequestStatus, + ) + + projectCmds = append(projectCmds, projectCmdContext) return } @@ -143,6 +146,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( parallelApply, parallelPlan, verbose, + ctx.PullRequestStatus, )) } @@ -165,6 +169,7 @@ func newProjectCommandContext(ctx *CommandContext, parallelApplyEnabled bool, parallelPlanEnabled bool, verbose bool, + pullStatus models.PullReqStatus, ) models.ProjectCommandContext { var projectPlanStatus models.ProjectPlanStatus @@ -199,7 +204,6 @@ func newProjectCommandContext(ctx *CommandContext, Steps: steps, HeadRepo: ctx.HeadRepo, Log: ctx.Log, - PullMergeable: ctx.PullMergeable, ProjectPlanStatus: projectPlanStatus, Pull: ctx.Pull, ProjectName: projCfg.Name, @@ -212,6 +216,7 @@ func newProjectCommandContext(ctx *CommandContext, Verbose: verbose, Workspace: projCfg.Workspace, PolicySets: policySets, + PullReqStatus: pullStatus, } } diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index 92f28ffa5a..4ece1f1946 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -20,7 +20,6 @@ import ( "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/core/runtime" - mocks2 "github.com/runatlantis/atlantis/server/core/runtime/mocks" tmocks "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/mocks" @@ -147,13 +146,11 @@ func TestDefaultProjectCommandRunner_ApplyNotCloned(t *testing.T) { func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) { RegisterMockTestingT(t) mockWorkingDir := mocks.NewMockWorkingDir() - mockApproved := mocks2.NewMockPullApprovedChecker() runner := &events.DefaultProjectCommandRunner{ WorkingDir: mockWorkingDir, WorkingDirLocker: events.NewDefaultWorkingDirLocker(), AggregateApplyRequirements: &events.AggregateApplyRequirements{ - PullApprovedChecker: mockApproved, - WorkingDir: mockWorkingDir, + WorkingDir: mockWorkingDir, }, } ctx := models.ProjectCommandContext{ @@ -162,9 +159,6 @@ func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) { tmp, cleanup := TempDir(t) defer cleanup() When(mockWorkingDir.GetWorkingDir(ctx.BaseRepo, ctx.Pull, ctx.Workspace)).ThenReturn(tmp, nil) - When(mockApproved.PullIsApproved(ctx.BaseRepo, ctx.Pull)).ThenReturn(models.ApprovalStatus{ - IsApproved: false, - }, nil) res := runner.Apply(ctx) Equals(t, "Pull request must be approved by at least one person other than the author before running apply.", res.Failure) @@ -182,7 +176,9 @@ func TestDefaultProjectCommandRunner_ApplyNotMergeable(t *testing.T) { }, } ctx := models.ProjectCommandContext{ - PullMergeable: false, + PullReqStatus: models.PullReqStatus{ + Mergeable: false, + }, ApplyRequirements: []string{"mergeable"}, } tmp, cleanup := TempDir(t) @@ -302,13 +298,11 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { mockApply := mocks.NewMockStepRunner() mockRun := mocks.NewMockCustomStepRunner() mockEnv := mocks.NewMockEnvStepRunner() - mockApproved := mocks2.NewMockPullApprovedChecker() mockWorkingDir := mocks.NewMockWorkingDir() mockLocker := mocks.NewMockProjectLocker() mockSender := mocks.NewMockWebhooksSender() applyReqHandler := &events.AggregateApplyRequirements{ - PullApprovedChecker: mockApproved, - WorkingDir: mockWorkingDir, + WorkingDir: mockWorkingDir, } runner := events.DefaultProjectCommandRunner{ @@ -338,7 +332,12 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { Workspace: "default", ApplyRequirements: c.applyReqs, RepoRelDir: ".", - PullMergeable: c.pullMergeable, + PullReqStatus: models.PullReqStatus{ + ApprovalStatus: models.ApprovalStatus{ + IsApproved: true, + }, + Mergeable: true, + }, } expEnvs := map[string]string{ "key": "value", @@ -348,9 +347,6 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { When(mockApply.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("apply", nil) When(mockRun.Run(ctx, "", repoDir, expEnvs)).ThenReturn("run", nil) When(mockEnv.Run(ctx, "", "value", repoDir, make(map[string]string))).ThenReturn("value", nil) - When(mockApproved.PullIsApproved(ctx.BaseRepo, ctx.Pull)).ThenReturn(models.ApprovalStatus{ - IsApproved: true, - }, nil) res := runner.Apply(ctx) Equals(t, c.expOut, res.ApplySuccess) @@ -358,8 +354,6 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { for _, step := range c.expSteps { switch step { - case "approved": - mockApproved.VerifyWasCalledOnce().PullIsApproved(ctx.BaseRepo, ctx.Pull) case "init": mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "plan": diff --git a/server/events/vcs/pull_status_fetcher.go b/server/events/vcs/pull_status_fetcher.go new file mode 100644 index 0000000000..2cbb75266e --- /dev/null +++ b/server/events/vcs/pull_status_fetcher.go @@ -0,0 +1,37 @@ +package vcs + +import ( + "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/events/models" +) + +type PullReqStatusFetcher interface { + FetchPullStatus(repo models.Repo, pull models.PullRequest) (models.PullReqStatus, error) +} + +type pullReqStatusFetcher struct { + client Client +} + +func NewPullReqStatusFetcher(client Client) PullReqStatusFetcher { + return &pullReqStatusFetcher{ + client: client, + } +} + +func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest) (pullStatus models.PullReqStatus, err error) { + approvalStatus, err := f.client.PullIsApproved(repo, pull) + if err != nil { + return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", repo.FullName, pull.Num) + } + + mergeable, err := f.client.PullIsMergeable(repo, pull) + if err != nil { + return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", repo.FullName, pull.Num) + } + + return models.PullReqStatus{ + ApprovalStatus: approvalStatus, + Mergeable: mergeable, + }, err +} diff --git a/server/server.go b/server/server.go index 5f2502b9e8..94ce797039 100644 --- a/server/server.go +++ b/server/server.go @@ -467,8 +467,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } applyRequirementHandler := &events.AggregateApplyRequirements{ - PullApprovedChecker: vcsClient, - WorkingDir: workingDir, + WorkingDir: workingDir, } projectCommandRunner := &events.DefaultProjectCommandRunner{ @@ -547,6 +546,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { boltdb, ) + pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient) applyCommandRunner := events.NewApplyCommandRunner( vcsClient, userConfig.DisableApplyAll, @@ -561,6 +561,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.ParallelPoolSize, userConfig.SilenceNoProjects, userConfig.SilenceVCSStatusNoProjects, + pullReqStatusFetcher, ) approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner(