Implement RFC 0020: PR-based chart sync workflows - #41
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds manually triggered Helm chart synchronisation and deletion workflows that create pull requests, alongside pull-request validation for archives, provenance, indexes, duplicates, and synchronisation structure. Makefile support adds chart presence validation. ChangesHelm chart automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant GitHubWorkflow
participant HelmRepository
participant PullRequest
Operator->>GitHubWorkflow: dispatch chart synchronisation or deletion
GitHubWorkflow->>HelmRepository: update chart package and index
GitHubWorkflow->>GitHubWorkflow: validate changes
GitHubWorkflow->>PullRequest: create pull request
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
.github/workflows/pr-chart-sync.yaml (1)
37-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUpgrade
actions/setup-goto v6 in the chart workflows.
.github/workflows/pr-chart-sync.yaml.github/workflows/pr-chart-delete.yaml.github/workflows/validate-chart-pr.yaml🤖 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/pr-chart-sync.yaml around lines 37 - 40, Upgrade the setup-go action from v4 to v6 in the Go setup steps of .github/workflows/pr-chart-sync.yaml (lines 37-40), .github/workflows/pr-chart-delete.yaml (lines 32-35), and .github/workflows/validate-chart-pr.yaml (lines 14-17), preserving the existing Go 1.21.x version configuration.Source: Linters/SAST tools
🤖 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/workflows/pr-chart-sync.yaml:
- Around line 29-34: Centralize validation for dispatch-controlled chart
identifiers before any shell use: in .github/workflows/pr-chart-sync.yaml lines
29-34, validate chart, version, and download URL before deriving BRANCH_NAME or
executing commands; in .github/workflows/pr-chart-delete.yaml lines 25-29, apply
the same chart and version validation before checking or deleting files. Reuse
one shared validation mechanism rather than duplicating inconsistent checks.
In @.github/workflows/validate-chart-pr.yaml:
- Around line 24-27: Update the “Install yq” workflow step to download a
specific immutable yq release instead of the mutable latest URL, fetch the
corresponding published checksum, and verify the downloaded binary before moving
or installing it under /usr/local/bin with sudo. Ensure installation proceeds
only after checksum validation succeeds.
- Around line 118-127: Update the provenance check in the chart validation
workflow to use helm verify with the trusted keyring for each chart instead of
grepping for a PGP header. Make both missing provenance files and failed
cryptographic verification exit non-zero, while retaining a clear success
message only after verification succeeds.
- Around line 165-186: The workflow currently validates only chart-sync PR
structure; add a separate chart-delete validation step conditioned on the
chart-delete label. Using the changed-files deleted and modified file outputs,
require exactly one deleted .tgz archive, exactly one matching .tgz.prov file,
and exactly one modified index.yaml, failing with a clear validation message
otherwise.
- Around line 43-55: Update the index comparison in the workflow to remove the
generated timestamp using yq and compare the resulting YAML documents
structurally or in canonical YAML form. Replace the current grep-and-sort
process while preserving the existing inconsistency message, diff output, and
failure behavior.
- Around line 59-66: Update the “Validate chart packages” step to avoid
interpolating steps.changed-files.outputs.all_changed_files directly into shell
source. Pass the changed-file list through an environment variable or safely
decoded JSON array, then iterate over the resulting filenames while preserving
the existing .tgz and .tgz.prov filtering.
In `@Makefile`:
- Around line 68-76: Update the validate-chart target to inspect the located
chart archive with Helm, using helm show chart or equivalent to validate the
archive and verify its metadata name and version match CHART_NAME and
CHART_VERSION. Only print the success message and return successfully after
these checks pass; retain the existing missing-file failure behavior.
- Line 34: Correct the Helm download command in the Makefile’s helm target by
removing the spaces that split curl options and the download URL, while
preserving the existing HELM_VERSION, OS, and ARCH substitutions so make helm
can retrieve the archive.
---
Nitpick comments:
In @.github/workflows/pr-chart-sync.yaml:
- Around line 37-40: Upgrade the setup-go action from v4 to v6 in the Go setup
steps of .github/workflows/pr-chart-sync.yaml (lines 37-40),
.github/workflows/pr-chart-delete.yaml (lines 32-35), and
.github/workflows/validate-chart-pr.yaml (lines 14-17), preserving the existing
Go 1.21.x version configuration.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efd37523-9615-446e-ae6b-a9222f58186b
📒 Files selected for processing (4)
.github/workflows/pr-chart-delete.yaml.github/workflows/pr-chart-sync.yaml.github/workflows/validate-chart-pr.yamlMakefile
| env: | ||
| BROWSER_DOWNLOAD_URL: ${{ github.event.inputs.browser_download_url }} | ||
| CHART_NAME: ${{ github.event.inputs.chart }} | ||
| CHART_VERSION: ${{ github.event.inputs.version }} | ||
| COMPONENT_REPO: ${{ github.event.inputs.component_repo }} | ||
| BRANCH_NAME: "sync/${{ github.event.inputs.chart }}-${{ github.event.inputs.version }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Centralise validation of dispatch-controlled chart identifiers.
Both privileged workflows derive shell commands, paths and branch names from unchecked dispatch inputs.
.github/workflows/pr-chart-sync.yaml#L29-L34: validate chart, version and download URL before any shell use..github/workflows/pr-chart-delete.yaml#L25-L29: apply the same chart/version validation before checking or deleting files.
📍 Affects 2 files
.github/workflows/pr-chart-sync.yaml#L29-L34(this comment).github/workflows/pr-chart-delete.yaml#L25-L29
🤖 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/pr-chart-sync.yaml around lines 29 - 34, Centralize
validation for dispatch-controlled chart identifiers before any shell use: in
.github/workflows/pr-chart-sync.yaml lines 29-34, validate chart, version, and
download URL before deriving BRANCH_NAME or executing commands; in
.github/workflows/pr-chart-delete.yaml lines 25-29, apply the same chart and
version validation before checking or deleting files. Reuse one shared
validation mechanism rather than duplicating inconsistent checks.
Source: Linters/SAST tools
| - name: Install yq | ||
| run: | | ||
| sudo wget -qO /usr/local/bin/yq https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 | ||
| sudo chmod +x /usr/local/bin/yq |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin and verify the privileged yq download.
Downloading a mutable latest binary and installing it as root without checksum verification creates a supply-chain risk. Pin an immutable release and verify its published checksum before installation.
🤖 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/validate-chart-pr.yaml around lines 24 - 27, Update the
“Install yq” workflow step to download a specific immutable yq release instead
of the mutable latest URL, fetch the corresponding published checksum, and
verify the downloaded binary before moving or installing it under /usr/local/bin
with sudo. Ensure installation proceeds only after checksum validation succeeds.
| # Backup current index | ||
| cp charts/index.yaml charts/index.yaml.backup | ||
|
|
||
| # Regenerate index from chart packages | ||
| make helm-index | ||
|
|
||
| # Compare original and regenerated index (ignore generated timestamp) | ||
| if ! diff -u <(grep -v 'generated:' charts/index.yaml.backup | sort) <(grep -v 'generated:' charts/index.yaml | sort) > /tmp/index_diff.txt; then | ||
| echo "Helm index is inconsistent with chart packages" | ||
| echo "Index differences:" | ||
| cat /tmp/index_diff.txt | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Compare the index structurally rather than sorting YAML lines.
Sorting raw lines discards YAML hierarchy, so values moved between chart entries can still compare equal. Remove only the generated timestamp with yq, then compare the resulting structured documents or canonical YAML.
🤖 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/validate-chart-pr.yaml around lines 43 - 55, Update the
index comparison in the workflow to remove the generated timestamp using yq and
compare the resulting YAML documents structurally or in canonical YAML form.
Replace the current grep-and-sort process while preserving the existing
inconsistency message, diff output, and failure behavior.
| - name: Validate chart packages | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| run: | | ||
| echo "Validating chart packages..." | ||
|
|
||
| # Find new/modified .tgz files | ||
| for file in ${{ steps.changed-files.outputs.all_changed_files }}; do | ||
| if [[ "$file" == *.tgz && "$file" != *.tgz.prov ]]; then |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not interpolate PR-controlled filenames into shell source.
A changed filename containing shell syntax is inserted directly into the script at Line 65. Pass the action output through an environment variable or JSON output and iterate with a safely decoded array.
🧰 Tools
🪛 zizmor (1.26.1)
[info] 65-65: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/validate-chart-pr.yaml around lines 59 - 66, Update the
“Validate chart packages” step to avoid interpolating
steps.changed-files.outputs.all_changed_files directly into shell source. Pass
the changed-file list through an environment variable or safely decoded JSON
array, then iterate over the resulting filenames while preserving the existing
.tgz and .tgz.prov filtering.
Source: Linters/SAST tools
| # Check for corresponding provenance file | ||
| prov_file="${file}.prov" | ||
| if [[ -f "$prov_file" ]]; then | ||
| echo "Found provenance file: $prov_file" | ||
| if ! grep -q "BEGIN PGP SIGNED MESSAGE" "$prov_file"; then | ||
| echo "Provenance file appears invalid (no PGP signature found)" | ||
| fi | ||
| else | ||
| echo "No provenance file found for $file" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail unless provenance is present and cryptographically verified.
Searching for a PGP header does not validate the signature, and an invalid or missing provenance file only emits a message. Consequently, the workflow can pass while claiming provenance validation succeeded. Use helm verify with the trusted keyring and exit non-zero on failure.
🤖 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/validate-chart-pr.yaml around lines 118 - 127, Update the
provenance check in the chart validation workflow to use helm verify with the
trusted keyring for each chart instead of grepping for a PGP header. Make both
missing provenance files and failed cryptographic verification exit non-zero,
while retaining a clear success message only after verification succeeds.
| - name: Validate chart sync PR structure | ||
| if: contains(github.event.pull_request.labels.*.name, 'chart-sync') | ||
| run: | | ||
| echo "Validating chart sync PR structure..." | ||
|
|
||
| # For chart sync PRs, expect exactly one new chart and updated index | ||
| new_charts=$(echo "${{ steps.changed-files.outputs.added_files }}" | grep -c '\.tgz$' || true) | ||
| new_provs=$(echo "${{ steps.changed-files.outputs.added_files }}" | grep -c '\.tgz\.prov$' || true) | ||
| modified_index=$(echo "${{ steps.changed-files.outputs.modified_files }}" | grep -c 'index\.yaml$' || true) | ||
|
|
||
| echo "Changes detected:" | ||
| echo " - New chart packages: $new_charts" | ||
| echo " - New provenance files: $new_provs" | ||
| echo " - Modified index: $modified_index" | ||
|
|
||
| if [[ "$new_charts" -eq 1 && "$new_provs" -eq 1 && "$modified_index" -eq 1 ]]; then | ||
| echo "Chart sync PR structure is valid" | ||
| else | ||
| echo "Invalid chart sync PR structure" | ||
| echo " Expected: 1 chart, 1 provenance file, 1 index update" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add structural validation for chart-delete PRs.
Deletion PRs are auto-merged, but this workflow only constrains chart-sync changes. A labelled deletion PR can remove additional packages and still pass if the regenerated index is consistent. Require exactly one deleted archive, its provenance file, and one modified index.
🧰 Tools
🪛 zizmor (1.26.1)
[info] 171-171: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 172-172: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 173-173: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/validate-chart-pr.yaml around lines 165 - 186, The
workflow currently validates only chart-sync PR structure; add a separate
chart-delete validation step conditioned on the chart-delete label. Using the
changed-files deleted and modified file outputs, require exactly one deleted
.tgz archive, exactly one matching .tgz.prov file, and exactly one modified
index.yaml, failing with a clear validation message otherwise.
4876c63 to
4b2d568
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
.github/workflows/validate-chart-pr.yaml (5)
39-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winLine-sort index comparison discards YAML structure — duplicate of prior finding.
Unchanged from the earlier review: sorting raw lines can mask real reordering-induced differences between chart entries.
🤖 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/validate-chart-pr.yaml around lines 39 - 57, Update the “Validate Helm index consistency” step to compare the backup and regenerated index as parsed YAML structures rather than sorting raw lines. Continue ignoring the generated timestamp, while preserving detection of meaningful chart-entry additions, removals, or changes regardless of ordering.
59-66: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnsafe interpolation of changed-file list into shell — duplicate of prior finding.
${{ steps.changed-files.outputs.all_changed_files }}is still interpolated directly into theforloop source at line 65, unchanged from the earlier review.🤖 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/validate-chart-pr.yaml around lines 59 - 66, Update the “Validate chart packages” shell loop to avoid directly interpolating steps.changed-files.outputs.all_changed_files into shell source. Pass the changed-file list through a safe environment variable or file-based mechanism, then iterate over it without allowing filenames to alter command parsing; preserve the existing .tgz and .tgz.prov filtering.
118-127: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winProvenance check only greps for a PGP header — duplicate of prior finding.
Still no cryptographic verification (
helm verify); a missing/invalid provenance file only logs a message rather than failing the job.🤖 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/validate-chart-pr.yaml around lines 118 - 127, Update the provenance validation block for each chart file to perform actual cryptographic verification with Helm, using the chart and its corresponding .prov file, instead of only grepping for a PGP header. Make missing provenance files and failed verification exit the validation job with a nonzero status, while retaining clear diagnostic messages.
24-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnpinned, unverified
yqbinary install — duplicate of prior finding.Downloading
latestoverwgetand installing as root with no checksum verification is unchanged from the earlier review.🤖 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/validate-chart-pr.yaml around lines 24 - 27, Update the “Install yq” workflow step to download a specific pinned yq release rather than the mutable latest URL, and verify the downloaded binary against its published checksum before installing it to /usr/local/bin/yq. Preserve executable permissions while ensuring installation occurs only after checksum validation.
165-186: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNo structural validation for auto-merged
chart-deletePRs — duplicate of prior finding.Unchanged: only
chart-sync-labelled PRs get structural constraints here; achart-deletePR can still auto-merge with unexpected extra changes.🤖 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/validate-chart-pr.yaml around lines 165 - 186, Extend the “Validate chart sync PR structure” workflow step to also validate PRs labeled “chart-delete”, applying the appropriate structural constraints for deletion PRs and rejecting unexpected extra changes before auto-merge. Preserve the existing chart-sync validation behavior and use the existing changed-files outputs for the checks.Makefile (1)
67-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
validate-chartonly checks the archive exists, not that it's valid or matches its filename — duplicate of prior finding.Unchanged from the earlier review: a corrupt
.tgz, or one whoseChart.yamlname/version disagrees with$(CHART_NAME)-$(CHART_VERSION), still prints a success message. This target is the CI gatepr-chart-sync.yamlrelies on before auto-merge, so the gap has real downstream impact.Note also that
$(CHART_NAME)/$(CHART_VERSION)are textually expanded into the recipe by Make itself (same injection surface as the workflow-level${{ }}issue) — validating the input format upstream in the calling workflow covers this too.♻️ Proposed direction
validate-chart: $(HELM) ## Validate a specific chart package `@echo` "Validating chart $(CHART_NAME)-$(CHART_VERSION)..." `@chart_file`="./charts/$(CHART_NAME)-$(CHART_VERSION).tgz"; \ if [[ ! -f "$$chart_file" ]]; then \ echo "Chart file not found: $$chart_file"; \ exit 1; \ - fi; \ - echo "✓ Chart file exists: $$chart_file" + fi; \ + name=$$($(HELM) show chart "$$chart_file" | yq eval '.name' -); \ + version=$$($(HELM) show chart "$$chart_file" | yq eval '.version' -); \ + if [[ "$$name" != "$(CHART_NAME)" || "$$version" != "$(CHART_VERSION)" ]]; then \ + echo "Chart metadata mismatch: got $$name-$$version"; \ + exit 1; \ + fi; \ + echo "✓ Chart file valid: $$chart_file"🤖 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 `@Makefile` around lines 67 - 76, Extend the validate-chart target beyond the existence check: validate that the archive is a readable Helm chart and inspect its Chart.yaml metadata, requiring its name and version to match CHART_NAME and CHART_VERSION before printing success. Preserve the existing missing-file failure behavior and ensure validation failures exit nonzero so pr-chart-sync.yaml cannot proceed with corrupt or mismatched archives..github/workflows/pr-chart-sync.yaml (1)
29-34: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnvalidated dispatch inputs (including
browser_download_url) interpolated via${{ }}throughout the job — duplicate of prior finding.Same root cause as flagged previously on this exact range:
CHART_NAME,CHART_VERSION,BROWSER_DOWNLOAD_URLare uncheckedworkflow_dispatchstrings, and everyrun:step below (51, 57-59, 68-69, 86-91, 96, 104-116, 131, 141, 147-148, 154) re-expands them with${{ env.X }}, which is literal text substitution vulnerable to injection under the privilegedKUADRANT_DEV_PAT.
browser_download_urlspecifically adds an SSRF angle: it's passed unchecked tomake get-chart, which fetches it — an attacker with dispatch access could point this at an internal/arbitrary endpoint. Validate the URL against an allow-listed host (e.g.github.com/objects.githubusercontent.com) and the chart/version charset before use, and switchrun:steps to reference$CHART_NAME/$CHART_VERSION/$BROWSER_DOWNLOAD_URLinstead of${{ env.X }}.🤖 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/pr-chart-sync.yaml around lines 29 - 34, Validate workflow_dispatch values before they reach the job: restrict CHART_NAME and CHART_VERSION to the permitted charset, and validate BROWSER_DOWNLOAD_URL as an HTTPS URL whose host is allow-listed to trusted GitHub download domains. In every run step using these values, reference the shell environment variables $CHART_NAME, $CHART_VERSION, and $BROWSER_DOWNLOAD_URL rather than re-expanding env.X through GitHub expression interpolation, while preserving the existing sync flow and KUADRANT_DEV_PAT usage.
🤖 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/workflows/pr-chart-delete.yaml:
- Around line 25-29: Validate the workflow_dispatch values before any shell
command uses them, restricting chart, version, and component_repo to the
required safe character set and failing on invalid input. In all downstream run
steps, including the path construction and git/registry commands, replace `${{
env.CHART_NAME }}`, `${{ env.CHART_VERSION }}`, `${{ env.COMPONENT_REPO }}`, and
related interpolations with the already-exported shell variables `$CHART_NAME`,
`$CHART_VERSION`, and `$COMPONENT_REPO`; keep BRANCH_NAME similarly
shell-expanded.
- Around line 115-153: Add a step before the “Create Pull Request” step to
ensure the chart-delete and automated labels exist in the repository, using the
workflow’s GH_TOKEN authentication and creating only missing labels. Keep the
existing gh pr create invocation and label assignments unchanged.
In @.github/workflows/pr-chart-sync.yaml:
- Line 1: Add the missing chart-sync, chart-delete, and automated labels before
the PR creation steps in both workflows, pr-chart-sync.yaml and
pr-chart-delete.yaml. Ensure each workflow creates or verifies these labels
before invoking gh pr create so label assignment succeeds.
In @.github/workflows/validate-chart-pr.yaml:
- Line 195: Update the chart validation PR comment text so the Markdown heading
in the workflow uses a space after the ## prefix, changing “##Chart Validation
Passed” to a properly formatted H2 while preserving the heading text.
- Around line 29-33: Update the tj-actions/changed-files reference in the “Get
changed files” workflow step from the mutable `@v41` tag to a full immutable
commit SHA, using the intended trusted release commit while preserving the
existing files: charts/** configuration.
---
Duplicate comments:
In @.github/workflows/pr-chart-sync.yaml:
- Around line 29-34: Validate workflow_dispatch values before they reach the
job: restrict CHART_NAME and CHART_VERSION to the permitted charset, and
validate BROWSER_DOWNLOAD_URL as an HTTPS URL whose host is allow-listed to
trusted GitHub download domains. In every run step using these values, reference
the shell environment variables $CHART_NAME, $CHART_VERSION, and
$BROWSER_DOWNLOAD_URL rather than re-expanding env.X through GitHub expression
interpolation, while preserving the existing sync flow and KUADRANT_DEV_PAT
usage.
In @.github/workflows/validate-chart-pr.yaml:
- Around line 39-57: Update the “Validate Helm index consistency” step to
compare the backup and regenerated index as parsed YAML structures rather than
sorting raw lines. Continue ignoring the generated timestamp, while preserving
detection of meaningful chart-entry additions, removals, or changes regardless
of ordering.
- Around line 59-66: Update the “Validate chart packages” shell loop to avoid
directly interpolating steps.changed-files.outputs.all_changed_files into shell
source. Pass the changed-file list through a safe environment variable or
file-based mechanism, then iterate over it without allowing filenames to alter
command parsing; preserve the existing .tgz and .tgz.prov filtering.
- Around line 118-127: Update the provenance validation block for each chart
file to perform actual cryptographic verification with Helm, using the chart and
its corresponding .prov file, instead of only grepping for a PGP header. Make
missing provenance files and failed verification exit the validation job with a
nonzero status, while retaining clear diagnostic messages.
- Around line 24-27: Update the “Install yq” workflow step to download a
specific pinned yq release rather than the mutable latest URL, and verify the
downloaded binary against its published checksum before installing it to
/usr/local/bin/yq. Preserve executable permissions while ensuring installation
occurs only after checksum validation.
- Around line 165-186: Extend the “Validate chart sync PR structure” workflow
step to also validate PRs labeled “chart-delete”, applying the appropriate
structural constraints for deletion PRs and rejecting unexpected extra changes
before auto-merge. Preserve the existing chart-sync validation behavior and use
the existing changed-files outputs for the checks.
In `@Makefile`:
- Around line 67-76: Extend the validate-chart target beyond the existence
check: validate that the archive is a readable Helm chart and inspect its
Chart.yaml metadata, requiring its name and version to match CHART_NAME and
CHART_VERSION before printing success. Preserve the existing missing-file
failure behavior and ensure validation failures exit nonzero so
pr-chart-sync.yaml cannot proceed with corrupt or mismatched archives.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8befd41-3cec-4344-b8a5-c6f31aeafde9
📒 Files selected for processing (4)
.github/workflows/pr-chart-delete.yaml.github/workflows/pr-chart-sync.yaml.github/workflows/validate-chart-pr.yamlMakefile
| env: | ||
| CHART_NAME: ${{ github.event.inputs.chart }} | ||
| CHART_VERSION: ${{ github.event.inputs.version }} | ||
| COMPONENT_REPO: ${{ github.event.inputs.component_repo }} | ||
| BRANCH_NAME: "delete/${{ github.event.inputs.chart }}-${{ github.event.inputs.version }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unvalidated dispatch inputs still interpolated via ${{ }} throughout the job — duplicate of prior finding.
CHART_NAME/CHART_VERSION/COMPONENT_REPO come straight from workflow_dispatch inputs with no format check, and every downstream run: step (lines 45, 50-51, 73-74, 101-108, 121-132, 150, 160, 166-167, 173, 180) re-expands them via ${{ env.X }} rather than reading the already-exported shell variable. GitHub performs literal text substitution for ${{ }} before the shell parses the script, so a value containing a quote/backtick can inject arbitrary commands that run with the privileged KUADRANT_DEV_PAT. It also enables path traversal (e.g. CHART_NAME=../../secret) into the charts/$CHART_NAME-$CHART_VERSION.tgz path built at line 50.
Add a validation step (regex-check chart/version for a safe charset) before any shell use, and switch the run: steps to reference $CHART_NAME, $CHART_VERSION, etc. (they're already exported via env:) instead of ${{ env.X }}.
🤖 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/pr-chart-delete.yaml around lines 25 - 29, Validate the
workflow_dispatch values before any shell command uses them, restricting chart,
version, and component_repo to the required safe character set and failing on
invalid input. In all downstream run steps, including the path construction and
git/registry commands, replace `${{ env.CHART_NAME }}`, `${{ env.CHART_VERSION
}}`, `${{ env.COMPONENT_REPO }}`, and related interpolations with the
already-exported shell variables `$CHART_NAME`, `$CHART_VERSION`, and
`$COMPONENT_REPO`; keep BRANCH_NAME similarly shell-expanded.
| - name: Create Pull Request | ||
| if: steps.chart-check.outputs.chart_exists == 'true' && steps.changes.outputs.has_changes == 'true' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.KUADRANT_DEV_PAT }} | ||
| run: | | ||
| gh pr create \ | ||
| --title "Remove chart: ${{ env.CHART_NAME }} v${{ env.CHART_VERSION }}" \ | ||
| --body "$(cat <<EOF | ||
| ## Chart Deletion | ||
|
|
||
| This PR removes a chart package from the repository. | ||
|
|
||
| - **Chart**: ${{ env.CHART_NAME }} | ||
| - **Version**: ${{ env.CHART_VERSION }} | ||
| - **Source Repository**: ${{ env.COMPONENT_REPO }} | ||
|
|
||
| ### Changes | ||
| - Removed chart package: \`${{ env.CHART_NAME }}-${{ env.CHART_VERSION }}.tgz\` | ||
| - Removed provenance file: \`${{ env.CHART_NAME }}-${{ env.CHART_VERSION }}.tgz.prov\` | ||
| - Updated Helm repository index | ||
|
|
||
| ### Validation | ||
| This PR will be validated by CI to ensure: | ||
| - Helm index consistency after deletion | ||
| - No broken references remain | ||
| - Deletion is clean and complete | ||
|
|
||
| ### Warning | ||
| This action will permanently remove the chart from the public repository. | ||
| Ensure this deletion is intentional and authorized. | ||
|
|
||
| --- | ||
| *This PR was created automatically by the chart deletion workflow.* | ||
| EOF | ||
| )" \ | ||
| --head "${{ env.BRANCH_NAME }}" \ | ||
| --base main \ | ||
| --label "chart-delete" \ | ||
| --label "automated" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh label list --repo "$(gh repo view --json nameWithOwner -q .nameWithOwner)" | grep -iE 'chart-delete|automated'Repository: Kuadrant/helm-charts
Length of output: 158
Create the PR labels before this step
gh pr create --label fails if chart-delete or automated do not already exist in the repository. Add a label-creation step, or ensure these labels are provisioned elsewhere, before this workflow reaches PR creation.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 121-121: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 121-121: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 127-127: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 128-128: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 129-129: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 132-132: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 132-132: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 132-132: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 132-132: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 150-150: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/pr-chart-delete.yaml around lines 115 - 153, Add a step
before the “Create Pull Request” step to ensure the chart-delete and automated
labels exist in the repository, using the workflow’s GH_TOKEN authentication and
creating only missing labels. Keep the existing gh pr create invocation and
label assignments unchanged.
| @@ -0,0 +1,155 @@ | |||
| name: PR Chart Sync | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh label list --repo "$(gh repo view --json nameWithOwner -q .nameWithOwner)" | grep -iE 'chart-sync|chart-delete|automated'Repository: Kuadrant/helm-charts
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- workflow label usage ---\n'
grep -nE 'chart-sync|chart-delete|automated|gh pr create|--label' .github/workflows/pr-chart-sync.yaml .github/workflows/pr-chart-delete.yaml || true
printf '\n--- label definitions in repo files ---\n'
rg -n --hidden --glob '!.git' --glob '!node_modules' --glob '!dist' --glob '!build' '^(chart-sync|chart-delete|automated)$|name:\s*(chart-sync|chart-delete|automated)\b|labels:' .Repository: Kuadrant/helm-charts
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant workflow lines ---'
sed -n '90,145p' .github/workflows/pr-chart-sync.yaml
printf '\n'
sed -n '108,165p' .github/workflows/pr-chart-delete.yaml
echo '\n--- repo label definitions ---'
fd -a -t f -I 'labels.*' .
rg -n --hidden --glob '!.git' --glob '!node_modules' --glob '!dist' --glob '!build' 'chart-sync|chart-delete|automated' .Repository: Kuadrant/helm-charts
Length of output: 5154
Add the missing PR labels gh pr create --label requires pre-existing labels. chart-sync, chart-delete, and automated are not defined, so both workflows will fail at PR creation.
pr-chart-sync.yaml#L98-L134pr-chart-delete.yaml#L115-L153
🤖 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/pr-chart-sync.yaml at line 1, Add the missing chart-sync,
chart-delete, and automated labels before the PR creation steps in both
workflows, pr-chart-sync.yaml and pr-chart-delete.yaml. Ensure each workflow
creates or verifies these labels before invoking gh pr create so label
assignment succeeds.
| - name: Get changed files | ||
| id: changed-files | ||
| uses: tj-actions/changed-files@v41 | ||
| with: | ||
| files: charts/** |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
[skip_cloning]
# Confirm current recommended/patched version and whether SHA-pinning guidance appliesRepository: Kuadrant/helm-charts
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow using the action
sed -n '1,120p' .github/workflows/validate-chart-pr.yaml
# Check whether this repository already pins similar actions by SHA
rg -n "uses: .+@v|uses: .+@[0-9a-f]{7,}" .github/workflows .github -g '!**/*.md'
# Look for any local notes about workflow security or action pinning
rg -n "changed-files|pin.*SHA|supply chain|CVE-2025-30066|dorny/paths-filter" .Repository: Kuadrant/helm-charts
Length of output: 6632
🌐 Web query:
tj-actions/changed-files March 2025 CVE-2025-30066 GitHub Actions SHA pinning recommendation
💡 Result:
CVE-2025-30066 was a supply chain attack that occurred on March 14–15, 2025, involving the popular GitHub Action tj-actions/changed-files [1][2]. During this incident, threat actors compromised the repository and retroactively modified existing version tags (v1 through v45.0.7) to point to a malicious commit (0e58ed8) [1][2][3]. This malicious code was designed to exfiltrate CI/CD secrets by printing them into GitHub Actions logs [1][4]. The core issue exploited was the mutable nature of Git tags, which allow a tag to be updated to reference different commit SHAs over time [5][6]. Because many users reference GitHub Actions by version tags (e.g., @v45), their workflows automatically pulled the compromised code without the users' knowledge [6]. To mitigate this risk and prevent similar supply chain attacks, the primary recommendation is to pin all third-party GitHub Actions to a full, immutable 40-character commit SHA [5][6][7]. Pinning to a commit SHA provides the following security benefits: * Immutability: Unlike a tag, a commit SHA is content-addressed and cannot be silently repointed by an attacker [5]. Once you have audited and pinned a specific SHA, your workflow will continue to use that exact version regardless of future modifications to the repository's tags [5][6]. * Operational Security: If an action repository is compromised, your workflows will remain on the last known-good SHA you pinned, protecting you from malicious updates pushed by attackers [5][6]. To manage the maintenance burden of SHA pinning, security experts and GitHub documentation recommend using automation tools like Dependabot or Renovate [8][5][6]. These tools can automatically keep your pinned SHAs updated by opening pull requests when new, legitimate releases are published, allowing you to review the changes before merging [5][6]. Furthermore, GitHub now provides policy features at the organization and enterprise level to enforce SHA pinning, ensuring that all workflows meet these security requirements [8].
Citations:
- 1: GHSA-mw4p-6x4p-x5m5
- 2: https://osv.dev/vulnerability/CVE-2025-30066
- 3: https://www.tenable.com/cve/CVE-2025-30066
- 4: GHSA-mrrh-fwg8-r2c3
- 5: https://safeguard.sh/resources/blog/tj-actions-changed-files-github-actions-supply-chain-compromise
- 6: https://emmer.dev/blog/pin-your-github-actions-to-protect-against-mutability/
- 7: https://www.opsmx.com/blog/github-actions-exploit-cve-2025-30066-what-this-means-for-your-software-supply-chain/
- 8: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
Pin tj-actions/changed-files to a full commit SHA. @v41 is mutable, and this action has a documented 2025 supply-chain compromise history; using an immutable SHA avoids a repeat of that attack pattern.
🤖 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/validate-chart-pr.yaml around lines 29 - 33, Update the
tj-actions/changed-files reference in the “Get changed files” workflow step from
the mutable `@v41` tag to a full immutable commit SHA, using the intended trusted
release commit while preserving the existing files: charts/** configuration.
297e829 to
4493f68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/pr-chart-delete.yaml (2)
159-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify auto-merge by targeting the branch directly.
The GitHub CLI allows merging a PR by referencing its head branch directly. This eliminates the need for an additional API call to look up the PR number, and prevents potential race conditions if the GitHub search index delays making the newly created PR immediately available.
.github/workflows/pr-chart-delete.yaml#L159-L161: simplify togh pr merge "${{ env.BRANCH_NAME }}" --auto --squash..github/workflows/pr-chart-sync.yaml#L138-L140: simplify togh pr merge "${{ env.BRANCH_NAME }}" --auto --squash.🤖 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/pr-chart-delete.yaml around lines 159 - 161, Update the auto-merge steps in .github/workflows/pr-chart-delete.yaml lines 159-161 and .github/workflows/pr-chart-sync.yaml lines 138-140 to pass "${{ env.BRANCH_NAME }}" directly to gh pr merge with --auto --squash; remove the gh pr list lookup and PR_NUMBER variable at both sites.
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
actions/setup-goto avoid deprecated runners.Version
v4ofactions/setup-goruns on Node 16, which is deprecated by GitHub Actions and will cause workflow warnings or eventual failures. Update tov5, which uses Node 20.
.github/workflows/pr-chart-delete.yaml#L32-L33: change toactions/setup-go@v5..github/workflows/pr-chart-sync.yaml#L37-L38: change toactions/setup-go@v5..github/workflows/validate-chart-pr.yaml#L14-L15: change toactions/setup-go@v5.🤖 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/pr-chart-delete.yaml around lines 32 - 33, Update the actions/setup-go reference from v4 to v5 in .github/workflows/pr-chart-delete.yaml lines 32-33, .github/workflows/pr-chart-sync.yaml lines 37-38, and .github/workflows/validate-chart-pr.yaml lines 14-15; leave the surrounding workflow configuration unchanged.Source: Linters/SAST tools
🤖 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/workflows/pr-chart-delete.yaml:
- Around line 37-38: Replace the nonexistent actions/checkout@v6 reference with
actions/checkout@v4 in the Checkout steps of
.github/workflows/pr-chart-delete.yaml (lines 37-38),
.github/workflows/pr-chart-sync.yaml (lines 43-44), and
.github/workflows/validate-chart-pr.yaml (lines 19-20).
In @.github/workflows/validate-chart-pr.yaml:
- Around line 171-173: Update the added_files and modified_files counting
commands for new_charts, new_provs, and modified_index to translate the
space-separated file lists into newline-separated entries before applying grep
-c, so each matching filename is counted individually.
---
Nitpick comments:
In @.github/workflows/pr-chart-delete.yaml:
- Around line 159-161: Update the auto-merge steps in
.github/workflows/pr-chart-delete.yaml lines 159-161 and
.github/workflows/pr-chart-sync.yaml lines 138-140 to pass "${{ env.BRANCH_NAME
}}" directly to gh pr merge with --auto --squash; remove the gh pr list lookup
and PR_NUMBER variable at both sites.
- Around line 32-33: Update the actions/setup-go reference from v4 to v5 in
.github/workflows/pr-chart-delete.yaml lines 32-33,
.github/workflows/pr-chart-sync.yaml lines 37-38, and
.github/workflows/validate-chart-pr.yaml lines 14-15; leave the surrounding
workflow configuration unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79bd9f4f-82cd-4fcb-a035-a45beced6b82
📒 Files selected for processing (4)
.github/workflows/pr-chart-delete.yaml.github/workflows/pr-chart-sync.yaml.github/workflows/validate-chart-pr.yamlMakefile
| - name: Checkout | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Downgrade actions/checkout to a valid version.
The actions/checkout@v6 action does not exist (the latest major version is v4). Specifying a non-existent version will cause the workflow to fail immediately upon initialization.
.github/workflows/pr-chart-delete.yaml#L37-L38: change toactions/checkout@v4..github/workflows/pr-chart-sync.yaml#L43-L44: change toactions/checkout@v4..github/workflows/validate-chart-pr.yaml#L19-L20: change toactions/checkout@v4.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 37-41: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 3 files
.github/workflows/pr-chart-delete.yaml#L37-L38(this comment).github/workflows/pr-chart-sync.yaml#L43-L44.github/workflows/validate-chart-pr.yaml#L19-L20
🤖 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/pr-chart-delete.yaml around lines 37 - 38, Replace the
nonexistent actions/checkout@v6 reference with actions/checkout@v4 in the
Checkout steps of .github/workflows/pr-chart-delete.yaml (lines 37-38),
.github/workflows/pr-chart-sync.yaml (lines 43-44), and
.github/workflows/validate-chart-pr.yaml (lines 19-20).
4493f68 to
020d447
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/workflows/pr-chart-delete.yaml:
- Around line 47-66: The chart-check step currently detects only the archive, so
deletion is skipped when provenance or index artefacts remain. Update the chart
existence logic in the “Check if chart exists” step to detect the chart archive,
its .prov file, and the corresponding index.yaml entry, setting
chart_exists=true and preserving the deletion/regeneration path when any
artefact exists.
- Around line 32-35: Upgrade the actions/setup-go reference from v4 to v6 in
every affected workflow: .github/workflows/pr-chart-delete.yaml lines 32-35,
.github/workflows/validate-chart-pr.yaml lines 14-17,
.github/workflows/sync-chart-created.yaml lines 33,
.github/workflows/sync-chart-deleted.yaml lines 28, and
.github/workflows/pr-chart-sync.yaml lines 38. Preserve the existing Go version
configuration.
In @.github/workflows/pr-chart-sync.yaml:
- Around line 37-41: Update the setup-go step in the PR chart sync workflow from
actions/setup-go@v4 to actions/setup-go@v6, preserving the existing Go version
and step configuration.
In @.github/workflows/validate-chart-pr.yaml:
- Around line 3-6: The workflow currently executes PR-controlled Makefile
targets with persisted checkout credentials and mixes validation with write
access. Update the checkout steps in the chart validation jobs to set
persist-credentials to false, and split PR commenting into a separate job with
the required write permission while keeping validation jobs read-only.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57818307-a645-47b3-9b0b-4369e810fa55
📒 Files selected for processing (4)
.github/workflows/pr-chart-delete.yaml.github/workflows/pr-chart-sync.yaml.github/workflows/validate-chart-pr.yamlMakefile
| - name: Set up Go 1.21.x | ||
| uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: 1.21.x |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== setup-go usages ==\n'
rg -n "actions/setup-go@" .github/workflows || true
printf '\n== pr-chart-delete.yaml ==\n'
sed -n '1,120p' .github/workflows/pr-chart-delete.yaml
printf '\n== validate-chart-pr.yaml ==\n'
sed -n '1,120p' .github/workflows/validate-chart-pr.yamlRepository: Kuadrant/helm-charts
Length of output: 8911
🌐 Web query:
actions/setup-go v4 node runtime v6 node24 release note supported current runners actionlint
💡 Result:
The actions/setup-go action has transitioned to using Node.js 24 as its runtime in its latest major version, v6 [1][2]. Key details regarding this update and related requirements: 1. Node 24 Requirement: The v6 edition of actions/setup-go utilizes Node.js 24 [1][2]. This upgrade required changes to the action's codebase, including stricter type checking [3]. The action's action.yml file has been updated to specify node24 to ensure correct execution [4]. 2. Runner Compatibility: To support the Node.js 24 runtime, your GitHub Actions runner must be version v2.327.1 or later [5][1]. Using an older runner version that does not support Node.js 24 will result in compatibility issues [1][2]. 3. v4 vs. v6: The v4 version of actions/setup-go, which preceded v6, operated on older Node.js runtimes (such as node16 or node20, depending on the specific v4 release/patch) [6][1]. If you are currently using v4 and wish to leverage the latest improvements and Node.js 24 runtime, you must upgrade to the v6 major release [1][2]. 4. Actionlint: While actionlint is a static analysis tool for GitHub Actions workflow files, it generally tracks updates to the platform. Ensure your local version of actionlint is up-to-date to correctly validate workflows that specify newer action versions or utilize modern features like node24 runtimes. For official details, refer to the actions/setup-go repository README and release notes [7][1].
Citations:
- 1: https://github.com/actions/setup-go/blob/main/README.md
- 2: https://github.com/actions/setup-go?tab=readme-ov-file
- 3: Node 24 upgrade actions/setup-go#624
- 4: Fix Node.js version in action.yml actions/setup-go#691
- 5: https://github.com/actions/setup-go
- 6: https://gitea.psi.ch/actions/setup-go/src/commit/40f1582b2485089dde7abd97c1529aa768e1baff
- 7: https://github.com/actions/setup-go/releases
🌐 Web query:
actions/setup-go v4 runtime node16 node20 node24 supported versions release notes
💡 Result:
The actions/setup-go action has transitioned through several Node.js runtime versions as it has evolved, with specific major versions tied to these upgrades: - actions/setup-go v5: Upgraded the runtime from Node 16 to Node 20 [1]. - actions/setup-go v6: Upgraded the runtime from Node 20 to Node 24 [2]. When using v6, it is required that your GitHub Actions runner is on version v2.327.1 or later to ensure compatibility [3][2]. The action does not support Node 16 in its current versions, as that runtime has reached its end of life and was removed in favor of newer Node.js versions [4][1].
Citations:
- 1: https://github.com/actions/setup-go/releases/tag/v5.0.0
- 2: https://github.com/actions/setup-go/releases/tag/v6.0.0
- 3: https://github.com/actions/setup-go?tab=readme-ov-file
- 4: feat: upgrade to use node 20 runtime and bump checkout action to v4 actions/setup-go#421
🌐 Web query:
site:github.com/actions/setup-go/releases v6 node24 setup-go v4 node20
💡 Result:
The actions/setup-go GitHub Action transitioned its underlying Node.js runtime from Node.js 20 to Node.js 24 with the release of version 6.0.0 [1][2]. Key details regarding these runtime updates include: - Version 6.0.0: Upgraded the Node.js runtime from version 20 to version 24 [1]. To ensure compatibility with this release, users must be running GitHub Actions runner version 2.327.1 or later [1][2]. - Version 5.0.0: Prior to this, the action upgraded its runtime from Node.js 16 to Node.js 20 [3]. These changes are part of the maintenance of the setup-go action to ensure compatibility with modern Node.js environments [1][3].
Citations:
- 1: https://github.com/actions/setup-go/releases/tag/v6.0.0
- 2: https://github.com/actions/setup-go/releases
- 3: https://github.com/actions/setup-go/releases/tag/v5.0.0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== actionlint references ==\n'
rg -n "actionlint|setup-go@v|setup-go" .github . || true
printf '\n== workflow runner images ==\n'
rg -n "runs-on:" .github/workflows || trueRepository: Kuadrant/helm-charts
Length of output: 1080
🌐 Web query:
actions/setup-go v4 node16 deprecation runner support current releases supported
💡 Result:
As of July 14, 2026, actions/setup-go v4 is considered deprecated because it relies on the Node.js 16 runtime, which reached its end-of-life on GitHub Actions runners on November 12, 2024 [1][2]. Key points regarding runner support and action versions: 1. Node.js 16 Deprecation: Node.js 16 has been fully removed from GitHub Actions runners [1][2]. Actions that were designed for Node.js 16 are no longer supported and may fail or produce deprecation warnings when executed in modern runner environments. 2. Node.js 20 Deprecation: Following the migration from Node.js 16, GitHub also initiated the deprecation of Node.js 20 [3][4]. As of mid-2026, Node.js 24 is the current standard for GitHub Actions [5][3], and Node.js 20 support is scheduled to be fully removed later in 2026 [4]. 3. Recommended Action Versions: Users should migrate to the latest major version of actions/setup-go to ensure compatibility with current runner runtimes (Node.js 24) [5]. The V6 edition of actions/setup-go explicitly includes an upgrade to the Node.js 24 runtime [5]. 4. Runner Compatibility: To ensure full compatibility with modern actions, workflows should be running on the latest available GitHub Actions runners, which support current Node.js standards [5][3]. For security and stability, it is strongly recommended to update any workflows still using actions/setup-go v4 to the latest version (v6 or newer) [5].
Citations:
- 1: https://github.blog/changelog/2024-09-25-end-of-life-for-actions-node16/
- 2: Remove node16 from the runner. actions/runner#3503
- 3: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- 4: https://github.com/orgs/community/discussions/189324
- 5: https://github.com/actions/setup-go?tab=readme-ov-file
Upgrade the actions/setup-go pins. v4 is deprecated because it relies on Node 16, which is no longer supported on GitHub Actions runners. Update every workflow using it to actions/setup-go@v6:
.github/workflows/pr-chart-delete.yaml#L33.github/workflows/validate-chart-pr.yaml#L15.github/workflows/sync-chart-created.yaml#L33.github/workflows/sync-chart-deleted.yaml#L28.github/workflows/pr-chart-sync.yaml#L38
🧰 Tools
🪛 actionlint (1.7.12)
[error] 33-33: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📍 Affects 2 files
.github/workflows/pr-chart-delete.yaml#L32-L35(this comment).github/workflows/validate-chart-pr.yaml#L14-L17
🤖 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/pr-chart-delete.yaml around lines 32 - 35, Upgrade the
actions/setup-go reference from v4 to v6 in every affected workflow:
.github/workflows/pr-chart-delete.yaml lines 32-35,
.github/workflows/validate-chart-pr.yaml lines 14-17,
.github/workflows/sync-chart-created.yaml lines 33,
.github/workflows/sync-chart-deleted.yaml lines 28, and
.github/workflows/pr-chart-sync.yaml lines 38. Preserve the existing Go version
configuration.
Source: Linters/SAST tools
| - name: Check if chart exists | ||
| id: chart-check | ||
| run: | | ||
| chart_file="charts/${{ env.CHART_NAME }}-${{ env.CHART_VERSION }}.tgz" | ||
| prov_file="charts/${{ env.CHART_NAME }}-${{ env.CHART_VERSION }}.tgz.prov" | ||
|
|
||
| if [[ -f "$chart_file" ]]; then | ||
| echo "chart_exists=true" >> $GITHUB_OUTPUT | ||
| echo "chart_file=$chart_file" >> $GITHUB_OUTPUT | ||
| echo "prov_file=$prov_file" >> $GITHUB_OUTPUT | ||
| echo "Found chart: $chart_file" | ||
| if [[ -f "$prov_file" ]]; then | ||
| echo "Found provenance: $prov_file" | ||
| else | ||
| echo "Provenance file not found: $prov_file" | ||
| fi | ||
| else | ||
| echo "chart_exists=false" >> $GITHUB_OUTPUT | ||
| echo "Chart not found: $chart_file" | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not skip deletion when only provenance or index artefacts remain.
The workflow reports “not found” whenever the archive is absent, even if its .prov file or index.yaml entry remains. Detect all three artefact types and run deletion/index regeneration when any exists.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 50-50: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 50-50: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 51-51: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 51-51: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/pr-chart-delete.yaml around lines 47 - 66, The chart-check
step currently detects only the archive, so deletion is skipped when provenance
or index artefacts remain. Update the chart existence logic in the “Check if
chart exists” step to detect the chart archive, its .prov file, and the
corresponding index.yaml entry, setting chart_exists=true and preserving the
deletion/regeneration path when any artefact exists.
| run: | | ||
| git checkout -b "${{ env.BRANCH_NAME }}" | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent script injection by using native shell variables.
Using GitHub Actions expressions (${{ env.VAR }}) inside inline bash scripts interpolates the values before execution, which allows for script injection if the environment variables contain malicious or unexpected characters. Ensure you use native shell variables (e.g., "$VAR") which safely pass the values.
.github/workflows/pr-chart-sync.yaml#L50-L52: Replace"${{ env.BRANCH_NAME }}"with"$BRANCH_NAME"..github/workflows/pr-chart-sync.yaml#L53-L60: Replace${{ env.VAR }}interpolation with$VARequivalents for the download URL, chart name, and version..github/workflows/pr-chart-sync.yaml#L65-L70: Replace${{ env.VAR }}interpolation with$VARequivalents for the chart name and version..github/workflows/pr-chart-sync.yaml#L80-L92: Replace${{ env.VAR }}interpolations inside thegit commitmessage with$VAR..github/workflows/pr-chart-sync.yaml#L93-L97: Replace"${{ env.BRANCH_NAME }}"with"$BRANCH_NAME"..github/workflows/pr-chart-sync.yaml#L98-L133: Replace${{ env.VAR }}interpolations in thegh pr createcommand and its heredoc body with$VARequivalents (using${VAR}inside backticks to preserve heredoc structure)..github/workflows/pr-chart-sync.yaml#L145-L152: Replace${{ env.VAR }}interpolations in theechostatements with$VAR..github/workflows/pr-chart-sync.yaml#L153-L158: Replace${{ env.VAR }}interpolations in theechostatements with$VAR.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 51-51: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
📍 Affects 1 file
.github/workflows/pr-chart-sync.yaml#L50-L52(this comment).github/workflows/pr-chart-sync.yaml#L53-L60.github/workflows/pr-chart-sync.yaml#L65-L70.github/workflows/pr-chart-sync.yaml#L80-L92.github/workflows/pr-chart-sync.yaml#L93-L97.github/workflows/pr-chart-sync.yaml#L98-L133.github/workflows/pr-chart-sync.yaml#L145-L152.github/workflows/pr-chart-sync.yaml#L153-L158
Source: Linters/SAST tools
1c3a7e4 to
06305bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/pr-chart-delete.yaml (4)
32-35: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUpgrade the
actions/setup-gopins.
v4is deprecated because it relies on Node 16, which is no longer supported on GitHub Actions runners. Update this toactions/setup-go@v6.🛠 Proposed fix
- name: Set up Go 1.21.x - uses: actions/setup-go@v4 + uses: actions/setup-go@v6 with:🤖 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/pr-chart-delete.yaml around lines 32 - 35, Update the setup-go action in the “Set up Go 1.21.x” workflow step from actions/setup-go@v4 to actions/setup-go@v6, keeping the existing go-version unchanged.Source: Linters/SAST tools
53-53: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not skip deletion when only provenance or index artefacts remain.
The workflow currently skips the deletion process if the chart archive (
.tgz) is absent, even if its.provfile remains or it is still listed inindex.yaml. Check for all artefact types so the repository is cleanly synchronised.🛠 Proposed fix
- if [[ -f "$chart_file" ]]; then + if [[ -f "$chart_file" ]] || [[ -f "$prov_file" ]] || { [[ -f "charts/index.yaml" ]] && grep -q "$CHART_NAME" charts/index.yaml; }; then🤖 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/pr-chart-delete.yaml at line 53, Update the deletion guard in the workflow’s chart cleanup logic to proceed when any related artefact exists: the chart archive, its provenance file, or its entry in index.yaml. Preserve the existing deletion behavior while ensuring cleanup is not skipped when only .prov or index.yaml artefacts remain.
37-38: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winDowngrade
actions/checkoutto a valid version.The
actions/checkout@v6action does not exist (the latest major version isv4). Specifying a non-existent version will cause the workflow to fail immediately upon initialisation.🛠 Proposed fix
- name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v4🤖 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/pr-chart-delete.yaml around lines 37 - 38, Update the Checkout step’s actions/checkout reference from the unavailable v6 release to the valid v4 major version, leaving the rest of the workflow unchanged.
25-29: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy liftUnvalidated dispatch inputs still interpolated via
${{ }}throughout the job.
CHART_NAME,CHART_VERSION, andCOMPONENT_REPOcome straight fromworkflow_dispatchinputs with no format check. Downstreamrun:steps re-expand them via${{ env.X }}rather than reading the already-exported shell variables. GitHub performs literal text substitution for${{ }}before the shell parses the script, which allows arbitrary command injection and path traversal.Add a validation step to restrict these inputs to a safe character set before any shell command runs, and switch all downstream
run:blocks to use standard shell variable syntax (e.g.,"$CHART_NAME") instead of${{ env.CHART_NAME }}.🛡️ Proposed input validation step
Add this validation step directly after the checkout step, and update all subsequent
run:blocks to use"$CHART_NAME","$CHART_VERSION","$COMPONENT_REPO", and"$BRANCH_NAME".- name: Validate inputs run: | if [[ ! "$CHART_NAME" =~ ^[a-zA-Z0-9.-]+$ ]] || [[ ! "$CHART_VERSION" =~ ^[a-zA-Z0-9.-]+$ ]]; then echo "Invalid chart name or version." exit 1 fi🤖 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/pr-chart-delete.yaml around lines 25 - 29, Add a validation step immediately after checkout that rejects CHART_NAME and CHART_VERSION unless they match the safe alphanumeric, dot, and hyphen pattern, before any other shell command runs. Update every subsequent run block to reference the exported shell variables "$CHART_NAME", "$CHART_VERSION", "$COMPONENT_REPO", and "$BRANCH_NAME" instead of GitHub expression interpolation such as "${{ env.* }}"; preserve the existing workflow behavior after validation.
🧹 Nitpick comments (1)
.github/workflows/validate-chart-pr.yaml (1)
14-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
actions/setup-gotov6.The
v4action uses a deprecated Node.js 20 runtime, which will trigger runner warnings. Update tov6to use the supported Node.js 24 runtime.💻 Proposed fix
- uses: actions/setup-go@v4 + uses: actions/setup-go@v6🤖 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/validate-chart-pr.yaml around lines 14 - 17, Update the setup-go action in the “Set up Go 1.21.x” workflow step from v4 to v6, preserving the existing Go version configuration.Source: Linters/SAST tools
🤖 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/workflows/validate-chart-pr.yaml:
- Line 166: Update the structural validation job condition in the workflow to
trigger based on the pull request branch name prefix used for chart-sync
branches instead of checking for the missing chart-sync label. Preserve the
existing validation steps and ensure only matching chart-sync branch names
activate the job.
---
Duplicate comments:
In @.github/workflows/pr-chart-delete.yaml:
- Around line 32-35: Update the setup-go action in the “Set up Go 1.21.x”
workflow step from actions/setup-go@v4 to actions/setup-go@v6, keeping the
existing go-version unchanged.
- Line 53: Update the deletion guard in the workflow’s chart cleanup logic to
proceed when any related artefact exists: the chart archive, its provenance
file, or its entry in index.yaml. Preserve the existing deletion behavior while
ensuring cleanup is not skipped when only .prov or index.yaml artefacts remain.
- Around line 37-38: Update the Checkout step’s actions/checkout reference from
the unavailable v6 release to the valid v4 major version, leaving the rest of
the workflow unchanged.
- Around line 25-29: Add a validation step immediately after checkout that
rejects CHART_NAME and CHART_VERSION unless they match the safe alphanumeric,
dot, and hyphen pattern, before any other shell command runs. Update every
subsequent run block to reference the exported shell variables "$CHART_NAME",
"$CHART_VERSION", "$COMPONENT_REPO", and "$BRANCH_NAME" instead of GitHub
expression interpolation such as "${{ env.* }}"; preserve the existing workflow
behavior after validation.
---
Nitpick comments:
In @.github/workflows/validate-chart-pr.yaml:
- Around line 14-17: Update the setup-go action in the “Set up Go 1.21.x”
workflow step from v4 to v6, preserving the existing Go version configuration.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80c61d8a-0399-4294-a219-c3dda6e7083e
📒 Files selected for processing (4)
.github/workflows/pr-chart-delete.yaml.github/workflows/pr-chart-sync.yaml.github/workflows/validate-chart-pr.yamlMakefile
| echo "No duplicate charts found" | ||
|
|
||
| - name: Validate chart sync PR structure | ||
| if: contains(github.event.pull_request.labels.*.name, 'chart-sync') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use branch prefix instead of missing labels.
The chart-sync label is no longer assigned during PR creation. As a result, this structural validation will be skipped entirely. Consider matching the branch name prefix instead of relying on the label.
💻 Proposed fix
- if: contains(github.event.pull_request.labels.*.name, 'chart-sync')
+ if: startsWith(github.head_ref, 'sync/')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if: contains(github.event.pull_request.labels.*.name, 'chart-sync') | |
| if: startsWith(github.head_ref, 'sync/') |
🤖 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/validate-chart-pr.yaml at line 166, Update the structural
validation job condition in the workflow to trigger based on the pull request
branch name prefix used for chart-sync branches instead of checking for the
missing chart-sync label. Preserve the existing validation steps and ensure only
matching chart-sync branch names activate the job.
06305bc to
f020243
Compare
Boomatang
left a comment
Boomatang
left a comment
There was a problem hiding this comment.
As all the workflows suffer from the go version and actions should be pinned, I only comment on those changes in the one file.
In general I think it is looking good.
| - name: Set up Go 1.21.x | ||
| uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: 1.21.x |
There was a problem hiding this comment.
This version of go is very old and we should be more inline with rest of the projects. This should be changed to 1.26.x
|
|
||
| steps: | ||
| - name: Set up Go 1.21.x | ||
| uses: actions/setup-go@v4 |
There was a problem hiding this comment.
We are move to pined version of the actions we use. The v4should be replaced with a commit. This will apply to very actions uses field.
| - name: Checkout | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| token: ${{ secrets.KUADRANT_DEV_PAT }} |
There was a problem hiding this comment.
We can checkout a branch with out using the PAT token.
Here is an example of doing such a thing. https://github.com/Kuadrant/authorino/blob/604ba6a0763c9e4c1a0f76c8a255c0c9107218e2/.github/workflows/pre-release.yaml#L16-L31
| fi | ||
|
|
||
| - name: Delete chart package | ||
| if: steps.chart-check.outputs.chart_exists == 'true' |
There was a problem hiding this comment.
Looking at this check. It seems to be on every step after here. Would it make more sense to exit with error if the chart that is trying to be removed does not exist anymore?
| - name: Create Pull Request | ||
| if: steps.chart-check.outputs.chart_exists == 'true' && steps.changes.outputs.has_changes == 'true' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.KUADRANT_DEV_PAT }} |
There was a problem hiding this comment.
This can be done using the same GitHub permission I method earlier. For testing in your fork you do need to enable a setting to allow the creation of the pr.
| - name: No changes detected | ||
| if: steps.changes.outputs.has_changes == 'false' | ||
| run: | | ||
| echo "No changes detected for ${{ env.CHART_NAME }} v${{ env.CHART_VERSION }}" | ||
| echo "Chart may already exist in the repository" |
There was a problem hiding this comment.
This check should really happen sooner so as not to need the `if`` check in a number of place. Possible should also have non-zero exit as it would be expect if a user calls the workflow that it would make changes.
| # Manual merge fallback - currently disabled | ||
| # Use this if auto-merge is not enabled in repository settings | ||
| # - name: Output PR details | ||
| # if: steps.changes.outputs.has_changes == 'true' | ||
| # run: | | ||
| # echo "Created PR for ${{ env.CHART_NAME }} v${{ env.CHART_VERSION }}" | ||
| # echo "Branch: ${{ env.BRANCH_NAME }}" | ||
| # echo "PR created successfully - manual merge required after CI validation passes" | ||
| # echo "To enable auto-merge: Repository Settings > General > Pull Requests > Allow auto-merge" |
|
|
||
| - name: Get changed files | ||
| id: changed-files | ||
| uses: tj-actions/changed-files@v41 |
There was a problem hiding this comment.
Where else is this action used? This feels like an unnecessary dependency. Some thing like git diff --name-only branch1 branch2 -- path/to/folder/ might do what you need without having to bring in a dependency.
Also I am not sure why this is needed here.
| rm -f ./charts/$(CHART_NAME)-$(CHART_VERSION).tgz* | ||
|
|
||
| .PHONY: validate-chart | ||
| validate-chart: $(HELM) ## Validate a specific chart package |
There was a problem hiding this comment.
I am questioning what is the need for this command when the Validate chart packages stage of the validate-chart-pr.yaml does so much more. I would like to see this improved.
e045c8b to
6d5f5d3
Compare
Boomatang
left a comment
Boomatang
left a comment
There was a problem hiding this comment.
Nothing major, only some clean up comments. The PR description is also now out of sync with the changes. With the requirement on the KUADRANT_DEV_PAT being the largest change.
Do you happen to have test PRs and pipeline test runs that I could look at?
| else | ||
| echo "has_changes=true" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
I think this can be removed now. The has_changes doesn't seem to be used any where else now.
| if [[ -n "${chart_versions[$key]:-}" ]]; then | ||
| echo "Duplicate chart found: $chart_name version $chart_version" | ||
| echo " Files: ${chart_versions[$key]} and $tgz_file" | ||
| exit 1 |
There was a problem hiding this comment.
I would not exit here. I would update a var to say a duplicate was found. And after the loop I would check that var, if true that stage then exit.
The reason I would do this is to it allows logging of multiple duplicates. Normally it is good to exit earlier, but this case is the exception to the rule.
Signed-off-by: fabikova <mfabikov@redhat.com>
6d5f5d3 to
d0ca482
Compare
|
I am not sure we should do this. This repo serves as chart registry and releasing concept does not apply to this repo IMHO. |
3 participants
This PR implements RFC 0020 to replace direct-push chart synchronization with a PR-based workflow that complies with branch protection rules and Red Hat's Segregation of Duties policy.
Addresses: #40RFC: Kuadrant/architecture#178
What Changed
Before (repository_dispatch):
repository_dispatchevent → Direct push to mainAfter (PR-based):
workflow_dispatchtrigger → PR creation → CI validation → Manual/automerge
Added Workflows
1. Chart Sync (
.github/workflows/pr-chart-sync.yaml)workflow_dispatchwith chart URL, name, versionsync-chart-created.yaml2. Chart Validation (
.github/workflows/validate-chart-pr.yaml)charts/3. Chart Deletion (
.github/workflows/pr-chart-delete.yaml)workflow_dispatchwith chart name, versionsync-chart-deleted.yamlManual Testing via GitHub UI - example
Browser download URL:
https://github.com/Kuadrant/authorino-operator/releases/download/v0.26.0/chart-authorino-operator-0.26.0.tgzauthorino-operator0.26.0authorino-operatorAuto-merge Setup (Optional)
For fully automated chart sync, enable auto-merge:
Complete Workflow Process
Chart Sync:
sync/{chart}-{version}after successful mergeChart Deletion:
delete/{chart}-{version}after successful mergeAuthentication
Uses
GITHUB_TOKENfor workflow operations, which provides:No additional PAT configuration required -
GITHUB_TOKENis automatically provided by GitHub Actions.Validation
All chart sync PRs undergo automated validation:
Addresses Adopt RFC 0020: Accept helm chart sync via PRs instead of direct pushes #40