incluster-comp-pr-merged: split multi-arch build into per-arch matrix + merge#88
incluster-comp-pr-merged: split multi-arch build into per-arch matrix + merge#88lyuval-armosec wants to merge 6 commits into
Conversation
… + merge Fixes 'error writing layer blob: failed to reserve cache' during docker/build-push-action, seen e.g. on kubevuln's pr-merged run (https://github.com/kubescape/kubevuln/actions/runs/29737569926/job/88346893168). Root cause: linux/amd64,linux/arm64 were built in one multi-platform build-push-action invocation (arm64 via QEMU) with cache-to/cache-from type=gha sharing a single cache scope across both platforms concurrently — a known trigger for GHA cache blob-reservation races (moby/buildkit#4188, docker/build-push-action#1044). Restructures the docker-build job into: - setup: prerelease tag + test names (unchanged logic, just relocated) - unit-test: go test, run once (was duplicated per platform before) - build: matrix over {amd64 -> ubuntu-large, arm64 -> ubuntu-large-arm64}, each leg builds natively (no QEMU) with its own GHA cache scope (component+arch scoped), pushing an arch-suffixed tag - merge-images: docker buildx imagetools create stitches the arch tags into the final manifest, then attestation/cosign move here to act on the merged digest Same pattern already validated in armosec/shared-workflows-v2's arc-runner-ci.yaml (PR #15/#16). workflow_call inputs schema is unchanged; this workflow declares no outputs and no consumer references internal job outputs, so the job-graph restructuring is contract-safe for all 6 known consumers (synchronizer, operator, prometheus-exporter, kubevuln, storage, http-request), all of which pass BUILD_PLATFORM: linux/amd64,linux/arm64. Single-platform callers (BUILD_PLATFORM without arm64) skip the arm64 leg via a job-level if, and merge-images degrades to a single-source manifest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workflow replaces the prior Docker build path with setup, multi-architecture image builds, manifest merging, provenance attestation, revised cosign signing, updated E2E input wiring, and release retagging dependencies. ChangesRelease workflow orchestration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Setup
participant Build
participant MergeImages
participant RunTests
participant Release
Setup->>Build: Export image tag and test outputs
Build->>MergeImages: Push architecture-specific images
MergeImages->>MergeImages: Create manifest and attest provenance
MergeImages->>MergeImages: Sign and verify image
Setup->>RunTests: Provide test selection and image tags
RunTests->>Release: Complete test dependency
Release->>Release: Retag prerelease images
Possibly related PRs
Suggested reviewers: 🚥 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 |
|
Summary:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/incluster-comp-pr-merged.yaml (3)
130-132: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueMitigate shell template injection risk.
Static analysis tools flag direct inline substitution of
inputs.IMAGE_TAGas a potential shell injection risk. To ensure robustness and silence the warnings, pass the variable safely viaenv.🛠️ Proposed fix
- name: Set prerelease image tag id: image-prerelease-tag - run: echo "IMAGE_TAG_PRERELEASE=${{ inputs.IMAGE_TAG }}-prerelease" >> $GITHUB_OUTPUT + env: + IMAGE_TAG: ${{ inputs.IMAGE_TAG }} + run: echo "IMAGE_TAG_PRERELEASE=${IMAGE_TAG}-prerelease" >> "$GITHUB_OUTPUT"🤖 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/incluster-comp-pr-merged.yaml around lines 130 - 132, Update the image-prerelease-tag step so inputs.IMAGE_TAG is passed through the step’s env configuration and referenced via the environment variable in the run command, avoiding direct GitHub expression interpolation in the shell. Preserve the existing IMAGE_TAG_PRERELEASE output format and prerelease suffix.Source: Linters/SAST tools
507-511: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueMitigate shell template injection warnings.
Static analysis tools highlight the direct string interpolation of workflow inputs inside
runblocks as a potential shell injection risk. To silence the warnings and ensure robustness, pass the variables safely usingenv.🛠️ Proposed fix
- name: Docker retag + env: + IMAGE_NAME: ${{ inputs.IMAGE_NAME }} + TAG_PRERELEASE: ${{ needs.setup.outputs.IMAGE_TAG_PRERELEASE }} + TAG: ${{ inputs.IMAGE_TAG }} run: | curl -L https://github.com/regclient/regclient/releases/download/v0.4.4/regctl-linux-amd64 >regctl chmod 755 regctl if [ ${{ inputs.COSIGN }} = true ]; then - ./regctl image copy ${{ inputs.IMAGE_NAME }}:${{ needs.setup.outputs.IMAGE_TAG_PRERELEASE }} ${{ inputs.IMAGE_NAME }}:${{ inputs.IMAGE_TAG }} --digest-tags - ./regctl image copy ${{ inputs.IMAGE_NAME }}:${{ needs.setup.outputs.IMAGE_TAG_PRERELEASE }} ${{ inputs.IMAGE_NAME }}:latest --digest-tags + ./regctl image copy "${IMAGE_NAME}:${TAG_PRERELEASE}" "${IMAGE_NAME}:${TAG}" --digest-tags + ./regctl image copy "${IMAGE_NAME}:${TAG_PRERELEASE}" "${IMAGE_NAME}:latest" --digest-tags else - ./regctl image copy ${{ inputs.IMAGE_NAME }}:${{ needs.setup.outputs.IMAGE_TAG_PRERELEASE }} ${{ inputs.IMAGE_NAME }}:${{ inputs.IMAGE_TAG }} - ./regctl image copy ${{ inputs.IMAGE_NAME }}:${{ needs.setup.outputs.IMAGE_TAG_PRERELEASE }} ${{ inputs.IMAGE_NAME }}:latest + ./regctl image copy "${IMAGE_NAME}:${TAG_PRERELEASE}" "${IMAGE_NAME}:${TAG}" + ./regctl image copy "${IMAGE_NAME}:${TAG_PRERELEASE}" "${IMAGE_NAME}:latest" 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/incluster-comp-pr-merged.yaml around lines 507 - 511, Update the workflow step containing the regctl image copy commands to pass IMAGE_NAME, IMAGE_TAG_PRERELEASE, and IMAGE_TAG through the step’s env configuration, then reference those environment variables in the shell commands instead of directly interpolating workflow inputs. Preserve the existing conditional behavior and latest-tag copy operations.Source: Linters/SAST tools
134-140: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse multiline EOF syntax for robust GitHub outputs.
If
inputs.REQUIRED_TESTScontains newlines (e.g., if it is passed as pretty-printed JSON),echo "TEST_NAMES=$input" >> $GITHUB_OUTPUTwill only assign the first line toTEST_NAMES, and the remaining lines will corrupt the$GITHUB_OUTPUTfile formatting.Using the standard multiline EOF heredoc syntax guarantees safe output assignment regardless of the string's format.
🛠️ Proposed fix
- id: export_tests_to_env name: set test name + env: + input: ${{ inputs.REQUIRED_TESTS }} run: | - echo "TEST_NAMES=$input" >> $GITHUB_OUTPUT - env: - input: ${{ inputs.REQUIRED_TESTS }} + { + echo 'TEST_NAMES<<EOF' + echo "$input" + echo 'EOF' + } >> "$GITHUB_OUTPUT"🤖 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/incluster-comp-pr-merged.yaml around lines 134 - 140, Update the export_tests_to_env step to write TEST_NAMES to GITHUB_OUTPUT using GitHub Actions’ multiline EOF delimiter syntax instead of a single-line echo, preserving the complete inputs.REQUIRED_TESTS value including embedded newlines.
🤖 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/incluster-comp-pr-merged.yaml:
- Around line 264-266: Update the password piping command in the cosign signing
step to use printf with a literal-string format specifier, passing
COSIGN_PRIVATE_KEY_PASSWORD as its value argument. Keep the existing cosign sign
invocation and key-file cleanup unchanged.
- Around line 160-173: Update the setup job to expose a BUILD_MATRIX output
generated by a set-matrix step, including the arm64 entry only when
inputs.BUILD_PLATFORM contains arm64 and always retaining amd64. In the build
job, remove the matrix-based job-level if condition and configure
strategy.matrix from needs.setup.outputs.BUILD_MATRIX so each intended platform
permutation runs.
---
Nitpick comments:
In @.github/workflows/incluster-comp-pr-merged.yaml:
- Around line 130-132: Update the image-prerelease-tag step so inputs.IMAGE_TAG
is passed through the step’s env configuration and referenced via the
environment variable in the run command, avoiding direct GitHub expression
interpolation in the shell. Preserve the existing IMAGE_TAG_PRERELEASE output
format and prerelease suffix.
- Around line 507-511: Update the workflow step containing the regctl image copy
commands to pass IMAGE_NAME, IMAGE_TAG_PRERELEASE, and IMAGE_TAG through the
step’s env configuration, then reference those environment variables in the
shell commands instead of directly interpolating workflow inputs. Preserve the
existing conditional behavior and latest-tag copy operations.
- Around line 134-140: Update the export_tests_to_env step to write TEST_NAMES
to GITHUB_OUTPUT using GitHub Actions’ multiline EOF delimiter syntax instead of
a single-line echo, preserving the complete inputs.REQUIRED_TESTS value
including embedded newlines.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 455016cf-b6b5-401a-a1da-ef6b0727880c
📒 Files selected for processing (1)
.github/workflows/incluster-comp-pr-merged.yaml
|
Summary:
|
… explicitly Previously the amd64 matrix leg's if-condition (matrix.arch == 'amd64') was unconditional, and merge-images always assumed an amd64 image existed — an arm64-only BUILD_PLATFORM would still build/reference an unrequested amd64 image. Adds a validation step in setup that resolves BUILD_PLATFORM into two explicit outputs (BUILD_AMD64, BUILD_ARM64), rejecting any value other than linux/amd64, linux/arm64, or linux/amd64,linux/arm64. build and merge-images now branch on those outputs instead of re-deriving arch membership from the raw string independently in two places. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Summary:
|
- Fix invalid matrix context in job-level if (build job): GH Actions
does not expose the matrix context to a job's own if condition.
Moved platform resolution into a setup step that emits a BUILD_MATRIX
JSON output (only the requested arch entries), consumed directly by
build's strategy.matrix via fromJson() — no job-level if needed.
- Fix printf format-string vulnerability in cosign signing step: a
COSIGN_PRIVATE_KEY_PASSWORD containing % or \ would have been
misinterpreted as a format directive. Now passed as a %s argument.
- Mitigate shell template-injection warnings by passing IMAGE_TAG,
IMAGE_NAME, and IMAGE_TAG_PRERELEASE through env instead of direct
${{ }} interpolation inside run: blocks (image-prerelease-tag step,
Docker retag step).
- Use multiline EOF syntax for the TEST_NAMES GITHUB_OUTPUT write so a
REQUIRED_TESTS value containing newlines doesn't corrupt the output
file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Yuval Levy <yuvalle@armosec.io>
|
Summary:
|
2 similar comments
|
Summary:
|
|
Summary:
|
…r-token loop Replaces the enumerated-whole-string case statement (three literal combos) and the resulting BUILD_AMD64/BUILD_ARM64 booleans with a single per-token loop over BUILD_PLATFORM split on commas. BUILD_MATRIX becomes the sole source of truth, consumed directly by both the build job's strategy.matrix and merge-images' source-tag list (previously each independently re-derived arch membership from the two booleans). Validation is intentionally minimal: only the arch suffix is checked against a known runner mapping (amd64/arm64), just enough to avoid silently scheduling an unrecognized platform onto the wrong runner. Everything else about platform-string validity is left to docker/build-push-action itself, which already rejects what it doesn't support. No duplicate-platform guard by design — a caller passing the same platform twice is a malformed input and the resulting failure is an acceptable consequence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Out of scope for this PR — the cache-race fix and the platform-matrix simplification shouldn't be bundled with an unrelated CodeRabbit nitpick fix. REQUIRED_TESTS is always passed as compact single-line JSON by every known consumer today, so the single-line echo is correct in practice; can be revisited separately if it's ever needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Summary:
|
Out of scope for this PR. Keeping only the printf %s fix for COSIGN_PRIVATE_KEY_PASSWORD (a real, critical-severity finding) from CodeRabbit's review — these two were lower-severity nitpicks unrelated to the cache-race fix or platform-matrix simplification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Summary:
|
3 similar comments
|
Summary:
|
|
Summary:
|
|
Summary:
|
|
Summary:
|
|
Closing this in favor of #89. Root cause turned out to be unrelated to multi-platform cache-export concurrency (the theory this PR was built on). Confirmed via GitHub's own changelog and a matching community report (docker/build-push-action#1571, comment docker/build-push-action#1571 (comment)): GitHub started issuing read-only Actions-cache tokens (as of 2026-06-26: https://github.blog/changelog/2026-06-26-read-only-actions-cache-for-untrusted-triggers/) to runs resolving to the default branch for any trigger not on a small trusted allowlist ( This happens regardless of job/matrix structure — every leg in this PR's matrix would inherit the same read-only token from the top-level triggering event, so splitting the build into per-arch jobs with per-arch GHA cache scopes doesn't address it. #89 (swap to The matrix-per-arch + native-arm64-runner restructuring here isn't wrong on its own terms, just doesn't fix this specific bug — closing to keep things simple. May revisit independently if there's a separate motivation for it later. |
2 participants
Summary
error writing layer blob: failed to reserve cacheseen duringdocker/build-push-actionon the sharedincluster-comp-pr-merged.yamlworkflow (e.g. kubevuln's pr-merged run: https://github.com/kubescape/kubevuln/actions/runs/29737569926/job/88346893168)linux/amd64,linux/arm64were built in a single multi-platformbuild-push-actioninvocation (arm64 via QEMU) withcache-to/cache-from: type=ghasharing one cache scope across both platforms concurrently — a known trigger for GHA cache blob-reservation races (migrate to new github.com/distribution/reference v0.5.0 moby/buildkit#4188, buildx failed with: ERROR: failed to solve: process "/bin/sh -c apk add --no-cache libc6-compat" did not complete successfully: exit code: 1 docker/build-push-action#1044)docker-buildjob intosetup(prerelease tag + test names) →unit-test(runs once, was duplicated per platform before) →build(matrix over{amd64 → ubuntu-large, arm64 → ubuntu-large-arm64}, each leg native — no QEMU — with its own component+arch-scoped GHA cache) →merge-images(docker buildx imagetools createstitches the arch tags into the final manifest; attestation + cosign move here to act on the merged digest)armosec/shared-workflows-v2'sarc-runner-ci.yaml(merged PRs Ignore covrall fail #15/ignore Coveralls fail #16)workflow_callinputs schema is unchanged and this workflow declares no outputs, so the internal job-graph restructure is contract-safe for all 6 known consumers (synchronizer,operator,prometheus-exporter,kubevuln,storage,http-request), all of which passBUILD_PLATFORM: linux/amd64,linux/arm64BUILD_PLATFORMwithout arm64) skip the arm64 leg via a job-levelif, andmerge-imagesdegrades to a single-source manifestTest plan
pr-mergedon a consumer repo (e.g. re-run kubevuln's, since that's the one with the original failing run) and confirm both matrix legs build/push andmerge-imagesproduces a valid manifest with no "failed to reserve cache" errorCOSIGN: true), and the retag/release flow increate-release-and-retagstill work against the merged manifest tagworkflow_dispatchrun (defaultBUILD_PLATFORM: linux/amd64) skips the arm64 leg cleanly and still produces a working image🤖 Generated with Claude Code
Summary by CodeRabbit