Skip to content

Commit

Permalink
WENGINES-4515 (#253)
Browse files Browse the repository at this point in the history
  • Loading branch information
Aayyush authored May 18, 2022
1 parent 30c09d9 commit 2953210
Show file tree
Hide file tree
Showing 16 changed files with 266 additions and 62 deletions.
2 changes: 1 addition & 1 deletion server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
DB: boltdb,
}

pullUpdater := &events.PullUpdater{
pullUpdater := &events.PullOutputUpdater{
HidePrevPlanComments: false,
VCSClient: vcsClient,
MarkdownRenderer: &events.MarkdownRenderer{},
Expand Down
10 changes: 5 additions & 5 deletions server/events/apply_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func NewApplyCommandRunner(
commitStatusUpdater CommitStatusUpdater,
prjCommandBuilder ProjectApplyCommandBuilder,
prjCmdRunner ProjectApplyCommandRunner,
pullUpdater *PullUpdater,
outputUpdater OutputUpdater,
dbUpdater *DBUpdater,
parallelPoolSize int,
pullReqStatusFetcher vcs.PullReqStatusFetcher,
Expand All @@ -29,7 +29,7 @@ func NewApplyCommandRunner(
commitStatusUpdater: commitStatusUpdater,
prjCmdBuilder: prjCommandBuilder,
prjCmdRunner: prjCmdRunner,
pullUpdater: pullUpdater,
outputUpdater: outputUpdater,
dbUpdater: dbUpdater,
parallelPoolSize: parallelPoolSize,
pullReqStatusFetcher: pullReqStatusFetcher,
Expand All @@ -43,7 +43,7 @@ type ApplyCommandRunner struct {
commitStatusUpdater CommitStatusUpdater
prjCmdBuilder ProjectApplyCommandBuilder
prjCmdRunner ProjectApplyCommandRunner
pullUpdater *PullUpdater
outputUpdater OutputUpdater
dbUpdater *DBUpdater
parallelPoolSize int
pullReqStatusFetcher vcs.PullReqStatusFetcher
Expand Down Expand Up @@ -105,7 +105,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *command.Comment) {
if statusErr := a.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil {
ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", statusErr))
}
a.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err})
a.outputUpdater.UpdateOutput(ctx, cmd, command.Result{Error: err})
return
}

Expand All @@ -118,7 +118,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *command.Comment) {
result = runProjectCmds(projectCmds, a.prjCmdRunner.Apply)
}

a.pullUpdater.UpdatePull(
a.outputUpdater.UpdateOutput(
ctx,
cmd,
result)
Expand Down
10 changes: 5 additions & 5 deletions server/events/approve_policies_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ func NewApprovePoliciesCommandRunner(
commitStatusUpdater CommitStatusUpdater,
prjCommandBuilder ProjectApprovePoliciesCommandBuilder,
prjCommandRunner ProjectApprovePoliciesCommandRunner,
pullUpdater *PullUpdater,
outputUpdater OutputUpdater,
dbUpdater *DBUpdater,
) *ApprovePoliciesCommandRunner {
return &ApprovePoliciesCommandRunner{
commitStatusUpdater: commitStatusUpdater,
prjCmdBuilder: prjCommandBuilder,
prjCmdRunner: prjCommandRunner,
pullUpdater: pullUpdater,
outputUpdater: outputUpdater,
dbUpdater: dbUpdater,
}
}

type ApprovePoliciesCommandRunner struct {
commitStatusUpdater CommitStatusUpdater
pullUpdater *PullUpdater
outputUpdater OutputUpdater
dbUpdater *DBUpdater
prjCmdBuilder ProjectApprovePoliciesCommandBuilder
prjCmdRunner ProjectApprovePoliciesCommandRunner
Expand All @@ -45,7 +45,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *command.Co
if statusErr := a.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.PolicyCheck); statusErr != nil {
ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", statusErr))
}
a.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err})
a.outputUpdater.UpdateOutput(ctx, cmd, command.Result{Error: err})
return
}

Expand All @@ -62,7 +62,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *command.Co

result := a.buildApprovePolicyCommandResults(ctx, projectCmds)

a.pullUpdater.UpdatePull(
a.outputUpdater.UpdateOutput(
ctx,
cmd,
result,
Expand Down
8 changes: 4 additions & 4 deletions server/events/command/apply/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ import (
"github.com/runatlantis/atlantis/server/events/command"
)

func NewDisabledRunner(pullUpdater *events.PullUpdater) *DisabledRunner {
func NewDisabledRunner(outputUpdater events.OutputUpdater) *DisabledRunner {
return &DisabledRunner{
pullUpdater: pullUpdater,
pullUpdater: outputUpdater,
}
}

type DisabledRunner struct {
pullUpdater *events.PullUpdater
pullUpdater events.OutputUpdater
}

func (r *DisabledRunner) Run(ctx *command.Context, cmd *command.Comment) {
r.pullUpdater.UpdatePull(
r.pullUpdater.UpdateOutput(
ctx,
cmd,
command.Result{
Expand Down
4 changes: 2 additions & 2 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var staleCommandChecker *mocks.MockStaleCommandChecker
// these were all split out from default command runner in an effort to improve
// readability however the tests were kept as is.
var dbUpdater *events.DBUpdater
var pullUpdater *events.PullUpdater
var pullUpdater events.OutputUpdater
var policyCheckCommandRunner *events.PolicyCheckCommandRunner
var approvePoliciesCommandRunner *events.ApprovePoliciesCommandRunner
var planCommandRunner *events.PlanCommandRunner
Expand Down Expand Up @@ -89,7 +89,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
DB: defaultBoltDB,
}

pullUpdater = &events.PullUpdater{
pullUpdater = &events.PullOutputUpdater{
HidePrevPlanComments: false,
VCSClient: vcsClient,
MarkdownRenderer: &events.MarkdownRenderer{},
Expand Down
127 changes: 127 additions & 0 deletions server/events/output_updater.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package events

import (
"fmt"

"github.com/runatlantis/atlantis/server/core/config/valid"
"github.com/runatlantis/atlantis/server/events/command"
"github.com/runatlantis/atlantis/server/events/models"
"github.com/runatlantis/atlantis/server/events/vcs"
"github.com/runatlantis/atlantis/server/events/vcs/types"
"github.com/runatlantis/atlantis/server/logging"
"github.com/runatlantis/atlantis/server/lyft/feature"
)

type OutputUpdater interface {
UpdateOutput(ctx *command.Context, cmd PullCommand, res command.Result)
}

// [WENGINES-4643] TODO: Remove PullOutputUpdater and default to checks once github checks is stable
// defaults to pull comments if checks is turned off
type FeatureAwareChecksOutputUpdater struct {
PullOutputUpdater
ChecksOutputUpdater

FeatureAllocator feature.Allocator
Logger logging.Logger
}

func (c *FeatureAwareChecksOutputUpdater) UpdateOutput(ctx *command.Context, cmd PullCommand, res command.Result) {
shouldAllocate, err := c.FeatureAllocator.ShouldAllocate(feature.GithubChecks, ctx.HeadRepo.FullName)
if err != nil {
c.Logger.Error(fmt.Sprintf("unable to allocate for feature: %s, error: %s", feature.GithubChecks, err))
}

// Github Checks turned on and github provider
if ctx.HeadRepo.VCSHost.Type == models.Github && shouldAllocate {
c.ChecksOutputUpdater.UpdateOutput(ctx, cmd, res)
return
}
c.PullOutputUpdater.UpdateOutput(ctx, cmd, res)
}

// Used to support checks type output (Github checks for example)
type ChecksOutputUpdater struct {
VCSClient vcs.Client
MarkdownRenderer *MarkdownRenderer
TitleBuilder vcs.StatusTitleBuilder
GlobalCfg valid.GlobalCfg
}

func (c *ChecksOutputUpdater) UpdateOutput(ctx *command.Context, cmd PullCommand, res command.Result) {
var templateOverrides map[string]string

// retrieve template override if configured
repoCfg := c.GlobalCfg.MatchingRepo(ctx.Pull.BaseRepo.ID())
if repoCfg != nil {
templateOverrides = repoCfg.TemplateOverrides
}

// iterate through all project results and the update the github check
for _, projectResult := range res.ProjectResults {
statusName := c.TitleBuilder.Build(cmd.CommandName().String(), vcs.StatusTitleOptions{
ProjectName: projectResult.ProjectName,
})

output := c.MarkdownRenderer.Render(res, cmd.CommandName(), ctx.Pull.BaseRepo.VCSHost.Type, templateOverrides)
updateStatusReq := types.UpdateStatusRequest{
Repo: ctx.HeadRepo,
Ref: ctx.Pull.HeadCommit,
StatusName: statusName,
PullNum: ctx.Pull.Num,
Description: output,
}

if err := c.VCSClient.UpdateStatus(ctx.RequestCtx, updateStatusReq); err != nil {
ctx.Log.Error("updable to update check run", map[string]interface{}{
"error": err.Error(),
})
}
}

}

// Default prj output updater which writes to the pull req comment
type PullOutputUpdater struct {
HidePrevPlanComments bool
VCSClient vcs.Client
MarkdownRenderer *MarkdownRenderer
GlobalCfg valid.GlobalCfg
}

func (c *PullOutputUpdater) UpdateOutput(ctx *command.Context, cmd PullCommand, res command.Result) {
// Log if we got any errors or failures.
if res.Error != nil {
ctx.Log.Error("", map[string]interface{}{
"error": res.Error.Error(),
})
} else if res.Failure != "" {
ctx.Log.Warn("", map[string]interface{}{
"failiure": res.Failure,
})
}

// HidePrevCommandComments will hide old comments left from previous runs to reduce
// clutter in a pull/merge request. This will not delete the comment, since the
// comment trail may be useful in auditing or backtracing problems.
if c.HidePrevPlanComments {
if err := c.VCSClient.HidePrevCommandComments(ctx.Pull.BaseRepo, ctx.Pull.Num, cmd.CommandName().TitleString()); err != nil {
ctx.Log.Error("unable to hide old comments", map[string]interface{}{
"error": err.Error(),
})
}
}

var templateOverrides map[string]string
repoCfg := c.GlobalCfg.MatchingRepo(ctx.Pull.BaseRepo.ID())
if repoCfg != nil {
templateOverrides = repoCfg.TemplateOverrides
}

comment := c.MarkdownRenderer.Render(res, cmd.CommandName(), ctx.Pull.BaseRepo.VCSHost.Type, templateOverrides)
if err := c.VCSClient.CreateComment(ctx.Pull.BaseRepo, ctx.Pull.Num, comment, cmd.CommandName().String()); err != nil {
ctx.Log.Error("unable to comment", map[string]interface{}{
"error": err.Error(),
})
}
}
14 changes: 7 additions & 7 deletions server/events/plan_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func NewPlanCommandRunner(
projectCommandBuilder ProjectPlanCommandBuilder,
projectCommandRunner ProjectPlanCommandRunner,
dbUpdater *DBUpdater,
pullUpdater *PullUpdater,
outputUpdater OutputUpdater,
policyCheckCommandRunner *PolicyCheckCommandRunner,
parallelPoolSize int,
) *PlanCommandRunner {
Expand All @@ -29,7 +29,7 @@ func NewPlanCommandRunner(
prjCmdBuilder: projectCommandBuilder,
prjCmdRunner: projectCommandRunner,
dbUpdater: dbUpdater,
pullUpdater: pullUpdater,
outputUpdater: outputUpdater,
policyCheckCommandRunner: policyCheckCommandRunner,
parallelPoolSize: parallelPoolSize,
}
Expand All @@ -43,7 +43,7 @@ type PlanCommandRunner struct {
prjCmdBuilder ProjectPlanCommandBuilder
prjCmdRunner ProjectPlanCommandRunner
dbUpdater *DBUpdater
pullUpdater *PullUpdater
outputUpdater OutputUpdater
policyCheckCommandRunner *PolicyCheckCommandRunner
parallelPoolSize int
}
Expand All @@ -57,7 +57,7 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
if statusErr := p.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil {
ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", statusErr))
}
p.pullUpdater.UpdatePull(ctx, AutoplanCommand{}, command.Result{Error: err})
p.outputUpdater.UpdateOutput(ctx, AutoplanCommand{}, command.Result{Error: err})
return
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
result = runProjectCmds(projectCmds, p.prjCmdRunner.Plan)
}

p.pullUpdater.UpdatePull(ctx, AutoplanCommand{}, result)
p.outputUpdater.UpdateOutput(ctx, AutoplanCommand{}, result)

pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults)
if err != nil {
Expand Down Expand Up @@ -132,7 +132,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *command.Comment) {
if statusErr := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil {
ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", statusErr))
}
p.pullUpdater.UpdatePull(ctx, cmd, command.Result{Error: err})
p.outputUpdater.UpdateOutput(ctx, cmd, command.Result{Error: err})
return
}

Expand All @@ -147,7 +147,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *command.Comment) {
result = runProjectCmds(projectCmds, p.prjCmdRunner.Plan)
}

p.pullUpdater.UpdatePull(
p.outputUpdater.UpdateOutput(
ctx,
cmd,
result)
Expand Down
8 changes: 4 additions & 4 deletions server/events/policy_check_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import (

func NewPolicyCheckCommandRunner(
dbUpdater *DBUpdater,
pullUpdater *PullUpdater,
outputUpdater OutputUpdater,
commitStatusUpdater CommitStatusUpdater,
projectCommandRunner ProjectPolicyCheckCommandRunner,
parallelPoolSize int,
) *PolicyCheckCommandRunner {
return &PolicyCheckCommandRunner{
dbUpdater: dbUpdater,
pullUpdater: pullUpdater,
outputUpdater: outputUpdater,
commitStatusUpdater: commitStatusUpdater,
prjCmdRunner: projectCommandRunner,
parallelPoolSize: parallelPoolSize,
Expand All @@ -26,7 +26,7 @@ func NewPolicyCheckCommandRunner(

type PolicyCheckCommandRunner struct {
dbUpdater *DBUpdater
pullUpdater *PullUpdater
outputUpdater OutputUpdater
commitStatusUpdater CommitStatusUpdater
prjCmdRunner ProjectPolicyCheckCommandRunner
parallelPoolSize int
Expand Down Expand Up @@ -57,7 +57,7 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj
result = runProjectCmds(cmds, p.prjCmdRunner.PolicyCheck)
}

p.pullUpdater.UpdatePull(ctx, PolicyCheckCommand{}, result)
p.outputUpdater.UpdateOutput(ctx, PolicyCheckCommand{}, result)

pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults)
if err != nil {
Expand Down
13 changes: 13 additions & 0 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,19 @@ func (g *GithubClient) UpdateStatus(ctx context.Context, request types.UpdateSta
return err
}

// [WENGINES-4643] TODO: Move the checks implementation to UpdateStatus once github checks is stable
// UpdateChecksStatus updates the status check
func (g *GithubClient) UpdateChecksStatus(ctx context.Context, request types.UpdateStatusRequest) error {
// TODO: Implement updating github checks
// - Get all checkruns for this SHA
// - Match the UpdateReqIdentifier with the check run. If it exists, update the checkrun. If it does not, create a new check run.

// Checks uses Status and Conlusion. Need to map models.CommitStatus to Status and Conclusion
// Status -> queued, in_progress, completed
// Conclusion -> failure, neutral, cancelled, timed_out, or action_required. (Optional. Required if you provide a status of "completed".)
return nil
}

// MarkdownPullLink specifies the string used in a pull request comment to reference another pull request.
func (g *GithubClient) MarkdownPullLink(pull models.PullRequest) (string, error) {
return fmt.Sprintf("#%d", pull.Num), nil
Expand Down
5 changes: 3 additions & 2 deletions server/events/vcs/instrumented_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import (
)

// NewInstrumentedGithubClient creates a client proxy responsible for gathering stats and logging
func NewInstrumentedGithubClient(client *GithubClient, statsScope tally.Scope, logger logging.Logger) IGithubClient {
func NewInstrumentedGithubClient(client *GithubClient, checksEnabledGHClient Client, statsScope tally.Scope, logger logging.Logger) IGithubClient {
scope := statsScope.SubScope("github")

// [WENGINES-4643] TODO: Remove checksEnabledGHClient with client once gitub checks is stable.
instrumentedGHClient := &InstrumentedClient{
Client: client,
Client: checksEnabledGHClient,
StatsScope: scope,
Logger: logger,
}
Expand Down
Loading

0 comments on commit 2953210

Please sign in to comment.