Skip to content

fix: Triggering PipelineRun on label addition in Github PRs#2034

Merged
pipelines-as-code[bot] merged 1 commit intotektoncd:mainfrom
zakisk:SRVKP-7378-fix-pr-trigger-on-label-add-update
Apr 9, 2025
Merged

fix: Triggering PipelineRun on label addition in Github PRs#2034
pipelines-as-code[bot] merged 1 commit intotektoncd:mainfrom
zakisk:SRVKP-7378-fix-pr-trigger-on-label-add-update

Conversation

@zakisk
Copy link
Copy Markdown
Member

@zakisk zakisk commented Apr 9, 2025

fixed an issue where PAC is triggering PipelineRuns on label addition on PRs when on-label annotation is not explicitly used and on-cel-expression is being used for matching event in PipelineRun definition.

Now, before matching annotations it is checking for label_update event type and on-label annoation and if it doesn't present then ignoring the PipelineRun with a log message and still preserving on-cel-expression precendence over on-label.

https://issues.redhat.com/browse/SRVKP-7378
https://issues.redhat.com/browse/SRVKP-7443

Changes

Submitter Checklist

  • 📝 Ensure your commit message is clear and informative. Refer to the How to write a git commit message guide. Include the commit message in the PR body rather than linking to an external site (e.g., Jira ticket).

  • ♽ Run make test lint before submitting a PR to avoid unnecessary CI processing. Consider installing pre-commit and running pre-commit install in the repository root for an efficient workflow.

  • ✨ We use linters to maintain clean and consistent code. Run make lint before submitting a PR. Some linters offer a --fix mode, executable with make fix-linters (ensure markdownlint and golangci-lint are installed).

  • 📖 Document any user-facing features or changes in behavior.

  • 🧪 While 100% coverage isn't required, we encourage unit tests for code changes where possible.

  • 🎁 If feasible, add an end-to-end test. See README for details.

  • 🔎 Address any CI test flakiness before merging, or provide a valid reason to bypass it (e.g., token rate limitations).

  • If adding a provider feature, fill in the following details:

Git Provider Supported
GitHub App ✅️
GitHub Webhook ❌️
Gitea ❌️
GitLab ❌️
Bitbucket Cloud ❌️
Bitbucket Data Center ❌️

(update the documentation accordingly)

@zakisk zakisk force-pushed the SRVKP-7378-fix-pr-trigger-on-label-add-update branch from c4feb31 to 1e70375 Compare April 9, 2025 06:54
Comment thread test/github_pullrequest_test.go Outdated
Copy link
Copy Markdown
Member

@chmouel chmouel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, feel free to address the nit

@zakisk zakisk force-pushed the SRVKP-7378-fix-pr-trigger-on-label-add-update branch from 1e70375 to 3f52651 Compare April 9, 2025 07:35
@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Apr 9, 2025

/test

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Apr 9, 2025

Actually the issue is causing when on-cel-expression is used but in my E2E I was using on-event and on-target-branch annotations. (updated it)

@chmouel
Copy link
Copy Markdown
Member

chmouel commented Apr 9, 2025

@zakisk do you think we could test both? on-cel and with on-event and on-target-bvranch ?

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Apr 9, 2025

@zakisk do you think we could test both? on-cel and with on-event and on-target-bvranch ?

@chmouel we can, but as per changes made in this PR, we will get same log message as it does with on-cel-expression and if you mean testing on-label annotation then it is being tested already in this test.

fixed an issue where PAC is triggering PipelineRuns on label
addition on PRs when on-label annotation is not explicitly
used and on-cel-expression is being used for matching event
in PipelineRun definition.

Now, before matching annotations it is checking for label_update
event type and on-label annoation and if it doesn't present then
ignoring the PipelineRun with a log message and still preserving
on-cel-expression precendence over on-label.

https://issues.redhat.com/browse/SRVKP-7378
https://issues.redhat.com/browse/SRVKP-7443

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
@zakisk zakisk force-pushed the SRVKP-7378-fix-pr-trigger-on-label-add-update branch from 3f52651 to 0625a58 Compare April 9, 2025 09:43
@chmouel
Copy link
Copy Markdown
Member

chmouel commented Apr 9, 2025

/lgtm

Copy link
Copy Markdown

@pipelines-as-code pipelines-as-code bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats @zakisk your PR Has been approved 🎉

✅ Pull Request Approved

Approval Status:

  • Required Approvals: 1
  • Current Approvals: 1

👥 Reviewers Who Approved:

Reviewer Permission Level Approval Status
@chmouel admin

📝 Next Steps

  • Ensure all required checks pass
  • Comply with branch protection rules
  • Request a maintainer to merge using the /merge command (or merge it
    directly if you have repository permission).

Automated by the PAC Boussole 🧭

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Apr 9, 2025

/merge

@pipelines-as-code pipelines-as-code bot merged commit c1233eb into tektoncd:main Apr 9, 2025
6 of 10 checks passed
@pipelines-as-code
Copy link
Copy Markdown

✅ PR Successfully Merged

  • Merge method: rebase
  • Merged by: @zakisk
  • Total approvals: 1/1

Approvals Summary:

Reviewer Permission Status
@chmouel admin

Thank you @zakisk for your valuable contribution! 🎉

Automated by the PAC Boussole 🧭

@zakisk zakisk deleted the SRVKP-7378-fix-pr-trigger-on-label-add-update branch April 9, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants