Skip to content

Commit

Permalink
Support pre-workflow hooks on all comment/auto triggered commands. (#43
Browse files Browse the repository at this point in the history
…) (#1418)

Fixes pre-workflow-hooks not logging errors #1371
Prevents pre-workflow-hook from locking and cloning the dir if there are no hooks registered. #1342
  • Loading branch information
nishkrishnan authored Feb 22, 2021
1 parent 5e4bde4 commit 8598c37
Show file tree
Hide file tree
Showing 16 changed files with 552 additions and 269 deletions.
27 changes: 16 additions & 11 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ type DefaultCommandRunner struct {
// 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
CommentCommandRunnerByCmd map[models.CommandName]CommentCommandRunner
Drainer *Drainer
SilenceForkPRErrorsFlag string
CommentCommandRunnerByCmd map[models.CommandName]CommentCommandRunner
Drainer *Drainer
PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner
}

// RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated.
Expand Down Expand Up @@ -139,12 +140,14 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
return
}

autoPlanRunner := buildCommentCommandRunner(c, models.PlanCommand)
if autoPlanRunner == nil {
ctx.Log.Err("invalid autoplan command")
return
err := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx)

if err != nil {
ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, models.PlanCommand)
}

autoPlanRunner := buildCommentCommandRunner(c, models.PlanCommand)

autoPlanRunner.Run(ctx, nil)
}

Expand Down Expand Up @@ -182,12 +185,14 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
return
}

cmdRunner := buildCommentCommandRunner(c, cmd.CommandName())
if cmdRunner == nil {
ctx.Log.Err("command %s is not supported", cmd.Name.String())
return
err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx)

if err != nil {
ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, cmd.Name.String())
}

cmdRunner := buildCommentCommandRunner(c, cmd.CommandName())

cmdRunner.Run(ctx, cmd)
}

Expand Down
26 changes: 16 additions & 10 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var approvePoliciesCommandRunner *events.ApprovePoliciesCommandRunner
var planCommandRunner *events.PlanCommandRunner
var applyCommandRunner *events.ApplyCommandRunner
var unlockCommandRunner *events.UnlockCommandRunner
var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner

func setup(t *testing.T) *vcsmocks.MockClient {
RegisterMockTestingT(t)
Expand Down Expand Up @@ -161,17 +162,22 @@ func setup(t *testing.T) *vcsmocks.MockClient {
models.UnlockCommand: unlockCommandRunner,
}

preWorkflowHooksCommandRunner = mocks.NewMockPreWorkflowHooksCommandRunner()

When(preWorkflowHooksCommandRunner.RunPreHooks(matchers.AnyPtrToEventsCommandContext())).ThenReturn(nil)

ch = events.DefaultCommandRunner{
VCSClient: vcsClient,
CommentCommandRunnerByCmd: commentCommandRunnerByCmd,
EventParser: eventParsing,
GithubPullGetter: githubGetter,
GitlabMergeRequestGetter: gitlabGetter,
AzureDevopsPullGetter: azuredevopsGetter,
Logger: logger,
AllowForkPRs: false,
AllowForkPRsFlag: "allow-fork-prs-flag",
Drainer: drainer,
VCSClient: vcsClient,
CommentCommandRunnerByCmd: commentCommandRunnerByCmd,
EventParser: eventParsing,
GithubPullGetter: githubGetter,
GitlabMergeRequestGetter: gitlabGetter,
AzureDevopsPullGetter: azuredevopsGetter,
Logger: logger,
AllowForkPRs: false,
AllowForkPRsFlag: "allow-fork-prs-flag",
Drainer: drainer,
PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner,
}
return vcsClient
}
Expand Down
2 changes: 2 additions & 0 deletions server/events/delete_lock_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNu
return err
}
if len(locks) == 0 {
l.Logger.Debug("No locks found for pull")
return nil
}

Expand All @@ -61,6 +62,7 @@ func (l *DefaultDeleteLockCommand) deleteWorkingDir(lock models.ProjectLock) {
// installations of Atlantis will have locks in their DB that do not have
// this field on PullRequest. We skip deleting the working dir in this case.
if lock.Pull.BaseRepo == (models.Repo{}) {
l.Logger.Debug("Not deleting the working dir.")
return
}
unlock, err := l.WorkingDirLocker.TryLock(lock.Pull.BaseRepo.FullName, lock.Pull.Num, lock.Workspace)
Expand Down
15 changes: 14 additions & 1 deletion server/events/mocks/matchers/ptr_to_events_commandcontext.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 23 additions & 28 deletions server/events/mocks/mock_pre_workflows_hooks_command_runner.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 8598c37

Please sign in to comment.