diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index aade27a029..62b59ef653 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1195,7 +1195,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers discardApprovalOnPlan, ) - e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient) + e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient, "atlantis-test") applyCommandRunner := events.NewApplyCommandRunner( e2eVCSClient, @@ -1211,7 +1211,6 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers parallelPoolSize, silenceNoProjects, false, - "atlantis-test", e2ePullReqStatusFetcher, ) @@ -1241,6 +1240,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers importCommandRunner := events.NewImportCommandRunner( pullUpdater, + e2ePullReqStatusFetcher, projectCommandBuilder, projectCommandRunner, ) diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index eae9ec8593..8a17f2a03a 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -21,7 +21,6 @@ func NewApplyCommandRunner( parallelPoolSize int, SilenceNoProjects bool, silenceVCSStatusNoProjects bool, - VCSStatusName string, pullReqStatusFetcher vcs.PullReqStatusFetcher, ) *ApplyCommandRunner { return &ApplyCommandRunner{ @@ -38,7 +37,6 @@ func NewApplyCommandRunner( parallelPoolSize: parallelPoolSize, SilenceNoProjects: SilenceNoProjects, silenceVCSStatusNoProjects: silenceVCSStatusNoProjects, - VCSStatusName: VCSStatusName, pullReqStatusFetcher: pullReqStatusFetcher, } } @@ -61,7 +59,6 @@ type ApplyCommandRunner struct { SilenceNoProjects bool // SilenceVCSStatusNoPlans is whether any plan should set commit status if no projects // are found - VCSStatusName string silenceVCSStatusNoProjects bool } @@ -105,7 +102,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { // required the Atlantis status checks to pass, then we've now changed // the mergeability status of the pull request. // This sets the approved, mergeable, and sqlocked status in the context. - ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull, a.VCSStatusName) + ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(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, diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index e65cdf5620..c2c830c6a1 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -23,7 +23,6 @@ import ( "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/core/db" "github.com/runatlantis/atlantis/server/events/command" - "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/metrics" @@ -52,6 +51,7 @@ var pendingPlanFinder *mocks.MockPendingPlanFinder var drainer *events.Drainer var deleteLockCommand *mocks.MockDeleteLockCommand var commitUpdater *mocks.MockCommitStatusUpdater +var pullReqStatusFetcher *vcsmocks.MockPullReqStatusFetcher // TODO: refactor these into their own unit tests. // these were all split out from default command runner in an effort to improve @@ -102,6 +102,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock workingDir = mocks.NewMockWorkingDir() pendingPlanFinder = mocks.NewMockPendingPlanFinder() commitUpdater = mocks.NewMockCommitStatusUpdater() + pullReqStatusFetcher = vcsmocks.NewMockPullReqStatusFetcher() tmp := t.TempDir() defaultBoltDB, err := db.New(tmp) Ok(t, err) @@ -156,8 +157,6 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock testConfig.discardApprovalOnPlan, ) - pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient) - applyCommandRunner = events.NewApplyCommandRunner( vcsClient, false, @@ -172,7 +171,6 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock testConfig.parallelPoolSize, testConfig.SilenceNoProjects, false, - testConfig.StatusName, pullReqStatusFetcher, ) @@ -202,6 +200,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock importCommandRunner = events.NewImportCommandRunner( pullUpdater, + pullReqStatusFetcher, projectCommandBuilder, projectCommandRunner, ) diff --git a/server/events/import_command_runner.go b/server/events/import_command_runner.go index fc4d09fbfa..6958568724 100644 --- a/server/events/import_command_runner.go +++ b/server/events/import_command_runner.go @@ -2,28 +2,46 @@ package events import ( "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/vcs" ) func NewImportCommandRunner( pullUpdater *PullUpdater, + pullReqStatusFetcher vcs.PullReqStatusFetcher, prjCmdBuilder ProjectImportCommandBuilder, prjCmdRunner ProjectImportCommandRunner, ) *ImportCommandRunner { return &ImportCommandRunner{ - pullUpdater: pullUpdater, - prjCmdBuilder: prjCmdBuilder, - prjCmdRunner: prjCmdRunner, + pullUpdater: pullUpdater, + pullReqStatusFetcher: pullReqStatusFetcher, + prjCmdBuilder: prjCmdBuilder, + prjCmdRunner: prjCmdRunner, } } type ImportCommandRunner struct { - pullUpdater *PullUpdater - prjCmdBuilder ProjectImportCommandBuilder - prjCmdRunner ProjectImportCommandRunner + pullUpdater *PullUpdater + pullReqStatusFetcher vcs.PullReqStatusFetcher + prjCmdBuilder ProjectImportCommandBuilder + prjCmdRunner ProjectImportCommandRunner } func (v *ImportCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { var err error + // 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 + // the mergeability status of the pull request. + // This sets the approved, mergeable, and sqlocked status in the context. + ctx.PullRequestStatus, err = v.pullReqStatusFetcher.FetchPullStatus(ctx.Pull) + if err != nil { + // On error we continue the request with mergeable assumed false. + // We want to continue because not all import will need this status, + // only if they rely on the mergeability requirement. + // 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) + } + var projectCmds []command.ProjectContext projectCmds, err = v.prjCmdBuilder.BuildImportCommands(ctx, cmd) if err != nil { diff --git a/server/events/import_command_runner_test.go b/server/events/import_command_runner_test.go new file mode 100644 index 0000000000..277c4bc9f8 --- /dev/null +++ b/server/events/import_command_runner_test.go @@ -0,0 +1,69 @@ +package events_test + +import ( + "testing" + + . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/models/fixtures" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/metrics" + . "github.com/runatlantis/atlantis/testing" +) + +func TestImportCommandRunner_Run(t *testing.T) { + RegisterMockTestingT(t) + + tests := []struct { + name string + pullReqStatus models.PullReqStatus + projectCmds []command.ProjectContext + expComment string + }{ + { + name: "success with zero projects", + pullReqStatus: models.PullReqStatus{ + ApprovalStatus: models.ApprovalStatus{IsApproved: true}, + Mergeable: true, + }, + projectCmds: []command.ProjectContext{}, + expComment: "Ran Import for 0 projects:\n\n\n\n", + }, + { + name: "failure with multiple projects", + pullReqStatus: models.PullReqStatus{ + ApprovalStatus: models.ApprovalStatus{IsApproved: true}, + Mergeable: true, + }, + projectCmds: []command.ProjectContext{{}, {}}, + expComment: "**Import Failed**: import cannot run on multiple projects. please specify one project.\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + vcsClient := setup(t) + + scopeNull, _, _ := metrics.NewLoggingScope(logger, "atlantis") + modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, State: models.OpenPullState, Num: fixtures.Pull.Num} + ctx := &command.Context{ + User: fixtures.User, + Log: logging.NewNoopLogger(t), + Scope: scopeNull, + Pull: modelPull, + HeadRepo: fixtures.GithubRepo, + Trigger: command.CommentTrigger, + } + cmd := &events.CommentCommand{Name: command.Import} + + When(pullReqStatusFetcher.FetchPullStatus(modelPull)).ThenReturn(tt.pullReqStatus, nil) + When(projectCommandBuilder.BuildImportCommands(ctx, cmd)).ThenReturn(tt.projectCmds, nil) + + importCommandRunner.Run(ctx, cmd) + + Assert(t, ctx.PullRequestStatus.Mergeable == true, "PullRequestStatus must be set for import_requirements") + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, tt.expComment, "import") + }) + } +} diff --git a/server/events/vcs/mocks/matchers/models_pullreqstatus.go b/server/events/vcs/mocks/matchers/models_pullreqstatus.go new file mode 100644 index 0000000000..289dbbed93 --- /dev/null +++ b/server/events/vcs/mocks/matchers/models_pullreqstatus.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + models "github.com/runatlantis/atlantis/server/events/models" +) + +func AnyModelsPullReqStatus() models.PullReqStatus { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PullReqStatus))(nil)).Elem())) + var nullValue models.PullReqStatus + return nullValue +} + +func EqModelsPullReqStatus(value models.PullReqStatus) models.PullReqStatus { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.PullReqStatus + return nullValue +} + +func NotEqModelsPullReqStatus(value models.PullReqStatus) models.PullReqStatus { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.PullReqStatus + return nullValue +} + +func ModelsPullReqStatusThat(matcher pegomock.ArgumentMatcher) models.PullReqStatus { + pegomock.RegisterMatcher(matcher) + var nullValue models.PullReqStatus + return nullValue +} diff --git a/server/events/vcs/mocks/mock_pull_req_status_fetcher.go b/server/events/vcs/mocks/mock_pull_req_status_fetcher.go new file mode 100644 index 0000000000..f4126d66f0 --- /dev/null +++ b/server/events/vcs/mocks/mock_pull_req_status_fetcher.go @@ -0,0 +1,109 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/events/vcs (interfaces: PullReqStatusFetcher) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockPullReqStatusFetcher struct { + fail func(message string, callerSkip ...int) +} + +func NewMockPullReqStatusFetcher(options ...pegomock.Option) *MockPullReqStatusFetcher { + mock := &MockPullReqStatusFetcher{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockPullReqStatusFetcher) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockPullReqStatusFetcher) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockPullReqStatusFetcher) FetchPullStatus(_param0 models.PullRequest) (models.PullReqStatus, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockPullReqStatusFetcher().") + } + params := []pegomock.Param{_param0} + result := pegomock.GetGenericMockFrom(mock).Invoke("FetchPullStatus", params, []reflect.Type{reflect.TypeOf((*models.PullReqStatus)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 models.PullReqStatus + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(models.PullReqStatus) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + +func (mock *MockPullReqStatusFetcher) VerifyWasCalledOnce() *VerifierMockPullReqStatusFetcher { + return &VerifierMockPullReqStatusFetcher{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockPullReqStatusFetcher) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockPullReqStatusFetcher { + return &VerifierMockPullReqStatusFetcher{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockPullReqStatusFetcher) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockPullReqStatusFetcher { + return &VerifierMockPullReqStatusFetcher{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockPullReqStatusFetcher) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockPullReqStatusFetcher { + return &VerifierMockPullReqStatusFetcher{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockPullReqStatusFetcher struct { + mock *MockPullReqStatusFetcher + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockPullReqStatusFetcher) FetchPullStatus(_param0 models.PullRequest) *MockPullReqStatusFetcher_FetchPullStatus_OngoingVerification { + params := []pegomock.Param{_param0} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "FetchPullStatus", params, verifier.timeout) + return &MockPullReqStatusFetcher_FetchPullStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockPullReqStatusFetcher_FetchPullStatus_OngoingVerification struct { + mock *MockPullReqStatusFetcher + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockPullReqStatusFetcher_FetchPullStatus_OngoingVerification) GetCapturedArguments() models.PullRequest { + _param0 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1] +} + +func (c *MockPullReqStatusFetcher_FetchPullStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.PullRequest) + } + } + return +} diff --git a/server/events/vcs/pull_status_fetcher.go b/server/events/vcs/pull_status_fetcher.go index e8a9c11d34..03d00c7ba6 100644 --- a/server/events/vcs/pull_status_fetcher.go +++ b/server/events/vcs/pull_status_fetcher.go @@ -5,29 +5,33 @@ import ( "github.com/runatlantis/atlantis/server/events/models" ) +//go:generate pegomock generate -m --package mocks -o mocks/mock_pull_req_status_fetcher.go PullReqStatusFetcher + type PullReqStatusFetcher interface { - FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (models.PullReqStatus, error) + FetchPullStatus(pull models.PullRequest) (models.PullReqStatus, error) } type pullReqStatusFetcher struct { - client Client + client Client + vcsStatusName string } -func NewPullReqStatusFetcher(client Client) PullReqStatusFetcher { +func NewPullReqStatusFetcher(client Client, vcsStatusName string) PullReqStatusFetcher { return &pullReqStatusFetcher{ - client: client, + client: client, + vcsStatusName: vcsStatusName, } } -func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (pullStatus models.PullReqStatus, err error) { - approvalStatus, err := f.client.PullIsApproved(repo, pull) +func (f *pullReqStatusFetcher) FetchPullStatus(pull models.PullRequest) (pullStatus models.PullReqStatus, err error) { + approvalStatus, err := f.client.PullIsApproved(pull.BaseRepo, pull) if err != nil { - return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", repo.FullName, pull.Num) + return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", pull.BaseRepo.FullName, pull.Num) } - mergeable, err := f.client.PullIsMergeable(repo, pull, vcsstatusname) + mergeable, err := f.client.PullIsMergeable(pull.BaseRepo, pull, f.vcsStatusName) if err != nil { - return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", repo.FullName, pull.Num) + return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", pull.BaseRepo.FullName, pull.Num) } return models.PullReqStatus{ diff --git a/server/server.go b/server/server.go index 61e00afde6..5b6158e73e 100644 --- a/server/server.go +++ b/server/server.go @@ -688,7 +688,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.DiscardApprovalOnPlanFlag, ) - pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient) + pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient, userConfig.VCSStatusName) applyCommandRunner := events.NewApplyCommandRunner( vcsClient, userConfig.DisableApplyAll, @@ -703,7 +703,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.ParallelPoolSize, userConfig.SilenceNoProjects, userConfig.SilenceVCSStatusNoProjects, - userConfig.VCSStatusName, pullReqStatusFetcher, ) @@ -733,6 +732,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { importCommandRunner := events.NewImportCommandRunner( pullUpdater, + pullReqStatusFetcher, projectCommandBuilder, instrumentedProjectCmdRunner, )