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

feat: annotation for matching PipelineRun on paths #1823

Merged

Conversation

chmouel
Copy link
Member

@chmouel chmouel commented Nov 18, 2024

Introduce easier-to-use annotations for matching PipelineRuns by file
path changes:

  • on-path-change: Matches PipelineRun if specified paths have
    changed.
  • on-path-change-ignore: Matches PipelineRun if specified paths do
    not
    have changed.

Examples:

  1. on-path-change: ["pkg/*", "cli/*"] matches if files in pkg or
    cli changed.
  2. on-path-change-ignore: ["docs/**"] matches if no changes occurred
    in the docs directory.

Annotations can be combined for more specific use cases:

  • on-path-change: ["docs/**"]
  • on-path-change-ignore: ["docs/generated/**"]

This setup triggers a PipelineRun when there are changes in the docs
directory, except for files under docs/generated.

Enhanced annotation options also support:

  • Targeting specific events (on-target-event: e.g., pull_request,
    push).
  • Matching specific branches (on-target-branch: e.g., main).

This improves usability over existing CEL-based configuration and makes
defining file-based triggers more intuitive.

Jira: https://issues.redhat.com/browse/SRVKP-6464

Signed-off-by: Chmouel Boudjnah [email protected]

pac-path-change-annotation-matcher.mp4

Changes

Submitter Checklist

  • 📝 Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).

  • ♽ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.

  • ✨ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).

  • 📖 If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.

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

  • 🎁 If feasible, please check if an end-to-end test can be added. See README for more details.

  • 🔎 If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).

@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch from 461f054 to 7077b35 Compare November 18, 2024 17:05
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 48.14815% with 14 lines in your changes missing coverage. Please review.

Project coverage is 65.28%. Comparing base (c9f9c84) to head (ac9c222).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/matcher/annotation_matcher.go 48.14% 9 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1823      +/-   ##
==========================================
- Coverage   65.31%   65.28%   -0.04%     
==========================================
  Files         177      177              
  Lines       13647    13674      +27     
==========================================
+ Hits         8914     8927      +13     
- Misses       4134     4143       +9     
- Partials      599      604       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch 2 times, most recently from 504d84b to 8702392 Compare November 18, 2024 18:07
@chmouel chmouel marked this pull request as ready for review November 18, 2024 18:12
@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch from 8702392 to 88a70ce Compare November 18, 2024 18:24
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

Looking good, just one question.

docs/content/docs/guide/authoringprs.md Show resolved Hide resolved
@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch from 88a70ce to eff66b3 Compare November 19, 2024 11:12
@chmouel chmouel changed the title feat: add user-friendly annotations for PipelineRun path matching feat: annotation for matching PipelineRun on paths Nov 19, 2024
@chmouel
Copy link
Member Author

chmouel commented Nov 19, 2024

I have added the on-path-change-ignore in there as well, please review 🙏🏻

@zakisk
Copy link
Contributor

zakisk commented Nov 21, 2024

@chmouel I've used following annotation and in my PR .tekton/pipelinerun.yaml, README.md, and main.go files have been changed but it didn't triggered PipelineRun, is that expected behaviour?

pipelinesascode.tekton.dev/on-path-change: "[***.go]"
pipelinesascode.tekton.dev/on-path-change-ignore: "[***.md, ***.yaml]"

@chmouel
Copy link
Member Author

chmouel commented Nov 21, 2024

Yeah since a yaml file has changed and you ask to be ignored when that's the case , ignore would take precedence there

@zakisk
Copy link
Contributor

zakisk commented Nov 21, 2024

@chmouel I've used following annotation and in my PR .tekton/pipelinerun.yaml, README.md, and main.go files have been changed but it didn't triggered PipelineRun, is that expected behaviour?

pipelinesascode.tekton.dev/on-path-change: "[***.go]"
pipelinesascode.tekton.dev/on-path-change-ignore: "[***.md, ***.yaml]"

I think this case should be covered in docs as well, otherwise /LGTM.

@chmouel
Copy link
Member Author

chmouel commented Nov 21, 2024

@zakisk it's seem to be already covered, or do you have any suggestions to phrase this differently?

https://github.com/openshift-pipelines/pipelines-as-code/pull/1823/files#diff-1d4f1dfbcc9a40a58e7ed79ec136ebf92d103db6be536b6a81266dbc4a0ef304R199-R207

@zakisk
Copy link
Contributor

zakisk commented Nov 21, 2024

pipelinesascode.tekton.dev/on-path-change: "[pkg/***.go]"
pipelinesascode.tekton.dev/on-path-change-ignore: "[docs/***.md, k8s/***.yaml]"

If files of all extensions have been changed on an event specified in both annotations, on-path-change-ignore will take precedence and Pipelines as Code will act according to it.

@chmouel nice to have added this, you can rephrase if I missed something.

@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch from eff66b3 to 82ea98d Compare November 21, 2024 10:55
@chmouel
Copy link
Member Author

chmouel commented Nov 21, 2024

@zakisk sounds good, i think it duplicate with the paragraph written before but maybe i am too much neck down into it and to me what seems obvious may not appear like that for people who read this for the first time. I have rephrased that whole section please have a look.

Introduce easier-to-use annotations for matching PipelineRuns by file
path changes:
- `on-path-changed`: Matches PipelineRun if specified paths have
  changes.
- `on-path-changed-ignore`: Matches PipelineRun if specified paths **do
  not** have changes.

Examples:
1. `on-path-changed: ["pkg/*", "cli/*"]` matches if files in `pkg` or
   `cli` changed.
2. `on-path-changed-ignore: ["docs/**"]` matches if no changes occurred
   in the `docs` directory.

Annotations can be combined for more specific use cases:
- `on-path-changed: ["docs/**"]`
- `on-path-changed-ignore: ["docs/generated/**"]`

This setup triggers a PipelineRun when there are changes in the `docs`
directory, except for files under `docs/generated`.

Enhanced annotation options also support:
- Targeting specific events (`on-target-event`: e.g., `pull_request`,
  `push`).
- Matching specific branches (`on-target-branch`: e.g., `main`).

This improves usability over existing CEL-based configuration and makes
defining file-based triggers more intuitive.

**Jira**: https://issues.redhat.com/browse/SRVKP-6464

Signed-off-by: Chmouel Boudjnah <[email protected]>
@chmouel chmouel force-pushed the SRVKP-6464-annotation-on-path-change branch from 82ea98d to ac9c222 Compare November 21, 2024 10:57
@chmouel chmouel merged commit eb50fac into openshift-pipelines:main Nov 21, 2024
7 checks passed
@chmouel chmouel deleted the SRVKP-6464-annotation-on-path-change branch November 21, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants