Red Hat Konflux update lvm-operator-bundle-4-23#2369
Red Hat Konflux update lvm-operator-bundle-4-23#2369red-hat-konflux[bot] wants to merge 1 commit into
Conversation
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
WalkthroughTwo Tekton ChangesLVM Operator Bundle Build Pipelines (PR and Push)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: red-hat-konflux[bot] The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.tekton/lvm-operator-bundle-4-23-pull-request.yaml:
- Line 5: The metadata annotation build.appstudio.openshift.io/repo is hardcoded
to openshift/lvm-operator causing fork PRs to record the wrong repo; change the
annotation value to use the template variable ({{source_url}}) that supplies the
actual git URL (and keep the {{revision}} token as-is), updating occurrences of
build.appstudio.openshift.io/repo in the file so the repo annotation matches the
cloned source for branch/PR builds.
- Around line 9-12: The on-cel-expression currently allows all pull_request
events to run; update pipelinesascode.tekton.dev/on-cel-expression to include an
authorization/maintainer gate so only approved PRs trigger runs (either use
PaC's native authorization feature or extend the CEL filter). Modify the
expression referenced by pipelinesascode.tekton.dev/on-cel-expression to require
maintainer membership or an explicit approval token/comment (for example add a
condition like author_association == "MEMBER" || comment =~ "ok-to-test" or
integrate the PaC authorization variable) so PR runs are blocked until the
maintainer gating is satisfied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 551e49bc-834a-4fd6-b481-b8745493b131
📒 Files selected for processing (2)
.tekton/lvm-operator-bundle-4-23-pull-request.yaml.tekton/lvm-operator-bundle-4-23-push.yaml
| kind: PipelineRun | ||
| metadata: | ||
| annotations: | ||
| build.appstudio.openshift.io/repo: https://github.com/openshift/lvm-operator?rev={{revision}} |
There was a problem hiding this comment.
Keep the repo annotation aligned with the actual clone source.
git-url is sourced from {{source_url}}, but the metadata repo is hardcoded to openshift/lvm-operator. Fork PRs will publish build metadata for a different repo than the one actually cloned.
Suggested fix
- build.appstudio.openshift.io/repo: https://github.com/openshift/lvm-operator?rev={{revision}}
+ build.appstudio.openshift.io/repo: '{{source_url}}?rev={{revision}}'Also applies to: 21-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.tekton/lvm-operator-bundle-4-23-pull-request.yaml at line 5, The metadata
annotation build.appstudio.openshift.io/repo is hardcoded to
openshift/lvm-operator causing fork PRs to record the wrong repo; change the
annotation value to use the template variable ({{source_url}}) that supplies the
actual git URL (and keep the {{revision}} token as-is), updating occurrences of
build.appstudio.openshift.io/repo in the file so the repo annotation matches the
cloned source for branch/PR builds.
| pipelinesascode.tekton.dev/cancel-in-progress: "true" | ||
| pipelinesascode.tekton.dev/max-keep-runs: "3" | ||
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | ||
| == "main" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Pipelines as Code: how do I require a /ok-to-testcomment before a PipelineRun is allowed to start for pull requests, and what trigger or annotation pattern should be used instead of a plainevent == "pull_request" CEL filter?
💡 Result:
To require a /ok-to-test comment before a PipelineRun starts in Tekton, you should use the built-in capabilities of Pipelines-as-Code or the GitHub Interceptor in Tekton Triggers, rather than manually constructing CEL filters for comment parsing [1][2]. Pipelines-as-Code (PaC) If you are using Pipelines-as-Code, this functionality is native [1]. When an unauthorized user (e.g., a contributor who is not an organization member or repository collaborator) opens a pull request, PaC automatically blocks the PipelineRun and posts a pending status check [1]. Authorized users can then initiate the PipelineRun by commenting /ok-to-test on the pull request [1]. Tekton Triggers (GitHub Interceptor) If you are using Tekton Triggers directly, you should use the GitHub Interceptor with the owners validation feature enabled [2]. This interceptor is designed to handle security gating for pull requests [3][2]. 1. Configuration: Configure your EventListener to use the GitHub Interceptor [2]. 2. Enable Owners Validation: Set the githubOwners parameter to enabled: true [2]. 3. Event Types: Ensure your interceptor is configured to listen for both pull_request and issue_comment events, as both are required for this workflow [2]. Example configuration snippet: triggers: - name: github-listener interceptors: - ref: name: "github" kind: ClusterInterceptor params: - name: "eventTypes" value: ["pull_request", "issue_comment"] - name: "githubOwners" value: enabled: true checkType: none Why avoid manual CEL filters? Using a plain event == "pull_request" CEL filter is insufficient because it does not provide the necessary security context (e.g., verifying if the commenter is an authorized owner) or the state management required to gate execution [2]. Manual CEL filters are prone to errors when parsing complex JSON payloads from webhooks and do not natively handle the "pending" status or the re-triggering logic that the GitHub Interceptor provides out-of-the-box [2][4][5]. Using the dedicated GitHub Interceptor ensures that your pipeline respects repository ownership and security policies automatically [3][2].
Citations:
- 1: https://pipelinesascode.com/docs/guide/running/
- 2: https://tekton.dev/docs/triggers/interceptors/
- 3: Github Interceptor to support Owners File Validation tektoncd/triggers#1516
- 4: cel filter error tektoncd/triggers#952
- 5: Validation for special characters for eventlistener cel filters tektoncd/triggers#746
🏁 Script executed:
# Read the full file to understand the context and check for PaC-specific annotations
cat -n .tekton/lvm-operator-bundle-4-23-pull-request.yaml | head -50Repository: openshift/lvm-operator
Length of output: 2888
🏁 Script executed:
# Continue reading the file and also check for any PR objectives or documentation
cat -n .tekton/lvm-operator-bundle-4-23-pull-request.yaml | tail -n +50 | head -50Repository: openshift/lvm-operator
Length of output: 2128
🏁 Script executed:
# Also search for PR objectives or any comment mentioning /ok-to-test requirement
find . -name "*.md" -o -name "*.txt" | xargs grep -l "ok-to-test" 2>/dev/null | head -5Repository: openshift/lvm-operator
Length of output: 48
🏁 Script executed:
# Check if there's any documentation or PR description files
git log --oneline --all -20 -- .tekton/lvm-operator-bundle-4-23-pull-request.yaml 2>/dev/null || echo "Git log not available"Repository: openshift/lvm-operator
Length of output: 122
🏁 Script executed:
# Check the commit that added this file for any message about /ok-to-test requirement
git show 8441793 --stat | head -30Repository: openshift/lvm-operator
Length of output: 1685
🏁 Script executed:
# Search the repository for any documentation about PipelineRun gates or /ok-to-test
rg "ok-to-test" --type md --type yaml -i 2>/dev/null | head -10Repository: openshift/lvm-operator
Length of output: 48
🏁 Script executed:
# Compare with other pull-request PipelineRun files to see if they have different CEL expressions
find .tekton -name "*pull-request.yaml" -exec basename {} \; | sortRepository: openshift/lvm-operator
Length of output: 252
🏁 Script executed:
# Check one of the other pull-request YAML files to see their CEL expressions
rg "on-cel-expression" .tekton/*.yaml -A 2Repository: openshift/lvm-operator
Length of output: 3957
🏁 Script executed:
# Look for PaC documentation or configuration in the repo that mentions authorization/gating
rg "pipelinesascode" . --type yaml -i -A 3 | head -50Repository: openshift/lvm-operator
Length of output: 48
🏁 Script executed:
# Search for any PaC documentation or configuration in the repository that explains how authorization works
find . -name "README*" -o -name "*.md" | xargs grep -l "PipelineRun\|Pipelines as Code" 2>/dev/null | head -3Repository: openshift/lvm-operator
Length of output: 48
🏁 Script executed:
# Check if there are any EventListener or other Tekton Triggers configurations that might handle /ok-to-test
find . -name "*.yaml" -o -name "*.yml" | xargs rg "EventListener\|GitHub" 2>/dev/null | head -10Repository: openshift/lvm-operator
Length of output: 48
🏁 Script executed:
# Look at the specific file again more carefully - check for any auth-related service account bindings
cat -n .tekton/lvm-operator-bundle-4-23-pull-request.yaml | grep -A 5 "serviceAccountName\|build-pipeline"Repository: openshift/lvm-operator
Length of output: 270
Add authorization check to the CEL expression for PR runs.
The plain event == "pull_request" && target_branch == "main" filter triggers on all PRs without the /ok-to-test comment gate. Use PaC's native authorization mechanism or add an additional CEL condition to enforce the maintainer gating described in the PR objective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.tekton/lvm-operator-bundle-4-23-pull-request.yaml around lines 9 - 12, The
on-cel-expression currently allows all pull_request events to run; update
pipelinesascode.tekton.dev/on-cel-expression to include an
authorization/maintainer gate so only approved PRs trigger runs (either use
PaC's native authorization feature or extend the CEL filter). Modify the
expression referenced by pipelinesascode.tekton.dev/on-cel-expression to require
maintainer membership or an explicit approval token/comment (for example add a
condition like author_association == "MEMBER" || comment =~ "ok-to-test" or
integrate the PaC authorization variable) so PR runs are blocked until the
maintainer gating is satisfied.
|
/retest |
|
@red-hat-konflux[bot]: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/retest |
Pipelines as Code configuration proposal
To start the PipelineRun, add a new comment with content
/ok-to-testFor more detailed information about running a PipelineRun, please refer to Pipelines as Code documentation Running the PipelineRun
To customize the proposed PipelineRuns after merge, please refer to Build Pipeline customization
Please follow the block sequence indentation style introduced by the proprosed PipelineRuns YAMLs, or keep using consistent indentation level through your customized PipelineRuns. When different levels are mixed, it will be changed to the proposed style.
Summary by CodeRabbit