Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the bug when getting files changed for pull_request_target event #26320

Merged
merged 9 commits into from
Aug 5, 2023
30 changes: 21 additions & 9 deletions modules/actions/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func GetEventsFromContent(content []byte) ([]*jobparser.Event, error) {
return events, nil
}

func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader) ([]*DetectedWorkflow, error) {
func DetectWorkflows(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader) ([]*DetectedWorkflow, error) {
entries, err := ListWorkflows(commit)
if err != nil {
return nil, err
Expand All @@ -114,7 +114,7 @@ func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventTy
}
for _, evt := range events {
log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent)
if detectMatched(commit, triggedEvent, payload, evt) {
if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
dwf := &DetectedWorkflow{
EntryName: entry.Name(),
TriggerEvent: evt.Name,
Expand All @@ -128,7 +128,7 @@ func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventTy
return workflows, nil
}

func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) bool {
func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent webhook_module.HookEventType, payload api.Payloader, evt *jobparser.Event) bool {
if !canGithubEventMatch(evt.Name, triggedEvent) {
return false
}
Expand Down Expand Up @@ -168,7 +168,7 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType
webhook_module.HookEventPullRequestSync,
webhook_module.HookEventPullRequestAssign,
webhook_module.HookEventPullRequestLabel:
return matchPullRequestEvent(commit, payload.(*api.PullRequestPayload), evt)
return matchPullRequestEvent(gitRepo, commit, payload.(*api.PullRequestPayload), evt)

case // pull_request_review
webhook_module.HookEventPullRequestReviewApproved,
Expand Down Expand Up @@ -331,7 +331,7 @@ func matchIssuesEvent(commit *git.Commit, issuePayload *api.IssuePayload, evt *j
return matchTimes == len(evt.Acts())
}

func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
func matchPullRequestEvent(gitRepo *git.Repository, commit *git.Commit, prPayload *api.PullRequestPayload, evt *jobparser.Event) bool {
acts := evt.Acts()
activityTypeMatched := false
matchTimes := 0
Expand Down Expand Up @@ -370,6 +370,18 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
}
}

var (
headCommit = commit
err error
)
if evt.Name == GithubEventPullRequestTarget && (len(acts["paths"]) > 0 || len(acts["paths-ignore"]) > 0) {
headCommit, err = gitRepo.GetCommit(prPayload.PullRequest.Head.Sha)
if err != nil {
log.Error("GetCommit [ref: %s]: %v", prPayload.PullRequest.Head.Sha, err)
return false
}
}

// all acts conditions should be satisfied
for cond, vals := range acts {
switch cond {
Expand All @@ -392,9 +404,9 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
matchTimes++
}
case "paths":
filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
if err != nil {
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err)
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err)
} else {
patterns, err := workflowpattern.CompilePatterns(vals...)
if err != nil {
Expand All @@ -405,9 +417,9 @@ func matchPullRequestEvent(commit *git.Commit, prPayload *api.PullRequestPayload
}
}
case "paths-ignore":
filesChanged, err := commit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
filesChanged, err := headCommit.GetFilesChangedSinceCommit(prPayload.PullRequest.Base.Ref)
if err != nil {
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", commit.ID.String(), err)
log.Error("GetFilesChangedSinceCommit [commit_sha1: %s]: %v", headCommit.ID.String(), err)
} else {
patterns, err := workflowpattern.CompilePatterns(vals...)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion modules/actions/workflows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestDetectMatched(t *testing.T) {
evts, err := GetEventsFromContent([]byte(tc.yamlOn))
assert.NoError(t, err)
assert.Len(t, evts, 1)
assert.Equal(t, tc.expected, detectMatched(tc.commit, tc.triggedEvent, tc.payload, evts[0]))
assert.Equal(t, tc.expected, detectMatched(nil, tc.commit, tc.triggedEvent, tc.payload, evts[0]))
})
}
}
4 changes: 2 additions & 2 deletions services/actions/notifier_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func notify(ctx context.Context, input *notifyInput) error {
}

var detectedWorkflows []*actions_module.DetectedWorkflow
workflows, err := actions_module.DetectWorkflows(commit, input.Event, input.Payload)
workflows, err := actions_module.DetectWorkflows(gitRepo, commit, input.Event, input.Payload)
if err != nil {
return fmt.Errorf("DetectWorkflows: %w", err)
}
Expand All @@ -164,7 +164,7 @@ func notify(ctx context.Context, input *notifyInput) error {
if err != nil {
return fmt.Errorf("gitRepo.GetCommit: %w", err)
}
baseWorkflows, err := actions_module.DetectWorkflows(baseCommit, input.Event, input.Payload)
baseWorkflows, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload)
if err != nil {
return fmt.Errorf("DetectWorkflows: %w", err)
}
Expand Down
54 changes: 53 additions & 1 deletion tests/integration/actions_trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
{
Operation: "create",
TreePath: ".gitea/workflows/pr.yml",
ContentReader: strings.NewReader("name: test\non: pull_request_target\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo helloworld\n"),
ContentReader: strings.NewReader("name: test\non:\n pull_request_target:\n paths:\n - 'file_*.txt'\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo helloworld\n"),
},
},
Message: "add workflow",
Expand Down Expand Up @@ -138,8 +138,60 @@ func TestPullRequestTargetEvent(t *testing.T) {
assert.NoError(t, err)

// load and compare ActionRun
assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}))
actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID})
assert.Equal(t, addFileToForkedResp.Commit.SHA, actionRun.CommitSHA)
assert.Equal(t, actions_module.GithubEventPullRequestTarget, actionRun.TriggerEvent)

// add another file whose name cannot match the specified path
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user3, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
TreePath: "foo.txt",
ContentReader: strings.NewReader("foo"),
},
},
Message: "add foo.txt",
OldBranch: "main",
NewBranch: "fork-branch-2",
Author: &files_service.IdentityOptions{
Name: user3.Name,
Email: user3.Email,
},
Committer: &files_service.IdentityOptions{
Name: user3.Name,
Email: user3.Email,
},
Dates: &files_service.CommitDateOptions{
Author: time.Now(),
Committer: time.Now(),
},
})
assert.NoError(t, err)
assert.NotEmpty(t, addFileToForkedResp)

// create Pull
pullIssue = &issues_model.Issue{
RepoID: baseRepo.ID,
Title: "A mismatched path cannot trigger pull-request-target-event",
PosterID: user3.ID,
Poster: user3,
IsPull: true,
}
pullRequest = &issues_model.PullRequest{
HeadRepoID: forkedRepo.ID,
BaseRepoID: baseRepo.ID,
HeadBranch: "fork-branch-2",
BaseBranch: "main",
HeadRepo: forkedRepo,
BaseRepo: baseRepo,
Type: issues_model.PullRequestGitea,
}
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
assert.NoError(t, err)

// the new pull request cannot trigger actions, so there is still only 1 record
assert.Equal(t, 1, unittest.GetCount(t, &actions_model.ActionRun{RepoID: baseRepo.ID}))
})
}