From 383aed7313c2a03ed85baf071874e16d1e42347a Mon Sep 17 00:00:00 2001 From: Samra Belachew Date: Wed, 2 Mar 2022 10:27:04 -0800 Subject: [PATCH 1/4] post implementation for gateway --- server/events/apply_command_runner.go | 4 +- .../events/approve_policies_command_runner.go | 4 +- .../mocks/matchers/models_pullrequest.go | 3 +- server/events/mocks/matchers/models_repo.go | 3 +- server/events/mocks/matchers/models_user.go | 3 +- .../matchers/ptr_to_events_commentcommand.go | 3 +- .../matchers/ptr_to_models_pullrequest.go | 3 +- .../mocks/matchers/ptr_to_models_repo.go | 3 +- server/events/mocks/matchers/time_time.go | 3 +- server/events/mocks/mock_command_runner.go | 5 +- server/events/plan_command_runner.go | 8 +- server/events/policy_check_command_runner.go | 2 +- server/events/pull_updater.go | 2 +- server/events/version_command_runner.go | 2 +- server/lyft/gateway/autoplan_builder.go | 166 +++++++++ server/lyft/gateway/autoplan_builder_test.go | 107 ++++++ server/lyft/gateway/events_controller.go | 287 +++++++++++++++ server/lyft/gateway/events_controller_test.go | 337 ++++++++++++++++++ .../mocks/matchers/models_pullrequest.go | 33 ++ .../gateway/mocks/matchers/models_repo.go | 33 ++ .../gateway/mocks/matchers/models_user.go | 33 ++ .../gateway/mocks/mock_autoplan_validator.go | 117 ++++++ 22 files changed, 1133 insertions(+), 28 deletions(-) create mode 100644 server/lyft/gateway/autoplan_builder.go create mode 100644 server/lyft/gateway/autoplan_builder_test.go create mode 100644 server/lyft/gateway/events_controller.go create mode 100644 server/lyft/gateway/events_controller_test.go create mode 100644 server/lyft/gateway/mocks/matchers/models_pullrequest.go create mode 100644 server/lyft/gateway/mocks/matchers/models_repo.go create mode 100644 server/lyft/gateway/mocks/matchers/models_user.go create mode 100644 server/lyft/gateway/mocks/mock_autoplan_validator.go diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 2cfbe51c4..4e148d2b2 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -115,7 +115,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } - a.pullUpdater.updatePull(ctx, cmd, command.Result{Error: err}) + a.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err}) return } @@ -143,7 +143,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { result = runProjectCmds(projectCmds, a.prjCmdRunner.Apply) } - a.pullUpdater.updatePull( + a.pullUpdater.UpdatePull( ctx, cmd, result) diff --git a/server/events/approve_policies_command_runner.go b/server/events/approve_policies_command_runner.go index 7e9a56e1a..c7f2b2af5 100644 --- a/server/events/approve_policies_command_runner.go +++ b/server/events/approve_policies_command_runner.go @@ -52,7 +52,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.PolicyCheck); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } - a.pullUpdater.updatePull(ctx, cmd, command.Result{Error: err}) + a.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err}) return } @@ -72,7 +72,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom result := a.buildApprovePolicyCommandResults(ctx, projectCmds) - a.pullUpdater.updatePull( + a.pullUpdater.UpdatePull( ctx, cmd, result, diff --git a/server/events/mocks/matchers/models_pullrequest.go b/server/events/mocks/matchers/models_pullrequest.go index db2666f02..9ae2a7e92 100644 --- a/server/events/mocks/matchers/models_pullrequest.go +++ b/server/events/mocks/matchers/models_pullrequest.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" models "github.com/runatlantis/atlantis/server/events/models" ) diff --git a/server/events/mocks/matchers/models_repo.go b/server/events/mocks/matchers/models_repo.go index 2ca60819c..fd44665f8 100644 --- a/server/events/mocks/matchers/models_repo.go +++ b/server/events/mocks/matchers/models_repo.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" models "github.com/runatlantis/atlantis/server/events/models" ) diff --git a/server/events/mocks/matchers/models_user.go b/server/events/mocks/matchers/models_user.go index e9bf1384b..0aa92b5d8 100644 --- a/server/events/mocks/matchers/models_user.go +++ b/server/events/mocks/matchers/models_user.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" models "github.com/runatlantis/atlantis/server/events/models" ) diff --git a/server/events/mocks/matchers/ptr_to_events_commentcommand.go b/server/events/mocks/matchers/ptr_to_events_commentcommand.go index 23d7794d0..55b3b5ba6 100644 --- a/server/events/mocks/matchers/ptr_to_events_commentcommand.go +++ b/server/events/mocks/matchers/ptr_to_events_commentcommand.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" events "github.com/runatlantis/atlantis/server/events" ) diff --git a/server/events/mocks/matchers/ptr_to_models_pullrequest.go b/server/events/mocks/matchers/ptr_to_models_pullrequest.go index 435081d1a..1f8678a09 100644 --- a/server/events/mocks/matchers/ptr_to_models_pullrequest.go +++ b/server/events/mocks/matchers/ptr_to_models_pullrequest.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" models "github.com/runatlantis/atlantis/server/events/models" ) diff --git a/server/events/mocks/matchers/ptr_to_models_repo.go b/server/events/mocks/matchers/ptr_to_models_repo.go index 720b22214..a85a0629a 100644 --- a/server/events/mocks/matchers/ptr_to_models_repo.go +++ b/server/events/mocks/matchers/ptr_to_models_repo.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" models "github.com/runatlantis/atlantis/server/events/models" ) diff --git a/server/events/mocks/matchers/time_time.go b/server/events/mocks/matchers/time_time.go index 755cf1bf8..461e1dd6d 100644 --- a/server/events/mocks/matchers/time_time.go +++ b/server/events/mocks/matchers/time_time.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" time "time" ) diff --git a/server/events/mocks/mock_command_runner.go b/server/events/mocks/mock_command_runner.go index ec7b5587c..80890c1af 100644 --- a/server/events/mocks/mock_command_runner.go +++ b/server/events/mocks/mock_command_runner.go @@ -4,12 +4,11 @@ package mocks import ( - "reflect" - "time" - pegomock "github.com/petergtz/pegomock" events "github.com/runatlantis/atlantis/server/events" models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" ) type MockCommandRunner struct { diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index 58ee51a43..f4e158aa3 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -75,7 +75,7 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { if statusErr := p.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } - p.pullUpdater.updatePull(ctx, AutoplanCommand{}, command.Result{Error: err}) + p.pullUpdater.UpdatePull(ctx, AutoplanCommand{}, command.Result{Error: err}) return } @@ -121,7 +121,7 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { result.PlansDeleted = true } - p.pullUpdater.updatePull(ctx, AutoplanCommand{}, result) + p.pullUpdater.UpdatePull(ctx, AutoplanCommand{}, result) pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults) if err != nil { @@ -160,7 +160,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { if statusErr := p.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } - p.pullUpdater.updatePull(ctx, cmd, command.Result{Error: err}) + p.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err}) return } @@ -195,7 +195,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { result.PlansDeleted = true } - p.pullUpdater.updatePull( + p.pullUpdater.UpdatePull( ctx, cmd, result) diff --git a/server/events/policy_check_command_runner.go b/server/events/policy_check_command_runner.go index f82470b92..7b49e161c 100644 --- a/server/events/policy_check_command_runner.go +++ b/server/events/policy_check_command_runner.go @@ -62,7 +62,7 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj result = runProjectCmds(cmds, p.prjCmdRunner.PolicyCheck) } - p.pullUpdater.updatePull(ctx, PolicyCheckCommand{}, result) + p.pullUpdater.UpdatePull(ctx, PolicyCheckCommand{}, result) pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults) if err != nil { diff --git a/server/events/pull_updater.go b/server/events/pull_updater.go index d526bcbd6..246fc62d7 100644 --- a/server/events/pull_updater.go +++ b/server/events/pull_updater.go @@ -13,7 +13,7 @@ type PullUpdater struct { GlobalCfg valid.GlobalCfg } -func (c *PullUpdater) updatePull(ctx *command.Context, cmd PullCommand, res command.Result) { +func (c *PullUpdater) UpdatePull(ctx *command.Context, cmd PullCommand, res command.Result) { // Log if we got any errors or failures. if res.Error != nil { ctx.Log.Err(res.Error.Error()) diff --git a/server/events/version_command_runner.go b/server/events/version_command_runner.go index 101ba2857..090e7ba59 100644 --- a/server/events/version_command_runner.go +++ b/server/events/version_command_runner.go @@ -52,7 +52,7 @@ func (v *VersionCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { result = runProjectCmds(projectCmds, v.prjCmdRunner.Version) } - v.pullUpdater.updatePull(ctx, cmd, result) + v.pullUpdater.UpdatePull(ctx, cmd, result) } func (v *VersionCommandRunner) isParallelEnabled(cmds []command.ProjectContext) bool { diff --git a/server/lyft/gateway/autoplan_builder.go b/server/lyft/gateway/autoplan_builder.go new file mode 100644 index 000000000..e052aa856 --- /dev/null +++ b/server/lyft/gateway/autoplan_builder.go @@ -0,0 +1,166 @@ +package gateway + +import ( + "fmt" + "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/metrics" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/recovery" + "github.com/uber-go/tally" + "strconv" +) + +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_autoplan_validator.go AutoplanValidator +type AutoplanValidator interface { + PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool +} + +// AutoplanBuilder handles setting up repo cloning and checking to verify of any terraform files have changed +type AutoplanBuilder struct { + Logger logging.SimpleLogging + Scope tally.Scope + VCSClient vcs.Client + PreWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner + DisableAutoplan bool + Drainer *events.Drainer + GlobalCfg valid.GlobalCfg + // AllowForkPRs controls whether we operate on pull requests from forks. + AllowForkPRs bool + // AllowForkPRsFlag is the name of the flag that controls fork PR's. We use + // this in our error message back to the user on a forked PR so they know + // how to enable this functionality. + AllowForkPRsFlag string + // SilenceForkPRErrors controls whether to comment on Fork PRs when AllowForkPRs = False + SilenceForkPRErrors bool + // SilenceForkPRErrorsFlag is the name of the flag that controls fork PR's. We use + // this in our error message back to the user on a forked PR so they know + // how to disable error comment + SilenceForkPRErrorsFlag string + // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans + // are found + silenceVCSStatusNoPlans bool + // SilenceVCSStatusNoPlans is whether any plan should set commit status if no projects + // are found + silenceVCSStatusNoProjects bool + CommitStatusUpdater events.CommitStatusUpdater + PrjCmdBuilder events.ProjectPlanCommandBuilder + PullUpdater *events.PullUpdater +} + +func (r *AutoplanBuilder) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { + if opStarted := r.Drainer.StartOp(); !opStarted { + if commentErr := r.VCSClient.CreateComment(baseRepo, pull.Num, ShutdownComment, command.Plan.String()); commentErr != nil { + r.Logger.Log(logging.Error, "unable to comment that Atlantis is shutting down: %s", commentErr) + } + return false + } + defer r.Drainer.OpDone() + + log := r.Logger.WithHistory( + "repository", baseRepo.FullName, + "pull-num", strconv.Itoa(pull.Num), + ) + defer r.logPanics(baseRepo, pull.Num, log) + + scope := r.Scope.SubScope("gateway-autoplan") + timer := scope.Timer(metrics.ExecutionTimeMetric).Start() + defer timer.Stop() + + ctx := &command.Context{ + User: user, + Log: log, + Scope: scope, + Pull: pull, + HeadRepo: headRepo, + Trigger: command.AutoTrigger, + } + if !r.validateCtxAndComment(ctx) { + return false + } + if r.DisableAutoplan { + return false + } + err := r.PreWorkflowHooksCommandRunner.RunPreHooks(ctx) + if err != nil { + ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, command.Plan) + } + + projectCmds, err := r.PrjCmdBuilder.BuildAutoplanCommands(ctx) + if err != nil { + if statusErr := r.CommitStatusUpdater.UpdateCombined(baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil { + ctx.Log.Warn("unable to update commit status: %s", statusErr) + } + r.PullUpdater.UpdatePull(ctx, events.AutoplanCommand{}, command.Result{Error: err}) + return false + } + if len(projectCmds) == 0 { + ctx.Log.Info("determined there was no project to run plan in") + if !(r.silenceVCSStatusNoPlans || r.silenceVCSStatusNoProjects) { + // If there were no projects modified, we set successful commit statuses + // with 0/0 projects planned/policy_checked/applied successfully because some users require + // the Atlantis status to be passing for all pull requests. + ctx.Log.Debug("setting VCS status to success with no projects found") + if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.Plan, 0, 0); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.PolicyCheck, 0, 0); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.Apply, 0, 0); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + } + ctx.Scope.Counter("tf_projects_found").Inc(1) + return false + } + ctx.Scope.Counter("tf_projects_not_found").Inc(1) + return true +} + +func (r *AutoplanBuilder) logPanics(baseRepo models.Repo, pullNum int, logger logging.SimpleLogging) { + if err := recover(); err != nil { + stack := recovery.Stack(3) + logger.Err("PANIC: %s\n%s", err, stack) + if commentErr := r.VCSClient.CreateComment( + baseRepo, + pullNum, + fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack), + "", + ); commentErr != nil { + logger.Err("unable to comment: %s", commentErr) + } + } +} + +func (r *AutoplanBuilder) validateCtxAndComment(ctx *command.Context) bool { + if !r.AllowForkPRs && ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner { + if r.SilenceForkPRErrors { + return false + } + ctx.Log.Info("command was run on a fork pull request which is disallowed") + if err := r.VCSClient.CreateComment(ctx.Pull.BaseRepo, ctx.Pull.Num, fmt.Sprintf("Atlantis commands can't be run on fork pull requests. To enable, set --%s or, to disable this message, set --%s", r.AllowForkPRsFlag, r.SilenceForkPRErrorsFlag), ""); err != nil { + ctx.Log.Err("unable to comment: %s", err) + } + return false + } + + if ctx.Pull.State != models.OpenPullState { + ctx.Log.Info("command was run on closed pull request") + if err := r.VCSClient.CreateComment(ctx.Pull.BaseRepo, ctx.Pull.Num, "Atlantis commands can't be run on closed pull requests", ""); err != nil { + ctx.Log.Err("unable to comment: %s", err) + } + return false + } + + repo := r.GlobalCfg.MatchingRepo(ctx.Pull.BaseRepo.ID()) + if !repo.BranchMatches(ctx.Pull.BaseBranch) { + ctx.Log.Info("command was run on a pull request which doesn't match base branches") + // just ignore it to allow us to use any git workflows without malicious intentions. + return false + } + return true +} diff --git a/server/lyft/gateway/autoplan_builder_test.go b/server/lyft/gateway/autoplan_builder_test.go new file mode 100644 index 000000000..948f0d65a --- /dev/null +++ b/server/lyft/gateway/autoplan_builder_test.go @@ -0,0 +1,107 @@ +package gateway_test + +import ( + "errors" + . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/core/config/valid" + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/mocks" + "github.com/runatlantis/atlantis/server/events/mocks/matchers" + "github.com/runatlantis/atlantis/server/events/models/fixtures" + vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/lyft/gateway" + "github.com/runatlantis/atlantis/server/metrics" + . "github.com/runatlantis/atlantis/testing" + "testing" +) + +var autoplanBuilder gateway.AutoplanBuilder +var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner +var projectCommandBuilder *mocks.MockProjectCommandBuilder +var drainer *events.Drainer +var commitStatusUpdater *mocks.MockCommitStatusUpdater + +func setupAutoplan(t *testing.T) *vcsmocks.MockClient { + RegisterMockTestingT(t) + projectCommandBuilder = mocks.NewMockProjectCommandBuilder() + commitStatusUpdater = mocks.NewMockCommitStatusUpdater() + vcsClient := vcsmocks.NewMockClient() + drainer = &events.Drainer{} + pullUpdater := &events.PullUpdater{ + HidePrevPlanComments: false, + VCSClient: vcsClient, + MarkdownRenderer: &events.MarkdownRenderer{}, + } + preWorkflowHooksCommandRunner = mocks.NewMockPreWorkflowHooksCommandRunner() + When(preWorkflowHooksCommandRunner.RunPreHooks(matchers.AnyPtrToEventsCommandContext())).ThenReturn(nil) + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{}) + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") + autoplanBuilder = gateway.AutoplanBuilder{ + Logger: logger, + Scope: scope, + VCSClient: vcsClient, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, + Drainer: drainer, + GlobalCfg: globalCfg, + AllowForkPRsFlag: "allow-fork-prs-flag", + PullUpdater: pullUpdater, + PrjCmdBuilder: projectCommandBuilder, + CommitStatusUpdater: commitStatusUpdater, + } + return vcsClient +} + +func TestPullRequestHasTerraformChanges_DrainOngoing(t *testing.T) { + t.Log("if drain is ongoing then a message should be displayed") + vcsClient := setupAutoplan(t) + drainer.ShutdownBlocking() + containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "Atlantis server is shutting down, please try again later.", "plan") + Assert(t, containsTerraformChanges == false, "should be false when an error occurs") +} + +func TestPullRequestHasTerraformChanges_ProjectBuilderError(t *testing.T) { + t.Log("if drain is ongoing then a message should be displayed") + vcsClient := setupAutoplan(t) + When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). + ThenReturn([]command.ProjectContext{}, errors.New("err")) + containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "**Plan Error**\n```\nerr\n```\n", "plan") + Assert(t, containsTerraformChanges == false, "should be false when an error occurs") +} + +func TestPullRequestHasTerraformChanges_TerraformChanges(t *testing.T) { + t.Log("verify returns true if terraform changes exist") + vcsClient := setupAutoplan(t) + When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). + ThenReturn([]command.ProjectContext{ + { + CommandName: command.Plan, + }, + { + CommandName: command.Plan, + }, + }, nil) + + containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + Assert(t, containsTerraformChanges == true, "should have terraform changes") + vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) +} + +func TestPullRequestHasTerraformChanges_NoTerraformChanges(t *testing.T) { + t.Log("verify returns false if terraform changes don't exist") + vcsClient := setupAutoplan(t) + containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + Assert(t, containsTerraformChanges == false, "should have no terraform changes") + vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) + commitStatusUpdater.VerifyWasCalled(Times(3)).UpdateCombinedCount( + matchers.AnyModelsRepo(), + matchers.AnyModelsPullRequest(), + matchers.AnyModelsCommitStatus(), + matchers.AnyModelsCommandName(), + AnyInt(), + AnyInt()) +} diff --git a/server/lyft/gateway/events_controller.go b/server/lyft/gateway/events_controller.go new file mode 100644 index 000000000..1e9aaf302 --- /dev/null +++ b/server/lyft/gateway/events_controller.go @@ -0,0 +1,287 @@ +package gateway + +import ( + "bytes" + "fmt" + "github.com/google/go-github/v31/github" + "github.com/pkg/errors" + events_controllers "github.com/runatlantis/atlantis/server/controllers/events" + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/lyft/aws/sns" + "github.com/uber-go/tally" + "net/http" +) + +const ( + GithubHeader = "X-Github-Event" + ShutdownComment = "Atlantis server is shutting down, please try again later." +) + +type HttpResponse struct { + body string + err HttpError +} + +type HttpError struct { + err error + code int +} + +// VCSEventsController handles all webhook requests which signify 'events' in the +// VCS host, ex. GitHub. +type VCSEventsController struct { + Logger logging.SimpleLogging + Scope tally.Scope + Parser events.EventParsing + CommentParser events.CommentParsing + // GithubWebhookSecret is the secret added to this webhook via the GitHub + // UI that identifies this call as coming from GitHub. If empty, no + // request validation is done. + GithubWebhookSecret []byte + GithubRequestValidator events_controllers.GithubRequestValidator + RepoAllowlistChecker *events.RepoAllowlistChecker + // SilenceAllowlistErrors controls whether we write an error comment on + // pull requests from non-allowlisted repos. + SilenceAllowlistErrors bool + VCSClient vcs.Client + SNSWriter sns.Writer + AutoplanValidator AutoplanValidator +} + +// Post handles POST webhook requests. +func (g *VCSEventsController) Post(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(GithubHeader) != "" { + g.Logger.Debug("handling GitHub post") + g.handleGithubPost(w, r) + return + } + g.respond(w, logging.Debug, http.StatusBadRequest, "Ignoring request") +} + +func (g *VCSEventsController) handleGithubPost(w http.ResponseWriter, r *http.Request) { + // Validate the request against the optional webhook secret. + payload, err := g.GithubRequestValidator.Validate(r, g.GithubWebhookSecret) + if err != nil { + g.respond(w, logging.Warn, http.StatusBadRequest, err.Error()) + return + } + githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery") + logger := g.Logger.With("gh-request-id", githubReqID) + scope := g.Scope.SubScope("github.event") + logger.Debug("request valid") + event, _ := github.ParseWebHook(github.WebHookType(r), payload) + + var resp HttpResponse + switch event := event.(type) { + case *github.IssueCommentEvent: + resp = g.HandleGithubCommentEvent(event, githubReqID, logger, r) + scope = scope.SubScope(fmt.Sprintf("comment.%s", *event.Action)) + case *github.PullRequestEvent: + resp = g.HandleGithubPullRequestEvent(logger, event, githubReqID, r) + scope = scope.SubScope(fmt.Sprintf("pr.%s", *event.Action)) + default: + resp = HttpResponse{ + body: fmt.Sprintf("Ignoring unsupported event %s", githubReqID), + } + } + + // We only count failures here as worker mode should count the successes. + if resp.err.code != 0 { + logger.Err("error handling gh post code: %d err: %s", resp.err.code, resp.err.err.Error()) + scope.Counter(fmt.Sprintf("error_%d", resp.err.code)).Inc(1) + w.WriteHeader(resp.err.code) + fmt.Fprintln(w, resp.err.err.Error()) + return + } + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, resp.body) +} + +// HandleGithubCommentEvent handles comment events from GitHub where Atlantis +// commands can come from. It's exported to make testing easier. +func (g *VCSEventsController) HandleGithubCommentEvent(event *github.IssueCommentEvent, githubReqID string, logger logging.SimpleLogging, r *http.Request) HttpResponse { + if event.GetAction() != "created" { + return HttpResponse{ + body: fmt.Sprintf("Ignoring comment event since action was not created %s", githubReqID), + } + } + baseRepo, _, pullNum, err := g.Parser.ParseGithubIssueCommentEvent(event) + wrapped := errors.Wrapf(err, "Failed parsing event: %s", githubReqID) + if err != nil { + return HttpResponse{ + body: wrapped.Error(), + err: HttpError{ + code: http.StatusBadRequest, + err: wrapped, + }, + } + } + // We pass in nil for maybeHeadRepo because the head repo data isn't + // available in the GithubIssueComment event. + return g.handleCommentEvent(logger, baseRepo, pullNum, event.Comment.GetBody(), models.Github, r) +} + +func (g *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, baseRepo models.Repo, pullNum int, comment string, vcsHost models.VCSHostType, r *http.Request) HttpResponse { + parseResult := g.CommentParser.Parse(comment, vcsHost) + if parseResult.Ignore { + truncated := comment + truncateLen := 40 + if len(truncated) > truncateLen { + truncated = comment[:truncateLen] + "..." + } + return HttpResponse{ + body: fmt.Sprintf("Ignoring non-command comment: %q", truncated), + } + } + logger.Info("parsed comment as %s", parseResult.Command) + + // At this point we know it's a command we're not supposed to ignore, so now + // we check if this repo is allowed to run commands in the first place. + if !g.RepoAllowlistChecker.IsAllowlisted(baseRepo.FullName, baseRepo.VCSHost.Hostname) { + g.commentNotAllowlisted(baseRepo, pullNum) + + err := errors.New("Repo not allowlisted") + return HttpResponse{ + body: err.Error(), + err: HttpError{ + err: err, + code: http.StatusForbidden, + }, + } + } + + // If the command isn't valid or doesn't require processing, ex. + // "atlantis help" then we just comment back immediately. + // We do this here rather than earlier because we need access to the pull + // variable to comment back on the pull request. + if parseResult.CommentResponse != "" { + if err := g.VCSClient.CreateComment(baseRepo, pullNum, parseResult.CommentResponse, ""); err != nil { + logger.Err("unable to comment on pull request: %s", err) + } + return HttpResponse{ + body: "Commenting back on pull request", + } + } + logger.Debug("executing command") + if err := g.SendToWorker(r); err != nil { + logger.With("err", err).Warn("failed to send comment request to Atlantis worker") + return HttpResponse{ + body: err.Error(), + err: HttpError{ + code: http.StatusBadRequest, + err: err, + }, + } + } + return HttpResponse{ + body: "Processing...", + } +} + +func (g *VCSEventsController) HandleGithubPullRequestEvent(logger logging.SimpleLogging, pullEvent *github.PullRequestEvent, githubReqID string, r *http.Request) HttpResponse { + pull, pullEventType, baseRepo, headRepo, user, err := g.Parser.ParseGithubPullEvent(pullEvent) + if err != nil { + wrapped := errors.Wrapf(err, "Error parsing pull data: %s %s", err, githubReqID) + return HttpResponse{ + body: wrapped.Error(), + err: HttpError{ + code: http.StatusBadRequest, + err: wrapped, + }, + } + } + logger.Debug("identified event as type %q", pullEventType.String()) + return g.handlePullRequestEvent(baseRepo, headRepo, pull, user, pullEventType, r) +} + +func (g *VCSEventsController) handlePullRequestEvent(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User, eventType models.PullRequestEventType, request *http.Request) HttpResponse { + if !g.RepoAllowlistChecker.IsAllowlisted(baseRepo.FullName, baseRepo.VCSHost.Hostname) { + // If the repo isn't allowlisted and we receive an opened pull request + // event we comment back on the pull request that the repo isn't + // allowlisted. This is because the user might be expecting Atlantis to + // autoplan. For other events, we just ignore them. + if eventType == models.OpenedPullEvent { + g.commentNotAllowlisted(baseRepo, pull.Num) + } + err := errors.New(fmt.Sprintf("Pull request event from non-allowlisted repo \"%s/%s\"", baseRepo.VCSHost.Hostname, baseRepo.FullName)) + return HttpResponse{ + body: err.Error(), + err: HttpError{ + code: http.StatusForbidden, + err: err, + }, + } + } + switch eventType { + case models.OpenedPullEvent, models.UpdatedPullEvent: + // If the pull request was opened or updated, we perform a pseudo-autoplan to determine if tf changes exist. + // If it exists, then we will forward request to the worker. + go g.handleOpenPullEvent(baseRepo, headRepo, pull, user, request) + return HttpResponse{ + body: "Processing...", + } + case models.ClosedPullEvent: + // If the pull request was closed, we route to worker to handle deleting locks. + if err := g.SendToWorker(request); err != nil { + return HttpResponse{ + body: err.Error(), + err: HttpError{ + code: http.StatusBadRequest, + err: err, + }, + } + } + case models.OtherPullEvent: + // Else we ignore the event. + return HttpResponse{ + body: "Ignoring non-actionable pull request event", + } + } + return HttpResponse{} +} + +func (g *VCSEventsController) handleOpenPullEvent(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User, request *http.Request) { + if hasTerraformChanges := g.AutoplanValidator.PullRequestHasTerraformChanges(baseRepo, headRepo, pull, user); hasTerraformChanges { + if err := g.SendToWorker(request); err != nil { + g.Logger.With("err", err).Warn("failed to send autoplan request to Atlantis worker") + } + } +} + +func (g *VCSEventsController) SendToWorker(r *http.Request) error { + buffer := bytes.NewBuffer([]byte{}) + if err := r.Write(buffer); err != nil { + g.Scope.SubScope("send").Counter("failure").Inc(1) + err = errors.Wrap(err, "marshalling gateway request to buffer") + return err + } + if err := g.SNSWriter.Write(buffer.Bytes()); err != nil { + g.Scope.SubScope("send").Counter("failure").Inc(1) + err = errors.Wrap(err, "marshalling gateway request to buffer") + return err + } + g.Scope.SubScope("send").Counter("success").Inc(1) + return nil +} + +func (g *VCSEventsController) respond(w http.ResponseWriter, lvl logging.LogLevel, code int, format string, args ...interface{}) { + response := fmt.Sprintf(format, args...) + g.Logger.Log(lvl, response) + w.WriteHeader(code) + fmt.Fprintln(w, response) +} + +// commentNotAllowlisted comments on the pull request that the repo is not +// allowlisted unless allowlist error comments are disabled. +func (g *VCSEventsController) commentNotAllowlisted(baseRepo models.Repo, pullNum int) { + if g.SilenceAllowlistErrors { + return + } + errMsg := "```\nError: This repo is not allowlisted for Atlantis.\n```" + if err := g.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); err != nil { + g.Logger.Err("unable to comment on pull request: %s", err) + } +} diff --git a/server/lyft/gateway/events_controller_test.go b/server/lyft/gateway/events_controller_test.go new file mode 100644 index 000000000..8795dee52 --- /dev/null +++ b/server/lyft/gateway/events_controller_test.go @@ -0,0 +1,337 @@ +package gateway_test + +import ( + "bytes" + "errors" + . "github.com/petergtz/pegomock" + events_controllers "github.com/runatlantis/atlantis/server/controllers/events" + "github.com/runatlantis/atlantis/server/controllers/events/mocks" + "github.com/runatlantis/atlantis/server/events" + emocks "github.com/runatlantis/atlantis/server/events/mocks" + "github.com/runatlantis/atlantis/server/events/mocks/matchers" + "github.com/runatlantis/atlantis/server/events/models" + vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" + "github.com/runatlantis/atlantis/server/logging" + sns_mocks "github.com/runatlantis/atlantis/server/lyft/aws/sns/mocks" + sns_matchers "github.com/runatlantis/atlantis/server/lyft/aws/sns/mocks/matchers" + "github.com/runatlantis/atlantis/server/lyft/gateway" + av_mocks "github.com/runatlantis/atlantis/server/lyft/gateway/mocks" + "github.com/runatlantis/atlantis/server/metrics" + . "github.com/runatlantis/atlantis/testing" + "io/ioutil" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" +) + +var secret = []byte("secret") + +func TestPost_NotGithub(t *testing.T) { + t.Log("when the request is not for github a 400 is returned") + e, _, _, _, _, _, _, _, _, _ := setup(t) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + e.Post(w, req) + ResponseContains(t, w, http.StatusBadRequest, "Ignoring request") +} + +func TestPost_InvalidGithubSecret(t *testing.T) { + t.Log("when the github payload can't be validated a 400 is returned") + e, v, _, _, _, _, _, _, _, _ := setup(t) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "value") + When(v.Validate(req, secret)).ThenReturn(nil, errors.New("err")) + e.Post(w, req) + ResponseContains(t, w, http.StatusBadRequest, "err") +} + +func TestPost_UnsupportedGithubEvent(t *testing.T) { + t.Log("when the event type is an unsupported github event we ignore it") + e, v, _, _, _, _, _, _, _, _ := setup(t) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "value") + When(v.Validate(req, nil)).ThenReturn([]byte(`{"not an event": ""}`), nil) + e.Post(w, req) + ResponseContains(t, w, http.StatusOK, "Ignoring unsupported event") +} + +func TestPost_GithubCommentNotCreated(t *testing.T) { + t.Log("when the event is a github comment but it's not a created event we ignore it") + e, v, _, _, _, _, _, _, _, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "issue_comment") + // comment action is deleted, not created + event := `{"action": "deleted"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusOK, "Ignoring comment event since action was not created") +} + +func TestPost_GithubInvalidComment(t *testing.T) { + t.Log("when the event is a github comment without all expected data we return a 400") + e, v, _, p, _, _, _, _, _, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "issue_comment") + event := `{"action": "created"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(models.Repo{}, models.User{}, 1, errors.New("err")) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusBadRequest, "Failed parsing event") +} + +func TestPost_GithubCommentNotAllowlisted(t *testing.T) { + t.Log("when the event is a github comment from a repo that isn't allowlisted we comment with an error") + RegisterMockTestingT(t) + vcsClient := vcsmocks.NewMockClient() + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "null") + e := gateway.VCSEventsController{ + Logger: logger, + Scope: scope, + GithubRequestValidator: &events_controllers.DefaultGithubRequestValidator{}, + CommentParser: &events.CommentParser{}, + Parser: &events.EventParser{}, + RepoAllowlistChecker: &events.RepoAllowlistChecker{}, + VCSClient: vcsClient, + } + requestJSON, err := ioutil.ReadFile(filepath.Join("../../controllers/events/testfixtures", "githubIssueCommentEvent_notAllowlisted.json")) + Ok(t, err) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(requestJSON)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set(gateway.GithubHeader, "issue_comment") + w := httptest.NewRecorder() + e.Post(w, req) + + Equals(t, http.StatusForbidden, w.Result().StatusCode) + body, _ := ioutil.ReadAll(w.Result().Body) + exp := "Repo not allowlisted" + Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) + expRepo, _ := models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "") + vcsClient.VerifyWasCalledOnce().CreateComment(expRepo, 2, "```\nError: This repo is not allowlisted for Atlantis.\n```", "") +} + +func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) { + t.Log("when the event is a github comment from a repo that isn't allowlisted and we are silencing errors, do not comment with an error") + RegisterMockTestingT(t) + vcsClient := vcsmocks.NewMockClient() + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "null") + e := gateway.VCSEventsController{ + Logger: logger, + Scope: scope, + GithubRequestValidator: &events_controllers.DefaultGithubRequestValidator{}, + CommentParser: &events.CommentParser{}, + Parser: &events.EventParser{}, + RepoAllowlistChecker: &events.RepoAllowlistChecker{}, + VCSClient: vcsClient, + SilenceAllowlistErrors: true, + } + requestJSON, err := ioutil.ReadFile(filepath.Join("../../controllers/events/testfixtures", "githubIssueCommentEvent_notAllowlisted.json")) + Ok(t, err) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(requestJSON)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set(gateway.GithubHeader, "issue_comment") + w := httptest.NewRecorder() + e.Post(w, req) + + Equals(t, http.StatusForbidden, w.Result().StatusCode) + body, _ := ioutil.ReadAll(w.Result().Body) + exp := "Repo not allowlisted" + Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) + vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) +} + +func TestPost_GithubCommentResponse(t *testing.T) { + t.Log("when the event is a github comment that warrants a comment response we comment back") + e, v, _, p, _, _, vcsClient, cp, _, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "issue_comment") + event := `{"action": "created"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + baseRepo := models.Repo{} + user := models.User{} + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) + When(cp.Parse("", models.Github)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"}) + w := httptest.NewRecorder() + + e.Post(w, req) + vcsClient.VerifyWasCalledOnce().CreateComment(baseRepo, 1, "a comment", "") + ResponseContains(t, w, http.StatusOK, "Commenting back on pull request") +} + +func TestPost_GithubPullRequestInvalid(t *testing.T) { + t.Log("when the event is a github pull request with invalid data we return a 400") + e, v, _, p, _, _, _, _, _, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + + event := `{"action": "closed"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(models.PullRequest{}, models.OpenedPullEvent, models.Repo{}, models.Repo{}, models.User{}, errors.New("err")) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusBadRequest, "Error parsing pull data: err") +} + +func TestPost_GithubPullRequestNotAllowlisted(t *testing.T) { + t.Log("when the event is a github pull request to a non-allowlisted repo we return a 400") + e, v, _, _, _, _, _, _, _, _ := setup(t) + var err error + e.RepoAllowlistChecker, err = events.NewRepoAllowlistChecker("github.com/nevermatch") + Ok(t, err) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + + event := `{"action": "closed"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusForbidden, "Pull request event from non-allowlisted repo") +} + +func setup(t *testing.T) (gateway.VCSEventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing, *sns_mocks.MockWriter, *av_mocks.MockAutoplanValidator) { + RegisterMockTestingT(t) + v := mocks.NewMockGithubRequestValidator() + gl := mocks.NewMockGitlabRequestParserValidator() + p := emocks.NewMockEventParsing() + cp := emocks.NewMockCommentParsing() + cr := emocks.NewMockCommandRunner() + c := emocks.NewMockPullCleaner() + vcsmock := vcsmocks.NewMockClient() + snsMock := sns_mocks.NewMockWriter() + avMock := av_mocks.NewMockAutoplanValidator() + repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") + Ok(t, err) + logger := logging.NewNoopLogger(t) + scope, _, _ := metrics.NewLoggingScope(logger, "null") + e := gateway.VCSEventsController{ + Logger: logger, + Scope: scope, + GithubRequestValidator: v, + Parser: p, + CommentParser: cp, + GithubWebhookSecret: secret, + RepoAllowlistChecker: repoAllowlistChecker, + VCSClient: vcsmock, + SNSWriter: snsMock, + AutoplanValidator: avMock, + } + return e, v, gl, p, cr, c, vcsmock, cp, snsMock, avMock +} + +func TestPost_GithubCommentSuccess(t *testing.T) { + t.Log("when the event is a github comment with a valid command we send request to SNS") + e, v, _, p, _, _, _, cp, sns, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "issue_comment") + event := `{"action": "created"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + baseRepo := models.Repo{} + user := models.User{} + cmd := events.CommentCommand{} + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) + When(cp.Parse("", models.Github)).ThenReturn(events.CommentParseResult{Command: &cmd}) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) + w := httptest.NewRecorder() + e.Post(w, req) + sns.VerifyWasCalledOnce().Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusOK, "Processing...") +} + +func TestPost_GithubCommentFailure(t *testing.T) { + t.Log("when the event is a github comment with a valid command we send request to SNS (failure)") + e, v, _, p, _, _, _, cp, sns, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "issue_comment") + event := `{"action": "created"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + baseRepo := models.Repo{} + user := models.User{} + cmd := events.CommentCommand{} + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) + When(cp.Parse("", models.Github)).ThenReturn(events.CommentParseResult{Command: &cmd}) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(errors.New("err")) + w := httptest.NewRecorder() + e.Post(w, req) + sns.VerifyWasCalledOnce().Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusBadRequest, "marshalling gateway request to buffer: err") +} + +func TestPost_GithubIgnorePR(t *testing.T) { + t.Log("when the event is not PR open/update/close, we ignore it") + e, v, _, p, _, _, _, _, sns, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + event := `{"action": "other"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + repo := models.Repo{} + pull := models.PullRequest{State: models.ClosedPullState} + When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OtherPullEvent, repo, repo, models.User{}, nil) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) + w := httptest.NewRecorder() + e.Post(w, req) + sns.VerifyWasCalled(Never()).Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusOK, "Ignoring non-actionable pull request event") +} + +func TestPost_GithubClosePR(t *testing.T) { + t.Log("when the event is a PR closed, we send request to SNS") + e, v, _, p, _, _, _, _, sns, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + event := `{"action": "closed"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + repo := models.Repo{} + pull := models.PullRequest{State: models.ClosedPullState} + When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.ClosedPullEvent, repo, repo, models.User{}, nil) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) + w := httptest.NewRecorder() + e.Post(w, req) + sns.VerifyWasCalledOnce().Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusOK, "") +} + +func TestPost_GithubOpenPR_WithTerraformChanges(t *testing.T) { + t.Log("when the event is a PR opened, we send request to SNS if there are tf changes") + e, v, _, p, _, _, _, _, sns, av := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + event := `{"action": "open"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + repo := models.Repo{} + pull := models.PullRequest{State: models.OpenPullState} + When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) + When(av.PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(true) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) + w := httptest.NewRecorder() + e.Post(w, req) + av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + sns.VerifyWasCalledEventually(Once(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusOK, "") +} + +func TestPost_GithubOpenPR_NoTerraformChanges(t *testing.T) { + t.Log("when the event is a PR opened, we don't send a request to SNS if there are no tf changes") + e, v, _, p, _, _, _, _, sns, av := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gateway.GithubHeader, "pull_request") + event := `{"action": "open"}` + When(v.Validate(req, secret)).ThenReturn([]byte(event), nil) + repo := models.Repo{} + pull := models.PullRequest{State: models.OpenPullState} + When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) + When(av.PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(false) + When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) + w := httptest.NewRecorder() + e.Post(w, req) + av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + sns.VerifyWasCalledEventually(Never(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) + ResponseContains(t, w, http.StatusOK, "") +} diff --git a/server/lyft/gateway/mocks/matchers/models_pullrequest.go b/server/lyft/gateway/mocks/matchers/models_pullrequest.go new file mode 100644 index 000000000..9ae2a7e92 --- /dev/null +++ b/server/lyft/gateway/mocks/matchers/models_pullrequest.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 AnyModelsPullRequest() models.PullRequest { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PullRequest))(nil)).Elem())) + var nullValue models.PullRequest + return nullValue +} + +func EqModelsPullRequest(value models.PullRequest) models.PullRequest { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.PullRequest + return nullValue +} + +func NotEqModelsPullRequest(value models.PullRequest) models.PullRequest { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.PullRequest + return nullValue +} + +func ModelsPullRequestThat(matcher pegomock.ArgumentMatcher) models.PullRequest { + pegomock.RegisterMatcher(matcher) + var nullValue models.PullRequest + return nullValue +} diff --git a/server/lyft/gateway/mocks/matchers/models_repo.go b/server/lyft/gateway/mocks/matchers/models_repo.go new file mode 100644 index 000000000..fd44665f8 --- /dev/null +++ b/server/lyft/gateway/mocks/matchers/models_repo.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 AnyModelsRepo() models.Repo { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.Repo))(nil)).Elem())) + var nullValue models.Repo + return nullValue +} + +func EqModelsRepo(value models.Repo) models.Repo { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.Repo + return nullValue +} + +func NotEqModelsRepo(value models.Repo) models.Repo { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.Repo + return nullValue +} + +func ModelsRepoThat(matcher pegomock.ArgumentMatcher) models.Repo { + pegomock.RegisterMatcher(matcher) + var nullValue models.Repo + return nullValue +} diff --git a/server/lyft/gateway/mocks/matchers/models_user.go b/server/lyft/gateway/mocks/matchers/models_user.go new file mode 100644 index 000000000..0aa92b5d8 --- /dev/null +++ b/server/lyft/gateway/mocks/matchers/models_user.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 AnyModelsUser() models.User { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.User))(nil)).Elem())) + var nullValue models.User + return nullValue +} + +func EqModelsUser(value models.User) models.User { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.User + return nullValue +} + +func NotEqModelsUser(value models.User) models.User { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.User + return nullValue +} + +func ModelsUserThat(matcher pegomock.ArgumentMatcher) models.User { + pegomock.RegisterMatcher(matcher) + var nullValue models.User + return nullValue +} diff --git a/server/lyft/gateway/mocks/mock_autoplan_validator.go b/server/lyft/gateway/mocks/mock_autoplan_validator.go new file mode 100644 index 000000000..173228a6a --- /dev/null +++ b/server/lyft/gateway/mocks/mock_autoplan_validator.go @@ -0,0 +1,117 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/lyft/gateway (interfaces: AutoplanValidator) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockAutoplanValidator struct { + fail func(message string, callerSkip ...int) +} + +func NewMockAutoplanValidator(options ...pegomock.Option) *MockAutoplanValidator { + mock := &MockAutoplanValidator{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockAutoplanValidator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockAutoplanValidator) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockAutoplanValidator) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockAutoplanValidator().") + } + params := []pegomock.Param{baseRepo, headRepo, pull, user} + result := pegomock.GetGenericMockFrom(mock).Invoke("PullRequestHasTerraformChanges", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + var ret0 bool + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(bool) + } + } + return ret0 +} + +func (mock *MockAutoplanValidator) VerifyWasCalledOnce() *VerifierMockAutoplanValidator { + return &VerifierMockAutoplanValidator{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockAutoplanValidator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockAutoplanValidator { + return &VerifierMockAutoplanValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockAutoplanValidator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockAutoplanValidator { + return &VerifierMockAutoplanValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockAutoplanValidator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockAutoplanValidator { + return &VerifierMockAutoplanValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockAutoplanValidator struct { + mock *MockAutoplanValidator + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockAutoplanValidator) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification { + params := []pegomock.Param{baseRepo, headRepo, pull, user} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PullRequestHasTerraformChanges", params, verifier.timeout) + return &MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification struct { + mock *MockAutoplanValidator + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { + baseRepo, headRepo, pull, user := c.GetAllCapturedArguments() + return baseRepo[len(baseRepo)-1], headRepo[len(headRepo)-1], pull[len(pull)-1], user[len(user)-1] +} + +func (c *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.Repo) + } + _param2 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]models.User, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(models.User) + } + } + return +} From 6abd5b5c72b4534545485d3e06583ff5d47a7ead Mon Sep 17 00:00:00 2001 From: Samra Belachew Date: Thu, 3 Mar 2022 09:21:28 -0800 Subject: [PATCH 2/4] comments --- server/lyft/gateway/autoplan_builder.go | 61 ++++++------------ server/lyft/gateway/autoplan_builder_test.go | 26 ++++---- server/lyft/gateway/events_controller.go | 63 ++++++++----------- server/lyft/gateway/events_controller_test.go | 11 ++-- .../gateway/mocks/mock_autoplan_validator.go | 16 ++--- 5 files changed, 69 insertions(+), 108 deletions(-) diff --git a/server/lyft/gateway/autoplan_builder.go b/server/lyft/gateway/autoplan_builder.go index e052aa856..8e16d2746 100644 --- a/server/lyft/gateway/autoplan_builder.go +++ b/server/lyft/gateway/autoplan_builder.go @@ -1,7 +1,6 @@ package gateway import ( - "fmt" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/command" @@ -14,13 +13,8 @@ import ( "strconv" ) -//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_autoplan_validator.go AutoplanValidator -type AutoplanValidator interface { - PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool -} - -// AutoplanBuilder handles setting up repo cloning and checking to verify of any terraform files have changed -type AutoplanBuilder struct { +// AutoplanValidatorBuilder handles setting up repo cloning and checking to verify of any terraform files have changed +type AutoplanValidatorBuilder struct { Logger logging.SimpleLogging Scope tally.Scope VCSClient vcs.Client @@ -51,11 +45,10 @@ type AutoplanBuilder struct { PullUpdater *events.PullUpdater } -func (r *AutoplanBuilder) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { +func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { if opStarted := r.Drainer.StartOp(); !opStarted { - if commentErr := r.VCSClient.CreateComment(baseRepo, pull.Num, ShutdownComment, command.Plan.String()); commentErr != nil { - r.Logger.Log(logging.Error, "unable to comment that Atlantis is shutting down: %s", commentErr) - } + r.Logger.Err("Atlantis is shutting down, cannot process current event") + r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) return false } defer r.Drainer.OpDone() @@ -64,24 +57,25 @@ func (r *AutoplanBuilder) PullRequestHasTerraformChanges(baseRepo models.Repo, h "repository", baseRepo.FullName, "pull-num", strconv.Itoa(pull.Num), ) - defer r.logPanics(baseRepo, pull.Num, log) + defer r.logPanics(log) - scope := r.Scope.SubScope("gateway-autoplan") - timer := scope.Timer(metrics.ExecutionTimeMetric).Start() + timer := r.Scope.Timer(metrics.ExecutionTimeMetric).Start() defer timer.Stop() ctx := &command.Context{ User: user, Log: log, - Scope: scope, + Scope: r.Scope, Pull: pull, HeadRepo: headRepo, Trigger: command.AutoTrigger, } if !r.validateCtxAndComment(ctx) { + r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) return false } if r.DisableAutoplan { + r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) return false } err := r.PreWorkflowHooksCommandRunner.RunPreHooks(ctx) @@ -95,6 +89,7 @@ func (r *AutoplanBuilder) PullRequestHasTerraformChanges(baseRepo models.Repo, h ctx.Log.Warn("unable to update commit status: %s", statusErr) } r.PullUpdater.UpdatePull(ctx, events.AutoplanCommand{}, command.Result{Error: err}) + r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) return false } if len(projectCmds) == 0 { @@ -104,55 +99,37 @@ func (r *AutoplanBuilder) PullRequestHasTerraformChanges(baseRepo models.Repo, h // with 0/0 projects planned/policy_checked/applied successfully because some users require // the Atlantis status to be passing for all pull requests. ctx.Log.Debug("setting VCS status to success with no projects found") - if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.Plan, 0, 0); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.PolicyCheck, 0, 0); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, command.Apply, 0, 0); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) + for _, cmd := range []command.Name{command.Plan, command.Apply, command.PolicyCheck} { + if err := r.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, cmd, 0, 0); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } } } - ctx.Scope.Counter("tf_projects_found").Inc(1) + r.Scope.Counter(metrics.ExecutionFailureMetric).Inc(1) return false } - ctx.Scope.Counter("tf_projects_not_found").Inc(1) + r.Scope.Counter(metrics.ExecutionSuccessMetric).Inc(1) return true } -func (r *AutoplanBuilder) logPanics(baseRepo models.Repo, pullNum int, logger logging.SimpleLogging) { +func (r *AutoplanValidatorBuilder) logPanics(logger logging.SimpleLogging) { if err := recover(); err != nil { stack := recovery.Stack(3) logger.Err("PANIC: %s\n%s", err, stack) - if commentErr := r.VCSClient.CreateComment( - baseRepo, - pullNum, - fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack), - "", - ); commentErr != nil { - logger.Err("unable to comment: %s", commentErr) - } } } -func (r *AutoplanBuilder) validateCtxAndComment(ctx *command.Context) bool { +func (r *AutoplanValidatorBuilder) validateCtxAndComment(ctx *command.Context) bool { if !r.AllowForkPRs && ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner { if r.SilenceForkPRErrors { return false } ctx.Log.Info("command was run on a fork pull request which is disallowed") - if err := r.VCSClient.CreateComment(ctx.Pull.BaseRepo, ctx.Pull.Num, fmt.Sprintf("Atlantis commands can't be run on fork pull requests. To enable, set --%s or, to disable this message, set --%s", r.AllowForkPRsFlag, r.SilenceForkPRErrorsFlag), ""); err != nil { - ctx.Log.Err("unable to comment: %s", err) - } return false } if ctx.Pull.State != models.OpenPullState { ctx.Log.Info("command was run on closed pull request") - if err := r.VCSClient.CreateComment(ctx.Pull.BaseRepo, ctx.Pull.Num, "Atlantis commands can't be run on closed pull requests", ""); err != nil { - ctx.Log.Err("unable to comment: %s", err) - } return false } diff --git a/server/lyft/gateway/autoplan_builder_test.go b/server/lyft/gateway/autoplan_builder_test.go index 948f0d65a..cece2bf6a 100644 --- a/server/lyft/gateway/autoplan_builder_test.go +++ b/server/lyft/gateway/autoplan_builder_test.go @@ -17,7 +17,7 @@ import ( "testing" ) -var autoplanBuilder gateway.AutoplanBuilder +var autoplanBuilder gateway.AutoplanValidatorBuilder var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner var projectCommandBuilder *mocks.MockProjectCommandBuilder var drainer *events.Drainer @@ -39,7 +39,7 @@ func setupAutoplan(t *testing.T) *vcsmocks.MockClient { globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{}) logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") - autoplanBuilder = gateway.AutoplanBuilder{ + autoplanBuilder = gateway.AutoplanValidatorBuilder{ Logger: logger, Scope: scope, VCSClient: vcsClient, @@ -54,28 +54,27 @@ func setupAutoplan(t *testing.T) *vcsmocks.MockClient { return vcsClient } -func TestPullRequestHasTerraformChanges_DrainOngoing(t *testing.T) { +func TestIsValid_DrainOngoing(t *testing.T) { t.Log("if drain is ongoing then a message should be displayed") - vcsClient := setupAutoplan(t) + _ = setupAutoplan(t) drainer.ShutdownBlocking() - containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) - vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "Atlantis server is shutting down, please try again later.", "plan") + containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } -func TestPullRequestHasTerraformChanges_ProjectBuilderError(t *testing.T) { - t.Log("if drain is ongoing then a message should be displayed") +func TestIsValid_ProjectBuilderError(t *testing.T) { + t.Log("projct builder error") vcsClient := setupAutoplan(t) When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). ThenReturn([]command.ProjectContext{}, errors.New("err")) - containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "**Plan Error**\n```\nerr\n```\n", "plan") Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } -func TestPullRequestHasTerraformChanges_TerraformChanges(t *testing.T) { +func TestIsValid_TerraformChanges(t *testing.T) { t.Log("verify returns true if terraform changes exist") - vcsClient := setupAutoplan(t) + _ = setupAutoplan(t) When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). ThenReturn([]command.ProjectContext{ { @@ -86,15 +85,14 @@ func TestPullRequestHasTerraformChanges_TerraformChanges(t *testing.T) { }, }, nil) - containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == true, "should have terraform changes") - vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) } func TestPullRequestHasTerraformChanges_NoTerraformChanges(t *testing.T) { t.Log("verify returns false if terraform changes don't exist") vcsClient := setupAutoplan(t) - containsTerraformChanges := autoplanBuilder.PullRequestHasTerraformChanges(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should have no terraform changes") vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) commitStatusUpdater.VerifyWasCalled(Times(3)).UpdateCombinedCount( diff --git a/server/lyft/gateway/events_controller.go b/server/lyft/gateway/events_controller.go index 1e9aaf302..a57b94e73 100644 --- a/server/lyft/gateway/events_controller.go +++ b/server/lyft/gateway/events_controller.go @@ -16,8 +16,7 @@ import ( ) const ( - GithubHeader = "X-Github-Event" - ShutdownComment = "Atlantis server is shutting down, please try again later." + GithubHeader = "X-Github-Event" ) type HttpResponse struct { @@ -30,6 +29,11 @@ type HttpError struct { code int } +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_autoplan_validator.go AutoplanValidator +type AutoplanValidator interface { + IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool +} + // VCSEventsController handles all webhook requests which signify 'events' in the // VCS host, ex. GitHub. type VCSEventsController struct { @@ -68,49 +72,45 @@ func (g *VCSEventsController) handleGithubPost(w http.ResponseWriter, r *http.Re g.respond(w, logging.Warn, http.StatusBadRequest, err.Error()) return } - githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery") - logger := g.Logger.With("gh-request-id", githubReqID) - scope := g.Scope.SubScope("github.event") - logger.Debug("request valid") event, _ := github.ParseWebHook(github.WebHookType(r), payload) + scope := g.Scope.SubScope("github.event") var resp HttpResponse switch event := event.(type) { case *github.IssueCommentEvent: - resp = g.HandleGithubCommentEvent(event, githubReqID, logger, r) + resp = g.HandleGithubCommentEvent(event, r) scope = scope.SubScope(fmt.Sprintf("comment.%s", *event.Action)) case *github.PullRequestEvent: - resp = g.HandleGithubPullRequestEvent(logger, event, githubReqID, r) + resp = g.HandleGithubPullRequestEvent(event, r) scope = scope.SubScope(fmt.Sprintf("pr.%s", *event.Action)) default: resp = HttpResponse{ - body: fmt.Sprintf("Ignoring unsupported event %s", githubReqID), + body: fmt.Sprintf("Ignoring unsupported event"), } } - - // We only count failures here as worker mode should count the successes. if resp.err.code != 0 { - logger.Err("error handling gh post code: %d err: %s", resp.err.code, resp.err.err.Error()) + g.Logger.Err("error handling gh post code: %d err: %s", resp.err.code, resp.err.err.Error()) scope.Counter(fmt.Sprintf("error_%d", resp.err.code)).Inc(1) w.WriteHeader(resp.err.code) fmt.Fprintln(w, resp.err.err.Error()) return } + scope.Counter(fmt.Sprintf("success_%d", http.StatusOK)).Inc(1) w.WriteHeader(http.StatusOK) fmt.Fprintln(w, resp.body) } // HandleGithubCommentEvent handles comment events from GitHub where Atlantis // commands can come from. It's exported to make testing easier. -func (g *VCSEventsController) HandleGithubCommentEvent(event *github.IssueCommentEvent, githubReqID string, logger logging.SimpleLogging, r *http.Request) HttpResponse { +func (g *VCSEventsController) HandleGithubCommentEvent(event *github.IssueCommentEvent, r *http.Request) HttpResponse { if event.GetAction() != "created" { return HttpResponse{ - body: fmt.Sprintf("Ignoring comment event since action was not created %s", githubReqID), + body: fmt.Sprintf("Ignoring comment event since action was not created"), } } baseRepo, _, pullNum, err := g.Parser.ParseGithubIssueCommentEvent(event) - wrapped := errors.Wrapf(err, "Failed parsing event: %s", githubReqID) if err != nil { + wrapped := errors.Wrap(err, "Failed parsing event") return HttpResponse{ body: wrapped.Error(), err: HttpError{ @@ -121,10 +121,10 @@ func (g *VCSEventsController) HandleGithubCommentEvent(event *github.IssueCommen } // We pass in nil for maybeHeadRepo because the head repo data isn't // available in the GithubIssueComment event. - return g.handleCommentEvent(logger, baseRepo, pullNum, event.Comment.GetBody(), models.Github, r) + return g.handleCommentEvent(baseRepo, pullNum, event.Comment.GetBody(), models.Github, r) } -func (g *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, baseRepo models.Repo, pullNum int, comment string, vcsHost models.VCSHostType, r *http.Request) HttpResponse { +func (g *VCSEventsController) handleCommentEvent(baseRepo models.Repo, pullNum int, comment string, vcsHost models.VCSHostType, r *http.Request) HttpResponse { parseResult := g.CommentParser.Parse(comment, vcsHost) if parseResult.Ignore { truncated := comment @@ -136,7 +136,6 @@ func (g *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b body: fmt.Sprintf("Ignoring non-command comment: %q", truncated), } } - logger.Info("parsed comment as %s", parseResult.Command) // At this point we know it's a command we're not supposed to ignore, so now // we check if this repo is allowed to run commands in the first place. @@ -159,15 +158,14 @@ func (g *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b // variable to comment back on the pull request. if parseResult.CommentResponse != "" { if err := g.VCSClient.CreateComment(baseRepo, pullNum, parseResult.CommentResponse, ""); err != nil { - logger.Err("unable to comment on pull request: %s", err) + g.Logger.Err("unable to comment on pull request: %s", err) } return HttpResponse{ body: "Commenting back on pull request", } } - logger.Debug("executing command") if err := g.SendToWorker(r); err != nil { - logger.With("err", err).Warn("failed to send comment request to Atlantis worker") + g.Logger.With("err", err).Err("failed to send comment request to Atlantis worker") return HttpResponse{ body: err.Error(), err: HttpError{ @@ -181,10 +179,10 @@ func (g *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b } } -func (g *VCSEventsController) HandleGithubPullRequestEvent(logger logging.SimpleLogging, pullEvent *github.PullRequestEvent, githubReqID string, r *http.Request) HttpResponse { +func (g *VCSEventsController) HandleGithubPullRequestEvent(pullEvent *github.PullRequestEvent, r *http.Request) HttpResponse { pull, pullEventType, baseRepo, headRepo, user, err := g.Parser.ParseGithubPullEvent(pullEvent) if err != nil { - wrapped := errors.Wrapf(err, "Error parsing pull data: %s %s", err, githubReqID) + wrapped := errors.Wrapf(err, "Error parsing pull data: %s", err) return HttpResponse{ body: wrapped.Error(), err: HttpError{ @@ -193,7 +191,6 @@ func (g *VCSEventsController) HandleGithubPullRequestEvent(logger logging.Simple }, } } - logger.Debug("identified event as type %q", pullEventType.String()) return g.handlePullRequestEvent(baseRepo, headRepo, pull, user, pullEventType, r) } @@ -244,9 +241,9 @@ func (g *VCSEventsController) handlePullRequestEvent(baseRepo models.Repo, headR } func (g *VCSEventsController) handleOpenPullEvent(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User, request *http.Request) { - if hasTerraformChanges := g.AutoplanValidator.PullRequestHasTerraformChanges(baseRepo, headRepo, pull, user); hasTerraformChanges { + if hasTerraformChanges := g.AutoplanValidator.IsValid(baseRepo, headRepo, pull, user); hasTerraformChanges { if err := g.SendToWorker(request); err != nil { - g.Logger.With("err", err).Warn("failed to send autoplan request to Atlantis worker") + g.Logger.With("err", err).Err("failed to send autoplan request to Atlantis worker") } } } @@ -254,16 +251,11 @@ func (g *VCSEventsController) handleOpenPullEvent(baseRepo models.Repo, headRepo func (g *VCSEventsController) SendToWorker(r *http.Request) error { buffer := bytes.NewBuffer([]byte{}) if err := r.Write(buffer); err != nil { - g.Scope.SubScope("send").Counter("failure").Inc(1) - err = errors.Wrap(err, "marshalling gateway request to buffer") - return err + return errors.Wrap(err, "marshalling gateway request to buffer") } if err := g.SNSWriter.Write(buffer.Bytes()); err != nil { - g.Scope.SubScope("send").Counter("failure").Inc(1) - err = errors.Wrap(err, "marshalling gateway request to buffer") - return err + return errors.Wrap(err, "marshalling gateway request to buffer") } - g.Scope.SubScope("send").Counter("success").Inc(1) return nil } @@ -280,8 +272,5 @@ func (g *VCSEventsController) commentNotAllowlisted(baseRepo models.Repo, pullNu if g.SilenceAllowlistErrors { return } - errMsg := "```\nError: This repo is not allowlisted for Atlantis.\n```" - if err := g.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); err != nil { - g.Logger.Err("unable to comment on pull request: %s", err) - } + g.Logger.With("repo", baseRepo.FullName, "pullNum", pullNum).Err("This repo is not allowlisted for Atlantis") } diff --git a/server/lyft/gateway/events_controller_test.go b/server/lyft/gateway/events_controller_test.go index 8795dee52..6d269e8a7 100644 --- a/server/lyft/gateway/events_controller_test.go +++ b/server/lyft/gateway/events_controller_test.go @@ -113,8 +113,6 @@ func TestPost_GithubCommentNotAllowlisted(t *testing.T) { body, _ := ioutil.ReadAll(w.Result().Body) exp := "Repo not allowlisted" Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) - expRepo, _ := models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "") - vcsClient.VerifyWasCalledOnce().CreateComment(expRepo, 2, "```\nError: This repo is not allowlisted for Atlantis.\n```", "") } func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) { @@ -145,7 +143,6 @@ func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) { body, _ := ioutil.ReadAll(w.Result().Body) exp := "Repo not allowlisted" Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) - vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) } func TestPost_GithubCommentResponse(t *testing.T) { @@ -308,11 +305,11 @@ func TestPost_GithubOpenPR_WithTerraformChanges(t *testing.T) { repo := models.Repo{} pull := models.PullRequest{State: models.OpenPullState} When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) - When(av.PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(true) + When(av.IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(true) When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) w := httptest.NewRecorder() e.Post(w, req) - av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) sns.VerifyWasCalledEventually(Once(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) ResponseContains(t, w, http.StatusOK, "") } @@ -327,11 +324,11 @@ func TestPost_GithubOpenPR_NoTerraformChanges(t *testing.T) { repo := models.Repo{} pull := models.PullRequest{State: models.OpenPullState} When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) - When(av.PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(false) + When(av.IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(false) When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) w := httptest.NewRecorder() e.Post(w, req) - av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).PullRequestHasTerraformChanges(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) sns.VerifyWasCalledEventually(Never(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) ResponseContains(t, w, http.StatusOK, "") } diff --git a/server/lyft/gateway/mocks/mock_autoplan_validator.go b/server/lyft/gateway/mocks/mock_autoplan_validator.go index 173228a6a..b31a12d7d 100644 --- a/server/lyft/gateway/mocks/mock_autoplan_validator.go +++ b/server/lyft/gateway/mocks/mock_autoplan_validator.go @@ -25,12 +25,12 @@ func NewMockAutoplanValidator(options ...pegomock.Option) *MockAutoplanValidator func (mock *MockAutoplanValidator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockAutoplanValidator) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockAutoplanValidator) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { +func (mock *MockAutoplanValidator) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { if mock == nil { panic("mock must not be nil. Use myMock := NewMockAutoplanValidator().") } params := []pegomock.Param{baseRepo, headRepo, pull, user} - result := pegomock.GetGenericMockFrom(mock).Invoke("PullRequestHasTerraformChanges", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("IsValid", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) var ret0 bool if len(result) != 0 { if result[0] != nil { @@ -77,23 +77,23 @@ type VerifierMockAutoplanValidator struct { timeout time.Duration } -func (verifier *VerifierMockAutoplanValidator) PullRequestHasTerraformChanges(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification { +func (verifier *VerifierMockAutoplanValidator) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockAutoplanValidator_IsValid_OngoingVerification { params := []pegomock.Param{baseRepo, headRepo, pull, user} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PullRequestHasTerraformChanges", params, verifier.timeout) - return &MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "IsValid", params, verifier.timeout) + return &MockAutoplanValidator_IsValid_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification struct { +type MockAutoplanValidator_IsValid_OngoingVerification struct { mock *MockAutoplanValidator methodInvocations []pegomock.MethodInvocation } -func (c *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { +func (c *MockAutoplanValidator_IsValid_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { baseRepo, headRepo, pull, user := c.GetAllCapturedArguments() return baseRepo[len(baseRepo)-1], headRepo[len(headRepo)-1], pull[len(pull)-1], user[len(user)-1] } -func (c *MockAutoplanValidator_PullRequestHasTerraformChanges_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { +func (c *MockAutoplanValidator_IsValid_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.Repo, len(c.methodInvocations)) From 399b2cf2b962d2f16ed7700822967818aa620dda Mon Sep 17 00:00:00 2001 From: Samra Belachew Date: Fri, 4 Mar 2022 14:06:35 -0800 Subject: [PATCH 3/4] comments --- server/lyft/gateway/autoplan_builder.go | 44 ++++--- server/lyft/gateway/autoplan_builder_test.go | 12 +- server/lyft/gateway/events_controller.go | 10 +- server/lyft/gateway/events_controller_test.go | 22 ++-- .../gateway/mocks/mock_autoplan_validator.go | 117 ------------------ .../gateway/mocks/mock_event_validator.go | 117 ++++++++++++++++++ 6 files changed, 163 insertions(+), 159 deletions(-) delete mode 100644 server/lyft/gateway/mocks/mock_autoplan_validator.go create mode 100644 server/lyft/gateway/mocks/mock_event_validator.go diff --git a/server/lyft/gateway/autoplan_builder.go b/server/lyft/gateway/autoplan_builder.go index 8e16d2746..6b578de80 100644 --- a/server/lyft/gateway/autoplan_builder.go +++ b/server/lyft/gateway/autoplan_builder.go @@ -1,6 +1,7 @@ package gateway import ( + "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/config/valid" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/command" @@ -13,13 +14,12 @@ import ( "strconv" ) -// AutoplanValidatorBuilder handles setting up repo cloning and checking to verify of any terraform files have changed -type AutoplanValidatorBuilder struct { +// AutoplanValidator handles setting up repo cloning and checking to verify of any terraform files have changed +type AutoplanValidator struct { Logger logging.SimpleLogging Scope tally.Scope VCSClient vcs.Client PreWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner - DisableAutoplan bool Drainer *events.Drainer GlobalCfg valid.GlobalCfg // AllowForkPRs controls whether we operate on pull requests from forks. @@ -45,11 +45,9 @@ type AutoplanValidatorBuilder struct { PullUpdater *events.PullUpdater } -func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { +func (r *AutoplanValidator) isValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) (bool, error) { if opStarted := r.Drainer.StartOp(); !opStarted { - r.Logger.Err("Atlantis is shutting down, cannot process current event") - r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) - return false + return false, errors.New("atlantis is shutting down, cannot process current event") } defer r.Drainer.OpDone() @@ -59,9 +57,6 @@ func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models ) defer r.logPanics(log) - timer := r.Scope.Timer(metrics.ExecutionTimeMetric).Start() - defer timer.Stop() - ctx := &command.Context{ User: user, Log: log, @@ -71,12 +66,7 @@ func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models Trigger: command.AutoTrigger, } if !r.validateCtxAndComment(ctx) { - r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) - return false - } - if r.DisableAutoplan { - r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) - return false + return false, errors.New("invalid command context") } err := r.PreWorkflowHooksCommandRunner.RunPreHooks(ctx) if err != nil { @@ -89,8 +79,7 @@ func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models ctx.Log.Warn("unable to update commit status: %s", statusErr) } r.PullUpdater.UpdatePull(ctx, events.AutoplanCommand{}, command.Result{Error: err}) - r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) - return false + return false, errors.Wrap(err, "Failed building project command") } if len(projectCmds) == 0 { ctx.Log.Info("determined there was no project to run plan in") @@ -105,6 +94,21 @@ func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models } } } + return false, nil + } + return true, nil +} + +func (r *AutoplanValidator) InstrumentedIsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { + timer := r.Scope.Timer(metrics.ExecutionTimeMetric).Start() + defer timer.Stop() + isValid, err := r.isValid(baseRepo, headRepo, pull, user) + if err != nil { + r.Logger.With("repo", baseRepo.FullName, "pull", pull.Num).Err(err.Error()) + r.Scope.Counter(metrics.ExecutionErrorMetric).Inc(1) + return false + } + if !isValid { r.Scope.Counter(metrics.ExecutionFailureMetric).Inc(1) return false } @@ -112,14 +116,14 @@ func (r *AutoplanValidatorBuilder) IsValid(baseRepo models.Repo, headRepo models return true } -func (r *AutoplanValidatorBuilder) logPanics(logger logging.SimpleLogging) { +func (r *AutoplanValidator) logPanics(logger logging.SimpleLogging) { if err := recover(); err != nil { stack := recovery.Stack(3) logger.Err("PANIC: %s\n%s", err, stack) } } -func (r *AutoplanValidatorBuilder) validateCtxAndComment(ctx *command.Context) bool { +func (r *AutoplanValidator) validateCtxAndComment(ctx *command.Context) bool { if !r.AllowForkPRs && ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner { if r.SilenceForkPRErrors { return false diff --git a/server/lyft/gateway/autoplan_builder_test.go b/server/lyft/gateway/autoplan_builder_test.go index cece2bf6a..d9855b733 100644 --- a/server/lyft/gateway/autoplan_builder_test.go +++ b/server/lyft/gateway/autoplan_builder_test.go @@ -17,7 +17,7 @@ import ( "testing" ) -var autoplanBuilder gateway.AutoplanValidatorBuilder +var autoplanBuilder gateway.AutoplanValidator var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner var projectCommandBuilder *mocks.MockProjectCommandBuilder var drainer *events.Drainer @@ -39,7 +39,7 @@ func setupAutoplan(t *testing.T) *vcsmocks.MockClient { globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{}) logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") - autoplanBuilder = gateway.AutoplanValidatorBuilder{ + autoplanBuilder = gateway.AutoplanValidator{ Logger: logger, Scope: scope, VCSClient: vcsClient, @@ -58,7 +58,7 @@ func TestIsValid_DrainOngoing(t *testing.T) { t.Log("if drain is ongoing then a message should be displayed") _ = setupAutoplan(t) drainer.ShutdownBlocking() - containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } @@ -67,7 +67,7 @@ func TestIsValid_ProjectBuilderError(t *testing.T) { vcsClient := setupAutoplan(t) When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). ThenReturn([]command.ProjectContext{}, errors.New("err")) - containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "**Plan Error**\n```\nerr\n```\n", "plan") Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } @@ -85,14 +85,14 @@ func TestIsValid_TerraformChanges(t *testing.T) { }, }, nil) - containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == true, "should have terraform changes") } func TestPullRequestHasTerraformChanges_NoTerraformChanges(t *testing.T) { t.Log("verify returns false if terraform changes don't exist") vcsClient := setupAutoplan(t) - containsTerraformChanges := autoplanBuilder.IsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should have no terraform changes") vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) commitStatusUpdater.VerifyWasCalled(Times(3)).UpdateCombinedCount( diff --git a/server/lyft/gateway/events_controller.go b/server/lyft/gateway/events_controller.go index a57b94e73..014b5c28d 100644 --- a/server/lyft/gateway/events_controller.go +++ b/server/lyft/gateway/events_controller.go @@ -29,9 +29,9 @@ type HttpError struct { code int } -//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_autoplan_validator.go AutoplanValidator -type AutoplanValidator interface { - IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_event_validator.go EventValidator +type EventValidator interface { + InstrumentedIsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool } // VCSEventsController handles all webhook requests which signify 'events' in the @@ -52,7 +52,7 @@ type VCSEventsController struct { SilenceAllowlistErrors bool VCSClient vcs.Client SNSWriter sns.Writer - AutoplanValidator AutoplanValidator + AutoplanValidator EventValidator } // Post handles POST webhook requests. @@ -241,7 +241,7 @@ func (g *VCSEventsController) handlePullRequestEvent(baseRepo models.Repo, headR } func (g *VCSEventsController) handleOpenPullEvent(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User, request *http.Request) { - if hasTerraformChanges := g.AutoplanValidator.IsValid(baseRepo, headRepo, pull, user); hasTerraformChanges { + if hasTerraformChanges := g.AutoplanValidator.InstrumentedIsValid(baseRepo, headRepo, pull, user); hasTerraformChanges { if err := g.SendToWorker(request); err != nil { g.Logger.With("err", err).Err("failed to send autoplan request to Atlantis worker") } diff --git a/server/lyft/gateway/events_controller_test.go b/server/lyft/gateway/events_controller_test.go index 6d269e8a7..21ebb66cb 100644 --- a/server/lyft/gateway/events_controller_test.go +++ b/server/lyft/gateway/events_controller_test.go @@ -15,7 +15,7 @@ import ( sns_mocks "github.com/runatlantis/atlantis/server/lyft/aws/sns/mocks" sns_matchers "github.com/runatlantis/atlantis/server/lyft/aws/sns/mocks/matchers" "github.com/runatlantis/atlantis/server/lyft/gateway" - av_mocks "github.com/runatlantis/atlantis/server/lyft/gateway/mocks" + ev_mocks "github.com/runatlantis/atlantis/server/lyft/gateway/mocks" "github.com/runatlantis/atlantis/server/metrics" . "github.com/runatlantis/atlantis/testing" "io/ioutil" @@ -193,7 +193,7 @@ func TestPost_GithubPullRequestNotAllowlisted(t *testing.T) { ResponseContains(t, w, http.StatusForbidden, "Pull request event from non-allowlisted repo") } -func setup(t *testing.T) (gateway.VCSEventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing, *sns_mocks.MockWriter, *av_mocks.MockAutoplanValidator) { +func setup(t *testing.T) (gateway.VCSEventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing, *sns_mocks.MockWriter, *ev_mocks.MockEventValidator) { RegisterMockTestingT(t) v := mocks.NewMockGithubRequestValidator() gl := mocks.NewMockGitlabRequestParserValidator() @@ -203,7 +203,7 @@ func setup(t *testing.T) (gateway.VCSEventsController, *mocks.MockGithubRequestV c := emocks.NewMockPullCleaner() vcsmock := vcsmocks.NewMockClient() snsMock := sns_mocks.NewMockWriter() - avMock := av_mocks.NewMockAutoplanValidator() + evMock := ev_mocks.NewMockEventValidator() repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") Ok(t, err) logger := logging.NewNoopLogger(t) @@ -218,9 +218,9 @@ func setup(t *testing.T) (gateway.VCSEventsController, *mocks.MockGithubRequestV RepoAllowlistChecker: repoAllowlistChecker, VCSClient: vcsmock, SNSWriter: snsMock, - AutoplanValidator: avMock, + AutoplanValidator: evMock, } - return e, v, gl, p, cr, c, vcsmock, cp, snsMock, avMock + return e, v, gl, p, cr, c, vcsmock, cp, snsMock, evMock } func TestPost_GithubCommentSuccess(t *testing.T) { @@ -297,7 +297,7 @@ func TestPost_GithubClosePR(t *testing.T) { func TestPost_GithubOpenPR_WithTerraformChanges(t *testing.T) { t.Log("when the event is a PR opened, we send request to SNS if there are tf changes") - e, v, _, p, _, _, _, _, sns, av := setup(t) + e, v, _, p, _, _, _, _, sns, ev := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gateway.GithubHeader, "pull_request") event := `{"action": "open"}` @@ -305,18 +305,18 @@ func TestPost_GithubOpenPR_WithTerraformChanges(t *testing.T) { repo := models.Repo{} pull := models.PullRequest{State: models.OpenPullState} When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) - When(av.IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(true) + When(ev.InstrumentedIsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(true) When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) w := httptest.NewRecorder() e.Post(w, req) - av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + ev.VerifyWasCalledEventually(Once(), 500*time.Millisecond).InstrumentedIsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) sns.VerifyWasCalledEventually(Once(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) ResponseContains(t, w, http.StatusOK, "") } func TestPost_GithubOpenPR_NoTerraformChanges(t *testing.T) { t.Log("when the event is a PR opened, we don't send a request to SNS if there are no tf changes") - e, v, _, p, _, _, _, _, sns, av := setup(t) + e, v, _, p, _, _, _, _, sns, ev := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gateway.GithubHeader, "pull_request") event := `{"action": "open"}` @@ -324,11 +324,11 @@ func TestPost_GithubOpenPR_NoTerraformChanges(t *testing.T) { repo := models.Repo{} pull := models.PullRequest{State: models.OpenPullState} When(p.ParseGithubPullEvent(matchers.AnyPtrToGithubPullRequestEvent())).ThenReturn(pull, models.OpenedPullEvent, repo, repo, models.User{}, nil) - When(av.IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(false) + When(ev.InstrumentedIsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser())).ThenReturn(false) When(sns.Write(sns_matchers.AnySliceOfByte())).ThenReturn(nil) w := httptest.NewRecorder() e.Post(w, req) - av.VerifyWasCalledEventually(Once(), 500*time.Millisecond).IsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) + ev.VerifyWasCalledEventually(Once(), 500*time.Millisecond).InstrumentedIsValid(matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsUser()) sns.VerifyWasCalledEventually(Never(), 500*time.Millisecond).Write(sns_matchers.AnySliceOfByte()) ResponseContains(t, w, http.StatusOK, "") } diff --git a/server/lyft/gateway/mocks/mock_autoplan_validator.go b/server/lyft/gateway/mocks/mock_autoplan_validator.go deleted file mode 100644 index b31a12d7d..000000000 --- a/server/lyft/gateway/mocks/mock_autoplan_validator.go +++ /dev/null @@ -1,117 +0,0 @@ -// Code generated by pegomock. DO NOT EDIT. -// Source: github.com/runatlantis/atlantis/server/lyft/gateway (interfaces: AutoplanValidator) - -package mocks - -import ( - pegomock "github.com/petergtz/pegomock" - models "github.com/runatlantis/atlantis/server/events/models" - "reflect" - "time" -) - -type MockAutoplanValidator struct { - fail func(message string, callerSkip ...int) -} - -func NewMockAutoplanValidator(options ...pegomock.Option) *MockAutoplanValidator { - mock := &MockAutoplanValidator{} - for _, option := range options { - option.Apply(mock) - } - return mock -} - -func (mock *MockAutoplanValidator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } -func (mock *MockAutoplanValidator) FailHandler() pegomock.FailHandler { return mock.fail } - -func (mock *MockAutoplanValidator) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockAutoplanValidator().") - } - params := []pegomock.Param{baseRepo, headRepo, pull, user} - result := pegomock.GetGenericMockFrom(mock).Invoke("IsValid", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) - var ret0 bool - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(bool) - } - } - return ret0 -} - -func (mock *MockAutoplanValidator) VerifyWasCalledOnce() *VerifierMockAutoplanValidator { - return &VerifierMockAutoplanValidator{ - mock: mock, - invocationCountMatcher: pegomock.Times(1), - } -} - -func (mock *MockAutoplanValidator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockAutoplanValidator { - return &VerifierMockAutoplanValidator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - } -} - -func (mock *MockAutoplanValidator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockAutoplanValidator { - return &VerifierMockAutoplanValidator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - inOrderContext: inOrderContext, - } -} - -func (mock *MockAutoplanValidator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockAutoplanValidator { - return &VerifierMockAutoplanValidator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - timeout: timeout, - } -} - -type VerifierMockAutoplanValidator struct { - mock *MockAutoplanValidator - invocationCountMatcher pegomock.InvocationCountMatcher - inOrderContext *pegomock.InOrderContext - timeout time.Duration -} - -func (verifier *VerifierMockAutoplanValidator) IsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockAutoplanValidator_IsValid_OngoingVerification { - params := []pegomock.Param{baseRepo, headRepo, pull, user} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "IsValid", params, verifier.timeout) - return &MockAutoplanValidator_IsValid_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockAutoplanValidator_IsValid_OngoingVerification struct { - mock *MockAutoplanValidator - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockAutoplanValidator_IsValid_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { - baseRepo, headRepo, pull, user := c.GetAllCapturedArguments() - return baseRepo[len(baseRepo)-1], headRepo[len(headRepo)-1], pull[len(pull)-1], user[len(user)-1] -} - -func (c *MockAutoplanValidator_IsValid_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(models.Repo) - } - _param1 = make([]models.Repo, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(models.Repo) - } - _param2 = make([]models.PullRequest, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(models.PullRequest) - } - _param3 = make([]models.User, len(c.methodInvocations)) - for u, param := range params[3] { - _param3[u] = param.(models.User) - } - } - return -} diff --git a/server/lyft/gateway/mocks/mock_event_validator.go b/server/lyft/gateway/mocks/mock_event_validator.go new file mode 100644 index 000000000..23d275fef --- /dev/null +++ b/server/lyft/gateway/mocks/mock_event_validator.go @@ -0,0 +1,117 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/lyft/gateway (interfaces: EventValidator) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockEventValidator struct { + fail func(message string, callerSkip ...int) +} + +func NewMockEventValidator(options ...pegomock.Option) *MockEventValidator { + mock := &MockEventValidator{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockEventValidator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockEventValidator) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockEventValidator) InstrumentedIsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) bool { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockEventValidator().") + } + params := []pegomock.Param{baseRepo, headRepo, pull, user} + result := pegomock.GetGenericMockFrom(mock).Invoke("InstrumentedIsValid", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + var ret0 bool + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(bool) + } + } + return ret0 +} + +func (mock *MockEventValidator) VerifyWasCalledOnce() *VerifierMockEventValidator { + return &VerifierMockEventValidator{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockEventValidator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockEventValidator { + return &VerifierMockEventValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockEventValidator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockEventValidator { + return &VerifierMockEventValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockEventValidator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockEventValidator { + return &VerifierMockEventValidator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockEventValidator struct { + mock *MockEventValidator + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockEventValidator) InstrumentedIsValid(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockEventValidator_InstrumentedIsValid_OngoingVerification { + params := []pegomock.Param{baseRepo, headRepo, pull, user} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "InstrumentedIsValid", params, verifier.timeout) + return &MockEventValidator_InstrumentedIsValid_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockEventValidator_InstrumentedIsValid_OngoingVerification struct { + mock *MockEventValidator + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockEventValidator_InstrumentedIsValid_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { + baseRepo, headRepo, pull, user := c.GetAllCapturedArguments() + return baseRepo[len(baseRepo)-1], headRepo[len(headRepo)-1], pull[len(pull)-1], user[len(user)-1] +} + +func (c *MockEventValidator_InstrumentedIsValid_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.Repo, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.Repo) + } + _param2 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.PullRequest) + } + _param3 = make([]models.User, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(models.User) + } + } + return +} From 893f0b2c7839acf5a121a0aa9e0bbc4f312ed300 Mon Sep 17 00:00:00 2001 From: Samra Belachew Date: Fri, 4 Mar 2022 14:11:18 -0800 Subject: [PATCH 4/4] nit --- server/lyft/gateway/autoplan_builder.go | 2 +- server/lyft/gateway/autoplan_builder_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/lyft/gateway/autoplan_builder.go b/server/lyft/gateway/autoplan_builder.go index 6b578de80..5a23f3950 100644 --- a/server/lyft/gateway/autoplan_builder.go +++ b/server/lyft/gateway/autoplan_builder.go @@ -79,7 +79,7 @@ func (r *AutoplanValidator) isValid(baseRepo models.Repo, headRepo models.Repo, ctx.Log.Warn("unable to update commit status: %s", statusErr) } r.PullUpdater.UpdatePull(ctx, events.AutoplanCommand{}, command.Result{Error: err}) - return false, errors.Wrap(err, "Failed building project command") + return false, errors.Wrap(err, "Failed building autoplan commands") } if len(projectCmds) == 0 { ctx.Log.Info("determined there was no project to run plan in") diff --git a/server/lyft/gateway/autoplan_builder_test.go b/server/lyft/gateway/autoplan_builder_test.go index d9855b733..c3522c9f6 100644 --- a/server/lyft/gateway/autoplan_builder_test.go +++ b/server/lyft/gateway/autoplan_builder_test.go @@ -17,7 +17,7 @@ import ( "testing" ) -var autoplanBuilder gateway.AutoplanValidator +var autoplanValidator gateway.AutoplanValidator var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner var projectCommandBuilder *mocks.MockProjectCommandBuilder var drainer *events.Drainer @@ -39,7 +39,7 @@ func setupAutoplan(t *testing.T) *vcsmocks.MockClient { globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{}) logger := logging.NewNoopLogger(t) scope, _, _ := metrics.NewLoggingScope(logger, "atlantis") - autoplanBuilder = gateway.AutoplanValidator{ + autoplanValidator = gateway.AutoplanValidator{ Logger: logger, Scope: scope, VCSClient: vcsClient, @@ -58,7 +58,7 @@ func TestIsValid_DrainOngoing(t *testing.T) { t.Log("if drain is ongoing then a message should be displayed") _ = setupAutoplan(t) drainer.ShutdownBlocking() - containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanValidator.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } @@ -67,7 +67,7 @@ func TestIsValid_ProjectBuilderError(t *testing.T) { vcsClient := setupAutoplan(t) When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())). ThenReturn([]command.ProjectContext{}, errors.New("err")) - containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanValidator.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "**Plan Error**\n```\nerr\n```\n", "plan") Assert(t, containsTerraformChanges == false, "should be false when an error occurs") } @@ -85,14 +85,14 @@ func TestIsValid_TerraformChanges(t *testing.T) { }, }, nil) - containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanValidator.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == true, "should have terraform changes") } func TestPullRequestHasTerraformChanges_NoTerraformChanges(t *testing.T) { t.Log("verify returns false if terraform changes don't exist") vcsClient := setupAutoplan(t) - containsTerraformChanges := autoplanBuilder.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) + containsTerraformChanges := autoplanValidator.InstrumentedIsValid(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) Assert(t, containsTerraformChanges == false, "should have no terraform changes") vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()) commitStatusUpdater.VerifyWasCalled(Times(3)).UpdateCombinedCount(