diff --git a/server/core/db/boltdb.go b/server/core/db/boltdb.go index 25abf6c966..48c1157553 100644 --- a/server/core/db/boltdb.go +++ b/server/core/db/boltdb.go @@ -22,16 +22,13 @@ type BoltDB struct { locksBucketName []byte pullsBucketName []byte globalLocksBucketName []byte - checkRunsBucketName []byte } const ( locksBucketName = "runLocks" pullsBucketName = "pulls" globalLocksBucketName = "globalLocks" - checkrunsBucketName = "checkRuns" pullKeySeparator = "::" - checkRunKeySeparator = "||" ) // New returns a valid locker. We need to be able to write to dataDir @@ -59,9 +56,6 @@ func New(dataDir string) (*BoltDB, error) { if _, err = tx.CreateBucketIfNotExists([]byte(globalLocksBucketName)); err != nil { return errors.Wrapf(err, "creating bucket %q", globalLocksBucketName) } - if _, err = tx.CreateBucketIfNotExists([]byte(checkrunsBucketName)); err != nil { - return errors.Wrapf(err, "creating bucket %q", checkrunsBucketName) - } return nil }) if err != nil { @@ -73,7 +67,6 @@ func New(dataDir string) (*BoltDB, error) { locksBucketName: []byte(locksBucketName), pullsBucketName: []byte(pullsBucketName), globalLocksBucketName: []byte(globalLocksBucketName), - checkRunsBucketName: []byte(checkrunsBucketName), }, nil } @@ -318,30 +311,6 @@ func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLoc return &lock, nil } -// Sets the checkRunID for a command -func (b *BoltDB) UpdateCheckRunForStatus(statusName string, repo models.Repo, ref string, checkRunStatus models.CheckRunStatus) error { - key := b.checkRunKey(statusName, repo, ref) - return b.db.Update(func(tx *bolt.Tx) error { - bucket := tx.Bucket(b.checkRunsBucketName) - return b.writeCheckRunToBucket(bucket, []byte(key), checkRunStatus) - }) -} - -// Returns nil if the checkrun dne in the db -func (b *BoltDB) GetCheckRunForStatus(statusName string, repo models.Repo, ref string) (*models.CheckRunStatus, error) { - key := b.checkRunKey(statusName, repo, ref) - - var checkRun *models.CheckRunStatus - err := b.db.View(func(tx *bolt.Tx) error { - bucket := tx.Bucket(b.checkRunsBucketName) - var txErr error - checkRun, txErr = b.getCheckRunFromBucket(bucket, []byte(key)) - return txErr - }) - - return checkRun, err -} - // UpdatePullWithResults updates pull's status with the latest project results. // It returns the new PullStatus object. func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []command.ProjectResult) (models.PullStatus, error) { @@ -493,10 +462,6 @@ func (b *BoltDB) lockKey(p models.Project, workspace string) string { return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace) } -func (b *BoltDB) checkRunKey(statusName string, repo models.Repo, ref string) string { - return fmt.Sprintf("%s||%s||%s", repo.FullName, ref, statusName) -} - func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) { serialized := bucket.Get(key) if serialized == nil { @@ -510,19 +475,6 @@ func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.Pul return &p, nil } -func (b *BoltDB) getCheckRunFromBucket(bucket *bolt.Bucket, key []byte) (*models.CheckRunStatus, error) { - serialized := bucket.Get(key) - if serialized == nil { - return nil, nil - } - - var p models.CheckRunStatus - if err := json.Unmarshal(serialized, &p); err != nil { - return nil, errors.Wrapf(err, "deserializing checkrun at %q with contents %q", key, serialized) - } - return &p, nil -} - func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error { serialized, err := json.Marshal(pull) if err != nil { @@ -531,15 +483,6 @@ func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models. return bucket.Put(key, serialized) } -func (b *BoltDB) writeCheckRunToBucket(bucket *bolt.Bucket, key []byte, checkRun models.CheckRunStatus) error { - serialized, err := json.Marshal(checkRun) - if err != nil { - return errors.Wrap(err, "serializing") - } - - return bucket.Put(key, serialized) -} - func (b *BoltDB) projectResultToProject(p command.ProjectResult) models.ProjectStatus { return models.ProjectStatus{ Workspace: p.Workspace, diff --git a/server/core/terraform/terraform_client_internal_test.go b/server/core/terraform/terraform_client_internal_test.go index 7cbf39d0ad..09e0e57ea5 100644 --- a/server/core/terraform/terraform_client_internal_test.go +++ b/server/core/terraform/terraform_client_internal_test.go @@ -64,7 +64,7 @@ func TestVersionLoader_buildsURL(t *testing.T) { v, _ := version.NewVersion("0.15.0") destPath := "some/path" - fullURL := fmt.Sprintf("https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_%s_%s.zip?checksum=file:https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_SHA256SUMS", runtime.GOOS, runtime.GOARCH) + fullURL := fmt.Sprintf("https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_%s_amd64.zip?checksum=file:https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_SHA256SUMS", runtime.GOOS) RegisterMockTestingT(t) diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 45237381f7..5e2419033b 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -80,7 +80,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *command.Comment) { return } - // Pending status creates a new checkrun if err = a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil { ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) } diff --git a/server/events/approve_policies_command_runner.go b/server/events/approve_policies_command_runner.go index 60c7a2c1d9..163d2d6da0 100644 --- a/server/events/approve_policies_command_runner.go +++ b/server/events/approve_policies_command_runner.go @@ -36,8 +36,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *command.Co baseRepo := ctx.Pull.BaseRepo pull := ctx.Pull - // Set ApprovePolicies to Pending - if err := a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.ApprovePolicies); err != nil { + if err := a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck); err != nil { ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) } diff --git a/server/events/command_runner.go b/server/events/command_runner.go index b4cac3b26b..0d821086d3 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -140,9 +140,6 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(ctx context.Context, baseRepo if err := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmdCtx); err != nil { c.Logger.ErrorContext(ctx, "Error running pre-workflow hooks", fields.PullRequestWithErr(pull, err)) - // Set to pending first to create a checkrun and populate the db - c.CommitStatusUpdater.UpdateCombined(ctx, cmdCtx.HeadRepo, cmdCtx.Pull, models.PendingCommitStatus, command.Plan) - c.CommitStatusUpdater.UpdateCombined(ctx, cmdCtx.HeadRepo, cmdCtx.Pull, models.FailedCommitStatus, command.Plan) return } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index db0b343037..ff420bd2a7 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -381,7 +381,7 @@ func TestRunAutoplanCommand_PreWorkflowHookError(t *testing.T) { ch.PreWorkflowHooksCommandRunner = preWorkflowHooksCommandRunner ch.RunAutoplanCommand(ctx, fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User, time.Now()) - _, _, _, status, cmdName := commitUpdater.VerifyWasCalled(&EqMatcher{Value: 2}).UpdateCombined(matchers.AnyContextContext(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsCommitStatus(), matchers.AnyCommandName()).GetCapturedArguments() + _, _, _, status, cmdName := commitUpdater.VerifyWasCalledOnce().UpdateCombined(matchers.AnyContextContext(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsCommitStatus(), matchers.AnyCommandName()).GetCapturedArguments() Equals(t, models.FailedCommitStatus, status) Equals(t, command.Plan, cmdName) } diff --git a/server/events/models/models.go b/server/events/models/models.go index 0c21db0989..cd20c5a8a1 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -394,17 +394,6 @@ type VersionSuccess struct { VersionOutput string } -// CheckRunStatus is the current status of a checkrun that is in progress -// It keeps track of the jobURL and checkRunOutput -type CheckRunStatus struct { - ID string - JobsURL string - - // Only need to persist for PolicyCheck commands since github does not persist the state of checkrun - // output - Output string -} - // PullStatus is the current status of a pull request that is in progress. type PullStatus struct { // Projects are the projects that have been modified in this pull request. diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index d51cb9ed5e..0b32496cca 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -52,11 +52,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { baseRepo := ctx.Pull.BaseRepo pull := ctx.Pull - // Pending status creates a new checkrun and populates the db - if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) - } - projectCmds, err := p.prjCmdBuilder.BuildAutoplanCommands(ctx) if err != nil { if statusErr := p.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil { @@ -76,25 +71,20 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.Plan, 0, 0); err != nil { ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) } - - // Pending status creates a new checkrun first - if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck, 0, 0); err != nil { - ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) - } if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.PolicyCheck, 0, 0); err != nil { ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) } - - // Pending status creates a new checkrun first - if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.Apply, 0, 0); err != nil { - ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) - } if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.Apply, 0, 0); err != nil { ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) } return } + // At this point we are sure Atlantis has work to do, so set commit status to pending + if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { + ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) + } + // Only run commands in parallel if enabled var result command.Result if p.isParallelEnabled(projectCmds) { diff --git a/server/events/policy_check_command_runner.go b/server/events/policy_check_command_runner.go index 7b2d4fac33..34fc906c70 100644 --- a/server/events/policy_check_command_runner.go +++ b/server/events/policy_check_command_runner.go @@ -33,12 +33,6 @@ type PolicyCheckCommandRunner struct { } func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.ProjectContext) { - - // Set policy_check commit status to pending - if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.PolicyCheck); err != nil { - ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) - } - if len(cmds) == 0 { ctx.Log.InfoContext(ctx.RequestCtx, "no projects to run policy_check in") // If there were no projects modified, we set successful commit statuses @@ -50,6 +44,11 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj return } + // So set policy_check commit status to pending + if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.PolicyCheck); err != nil { + ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) + } + var result command.Result if p.isParallelEnabled(cmds) { ctx.Log.InfoContext(ctx.RequestCtx, "Running policy_checks in parallel") @@ -58,9 +57,6 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj result = runProjectCmds(cmds, p.prjCmdRunner.PolicyCheck) } - // Set project level statuses to pending to simplify handling status updates in checks/github_client - p.setProjectLevelStatusesToPending(*ctx, result) - p.outputUpdater.UpdateOutput(ctx, PolicyCheckCommand{}, result) pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults) @@ -91,22 +87,3 @@ func (p *PolicyCheckCommandRunner) updateCommitStatus(ctx *command.Context, pull func (p *PolicyCheckCommandRunner) isParallelEnabled(cmds []command.ProjectContext) bool { return len(cmds) > 0 && cmds[0].ParallelPolicyCheckEnabled } - -func (p *PolicyCheckCommandRunner) setProjectLevelStatusesToPending(ctx command.Context, result command.Result) { - - for _, prjResult := range result.ProjectResults { - - // Rebuild the project ctx after result - prjCtx := command.ProjectContext{ - ProjectName: prjResult.ProjectName, - RepoRelDir: prjResult.RepoRelDir, - Workspace: prjResult.Workspace, - BaseRepo: ctx.Pull.BaseRepo, - Pull: ctx.Pull, - } - - if err := p.commitStatusUpdater.UpdateProject(ctx.RequestCtx, prjCtx, prjResult.Command, models.PendingCommitStatus, ""); err != nil { - ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err)) - } - } -} diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 1c467865ba..8be360947e 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -484,15 +484,143 @@ func (g *GithubClient) UpdateStatus(ctx context.Context, request types.UpdateSta } // [WENGINES-4643] TODO: Move the checks implementation to UpdateStatus once github checks is stable -func (g *GithubClient) UpdateCheckStatus(ctx context.Context, repo models.Repo, checkRunId int64, updateCheckRunOpts github.UpdateCheckRunOptions) (github.CheckRun, error) { - checkRun, _, err := g.client.Checks.UpdateCheckRun(ctx, repo.Owner, repo.Name, checkRunId, updateCheckRunOpts) - return *checkRun, err +func (g *GithubClient) UpdateChecksStatus(ctx context.Context, request types.UpdateStatusRequest) error { + checkRuns, err := g.GetRepoChecks(request.Repo, request.Ref) + if err != nil { + return err + } + + // Update checkrun if it exists and if it's not a rerun + // request.state is pending only when an operation starts. So, if the checkrun exists and the state is pending, it is a rerun. + if checkRun := g.findCheckRun(request.StatusName, checkRuns); checkRun != nil && request.State != models.PendingCommitStatus { + return g.updateChecksStatus(ctx, request, checkRun) + } + + return g.createChecksStatus(ctx, request) } -// [WENGINES-4643] TODO: Move the checks implementation to UpdateStatus once github checks is stable -func (g *GithubClient) CreateCheckStatus(ctx context.Context, repo models.Repo, createCheckRunOpts github.CreateCheckRunOptions) (github.CheckRun, error) { - checkRun, _, err := g.client.Checks.CreateCheckRun(ctx, repo.Owner, repo.Name, createCheckRunOpts) - return *checkRun, err +// Update existing checkrun +func (g *GithubClient) updateChecksStatus(ctx context.Context, request types.UpdateStatusRequest, checkRun *github.CheckRun) error { + + var fallBackURL string + if checkRun.DetailsURL != nil { + fallBackURL = *checkRun.DetailsURL + } + + ouptut := g.capCheckRunOutput(request.Output) + status, conclusion := g.resolveChecksStatus(request.State) + summary := g.summaryWithJobURL(request, fallBackURL) + + checkRunOutput := github.CheckRunOutput{ + Title: &request.StatusName, + Text: &ouptut, + Summary: &summary, + } + + updateCheckRunOpts := github.UpdateCheckRunOptions{ + Name: request.StatusName, + Status: &status, + Output: &checkRunOutput, + } + + // URL in update request takes precedence. + // fall back to checkRun details URL + if request.DetailsURL != "" { + updateCheckRunOpts.DetailsURL = &request.DetailsURL + } else if checkRun.DetailsURL != nil { + updateCheckRunOpts.DetailsURL = checkRun.DetailsURL + } + + // Conclusion is required if status is Completed + if status == Completed.String() { + updateCheckRunOpts.Conclusion = &conclusion + } + _, _, err := g.client.Checks.UpdateCheckRun(ctx, request.Repo.Owner, request.Repo.Name, *checkRun.ID, updateCheckRunOpts) + return err +} + +// create a new checkrun +func (g *GithubClient) createChecksStatus(ctx context.Context, request types.UpdateStatusRequest) error { + ouptut := g.capCheckRunOutput(request.Output) + status, conclusion := g.resolveChecksStatus(request.State) + summary := g.summaryWithJobURL(request, "") + + checkRunOutput := github.CheckRunOutput{ + Title: &request.StatusName, + Text: &ouptut, + Summary: &summary, + } + + createCheckRunOpts := github.CreateCheckRunOptions{ + Name: request.StatusName, + HeadSHA: request.Ref, + Status: &status, + Output: &checkRunOutput, + } + + // Conclusion is required if status is Completed + if status == Completed.String() { + createCheckRunOpts.Conclusion = &conclusion + } + + _, _, err := g.client.Checks.CreateCheckRun(ctx, request.Repo.Owner, request.Repo.Name, createCheckRunOpts) + return err +} + +// Cap the output string if it exceeds the max checks output length +func (g *GithubClient) capCheckRunOutput(output string) string { + if len(output) > maxChecksOutputLength { + return output[:maxChecksOutputLength] + } + return output +} + +// Append job URL to summary if it's a project plan or apply operation bc we currently only stream logs for these two operations +func (g *GithubClient) summaryWithJobURL(request types.UpdateStatusRequest, fallBackURL string) string { + if strings.Contains(request.StatusName, ":") && + (strings.Contains(request.StatusName, "plan") || strings.Contains(request.StatusName, "apply")) { + + // URL in update request takes precedence + // fallbackURL i.e checkrun URL could be stale from previous operation + if request.DetailsURL != "" { + return fmt.Sprintf("%s\n[Logs](%s)", request.Description, request.DetailsURL) + } else if fallBackURL != "" { + return fmt.Sprintf("%s\n[Logs](%s)", request.Description, fallBackURL) + } + } + return request.Description +} + +// Github Checks uses Status and Conclusion to report status of the check run. 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".) +func (g *GithubClient) resolveChecksStatus(state models.CommitStatus) (string, string) { + status := Queued + conclusion := Neutral + + switch state { + case models.SuccessCommitStatus: + status = Completed + conclusion = Success + + case models.PendingCommitStatus: + status = InProgress + + case models.FailedCommitStatus: + status = Completed + conclusion = Failure + } + + return status.String(), conclusion.String() +} + +func (g *GithubClient) findCheckRun(statusName string, checkRuns []*github.CheckRun) *github.CheckRun { + for _, checkRun := range checkRuns { + if *checkRun.Name == statusName { + return checkRun + } + } + return nil } // MarkdownPullLink specifies the string used in a pull request comment to reference another pull request. diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 0a56156925..7616515740 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -14,8 +14,11 @@ import ( "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/testing" + "github.com/stretchr/testify/assert" + "golang.org/x/net/context" "github.com/shurcooL/githubv4" ) @@ -330,6 +333,455 @@ func TestGithubClient_HideOldComments(t *testing.T) { Equals(t, githubv4.ReportedContentClassifiersOutdated, gotMinimizeCalls[0].Variables.Input.Classifier) } +func TestGithubClient_UpdateChecksStatus(t *testing.T) { + + listCheckRunRespFormat := ` + { + "total_count": 1, + "check_runs": [ + { + "id": 1, + "head_sha": "ce587453ced02b1526dfb4cb910479d431683101", + "status": "completed", + "conclusion": "neutral", + "started_at": "2018-05-04T01:14:52Z", + "completed_at": "2018-05-04T01:14:52Z", + "name": "%s", + "check_suite": { + "id": 5 + } + } + ] + } + ` + + cases := []struct { + name string + newCheckRunName string + existingCheckRunName string + listCheckRunResp string + }{ + { + name: "create new check run when check run dne", + newCheckRunName: "atlantis/apply", + listCheckRunResp: fmt.Sprintf(listCheckRunRespFormat, "atlantis/plan"), + }, + { + name: "update check run when check run exists", + existingCheckRunName: "atlantis/apply", + listCheckRunResp: fmt.Sprintf(listCheckRunRespFormat, "atlantis/apply"), + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/sha/check-runs?per_page=100": + _, err := w.Write([]byte(c.listCheckRunResp)) + Ok(t, err) + + case "/api/v3/repos/owner/repo/check-runs": + // parse req and assert CreateNewCheckRun was called for new check run + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + m := make(map[string]interface{}) + err = json.Unmarshal(body, &m) + Ok(t, err) + assert.Equal(t, c.newCheckRunName, m["name"]) + case "/api/v3/repos/owner/repo/check-runs/1": + // parse req and assert UpdateCheckRun was called for existing check run + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + m := make(map[string]interface{}) + err = json.Unmarshal(body, &m) + Ok(t, err) + assert.Equal(t, c.existingCheckRunName, m["name"]) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + req := types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: models.SuccessCommitStatus, + Description: "description", + DetailsURL: "https://google.com", + Ref: "sha", + } + + if c.newCheckRunName != "" { + req.StatusName = c.newCheckRunName + } else { + req.StatusName = c.existingCheckRunName + } + + err = client.UpdateChecksStatus(context.TODO(), req) + Ok(t, err) + }) + } +} + +func TestGithubClient_UpdateChecksStatus_ConclusionWhenStatusComplete(t *testing.T) { + checkRunName := "atlantis/apply" + listCheckRunResp := ` + { + "total_count": 1, + "check_runs": [ + { + "id": 1, + "head_sha": "ce587453ced02b1526dfb4cb910479d431683101", + "status": "completed", + "conclusion": "neutral", + "started_at": "2018-05-04T01:14:52Z", + "completed_at": "2018-05-04T01:14:52Z", + "name": "%s", + "check_suite": { + "id": 5 + } + } + ] + } + ` + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/sha/check-runs?per_page=100": + _, err := w.Write([]byte(fmt.Sprintf(listCheckRunResp, checkRunName))) + Ok(t, err) + case "/api/v3/repos/owner/repo/check-runs/1": + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + m := make(map[string]interface{}) + err = json.Unmarshal(body, &m) + Ok(t, err) + + // assert conclusion was set to success when status is complete + assert.Equal(t, checkRunName, m["name"]) + assert.Equal(t, "success", m["conclusion"]) + + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + err = client.UpdateChecksStatus(context.TODO(), types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: models.SuccessCommitStatus, + StatusName: checkRunName, + Description: "description", + DetailsURL: "https://google.com", + Ref: "sha", + }) + Ok(t, err) + +} + +// Test to ensure we are creating a new check run when a checkrun with the same name exists +// but the state is Pending, i.e it is a re-run for the same operation. +func TestGithubClient_UpdateChecksStatus_CreateNewCheckRunWhenPendingStatus(t *testing.T) { + checkRunName := "atlantis/apply" + listCheckRunResp := ` + { + "total_count": 1, + "check_runs": [ + { + "id": 1, + "head_sha": "ce587453ced02b1526dfb4cb910479d431683101", + "status": "completed", + "conclusion": "neutral", + "started_at": "2018-05-04T01:14:52Z", + "completed_at": "2018-05-04T01:14:52Z", + "name": "%s", + "check_suite": { + "id": 5 + } + } + ] + } + ` + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/sha/check-runs?per_page=100": + _, err := w.Write([]byte(fmt.Sprintf(listCheckRunResp, checkRunName))) + Ok(t, err) + case "/api/v3/repos/owner/repo/check-runs": + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + m := make(map[string]interface{}) + err = json.Unmarshal(body, &m) + Ok(t, err) + + // assert new checkrun was created + assert.Equal(t, checkRunName, m["name"]) + + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + err = client.UpdateChecksStatus(context.TODO(), types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: models.PendingCommitStatus, + StatusName: checkRunName, + Description: "description", + DetailsURL: "https://google.com", + Ref: "sha", + }) + Ok(t, err) + +} + +func TestGithubClient_UpdateChecksStatus_ErrorWhenListCheckRunsFails(t *testing.T) { + listCheckRunResp := `error response` + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/sha/check-runs?per_page=100": + _, err := w.Write([]byte(listCheckRunResp)) + Ok(t, err) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + err = client.UpdateChecksStatus(context.TODO(), types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: models.PendingCommitStatus, + StatusName: "atlantis/plan", + Description: "description", + DetailsURL: "https://google.com", + Ref: "sha", + }) + assert.Error(t, err) +} + +func TestGithubClient_UpdateChecksStatus_RequestDetailsURLTakesPrecedence(t *testing.T) { + checkRunName := "atlantis/apply: my-project" + detailsURL := "https://google.com" + listCheckRunResp := ` + { + "total_count": 1, + "check_runs": [ + { + "id": 1, + "head_sha": "ce587453ced02b1526dfb4cb910479d431683101", + "status": "completed", + "conclusion": "neutral", + "started_at": "2018-05-04T01:14:52Z", + "completed_at": "2018-05-04T01:14:52Z", + "details_url": "https://example.com", + "name": "%s", + "check_suite": { + "id": 5 + } + } + ] + } + ` + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/commits/sha/check-runs?per_page=100": + _, err := w.Write([]byte(fmt.Sprintf(listCheckRunResp, checkRunName))) + Ok(t, err) + case "/api/v3/repos/owner/repo/check-runs/1": + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + m := make(map[string]interface{}) + err = json.Unmarshal(body, &m) + Ok(t, err) + + // assert updateReq details URL takes precedence over checkRun's details URL + assert.Equal(t, checkRunName, m["name"]) + assert.Equal(t, detailsURL, m["details_url"]) + + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + err = client.UpdateChecksStatus(context.TODO(), types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: models.SuccessCommitStatus, + StatusName: checkRunName, + Description: "description", + DetailsURL: detailsURL, + Ref: "sha", + }) + Ok(t, err) +} + +func TestGithubClient_UpdateStatus(t *testing.T) { + cases := []struct { + status models.CommitStatus + expState string + }{ + { + models.PendingCommitStatus, + "pending", + }, + { + models.SuccessCommitStatus, + "success", + }, + { + models.FailedCommitStatus, + "failure", + }, + } + + for _, c := range cases { + t.Run(c.status.String(), func(t *testing.T) { + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v3/repos/owner/repo/statuses/": + body, err := ioutil.ReadAll(r.Body) + Ok(t, err) + exp := fmt.Sprintf(`{"state":"%s","target_url":"https://google.com","description":"description","context":"src"}%s`, c.expState, "\n") + Equals(t, exp, string(body)) + defer r.Body.Close() // nolint: errcheck + w.WriteHeader(http.StatusOK) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") + client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) + Ok(t, err) + defer disableSSLVerification()() + + err = client.UpdateStatus(context.TODO(), types.UpdateStatusRequest{ + Repo: models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.Github, + Hostname: "github.com", + }, + }, + PullNum: 1, + State: c.status, + StatusName: "src", + Description: "description", + DetailsURL: "https://google.com", + }) + Ok(t, err) + }) + } +} + func TestGithubClient_PullIsApproved(t *testing.T) { respTemplate := `[ { diff --git a/server/lyft/checks/github_client.go b/server/lyft/checks/github_client.go index e911560ad1..b053678429 100644 --- a/server/lyft/checks/github_client.go +++ b/server/lyft/checks/github_client.go @@ -3,151 +3,21 @@ package checks import ( "context" "fmt" - "strconv" - "strings" - "github.com/google/go-github/v45/github" - "github.com/pkg/errors" - "github.com/runatlantis/atlantis/server/core/db" - "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" ) -// Reference: https://github.com/github/docs/issues/3765 -const maxChecksOutputLength = 65535 - -// github checks status -type CheckStatus int - -const ( - Queued CheckStatus = iota - InProgress - Completed -) - -func (e CheckStatus) String() string { - switch e { - case Queued: - return "queued" - case InProgress: - return "in_progress" - case Completed: - return "completed" - } - return "" -} - -// github checks conclusion -type ChecksConclusion int - -const ( - Neutral ChecksConclusion = iota - TimedOut - ActionRequired - Cancelled - Failure - Success -) - -func (e ChecksConclusion) String() string { - switch e { - case Neutral: - return "neutral" - case TimedOut: - return "timed_out" - case ActionRequired: - return "action_required" - case Cancelled: - return "cancelled" - case Failure: - return "failure" - case Success: - return "success" - } - return "" -} - // [WENGINES-4643] TODO: Remove this wrapper and add checks implementation to UpdateStatus() directly after github checks is stable type ChecksClientWrapper struct { *vcs.GithubClient FeatureAllocator feature.Allocator Logger logging.Logger - Db *db.BoltDB } func (c *ChecksClientWrapper) UpdateStatus(ctx context.Context, request types.UpdateStatusRequest) error { - - if !c.isChecksEnabled(ctx, request) { - return c.GithubClient.UpdateStatus(ctx, request) - } - - // Pending state when it's a new run. - if request.State == models.PendingCommitStatus { - return c.createCheckRun(ctx, request) - } - - // Get checkrun from db and update the existing checkrun - checkRun, err := c.Db.GetCheckRunForStatus(request.StatusName, request.Repo, request.Ref) - if err != nil { - return errors.Wrapf(err, "getting checkrun Id from db for %s", request.StatusName) - } - - // This is likely a bug since all for every new checkrun, we first set it to Pending and populate the db - if checkRun == nil { - return errors.New("checkrun dne in db") - } - - return c.updateCheckRun(ctx, *checkRun, request) -} - -func (c *ChecksClientWrapper) createCheckRun(ctx context.Context, request types.UpdateStatusRequest) error { - checkRun, err := c.GithubClient.CreateCheckStatus(ctx, request.Repo, c.populateCreateCheckRunOptions(request)) - if err != nil { - return errors.Wrapf(err, "creating checkrun for %s", request.StatusName) - } - - return c.updateCheckRunInDb(checkRun, request) -} - -func (c *ChecksClientWrapper) updateCheckRun(ctx context.Context, checkRun models.CheckRunStatus, request types.UpdateStatusRequest) error { - checkRunIdInt, err := strconv.ParseInt(checkRun.ID, 10, 64) - if err != nil { - return errors.Wrapf(err, "parsing checkrunId for %s", request.StatusName) - } - - updatedCheckRun, err := c.GithubClient.UpdateCheckStatus(ctx, request.Repo, checkRunIdInt, c.populateUpdateCheckRunOptions(request, checkRun)) - if err != nil { - return errors.Wrapf(err, "updating checkrun for %s", request.StatusName) - } - - return c.updateCheckRunInDb(updatedCheckRun, request) -} - -func (c *ChecksClientWrapper) updateCheckRunInDb(checkRun github.CheckRun, request types.UpdateStatusRequest) error { - - checkRunStatus := models.CheckRunStatus{ - ID: strconv.FormatInt(*checkRun.ID, 10), - JobsURL: request.DetailsURL, - } - - // Persist the output for policy check commands only since github does not persist the state of the checkrun output - // Project plan/apply commands output the logs when the operation is complete, so we don't need to persist the output - // for these commands. - if strings.Contains(request.StatusName, "policy_check") && checkRun.Output != nil && checkRun.Output.Text != nil { - checkRunStatus.Output = *checkRun.Output.Text - } - - // Store the checkrun ID in boltdb - if err := c.Db.UpdateCheckRunForStatus(request.StatusName, request.Repo, request.Ref, checkRunStatus); err != nil { - return errors.Wrapf(err, "updating checkrun id in db for %s", request.StatusName) - } - return nil -} - -func (c *ChecksClientWrapper) isChecksEnabled(ctx context.Context, request types.UpdateStatusRequest) bool { shouldAllocate, err := c.FeatureAllocator.ShouldAllocate(feature.GithubChecks, feature.FeatureContext{ RepoName: request.Repo.FullName, PullCreationTime: request.PullCreationTime, @@ -158,127 +28,9 @@ func (c *ChecksClientWrapper) isChecksEnabled(ctx context.Context, request types }) } - return shouldAllocate -} - -func (c *ChecksClientWrapper) populateCreateCheckRunOptions(request types.UpdateStatusRequest) github.CreateCheckRunOptions { - status, conclusion := c.resolveChecksStatus(request.State) - output := c.capCheckRunOutput(request.Output) - summary := c.summaryWithJobURL(request.StatusName, request.Description, request.DetailsURL) - - checkRunOutput := &github.CheckRunOutput{ - Title: &request.StatusName, - Summary: &summary, - } - - // Only add text if output is not empty to avoid an empty output box in the checkrun UI - if output != "" { - checkRunOutput.Text = &output - } - - createCheckRunOptions := github.CreateCheckRunOptions{ - Name: request.StatusName, - HeadSHA: request.Ref, - Status: &status, - Output: checkRunOutput, - } - - // Add details URL is in the req - if request.DetailsURL != "" { - createCheckRunOptions.DetailsURL = &request.DetailsURL - } - - // Conclusion is required if status is Completed - if status == Completed.String() { - createCheckRunOptions.Conclusion = &conclusion - } - - return createCheckRunOptions -} - -func (c *ChecksClientWrapper) populateUpdateCheckRunOptions(request types.UpdateStatusRequest, checkRunStatus models.CheckRunStatus) github.UpdateCheckRunOptions { - // Populate the output for policy_check command if the output is empty - if strings.Contains(request.StatusName, "policy_check") && request.Output == "" { - request.Output = checkRunStatus.Output - } - - // Populate the DetailsURL if empty - if request.DetailsURL == "" { - request.DetailsURL = checkRunStatus.JobsURL - } - - status, conclusion := c.resolveChecksStatus(request.State) - output := c.capCheckRunOutput(request.Output) - summary := c.summaryWithJobURL(request.StatusName, request.Description, request.DetailsURL) - - checkRunOutput := &github.CheckRunOutput{ - Title: &request.StatusName, - Summary: &summary, - } - - // Only add text if output is not empty to avoid an empty output box in the checkrun UI - if output != "" { - checkRunOutput.Text = &output - } - - updateCheckRunOptions := github.UpdateCheckRunOptions{ - Name: request.StatusName, - Status: &status, - Output: checkRunOutput, - } - - // Add details URL is in the req - if request.DetailsURL != "" { - updateCheckRunOptions.DetailsURL = &request.DetailsURL - } - - // Conclusion is required if status is Completed - if status == Completed.String() { - updateCheckRunOptions.Conclusion = &conclusion - } - - return updateCheckRunOptions -} - -// Github Checks uses Status and Conclusion to report status of the check run. 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".) -func (c *ChecksClientWrapper) resolveChecksStatus(state models.CommitStatus) (string, string) { - status := Queued - conclusion := Neutral - - switch state { - case models.SuccessCommitStatus: - status = Completed - conclusion = Success - - case models.PendingCommitStatus: - status = InProgress - - case models.FailedCommitStatus: - status = Completed - conclusion = Failure - } - - return status.String(), conclusion.String() -} - -// Cap the output string if it exceeds the max checks output length -func (c *ChecksClientWrapper) capCheckRunOutput(output string) string { - if len(output) > maxChecksOutputLength { - return output[:maxChecksOutputLength] + if !shouldAllocate { + return c.GithubClient.UpdateStatus(ctx, request) } - return output -} -// Append job URL to summary if it's a project plan or apply operation bc we currently only stream logs for these two operations -func (g *ChecksClientWrapper) summaryWithJobURL(statusName string, summary string, jobsURL string) string { - if strings.Contains(statusName, ":") && - (strings.Contains(statusName, "plan") || strings.Contains(statusName, "apply")) { - - if jobsURL != "" { - return fmt.Sprintf("%s\n[Logs](%s)", summary, jobsURL) - } - } - return summary + return c.GithubClient.UpdateChecksStatus(ctx, request) } diff --git a/server/lyft/checks/github_client_test.go b/server/lyft/checks/github_client_test.go deleted file mode 100644 index b604ffd9b0..0000000000 --- a/server/lyft/checks/github_client_test.go +++ /dev/null @@ -1,336 +0,0 @@ -package checks_test - -import ( - "context" - "crypto/tls" - "encoding/json" - "fmt" - "io/ioutil" - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/runatlantis/atlantis/server/core/db" - "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/checks" - "github.com/runatlantis/atlantis/server/lyft/feature" - - . "github.com/runatlantis/atlantis/testing" -) - -var checkRunRespFormat = `{ - "id": 4, - "head_sha": "ce587453ced02b1526dfb4cb910479d431683101", - "node_id": "MDg6Q2hlY2tSdW40", - "external_id": "42", - "url": "https://api.github.com/repos/github/hello-world/check-runs/4", - "html_url": "https://github.com/github/hello-world/runs/4", - "details_url": "https://example.com", - "status": "in_progress", - "conclusion": null, - "started_at": "2018-05-04T01:14:52Z", - "completed_at": null, - "name": "%s", - "check_suite": { - "id": 5 - }, - "output": { - "title": "Mighty Readme report", - "summary": "There are 0 failures, 2 warnings, and 1 notice.", - "text": "Output text" - } - }` - -func TestUpdateStatus_FeatureAllocation(t *testing.T) { - - cases := []struct { - name string - shouldAllocate bool - isCommitStatus bool - isCheckRunStatus bool - }{ - { - name: "use default status update when checks is not enabled", - shouldAllocate: false, - isCommitStatus: true, - isCheckRunStatus: false, - }, - { - name: "use github checks when checks is enabled", - shouldAllocate: true, - isCommitStatus: false, - isCheckRunStatus: true, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - - // Reset at the start of each test - commitStatus := false - checkRunStatus := false - statusName := "atlantis/plan" - - checksClientWrapper, boltdb, repo := setup(t, c.shouldAllocate, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.RequestURI { - case "/api/v3/repos/owner/repo/statuses/ref": - commitStatus = true - - case "/api/v3/repos/owner/repo/check-runs": - checkRunStatus = true - w.Write([]byte(fmt.Sprintf(checkRunRespFormat, statusName))) - - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - return - } - })) - defer disableSSLVerification()() - - checksClientWrapper.UpdateStatus(context.TODO(), types.UpdateStatusRequest{ - Repo: repo, - Ref: "ref", - State: models.PendingCommitStatus, - StatusName: statusName, - }) - - // Assert the right status update is used - if commitStatus != c.isCommitStatus || checkRunStatus != c.isCheckRunStatus { - t.FailNow() - } - - // Check if it was persisted to boltdb - persistedCheckRunStatus, err := boltdb.GetCheckRunForStatus("atlantis/plan", repo, "ref") - if c.isCheckRunStatus && (err != nil || persistedCheckRunStatus == nil) { - t.FailNow() - } - }) - } -} - -func TestUpdateStatus_PersistCheckRunOutput(t *testing.T) { - - cases := []struct { - name string - statusName string - shouldPersistOutput bool - }{ - { - name: "persist checkrun output in bolt db when policy_check command", - statusName: "atlantis/plan", - shouldPersistOutput: false, - }, - { - name: "should not perist checkrun output when not policy_check", - statusName: "atlantis/policy_check", - shouldPersistOutput: true, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - - checksClientWrapper, boltdb, repo := setup(t, true, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.RequestURI { - - case "/api/v3/repos/owner/repo/check-runs": - w.Write([]byte(fmt.Sprintf(checkRunRespFormat, c.statusName))) - - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - return - } - })) - defer disableSSLVerification()() - - checksClientWrapper.UpdateStatus(context.TODO(), types.UpdateStatusRequest{ - Repo: repo, - Ref: "ref", - State: models.PendingCommitStatus, - StatusName: c.statusName, - }) - - checkRunStatus, err := boltdb.GetCheckRunForStatus(c.statusName, repo, "ref") - Ok(t, err) - - // Assert checkrun was persisted - if checkRunStatus == nil { - t.FailNow() - } - - // Assert checkrun output was persisted when necessary - if (c.shouldPersistOutput && checkRunStatus.Output == "") || - (!c.shouldPersistOutput && checkRunStatus.Output != "") { - t.FailNow() - } - - }) - } -} - -func TestUpdateStatus_PopulatesOutputWhenEmpty(t *testing.T) { - cases := []struct { - name string - expectedOutput string - populateOutputFromBoltDb bool - output string - }{ - { - name: "populate output from boltdb for policy_check when output in req is empty", - populateOutputFromBoltDb: true, - output: "", - expectedOutput: "Original output", - }, - { - name: "do not populate output from boltdb for policy_check when output in req is not empty", - populateOutputFromBoltDb: false, - output: "Updated output", - expectedOutput: "Updated output", - }, - } - statusName := "atlantis/policy_check" - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - - var output string - checksClientWrapper, boltdb, repo := setup(t, true, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.RequestURI { - - case "/api/v3/repos/owner/repo/check-runs/4": - body, err := ioutil.ReadAll(r.Body) - Ok(t, err) - m := make(map[string]interface{}) - err = json.Unmarshal(body, &m) - Ok(t, err) - - receivedOutput := m["output"].(map[string]interface{}) - output = receivedOutput["text"].(string) - - w.Write([]byte(fmt.Sprintf(checkRunRespFormat, statusName))) - - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - return - } - })) - defer disableSSLVerification()() - - // Populate boltdb - boltdb.UpdateCheckRunForStatus("atlantis/policy_check", repo, "ref", models.CheckRunStatus{ - ID: "4", - Output: "Original output", - }) - - updateStatusReq := types.UpdateStatusRequest{ - Repo: repo, - Ref: "ref", - State: models.SuccessCommitStatus, - StatusName: statusName, - } - - if c.output != "" { - updateStatusReq.Output = c.output - } - - checksClientWrapper.UpdateStatus(context.TODO(), updateStatusReq) - - if c.expectedOutput != output { - t.FailNow() - } - - // Assert last status update is persisted to bolt db - checkRunStatus, err := boltdb.GetCheckRunForStatus(statusName, repo, "ref") - Ok(t, err) - - if checkRunStatus.Output != "Output text" { - t.FailNow() - } - }) - } -} - -func TestUpdateStatus_ErrorWhenCheckRunDoesNotExist(t *testing.T) { - dataDir, cleanup := TempDir(t) - defer cleanup() - - boltdb, err := db.New(dataDir) - Ok(t, err) - - checksClientWrapper := checks.ChecksClientWrapper{ - FeatureAllocator: &mockFeatureAllocator{shouldAllocate: true}, - Logger: logging.NewNoopCtxLogger(t), - Db: boltdb, - } - - repo := models.Repo{ - Owner: "owner", - Name: "repo", - FullName: "owner/repo", - } - - err = checksClientWrapper.UpdateStatus(context.TODO(), types.UpdateStatusRequest{ - Repo: repo, - Ref: "ref", - State: models.SuccessCommitStatus, - StatusName: "atlantis/plan", - Description: "Hello World", - }) - - // assert same error - ErrEquals(t, "checkrun dne in db", err) -} - -// disableSSLVerification disables ssl verification for the global http client -// and returns a function to be called in a defer that will re-enable it. -func disableSSLVerification() func() { - orig := http.DefaultTransport.(*http.Transport).TLSClientConfig - // nolint: gosec - http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - return func() { - http.DefaultTransport.(*http.Transport).TLSClientConfig = orig - } -} - -func setup(t *testing.T, shouldAllocate bool, handlerFunc http.HandlerFunc) (checks.ChecksClientWrapper, db.BoltDB, models.Repo) { - testServer := httptest.NewTLSServer(handlerFunc) - - testServerURL, err := url.Parse(testServer.URL) - Ok(t, err) - mergeabilityChecker := vcs.NewPullMergeabilityChecker("atlantis") - client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopCtxLogger(t), mergeabilityChecker) - Ok(t, err) - - dataDir, cleanup := TempDir(t) - defer cleanup() - - boltdb, err := db.New(dataDir) - Ok(t, err) - - repo := models.Repo{ - Owner: "owner", - Name: "repo", - } - - return checks.ChecksClientWrapper{ - GithubClient: client, - FeatureAllocator: &mockFeatureAllocator{shouldAllocate: shouldAllocate}, - Logger: logging.NewNoopCtxLogger(t), - Db: boltdb, - }, *boltdb, repo -} - -type mockFeatureAllocator struct { - shouldAllocate bool -} - -func (m *mockFeatureAllocator) ShouldAllocate(featureID feature.Name, featureCtx feature.FeatureContext) (bool, error) { - return m.shouldAllocate, nil -} diff --git a/server/lyft/gateway/autoplan_builder.go b/server/lyft/gateway/autoplan_builder.go index cdbc931e00..6801f5ffe3 100644 --- a/server/lyft/gateway/autoplan_builder.go +++ b/server/lyft/gateway/autoplan_builder.go @@ -56,11 +56,6 @@ func (r *AutoplanValidator) isValid(ctx context.Context, logger logging.Logger, cmdCtx.Log.ErrorContext(cmdCtx.RequestCtx, fmt.Sprintf("Error running pre-workflow hooks %s. Proceeding with %s command.", err, command.Plan)) } - // Set to pending to create checkrun - if statusErr := r.CommitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.Plan); statusErr != nil { - cmdCtx.Log.WarnContext(cmdCtx.RequestCtx, fmt.Sprintf("unable to update commit status: %v", statusErr)) - } - projectCmds, err := r.PrjCmdBuilder.BuildAutoplanCommands(cmdCtx) if err != nil { if statusErr := r.CommitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil { diff --git a/server/server.go b/server/server.go index 317a9f138c..916b5e1440 100644 --- a/server/server.go +++ b/server/server.go @@ -19,6 +19,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/runatlantis/atlantis/server/events/terraform/filter" "io" "io/ioutil" "log" @@ -32,8 +33,6 @@ import ( "syscall" "time" - "github.com/runatlantis/atlantis/server/events/terraform/filter" - assetfs "github.com/elazarl/go-bindata-assetfs" "github.com/runatlantis/atlantis/server/instrumentation" "github.com/runatlantis/atlantis/server/static" @@ -188,11 +187,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { globalCfg = globalCfg.EnablePlatformMode() } - boltdb, err := db.New(userConfig.DataDir) - if err != nil { - return nil, err - } - if userConfig.RepoConfig != "" { globalCfg, err = validator.ParseGlobalCfg(userConfig.RepoConfig, globalCfg) if err != nil { @@ -268,7 +262,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { FeatureAllocator: featureAllocator, Logger: ctxLogger, GithubClient: rawGithubClient, - Db: boltdb, } githubClient = vcs.NewInstrumentedGithubClient(rawGithubClient, checksWrapperGhClient, statsScope, ctxLogger) @@ -437,6 +430,10 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { TemplateResolver: templateResolver, } + boltdb, err := db.New(userConfig.DataDir) + if err != nil { + return nil, err + } var lockingClient locking.Locker var applyLockingClient locking.ApplyLocker