From 7199ea4a7e80aad2f7729fb40b7f098f3d319ff4 Mon Sep 17 00:00:00 2001 From: PePe Amengual Date: Thu, 31 Mar 2022 08:51:49 -0700 Subject: [PATCH] fix: reverting change to command status logic (#2173) --- server/events/apply_command_runner.go | 8 +- server/events/plan_command_runner.go | 11 -- .../fixtures/github-commit-status-empty.json | 77 --------- .../fixtures/github-commit-status-full.json | 148 ------------------ server/events/vcs/github_client.go | 28 ---- server/events/vcs/github_client_test.go | 28 ---- 6 files changed, 4 insertions(+), 296 deletions(-) delete mode 100644 server/events/vcs/fixtures/github-commit-status-empty.json delete mode 100644 server/events/vcs/fixtures/github-commit-status-full.json diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 1cd1f5baab..db8e27c161 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -94,6 +94,10 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { return } + if err = a.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + // Get the mergeable status before we set any build statuses of our own. // We do this here because when we set a "Pending" status, if users have // required the Atlantis status checks to pass, then we've now changed @@ -108,10 +112,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) } - if err = a.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - var projectCmds []command.ProjectContext projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index 526d0994d2..bef34ec5fe 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -105,9 +105,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { if err := p.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { ctx.Log.Warn("unable to update plan commit status: %s", err) } - if err := p.commitStatusUpdater.UpdateCombinedCount(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply, 0, len(projectCmds)); err != nil { - ctx.Log.Warn("unable to update apply commit status: %s", err) - } // Only run commands in parallel if enabled var result command.Result @@ -183,14 +180,6 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { projectCmds, policyCheckCmds := p.partitionProjectCmds(ctx, projectCmds) - // At this point we are sure Atlantis has work to do, so set commit status to pending - if err := p.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - if err := p.commitStatusUpdater.UpdateCombinedCount(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply, 0, len(projectCmds)); err != nil { - ctx.Log.Warn("unable to update apply commit status: %s", err) - } - // Only run commands in parallel if enabled var result command.Result if p.isParallelEnabled(projectCmds) { diff --git a/server/events/vcs/fixtures/github-commit-status-empty.json b/server/events/vcs/fixtures/github-commit-status-empty.json deleted file mode 100644 index 986f31dc4f..0000000000 --- a/server/events/vcs/fixtures/github-commit-status-empty.json +++ /dev/null @@ -1,77 +0,0 @@ -{ - "state": "pending", - "statuses": [ - - ], - "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", - "total_count": 0, - "repository": { - "id": 1296269, - "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", - "name": "Hello-World", - "full_name": "octocat/Hello-World", - "private": false, - "owner": { - "login": "octocat", - "id": 583231, - "node_id": "MDQ6VXNlcjU4MzIzMQ==", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/octocat", - "html_url": "https://github.com/octocat", - "followers_url": "https://api.github.com/users/octocat/followers", - "following_url": "https://api.github.com/users/octocat/following{/other_user}", - "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", - "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", - "organizations_url": "https://api.github.com/users/octocat/orgs", - "repos_url": "https://api.github.com/users/octocat/repos", - "events_url": "https://api.github.com/users/octocat/events{/privacy}", - "received_events_url": "https://api.github.com/users/octocat/received_events", - "type": "User", - "site_admin": false - }, - "html_url": "https://github.com/octocat/Hello-World", - "description": "My first repository on GitHub!", - "fork": false, - "url": "https://api.github.com/repos/octocat/Hello-World", - "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", - "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", - "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", - "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", - "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", - "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", - "events_url": "https://api.github.com/repos/octocat/Hello-World/events", - "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", - "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", - "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", - "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", - "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", - "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", - "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", - "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", - "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", - "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", - "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", - "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", - "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", - "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", - "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", - "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", - "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", - "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", - "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", - "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", - "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", - "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", - "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", - "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", - "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", - "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", - "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", - "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", - "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments" - }, - "commit_url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/status" -} diff --git a/server/events/vcs/fixtures/github-commit-status-full.json b/server/events/vcs/fixtures/github-commit-status-full.json deleted file mode 100644 index 97612149bf..0000000000 --- a/server/events/vcs/fixtures/github-commit-status-full.json +++ /dev/null @@ -1,148 +0,0 @@ -{ - "state": "blocked", - "statuses": [ - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230299674, - "node_id": "SC_kwDOFRFvL88AAAADx2a4Gg", - "state": "success", - "description": "Plan succeeded.", - "target_url": "https://localhost/jobs/octocat/Hello-World/1/project1", - "context": "atlantis/plan: project1", - "created_at": "2022-02-10T15:26:01Z", - "updated_at": "2022-02-10T15:26:01Z" - }, - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230303174, - "node_id": "SC_kwDOFRFvL88AAAADx2bFxg", - "state": "success", - "description": "Plan succeeded.", - "target_url": "https://localhost/jobs/octocat/Hello-World/1/project2", - "context": "atlantis/plan: project2", - "created_at": "2022-02-10T15:26:12Z", - "updated_at": "2022-02-10T15:26:12Z" - }, - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230303679, - "node_id": "SC_kwDOFRFvL88AAAADx2bHvw", - "state": "success", - "description": "2/2 projects planned successfully.", - "target_url": "", - "context": "atlantis/plan", - "created_at": "2022-02-10T15:26:13Z", - "updated_at": "2022-02-10T15:26:13Z" - }, - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230307923, - "node_id": "SC_kwDOFRFvL88AAAADx2bYUw", - "state": "failure", - "description": "Apply failed.", - "target_url": "https://localhost/jobs/octocat/Hello-World/1/project1", - "context": "atlantis/apply: project1", - "created_at": "2022-02-10T15:26:27Z", - "updated_at": "2022-02-10T15:26:27Z" - }, - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230308153, - "node_id": "SC_kwDOFRFvL88AAAADx2bZOQ", - "state": "failure", - "description": "Apply failed.", - "target_url": "https://localhost/jobs/octocat/Hello-World/1/project2", - "context": "atlantis/apply: project2", - "created_at": "2022-02-10T15:26:27Z", - "updated_at": "2022-02-10T15:26:27Z" - }, - { - "url": "https://api.github.com/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "id": 16230308528, - "node_id": "SC_kwDOFRFvL88AAAADx2basA", - "state": "failure", - "description": "0/2 projects applied successfully.", - "target_url": "", - "context": "atlantis/apply", - "created_at": "2022-02-10T15:26:28Z", - "updated_at": "2022-02-10T15:26:28Z" - } - ], - "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", - "total_count": 0, - "repository": { - "id": 1296269, - "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", - "name": "Hello-World", - "full_name": "octocat/Hello-World", - "private": false, - "owner": { - "login": "octocat", - "id": 583231, - "node_id": "MDQ6VXNlcjU4MzIzMQ==", - "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/octocat", - "html_url": "https://github.com/octocat", - "followers_url": "https://api.github.com/users/octocat/followers", - "following_url": "https://api.github.com/users/octocat/following{/other_user}", - "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", - "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", - "organizations_url": "https://api.github.com/users/octocat/orgs", - "repos_url": "https://api.github.com/users/octocat/repos", - "events_url": "https://api.github.com/users/octocat/events{/privacy}", - "received_events_url": "https://api.github.com/users/octocat/received_events", - "type": "User", - "site_admin": false - }, - "html_url": "https://github.com/octocat/Hello-World", - "description": "My first repository on GitHub!", - "fork": false, - "url": "https://api.github.com/repos/octocat/Hello-World", - "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", - "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", - "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", - "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", - "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", - "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", - "events_url": "https://api.github.com/repos/octocat/Hello-World/events", - "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", - "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", - "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", - "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", - "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", - "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", - "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", - "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", - "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", - "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", - "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", - "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", - "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", - "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", - "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", - "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", - "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", - "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", - "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", - "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", - "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", - "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", - "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", - "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", - "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", - "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", - "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", - "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", - "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments" - }, - "commit_url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/status" -} diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index a1b9f19f9c..d3615ad172 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -303,8 +303,6 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest return false, errors.Wrap(err, "getting pull request") } state := githubPR.GetMergeableState() - status, _ := g.GetCombinedStatus(repo, githubPR.GetHead().GetSHA()) - // We map our mergeable check to when the GitHub merge button is clickable. // This corresponds to the following states: // clean: No conflicts, all requirements satisfied. @@ -314,27 +312,9 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest // has_hooks: GitHub Enterprise only, if a repo has custom pre-receive // hooks. Merging is allowed (green box). // See: https://github.com/octokit/octokit.net/issues/1763 - // - // We should not dismiss the PR if the only our "atlantis/apply" status is pending/failing - if state == "blocked" { - applyStatus := false - for _, s := range status.Statuses { - if strings.Contains(s.GetContext(), "atlantis/apply") { - applyStatus = true - continue - } - if s.GetContext() != "atlantis/apply" && s.GetState() != "success" { - // If any other status is pending/failing mark as non-mergeable - return false, nil - } - } - return applyStatus, nil - } - if state != "clean" && state != "unstable" && state != "has_hooks" { return false, nil } - return true, nil } @@ -365,14 +345,6 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe return pull, err } -func (g *GithubClient) GetCombinedStatus(repo models.Repo, ref string) (*github.CombinedStatus, error) { - opts := github.ListOptions{ - PerPage: 300, - } - status, _, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, ref, &opts) - return status, err -} - // UpdateStatus updates the status badge on the pull request. // See https://github.com/blog/1227-commit-status-api. func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error { diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index ccabb41ad3..42583f70fc 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -478,57 +478,42 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { cases := []struct { state string expMergeable bool - useStatus bool }{ { "dirty", false, - false, }, { "unknown", false, - false, - }, - { - "blocked", - true, - true, }, { "blocked", false, - false, }, { "behind", false, - false, }, { "random", false, - false, }, { "unstable", true, - false, }, { "has_hooks", true, - false, }, { "clean", true, - false, }, { "", false, - false, }, } @@ -545,25 +530,12 @@ func TestGithubClient_PullIsMergeable(t *testing.T) { 1, ) - var statusBytes []byte - var err error - if c.useStatus { - statusBytes, err = os.ReadFile("fixtures/github-commit-status-full.json") - } else { - statusBytes, err = os.ReadFile("fixtures/github-commit-status-empty.json") - } - Ok(t, err) - statusJSON := string(statusBytes) - testServer := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.RequestURI { case "/api/v3/repos/owner/repo/pulls/1": w.Write([]byte(response)) // nolint: errcheck return - case "/api/v3/repos/owner/repo/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/status?per_page=300": - w.Write([]byte(statusJSON)) // nolint: errcheck - return default: t.Errorf("got unexpected request at %q", r.RequestURI) http.Error(w, "not found", http.StatusNotFound)