Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Add PullStatusFetcher interface #1904

Merged
merged 3 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -984,6 +984,8 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
boltdb,
)

e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient)

applyCommandRunner := events.NewApplyCommandRunner(
e2eVCSClient,
false,
Expand All @@ -998,6 +1000,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
parallelPoolSize,
silenceNoProjects,
false,
e2ePullReqStatusFetcher,
)

approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner(
Expand Down
34 changes: 18 additions & 16 deletions server/events/apply_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func NewApplyCommandRunner(
parallelPoolSize int,
SilenceNoProjects bool,
silenceVCSStatusNoProjects bool,
pullReqStatusFetcher vcs.PullReqStatusFetcher,
) *ApplyCommandRunner {
return &ApplyCommandRunner{
vcsClient: vcsClient,
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down
14 changes: 3 additions & 11 deletions server/events/apply_requirement_handler.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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.
Expand All @@ -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:
Expand Down
5 changes: 0 additions & 5 deletions server/events/command_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -131,6 +132,8 @@ func setup(t *testing.T) *vcsmocks.MockClient {
defaultBoltDB,
)

pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient)

applyCommandRunner = events.NewApplyCommandRunner(
vcsClient,
false,
Expand All @@ -145,6 +148,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
parallelPoolSize,
SilenceNoProjects,
false,
pullReqStatusFetcher,
)

approvePoliciesCommandRunner = events.NewApprovePoliciesCommandRunner(
Expand Down
13 changes: 6 additions & 7 deletions server/events/models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading