-
Notifications
You must be signed in to change notification settings - Fork 117
feat: Enable cel: template expressions #2363
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
base: main
Are you sure you want to change the base?
feat: Enable cel: template expressions #2363
Conversation
CEL evaluation marshaled the event body then unmarshaled into a map. When
body was nil (or marshaled to JSON null), json.Unmarshal failed with
"cannot unmarshal null into map[string]interface{}". This caused cel:
placeholders to be replaced with an empty string, breaking pac-only
expressions in contexts without a webhook payload.
Treat nil/null body as an empty object for CEL evaluation and add a unit
test to cover pac-only expressions with a nil body.
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Assisted-by: GPT-5.2 (via Cursor)
Introduced the `cel:` prefix to allow evaluating complex Common Expression Language (CEL) expressions within PipelineRun templates. Enabled access to the `body`, `headers`, `files`, or `pac` namespaces to facilitate advanced logic like ternary operations, safe field access with `has()`, or collection processing. Ensured expressions return an empty string upon evaluation or syntax errors to prevent pipeline disruptions. Added comprehensive documentation, unit tests, plus integration tests for various use cases. E2E Tests was added for Gitea, GitHub and GitLab to validate the feature. Jira: https://issues.redhat.com/browse/SRVKP-8619 Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Summary of ChangesHello @chmouel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the templating capabilities by integrating Common Expression Language (CEL) expressions. This allows users to embed complex logic directly into templates, making them more powerful and flexible for various use cases like conditional environment selection or safe access to optional fields. Additionally, it addresses a critical bug to ensure CEL expressions function reliably even when no webhook body is available, improving the overall robustness of the system. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 PR Lint Feedback
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a powerful new feature, the cel: prefix for template expressions, which significantly enhances the expressiveness of templates. It also commendably includes a fix to ensure CEL evaluation works correctly even without a webhook payload. The implementation is robust, backed by a comprehensive suite of unit and end-to-end tests across multiple Git providers. The documentation has also been updated to reflect these new capabilities. I have a couple of minor suggestions to further refine the code and documentation.
| ```yaml | ||
| params: | ||
| - name: run-go-tests | ||
| value: "{{ cel: files.all.exists(f, f.endsWith(\".go\")) ? \"true\" : \"false\" }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ternary operator ? \"true\" : \"false\" is redundant in this CEL expression. Boolean results from CEL expressions are automatically converted to the strings "true" or "false" during template rendering. Simplifying the expression makes the example cleaner and demonstrates a more idiomatic usage of the new feature.
| value: "{{ cel: files.all.exists(f, f.endsWith(\".go\")) ? \"true\" : \"false\" }}" | |
| value: "{{ cel: files.all.exists(f, f.endsWith(\".go\")) }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not so sure about that, i let this be explicit
| if jsonMap == nil { | ||
| jsonMap = map[string]any{} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check for jsonMap == nil is redundant. The json.Unmarshal function, when unmarshaling a JSON object into a *map[string]any, allocates a new map if the pointer is nil. The case where the JSON content is null (which would make jsonMap nil after unmarshaling) is already handled on line 54. Therefore, if json.Unmarshal succeeds at this point, jsonMap is guaranteed to be non-nil. Removing this check simplifies the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd rather be explicit than caught by surprise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enables the use of cel: prefix in templates for evaluating arbitrary CEL expressions with access to body, headers, files, and pac namespaces. It also includes a critical bug fix that prevents CEL evaluation failures when no webhook payload body is present (nil/null), allowing PAC-only expressions to work consistently.
Key changes:
- Added
cel:prefix support for inline CEL expressions in templates, enabling conditional logic, string operations, and safe field access without requiring precomputed PAC parameters - Fixed CEL evaluation to treat nil/null webhook bodies as empty objects instead of failing, enabling reliable evaluation of PAC-only expressions
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cel/cel.go | Enhanced body handling to treat nil/null values as empty objects, enabling CEL evaluation without webhook payloads |
| pkg/templates/templating.go | Added cel: prefix support for arbitrary CEL expressions with access to pac namespace via dico parameter |
| pkg/cel/cel_test.go | Added test for pac-only queries with nil body |
| pkg/templates/templating_test.go | Added comprehensive unit tests covering cel: prefix functionality including ternary expressions, pac namespace access, has() function, files access, headers access, and error handling |
| test/testdata/pipelinerun-cel-prefix-test.yaml | Test pipeline for Gitea demonstrating cel: prefix with various expression types |
| test/testdata/pipelinerun-cel-prefix-gitlab.yaml | Test pipeline for GitLab demonstrating cel: prefix with GitLab-specific expressions |
| test/testdata/pipelinerun-cel-prefix-github.yaml | Test pipeline for GitHub demonstrating cel: prefix with GitHub-specific expressions |
| test/testdata/TestGiteaParamsCelPrefix.golden | Expected output for Gitea cel: prefix test |
| test/testdata/TestGitlabMergeRequestCelPrefix.golden | Expected output for GitLab cel: prefix test |
| test/testdata/TestGithubPullRequestCelPrefix.golden | Expected output for GitHub cel: prefix test |
| test/gitlab_merge_request_test.go | E2E test validating cel: prefix expressions in GitLab merge request context |
| test/github_pullrequest_test.go | E2E test validating cel: prefix expressions in GitHub pull request context |
| test/gitea_params_test.go | E2E test validating cel: prefix expressions in Gitea pull request context |
| docs/content/docs/guide/authoringprs.md | User documentation with examples of cel: prefix usage including conditionals, safe field access, file checks, and string concatenation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| _ = mr // use mr variable to avoid unused variable error |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable mr is fetched but never actually used in the test. The comment says it's to "avoid unused variable error", but this is unnecessary. If the variable isn't needed for the test logic, you should either remove the fetch operation entirely or use _, _, err := to explicitly ignore it. This fetch operation makes an extra API call that's not needed for the test.
| _ = mr // use mr variable to avoid unused variable error |
📝 Description of the Change
This PR adds support for evaluating CEL expressions inside templates using a
cel:prefix. It’s meant to make templates more expressive without forcing everything to be precomputed as PAC params: it allows doing simple conditionals, string ops, and presence checks right where the value is used.It also includes a small but important follow-up fix found during review: CEL evaluation used to fail when there was no webhook payload body (
rawEvent == nil). In that situation,cel:expressions would silently resolve to an empty string, which broke “pac-only” expressions (anything that only depends onpac.*,headers, orfiles). This PR makes CEL evaluation treat a missing body as an empty object so these expressions work consistently across all execution paths.🚀 Feature:
cel:expressions in templatesWith the
cel:prefix, let us evaluate arbitrary CEL expressions with access to:body: webhook JSON payload (when available)headers: request headersfiles: changed-files data (when available)pac: standard PAC parameters (pac.target_branch,pac.revision, …)revision,target_branch, …) mapped from PAC paramsWhat this unlocks in practice:
{{ cel: pac.target_branch == "main" ? "prod" : "dev" }}{{ cel: has(body.pull_request) ? body.pull_request.number : "" }}🐛 Fix: CEL works when there’s no webhook body
Previously, CEL evaluation marshaled the event body and unmarshaled it into a map for the
bodynamespace. WhenrawEventwasnil(or effectively JSONnull), JSON unmarshalling failed, and the templating layer would replacecel:results with an empty string.Now, a
nil/nullbody is treated as an empty object for CEL (body == {}), so expressions that only depend on PAC data (or headers/files) behave as expected, even when there is no incoming webhook payload.Example that now works reliably in PAC-only contexts:
{{ cel: pac.revision }}{{ cel: pac.target_branch == "main" ? "production" : "staging" }}👨🏻 Linked Jira
https://issues.redhat.com/browse/SRVKP-8619
🔗 Linked GitHub Issue
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
Important
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com
**💡You can use the script
./hack/add-llm-coauthor.shto automatically addthese co-author trailers to your commits.
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.