ci: enable copy-pr-bot and add helm chart validation#281
Open
saturley-hall wants to merge 3 commits into
Open
Conversation
Add .github/copy-pr-bot.yaml so the org-level copy-pr-bot App mirrors signed PR commits to canonical pull-request/<id> branches. Switch ci, codeql, copyright-checks, and trigger_ci workflows from pull_request: to push: pull-request/* so all PR CI (fork or internal) runs in trusted context against the mirror branch. lint-pr-title is left on pull_request: since it needs PR title metadata. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a helm-chart job that runs helm dependency build, helm lint against every values variant, helm template to catch rendering errors, and helm package. Upload Chart.yaml, rendered manifests, the packaged tarball, and a deduplicated images.txt as a helm-chart-dependencies artifact for future SBOM/CVE compliance work to consume. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
WalkthroughThis PR updates GitHub Actions workflow configurations to consolidate branch trigger patterns across multiple workflows, adds a new Helm chart validation job to the CI pipeline, and enables the copy PR bot configuration. ChangesGitHub Actions Workflow Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
19-20:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix the concurrency configuration for push-based PR workflow.
The concurrency block checks
github.event_name == 'pull_request', but this workflow no longer triggers onpull_requestevents. The condition will always be false for push events topull-request/*branches, which means:
- Each run gets a unique concurrency group (based on
run_id)- Previous runs won't be auto-cancelled
- The intended cancellation logic for PRs is broken
For push events to
pull-request/*branches, extract the PR number from the branch name.🔧 Proposed fix
concurrency: - group: ${{ github.event_name == 'pull_request' && format('{0}-{1}', github.workflow, github.event.pull_request.number) || format('{0}-{1}', github.workflow, github.run_id) }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: ${{ startsWith(github.ref, 'refs/heads/pull-request/') && format('{0}-pr-{1}', github.workflow, github.ref) || format('{0}-{1}', github.workflow, github.run_id) }} + cancel-in-progress: ${{ startsWith(github.ref, 'refs/heads/pull-request/') }}This groups runs by the full
pull-request/*branch ref and enables cancellation for those branches.🤖 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 @.github/workflows/ci.yml around lines 19 - 20, The concurrency group is using github.event_name == 'pull_request' which is always false for push-triggered runs on pull-request/* branches, so change the concurrency.group expression to handle both PR events and push-to-pull-request branches: detect either github.event_name == 'pull_request' or startsWith(github.ref, 'refs/heads/pull-request/'), and when it's a push to pull-request/* use the branch ref (e.g., github.ref) or extract the PR number from github.ref (split on '/' and take the last segment) to form a stable group key; keep cancel-in-progress true for the same condition so previous runs on the same pull-request branch are cancelled.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
251-253: 💤 Low valueImage extraction pattern may miss non-standard YAML formatting.
The grep/sed pipeline assumes a simple
image: <value>format. It may not capture images defined using YAML anchors, multiline strings, or complex quoting. Since this builds an informational artifact rather than critical functionality, the risk is low.Consider using a YAML-aware tool like
yqfor more robust extraction:Alternative approach using yq
- grep -hE '^\s*image:\s*' helm-artifacts/rendered/*.rendered.yaml \ - | sed -E 's/^[[:space:]]*image:[[:space:]]*"?([^"]+)"?[[:space:]]*$/\1/' \ - | sort -u > helm-artifacts/images.txt + yq eval '.. | select(has("image")) | .image' helm-artifacts/rendered/*.rendered.yaml \ + | sort -u > helm-artifacts/images.txt🤖 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 @.github/workflows/ci.yml around lines 251 - 253, The current grep/sed pipeline that pulls "image:" values from helm-artifacts/rendered/*.rendered.yaml is fragile and can miss images defined via YAML anchors, multiline strings, or complex quoting; replace that pipeline with a YAML-aware extraction using yq: parse all rendered YAML documents under helm-artifacts/rendered/*.rendered.yaml, recursively select all image keys (including within nested objects and arrays), collapse/flatten the results, deduplicate (sort -u) and write the output to helm-artifacts/images.txt so anchors, multiline values and varied quoting are handled correctly.
🤖 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 @.github/copy-pr-bot.yaml:
- Line 1: Add explicit optional config fields to make sync behavior clear: keep
enabled: true and add auto_sync_draft: false and auto_sync_ready: true (or your
chosen explicit values) so the intent is not left to defaults; update the
.github/copy-pr-bot.yaml YAML to include these keys alongside enabled and leave
signature verification out since the app handles it.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 19-20: The concurrency group is using github.event_name ==
'pull_request' which is always false for push-triggered runs on pull-request/*
branches, so change the concurrency.group expression to handle both PR events
and push-to-pull-request branches: detect either github.event_name ==
'pull_request' or startsWith(github.ref, 'refs/heads/pull-request/'), and when
it's a push to pull-request/* use the branch ref (e.g., github.ref) or extract
the PR number from github.ref (split on '/' and take the last segment) to form a
stable group key; keep cancel-in-progress true for the same condition so
previous runs on the same pull-request branch are cancelled.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 251-253: The current grep/sed pipeline that pulls "image:" values
from helm-artifacts/rendered/*.rendered.yaml is fragile and can miss images
defined via YAML anchors, multiline strings, or complex quoting; replace that
pipeline with a YAML-aware extraction using yq: parse all rendered YAML
documents under helm-artifacts/rendered/*.rendered.yaml, recursively select all
image keys (including within nested objects and arrays), collapse/flatten the
results, deduplicate (sort -u) and write the output to helm-artifacts/images.txt
so anchors, multiline values and varied quoting are handled correctly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d2798a5-1e35-4e6f-9883-5e3c9e520ea2
📒 Files selected for processing (5)
.github/copy-pr-bot.yaml.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/copyright-checks.yml.github/workflows/trigger_ci.yml
Contributor
|
Is this ready for review? |
Signed-off-by: Harrison Saturley-Hall <hsaturleyhal@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves OPS-6320.
Summary
copy-pr-botGitHub App by adding.github/copy-pr-bot.yaml. The bot mirrors signed PR commits (fork or internal) onto a canonicalpull-request/<id>branch in this repo.ci.yml,codeql.yml,copyright-checks.yml, andtrigger_ci.ymlfrompull_request:triggers topush:onpull-request/*so all PR CI runs against the trusted mirror branch.lint-pr-title.ymlstays onpull_request:because it needs PR-title metadata.helm-chartjob toci.ymlthat lints the chart with every values file variant, renders templates viahelm template, packages the chart, and uploadsChart.yaml, rendered manifests, the.tgz, and a deduplicatedimages.txtas ahelm-chart-dependenciesartifact for future SBOM/CVE compliance work to consume.Test plan
copy-pr-botmirrors this PR topull-request/<N>and posts the mirror branch name.ci,codeql,copyright-checks, andtrigger_ciworkflows trigger on the push topull-request/<N>and pass.lint-pr-titlestill runs on the originalpull_requestevent.helm-chart-dependenciesartifact: containsChart.yaml,rendered/*.rendered.yamlfor all 5 values files,package/modelexpress-0.3.0.tgz, andimages.txtlistingnvcr.io/nvidia/ai-dynamo/modelexpress-server:0.3.0,modelexpress:test,ghcr.io/grpc-ecosystem/grpc-health-probe:v0.4.35.Generated with Claude Code
Summary by CodeRabbit