Skip to content

Conversation

@chmouel
Copy link
Member

@chmouel chmouel commented Dec 18, 2025

📝 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 on pac.*, headers, or files). 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 templates

With the cel: prefix, let us evaluate arbitrary CEL expressions with access to:

  • body: webhook JSON payload (when available)
  • headers: request headers
  • files: changed-files data (when available)
  • pac: standard PAC parameters (pac.target_branch, pac.revision, …)
  • plus direct convenience variables (e.g. revision, target_branch, …) mapped from PAC params

What this unlocks in practice:

  • Branch/event-based selection:
    • {{ cel: pac.target_branch == "main" ? "prod" : "dev" }}
  • Safe access to optional payload fields:
    • {{ cel: has(body.pull_request) ? body.pull_request.number : "" }}
  • Readable, localized logic without adding extra params or custom glue code.

🐛 Fix: CEL works when there’s no webhook body

Previously, CEL evaluation marshaled the event body and unmarshaled it into a map for the body namespace. When rawEvent was nil (or effectively JSON null), JSON unmarshalling failed, and the templating layer would replace cel: results with an empty string.

Now, a nil/null body 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

  • 🐛 Bug fix (fix:)
  • ✨ New feature (feat:)
  • 💥 Breaking change (feat!:, fix!:)
  • 📚 Documentation update (docs:)
  • ⚙️ Chore (chore:)
  • 💅 Refactor (refactor:)
  • 🔧 Enhancement (enhance:)
  • 📦 Dependency update (deps:)

🧪 Testing Strategy

  • Unit tests
  • Integration tests
  • End-to-end tests
  • Manual testing
  • Not Applicable

🤖 AI Assistance

  • I have not used any AI assistance for this PR.
  • I have used AI assistance for this PR.

If you have used AI assistance, please provide the following details:

Which LLM was used?

  • GitHub Copilot
  • ChatGPT (OpenAI)
  • Claude (Anthropic)
  • Cursor
  • Gemini (Google)
  • Other: ____________

Extent of AI Assistance:

  • Documentation and research only
  • Unit tests or E2E tests only
  • Code generation (parts of the code)
  • Full code generation (most of the PR)
  • PR description and comments
  • Commit message(s)

Important

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer 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.sh to automatically add
these co-author trailers to your commits.

✅ Submitter Checklist

  • 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
  • ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
  • ♽ I have run make test and make lint locally to check for and fix any
    issues. For an efficient workflow, I have considered installing
    pre-commit and running pre-commit install to
    automate these checks.
  • 📖 I have added or updated documentation for any user-facing changes.
  • 🧪 I have added sufficient unit tests for my code changes.
  • 🎁 I have added end-to-end tests where feasible. See README for more details.
  • 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
  • If adding a provider feature, I have filled in the following and updated the provider documentation:
    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

chmouel and others added 2 commits December 18, 2025 18:08
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>
Copilot AI review requested due to automatic review settings December 18, 2025 11:13
@gemini-code-assist
Copy link

Summary of Changes

Hello @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

  • Feature: cel: prefix for templates: Introduces support for Common Expression Language (CEL) expressions directly within templates using a cel: prefix. This allows for more dynamic and expressive conditional logic, string operations, and presence checks, making templates more powerful without precomputing everything as PAC parameters.
  • Improved CEL evaluation for nil webhook bodies: Fixes an issue where CEL expressions would fail or silently resolve to an empty string when no webhook payload body (rawEvent == nil) was present. This ensures consistent and reliable behavior for PAC-only expressions that depend on pac.*, headers, or files.

🧠 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@pipelines-as-code
Copy link

🔍 PR Lint Feedback

Note: This automated check helps ensure your PR follows our contribution guidelines.

⚠️ Items that need attention:

📝 PR title format

Current title: Enable cel: template expressions

Expected format <type>(<scope>): <subject> with <type> one of [build, chore, ci, docs, deps, enhance, feat, dnm, fix, perf, refactor, release, revert, style, test].

Expected pattern: <type>(<scope>): <subject>
Allowed types: build, chore, ci, docs, deps, enhance, feat, dnm, fix, perf, refactor, release, revert, style, test

Examples:

  • fix(controller): ensure reconciler handles nil spec
  • docs: update contributing guide with lint instructions

ℹ️ Next Steps

  • Review and address the items above
  • Push new commits to update this PR
  • This comment will be automatically updated when issues are resolved
🔧 Admin Tools (click to expand)

Automated Issue/Ticket Creation:

  • /issue-create - Generate a GitHub issue from this PR content using AI
  • /jira-create - Create a SRVKP Jira ticket from this PR content using AI

⚠️ Important: Always review and edit generated content before finalizing tickets/issues.
The AI-generated content should be used as a starting point and may need adjustments.

These commands are available to maintainers and will post the generated content as PR comments for review.

🤖 This feedback was generated automatically by the PR CI system

@pipelines-as-code pipelines-as-code bot added documentation Improvements or additions to documentation e2e labels Dec 18, 2025
Copy link

@gemini-code-assist gemini-code-assist bot left a 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\" }}"

Choose a reason for hiding this comment

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

medium

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.

Suggested change
value: "{{ cel: files.all.exists(f, f.endsWith(\".go\")) ? \"true\" : \"false\" }}"
value: "{{ cel: files.all.exists(f, f.endsWith(\".go\")) }}"

Copy link
Member Author

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

Comment on lines +61 to +63
if jsonMap == nil {
jsonMap = map[string]any{}
}

Choose a reason for hiding this comment

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

medium

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.

Copy link
Member Author

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

@chmouel chmouel changed the title Enable cel: template expressions feat: Enable cel: template expressions Dec 18, 2025
Copy link
Contributor

Copilot AI left a 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.

Comment on lines +1040 to +1041

_ = mr // use mr variable to avoid unused variable error
Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
_ = mr // use mr variable to avoid unused variable error

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation e2e

Development

Successfully merging this pull request may close these issues.

1 participant