Skip to content

Commit c05e765

Browse files
zakiskchmouel
authored andcommitted
feat: support pull_request_number variable on push events
added support for pull_request_number dynamic variable when push event is occurred on pull request merge. upstream issue: #1353 https://issues.redhat.com/browse/SRVKP-7834 Signed-off-by: Zaki Shaikh <[email protected]>
1 parent 7e1d03c commit c05e765

File tree

4 files changed

+162
-84
lines changed

4 files changed

+162
-84
lines changed

docs/content/docs/guide/authoringprs.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ check out the code that is being tested.
5353
| event_type | The event type (eg: `pull_request` or `push`) | `{{event_type}}` | pull_request (see the note for GitOps Comments [here]({{< relref "/docs/guide/gitops_commands.md#event-type-annotation-and-dynamic-variables" >}}) ) |
5454
| git_auth_secret | The secret name auto-generated with provider token to check out private repos. | `{{git_auth_secret}}` | pac-gitauth-xkxkx |
5555
| headers | The request headers (see [below](#using-the-body-and-headers-in-a-pipelines-as-code-parameter)) | `{{headers['x-github-event']}}` | push |
56-
| pull_request_number | The pull or merge request number, only defined when we are in a `pull_request` event type. | `{{pull_request_number}}` | 1 |
56+
| pull_request_number | The pull or merge request number, only defined when we are in a `pull_request` event or push event occurred when pull request is merged. | `{{pull_request_number}}` | 1 |
5757
| repo_name | The repository name. | `{{repo_name}}` | pipelines-as-code |
5858
| repo_owner | The repository owner. | `{{repo_owner}}` | openshift-pipelines |
5959
| repo_url | The repository full URL. | `{{repo_url}}` | https:/github.com/repo/owner |
@@ -67,6 +67,11 @@ check out the code that is being tested.
6767
| trigger_comment | The comment triggering the PipelineRun when using a [GitOps command]({{< relref "/docs/guide/running.md#gitops-command-on-pull-or-merge-request" >}}) (like `/test`, `/retest`) | `{{trigger_comment}}` | /merge-pr branch |
6868
| pull_request_labels | The labels of the pull request separated by a newline | `{{pull_request_labels}}` | bugs\nenhancement |
6969

70+
Note: When using the `{{ pull_request_number }}` variable in a push-triggered PipelineRun when a pull request is merged and the commit is associated with multiple pull requests
71+
the git provider API may return more than one pull request. In such cases, the `{{ pull_request_number }}` variable will contain the number of the first pull request returned by the API.
72+
73+
The `{{ pull_request_number }}` variable is currently supported only for the GitHub provider when used in a push event.
74+
7075
### Defining Parameters with Object Values in YAML
7176

7277
When working with YAML, particularly when defining parameters, you might encounter situations where you need to pass an object or a dynamic variable (e.g., `{{ body }}`) as the value of a parameter. However, YAML's validation rules prevent such values from being defined inline.

pkg/provider/github/github_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1416,7 +1416,7 @@ func TestSkipPushEventForPRCommits(t *testing.T) {
14161416
},
14171417
isPartOfPR: false,
14181418
wantErr: false,
1419-
skipWarnLogContains: "Error checking if push commit is part of PR",
1419+
skipWarnLogContains: "Error getting pull requests associated with the commit in this push event",
14201420
},
14211421
}
14221422

pkg/provider/github/parse_payload.go

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -208,45 +208,39 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h
208208
return processedEvent, nil
209209
}
210210

211-
// isCommitPartOfPullRequest checks if the commit from a push event is part of an open pull request
212-
// If it is, it returns true and the PR number.
213-
func (v *Provider) isCommitPartOfPullRequest(ctx context.Context, sha, org, repo string) (bool, int, error) {
211+
// getPullRequestsWithCommit lists the all pull requests associated with given commit.
212+
func (v *Provider) getPullRequestsWithCommit(ctx context.Context, sha, org, repo string) ([]*github.PullRequest, error) {
214213
if v.ghClient == nil {
215-
return false, 0, fmt.Errorf("github client is not initialized")
214+
return nil, fmt.Errorf("github client is not initialized")
216215
}
217216

218217
// Validate input parameters
219218
if sha == "" {
220-
return false, 0, fmt.Errorf("sha cannot be empty")
219+
return nil, fmt.Errorf("sha cannot be empty")
221220
}
222221
if org == "" {
223-
return false, 0, fmt.Errorf("organization cannot be empty")
222+
return nil, fmt.Errorf("organization cannot be empty")
224223
}
225224
if repo == "" {
226-
return false, 0, fmt.Errorf("repository cannot be empty")
225+
return nil, fmt.Errorf("repository cannot be empty")
227226
}
228227

229-
// Use pagination to handle repositories with many PRs
230228
opts := &github.ListOptions{
231229
PerPage: 100, // GitHub's maximum per page
232230
}
233231

232+
pullRequests := []*github.PullRequest{}
233+
234234
for {
235235
// Use the "List pull requests associated with a commit" API to check if the commit is part of any open PR
236236
prs, resp, err := v.ghClient.PullRequests.ListPullRequestsWithCommit(ctx, org, repo, sha, opts)
237237
if err != nil {
238238
// Log the error for debugging purposes
239239
v.Logger.Debugf("Failed to list pull requests for commit %s in %s/%s: %v", sha, org, repo, err)
240-
return false, 0, fmt.Errorf("failed to list pull requests for commit %s: %w", sha, err)
240+
return nil, fmt.Errorf("failed to list pull requests for commit %s: %w", sha, err)
241241
}
242242

243-
// Check if any of the returned PRs are open
244-
for _, pr := range prs {
245-
if pr.GetState() == "open" {
246-
v.Logger.Debugf("Commit %s is part of open PR #%d in %s/%s", sha, pr.GetNumber(), org, repo)
247-
return true, pr.GetNumber(), nil
248-
}
249-
}
243+
pullRequests = append(pullRequests, prs...)
250244

251245
// Check if there are more pages
252246
if resp.NextPage == 0 {
@@ -255,8 +249,22 @@ func (v *Provider) isCommitPartOfPullRequest(ctx context.Context, sha, org, repo
255249
opts.Page = resp.NextPage
256250
}
257251

252+
return pullRequests, nil
253+
}
254+
255+
// isCommitPartOfPullRequest checks if the commit from a push event is part of an open pull request
256+
// If it is, it returns true and the PR number.
257+
func (v *Provider) isCommitPartOfPullRequest(sha, org, repo string, prs []*github.PullRequest) (bool, int) {
258+
// Check if any of the returned PRs are open
259+
for _, pr := range prs {
260+
if pr.GetState() == "open" {
261+
v.Logger.Debugf("Commit %s is part of open PR #%d in %s/%s", sha, pr.GetNumber(), org, repo)
262+
return true, pr.GetNumber()
263+
}
264+
}
265+
258266
v.Logger.Debugf("Commit %s is not part of any open pull request in %s/%s", sha, org, repo)
259-
return false, 0, nil
267+
return false, 0
260268
}
261269

262270
func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt any) (*info.Event, error) {
@@ -327,12 +335,17 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
327335
org := gitEvent.GetRepo().GetOwner().GetLogin()
328336
repoName := gitEvent.GetRepo().GetName()
329337

330-
// Only check if the flag is enabled
331-
if v.pacInfo.SkipPushEventForPRCommits {
332-
isPartOfPR, prNumber, err := v.isCommitPartOfPullRequest(ctx, sha, org, repoName)
333-
if err != nil {
334-
v.Logger.Warnf("Error checking if push commit is part of PR: %v", err)
335-
}
338+
// First get all the pull requests associated with this commit so that we can reuse the output to check
339+
// whether the commit is included in any PR or not, and if this push is generated on PR merge event, we can
340+
// assign PR number to `pull_request_number` variable.
341+
prs, err := v.getPullRequestsWithCommit(ctx, sha, org, repoName)
342+
if err != nil {
343+
v.Logger.Warnf("Error getting pull requests associated with the commit in this push event: %v", err)
344+
}
345+
346+
// Only check if the flag is enabled and there are pull requests associated with this commit.
347+
if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 {
348+
isPartOfPR, prNumber := v.isCommitPartOfPullRequest(sha, org, repoName, prs)
336349

337350
// If the commit is part of a PR, skip processing the push event
338351
if isPartOfPR {
@@ -341,6 +354,12 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
341354
}
342355
}
343356

357+
// if there are pull requests associated with this commit, first pull request number will be used
358+
// for `pull_request_number` dynamic variable.
359+
if len(prs) > 0 {
360+
processedEvent.PullRequestNumber = *prs[0].Number
361+
}
362+
344363
processedEvent.Organization = gitEvent.GetRepo().GetOwner().GetLogin()
345364
processedEvent.Repository = gitEvent.GetRepo().GetName()
346365
processedEvent.DefaultBranch = gitEvent.GetRepo().GetDefaultBranch()

0 commit comments

Comments
 (0)