From d624a5edd6a34cecb0109165e9d3fd400b4a7615 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 3 Nov 2024 23:03:19 +0100 Subject: [PATCH] fix: Actions PR workflows must update the commit status When a workflow has on: pull_request: types: - labeled - unlabeled The outcome of the workflow (success or failure) must be associated with the head sha commit status. Otherwise it cannot be used as a requirement for merging the pull request (branch protections). (cherry picked from commit 66c85b7d8b40d4c1c504b01ab70b6282059b1c21) --- models/actions/run.go | 6 +- services/actions/commit_status.go | 2 +- tests/integration/actions_trigger_test.go | 236 ++++++++++++++++++++++ 3 files changed, 242 insertions(+), 2 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 8b40cb7ba8..f637634575 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -146,7 +146,11 @@ func (run *ActionRun) GetPushEventPayload() (*api.PushPayload, error) { } func (run *ActionRun) GetPullRequestEventPayload() (*api.PullRequestPayload, error) { - if run.Event == webhook_module.HookEventPullRequest || run.Event == webhook_module.HookEventPullRequestSync { + if run.Event == webhook_module.HookEventPullRequest || + run.Event == webhook_module.HookEventPullRequestSync || + run.Event == webhook_module.HookEventPullRequestAssign || + run.Event == webhook_module.HookEventPullRequestMilestone || + run.Event == webhook_module.HookEventPullRequestLabel { var payload api.PullRequestPayload if err := json.Unmarshal([]byte(run.EventPayload), &payload); err != nil { return nil, err diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 2698059e94..04dffbac88 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -53,7 +53,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return fmt.Errorf("head commit is missing in event payload") } sha = payload.HeadCommit.ID - case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync: + case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync, webhook_module.HookEventPullRequestLabel, webhook_module.HookEventPullRequestAssign, webhook_module.HookEventPullRequestMilestone: if run.TriggerEvent == actions_module.GithubEventPullRequestTarget { event = "pull_request_target" } else { diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index de646f05b5..c57a64e7dd 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -5,12 +5,14 @@ package integration import ( "fmt" + "net/http" "net/url" "strings" "testing" "time" actions_model "code.gitea.io/gitea/models/actions" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" @@ -22,9 +24,11 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" webhook_module "code.gitea.io/gitea/modules/webhook" actions_service "code.gitea.io/gitea/services/actions" + issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" release_service "code.gitea.io/gitea/services/release" repo_service "code.gitea.io/gitea/services/repository" @@ -35,6 +39,238 @@ import ( "github.com/stretchr/testify/require" ) +func TestPullRequestCommitStatus(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the base repo + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + // prepare the repository + files := make([]*files_service.ChangeRepoFile, 0, 10) + for _, onType := range []string{ + "opened", + "synchronize", + "labeled", + "unlabeled", + "assigned", + "unassigned", + "milestoned", + "demilestoned", + "closed", + "reopened", + } { + files = append(files, &files_service.ChangeRepoFile{ + Operation: "create", + TreePath: fmt.Sprintf(".forgejo/workflows/%s.yml", onType), + ContentReader: strings.NewReader(fmt.Sprintf(`name: %[1]s +on: + pull_request: + types: + - %[1]s +jobs: + %[1]s: + runs-on: docker + steps: + - run: true +`, onType)), + }) + } + baseRepo, _, f := tests.CreateDeclarativeRepo(t, user2, "repo-pull-request", + []unit_model.Type{unit_model.TypeActions}, nil, files) + defer f() + baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) + require.NoError(t, err) + defer func() { + baseGitRepo.Close() + }() + + // prepare the pull request + testEditFileToNewBranch(t, session, "user2", "repo-pull-request", "main", "wip-something", "README.md", "Hello, world 1") + testPullCreate(t, session, "user2", "repo-pull-request", true, "main", "wip-something", "Commit status PR") + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: baseRepo.ID}) + require.NoError(t, pr.LoadIssue(db.DefaultContext)) + + // prepare the assignees + issueURL := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%s", "user2", "repo-pull-request", fmt.Sprintf("%d", pr.Issue.Index)) + + // prepare the labels + labelStr := "/api/v1/repos/user2/repo-pull-request/labels" + req := NewRequestWithJSON(t, "POST", labelStr, &api.CreateLabelOption{ + Name: "mylabel", + Color: "abcdef", + Description: "description mylabel", + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusCreated) + label := new(api.Label) + DecodeJSON(t, resp, &label) + labelURL := fmt.Sprintf("%s/labels", issueURL) + + // prepare the milestone + milestoneStr := "/api/v1/repos/user2/repo-pull-request/milestones" + req = NewRequestWithJSON(t, "POST", milestoneStr, &api.CreateMilestoneOption{ + Title: "mymilestone", + State: "open", + }).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusCreated) + milestone := new(api.Milestone) + DecodeJSON(t, resp, &milestone) + + // check that one of the status associated with the commit sha matches both + // context & state + checkCommitStatus := func(sha, context string, state api.CommitStatusState) bool { + commitStatuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, pr.BaseRepoID, sha, db.ListOptionsAll) + require.NoError(t, err) + for _, commitStatus := range commitStatuses { + if state == commitStatus.State && context == commitStatus.Context { + return true + } + } + return false + } + + count := 0 + for _, testCase := range []struct { + onType string + jobID string + doSomething func() + action api.HookIssueAction + hasLabel bool + }{ + { + onType: "opened", + doSomething: func() {}, + action: api.HookIssueOpened, + }, + { + onType: "synchronize", + doSomething: func() { + testEditFile(t, session, "user2", "repo-pull-request", "wip-something", "README.md", "Hello, world 2") + }, + action: api.HookIssueSynchronized, + }, + { + onType: "labeled", + doSomething: func() { + req := NewRequestWithJSON(t, "POST", labelURL, &api.IssueLabelsOption{ + Labels: []any{label.ID}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + }, + action: api.HookIssueLabelUpdated, + hasLabel: true, + }, + { + onType: "unlabeled", + doSomething: func() { + req := NewRequestWithJSON(t, "PUT", labelURL, &api.IssueLabelsOption{ + Labels: []any{}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + }, + action: api.HookIssueLabelCleared, + hasLabel: true, + }, + { + onType: "assigned", + doSomething: func() { + req := NewRequestWithJSON(t, "PATCH", issueURL, &api.EditIssueOption{ + Assignees: []string{"user2"}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + }, + action: api.HookIssueAssigned, + }, + { + onType: "unassigned", + doSomething: func() { + req := NewRequestWithJSON(t, "PATCH", issueURL, &api.EditIssueOption{ + Assignees: []string{}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + }, + action: api.HookIssueUnassigned, + }, + { + onType: "milestoned", + doSomething: func() { + req := NewRequestWithJSON(t, "PATCH", issueURL, &api.EditIssueOption{ + Milestone: &milestone.ID, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + }, + action: api.HookIssueMilestoned, + }, + { + onType: "demilestoned", + doSomething: func() { + var zero int64 + req := NewRequestWithJSON(t, "PATCH", issueURL, &api.EditIssueOption{ + Milestone: &zero, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + }, + action: api.HookIssueDemilestoned, + }, + { + onType: "closed", + doSomething: func() { + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + require.NoError(t, err) + err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, true) + require.NoError(t, err) + }, + action: api.HookIssueClosed, + }, + { + onType: "reopened", + doSomething: func() { + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + require.NoError(t, err) + err = issue_service.ChangeStatus(db.DefaultContext, pr.Issue, user2, sha, false) + require.NoError(t, err) + }, + action: api.HookIssueReOpened, + }, + } { + t.Run(testCase.onType, func(t *testing.T) { + // trigger the onType event + testCase.doSomething() + count++ + context := fmt.Sprintf("%[1]s / %[1]s (pull_request)", testCase.onType) + + // wait for a new ActionRun to be created + assert.Eventually(t, func() bool { + return count == unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) + }, 30*time.Second, 1*time.Second) + + // verify the expected ActionRun was created + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + require.NoError(t, err) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, WorkflowID: fmt.Sprintf("%s.yml", testCase.onType)}) + + assert.Equal(t, sha, actionRun.CommitSHA) + assert.Equal(t, actions_module.GithubEventPullRequest, actionRun.TriggerEvent) + event, err := actionRun.GetPullRequestEventPayload() + if testCase.hasLabel { + assert.NotNil(t, event.Label) + } + require.NoError(t, err) + assert.Equal(t, testCase.action, event.Action) + + // verify the expected ActionRunJob was created and is StatusWaiting + job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{JobID: testCase.onType, CommitSHA: sha}) + assert.Equal(t, actions_model.StatusWaiting, job.Status) + + // verify the commit status changes to CommitStatusSuccess when the job changes to StatusSuccess + assert.True(t, checkCommitStatus(sha, context, api.CommitStatusPending)) + job.Status = actions_model.StatusSuccess + actions_service.CreateCommitStatus(db.DefaultContext, job) + assert.True(t, checkCommitStatus(sha, context, api.CommitStatusSuccess)) + }) + } + }) +} + func TestPullRequestTargetEvent(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the base repo