-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add runner and caching inputs for ARC support #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds inputs for per-architecture runners, mirrors, registries, proxies, and cache controls; generates BuildKit/Buildx config; injects npm/apt configs and secrets; produces per-arch builds and a merge job that creates/pushes multi-platform manifests and exposes a final Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Setup as Setup Job
participant Build as Build Job (per-arch)
participant Merge as Merge Job
participant Reg as Container Registry
Note over GH,Setup: parse workflow inputs (runners, mirrors, registries, caches, secrets)
GH->>Setup: run setup -> generate matrix & BuildKit config
Setup-->>GH: outputs (matrix JSON, buildkit config, multi_platform flag)
par Per-arch builds
GH->>Build: start build for each matrix entry (runs-on per-arch)
Build->>Build: install Buildx using generated BuildKit config, inject npm/apt configs, apply cache-from/cache-to, use docker-secrets
Build->>Reg: build & push per-arch image (emit digest/artifact)
Build-->>GH: upload digest/artifact and set job output (per-arch image_tag)
end
alt multi-platform merge
GH->>Merge: run merge job (consume per-arch digests & tag configuration)
Merge->>Reg: create manifest list from per-arch images and push manifest + tags
Merge->>Reg: inspect pushed manifest/image, retrieve final digest/tag
Merge-->>GH: output final image_tag
else single-platform
Merge-->>GH: forward build-provided image_tag
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/build-and-push-docker-image.yml (2)
189-226: Shell injection risk with direct input interpolation.The
${{ inputs.tags }}is directly interpolated into the shell script (lines 201, 203). If a malicious or malformed value is passed, it could break parsing or execute unintended commands. Use an environment variable to safely pass the input.Recommended fix using environment variable
- name: Prepare tag configuration id: tag_config + env: + TAGS_INPUT: ${{ inputs.tags }} run: | # Use TAGS_CONFIG environment variable to store tags configuration echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV # Default tagging configuration echo "type=raw,value=latest,enable={{is_default_branch}}" >> $GITHUB_ENV echo "type=ref,event=branch" >> $GITHUB_ENV echo "type=ref,event=pr" >> $GITHUB_ENV # Check if tags input is provided - if [ -n "${{ inputs.tags }}" ]; then + if [ -n "$TAGS_INPUT" ]; then # Process input line by line - echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + echo "$TAGS_INPUT" | while IFS= read -r line || [[ -n "$line" ]]; do # Trim whitespace line=$(echo "$line" | xargs) if [ -n "$line" ]; then if [[ "$line" == *"type="* ]]; then # Line contains configuration, add it directly echo "$line" >> $GITHUB_ENV else # Line contains simple tags, process as comma-separated list IFS=',' read -ra TAG_ARRAY <<< "$line" for tag in "${TAG_ARRAY[@]}"; do # Trim whitespace from each tag tag=$(echo "$tag" | xargs) if [ -n "$tag" ]; then echo "type=raw,value=${tag}" >> $GITHUB_ENV fi done fi fi done fi echo "EOF" >> $GITHUB_ENV
178-183: Registry login is hardcoded to ghcr.io, inconsistent with build job.The merge job hardcodes
ghcr.iowhile the build job respectsinputs.registry. This would cause failures when pushing to a different registry.Suggested fix for consistency
- name: Login to GHCR uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
🧹 Nitpick comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
138-143: Consider quoting the platform variable assignment.While the current platform values are safe, quoting the assignment provides defense against future changes.
Suggested improvement
- name: Prepare run: | - platform=${{ matrix.platform }} + platform="${{ matrix.platform }}" echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV image="${{ inputs.image-name }}" echo "IMAGE_NAME=${image//\//-}" >> $GITHUB_ENV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 160-171: The workflow references inputs.skip-merge but that input
is not declared; add a workflow_call.inputs entry named skip-merge (e.g., type:
boolean, default: false, description: "Skip multi-platform image manifest merge
and push per-platform images") so the expressions using inputs.skip-merge (used
in outputs and the other references) validate; ensure the exact key is
"skip-merge" to match uses in the template and set an appropriate default and
type to avoid “Unrecognized named-value” errors.
- Around line 84-86: The jobs 'setup' and 'merge' currently hardcode runs-on to
inputs.runner-amd64 which causes the workflow to fail when build-amd64 is false
and that runner label isn't available; change both jobs to select their runner
dynamically: instead of always using inputs.runner-amd64, make runs-on
conditional or use a fallback input (e.g., inputs.runner-arm64 or a default like
ubuntu-latest) driven by inputs.build-amd64 so the job picks the amd64 runner
only when build-amd64 is true and otherwise uses the arm64/default runner.
🧹 Nitpick comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
91-100: Matrixrunneris unused — either wire it or drop it.The matrix JSON includes a
runnerfield, butruns-onignores it. This is dead config and can confuse future edits.♻️ Option A: Use matrix.runner
- matrix_json="${matrix_json} {\"platform\": \"linux/amd64\", \"runner\": \"ubuntu-24.04\"}," + matrix_json="${matrix_json} {\"platform\": \"linux/amd64\", \"runner\": \"${{ inputs.runner-amd64 }}\"}," ... - matrix_json="${matrix_json} {\"platform\": \"linux/arm64\", \"runner\": \"ubuntu-24.04-arm\"}," + matrix_json="${matrix_json} {\"platform\": \"linux/arm64\", \"runner\": \"${{ inputs.runner-arm64 }}\"}," ... - runs-on: ${{ matrix.platform == 'linux/amd64' && inputs.runner-amd64 || inputs.runner-arm64 }} + runs-on: ${{ matrix.runner }}Also applies to: 111-113
| # When skip-merge is enabled, we push the image manifest directly to the registry | ||
| # The caller is responsible for ensuring tags are unique per platform (e.g. by appending -amd64) | ||
| # Otherwise they will overwrite each other. | ||
| tags: ${{ inputs.registry }}/${{ inputs.image-name }} | ||
| labels: ${{ steps.meta.outputs.labels }} | ||
| build-args: ${{ inputs.build-args }} | ||
| network: ${{ inputs.network }} | ||
| outputs: type=image,"name=${{ inputs.registry }}/${{ inputs.image-name }}",push-by-digest=true,name-canonical=true,push=true | ||
| platforms: ${{ matrix.platform }} | ||
| cache-from: ${{ inputs.cache-from }} | ||
| cache-to: ${{ inputs.cache-to }} | ||
| # Push immediately if skipping merge, otherwise push by digest | ||
| outputs: ${{ inputs.skip-merge && 'type=image,push=true' || format('type=image,"name={0}/{1}",push-by-digest=true,name-canonical=true,push=true', inputs.registry, inputs.image-name) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define skip-merge input (currently undefined).
inputs.skip-merge is referenced in expressions, but the input is not declared under workflow_call.inputs. This will break workflow validation with “Unrecognized named-value”.
🐛 Proposed fix (declare the missing input)
build-arm64:
type: boolean
default: true
description: "Enable linux/arm64 build"
+ skip-merge:
+ type: boolean
+ default: false
+ description: "Skip multi-arch manifest merge and push per-platform images directly"
cache-from:
type: string
default: ''
description: "List of external cache sources for buildx (e.g., type=registry,ref=...)"Also applies to: 195-196
🧰 Tools
🪛 actionlint (1.7.10)
171-171: property "skip-merge" is not defined in object type {build-amd64: bool; build-args: string; build-arm64: bool; cache-from: string; cache-to: string; docker-context: string; docker-file: string; image-name: string; labels: string; network: string; ref: string; registry: string; runner-amd64: string; runner-arm64: string; tags: string}
(expression)
🤖 Prompt for AI Agents
In @.github/workflows/build-and-push-docker-image.yml around lines 160 - 171,
The workflow references inputs.skip-merge but that input is not declared; add a
workflow_call.inputs entry named skip-merge (e.g., type: boolean, default:
false, description: "Skip multi-platform image manifest merge and push
per-platform images") so the expressions using inputs.skip-merge (used in
outputs and the other references) validate; ensure the exact key is "skip-merge"
to match uses in the template and set an appropriate default and type to avoid
“Unrecognized named-value” errors.
c239d9e to
c808049
Compare
…c matrix with custom runner inputs
…tform builds - Add build-amd64 and build-arm64 boolean inputs (default: true) - Dynamic matrix generation based on enabled platforms - Single-platform builds push directly without merge job - Multi-platform builds use digest artifacts and merge manifest - Output handling works for both single and multi-platform cases Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
262-267: Use consistent registry and credentials across jobs.The merge job hardcodes
ghcr.ioandgithub.repository_owner, ignoring theinputs.registryand secret inputs. This breaks workflows using non-GHCR registries (Docker Hub, AWS ECR, etc.).The build job correctly uses
inputs.registryand credential fallbacks at lines 164-166.🐛 Proposed fix
- name: Login to GHCR uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
🤖 Fix all issues with AI agents
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 217-223: The bash parameter expansions that set PLATFORM_PAIR and
IMAGE_NAME use incorrect escaping (they use "\\/" which matches backslashes) so
forward slashes in the `platform` and `image` variables won't be replaced;
update the expansions to use proper forward-slash escaping (e.g.,
`${platform//\//-}` and `${image//\//-}`) when writing to GITHUB_ENV so
`linux/amd64` becomes `linux-amd64` and artifact names match; modify the
commands that create PLATFORM_PAIR and IMAGE_NAME to use the corrected
replacements for `platform` and `image`.
🧹 Nitpick comments (2)
.github/workflows/build-and-push-docker-image.yml (2)
272-310: Tag configuration logic is functional but duplicates build job metadata.The tag processing logic correctly handles both simple tags and advanced
type=configuration. However, this duplicates the metadata extraction logic from the build job (lines 173-177). Consider extracting this to a reusable action or script if the workflow grows.
241-243: Consider dynamic runner selection for arm64-only scenarios.The merge job always uses
runner-amd64(or its defaultubuntu-24.04). In arm64-only ARC deployments whereubuntu-24.04isn't available, this could fail. The setup job's approach of usingubuntu-latestis more portable.♻️ Alternative: match setup job's approach
merge: - runs-on: ${{ inputs.runner-amd64 || 'ubuntu-latest' }} + runs-on: ubuntu-latestOr for explicit runner preference:
merge: - runs-on: ${{ inputs.runner-amd64 || 'ubuntu-latest' }} + runs-on: ${{ inputs.build-amd64 && inputs.runner-amd64 || inputs.runner-arm64 }}
The setup job was still using ubuntu-latest which can have 20+ minute queue times. Now uses runner-amd64 input so callers get consistent fast queue times across all jobs. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Adds optional docker-secrets input to pass secrets to docker/build-push-action. This enables Dockerfiles that use --mount=type=secret to work with the reusable workflow. Backward compatible - defaults to empty string.
Secrets cannot be passed via inputs in reusable workflows. Move docker-secrets to the secrets block to allow callers to properly pass secret values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/build-and-push-docker-image.yml (2)
267-272: Use the configured registry credentials in merge.The merge job always logs into GHCR with
GITHUB_TOKEN, so non‑GHCR registries or custom creds will fail.🔧 Proposed fix
- - name: Login to GHCR + - name: Log in to Container registry uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
171-180: Normalize simple tags in thebuildjob to match themergejob's handling.The
inputs.tagsdescription promises support for comma-separated simple tags (e.g.,"v1.0,latest"), but thebuildjob at lines 171–180 passes them directly todocker/metadata-actionwithout normalization. Sincedocker/metadata-actionrequires each line to havetype=...format, simple tags will be rejected or ignored. Themergejob already implements the correct normalization; apply the same approach to thebuildjob.♻️ Suggested fix (reuse the tag normalization from merge job)
+ - name: Prepare tag configuration + id: tag_config + run: | + echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV + echo "type=raw,value=latest,enable={{is_default_branch}}" >> $GITHUB_ENV + echo "type=ref,event=branch" >> $GITHUB_ENV + echo "type=ref,event=pr" >> $GITHUB_ENV + if [ -n "${{ inputs.tags }}" ]; then + echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + line=$(echo "$line" | xargs) + if [ -n "$line" ]; then + if [[ "$line" == *"type="* ]]; then + echo "$line" >> $GITHUB_ENV + else + IFS=',' read -ra TAG_ARRAY <<< "$line" + for tag in "${TAG_ARRAY[@]}"; do + tag=$(echo "$tag" | xargs) + if [ -n "$tag" ]; then + echo "type=raw,value=${tag}" >> $GITHUB_ENV + fi + done + fi + fi + done + fi + echo "EOF" >> $GITHUB_ENV + - name: Extract metadata (tags, labels) for Docker id: meta uses: docker/metadata-action@v5 with: images: ${{ inputs.registry }}/${{ inputs.image-name }} tags: | - type=raw,value=latest,enable={{is_default_branch}} - type=ref,event=branch - type=ref,event=pr - ${{ inputs.tags }} + ${{ env.TAGS_CONFIG }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
277-282: Registry mismatch: merge job hardcodesghcr.ioinstead of usinginputs.registry.The build job correctly uses
${{ inputs.registry }}(line 177), but the merge job hardcodesghcr.io. If a user specifies a different registry, the merge job will fail to authenticate and push the manifest correctly.🐛 Proposed fix
- name: Login to GHCR uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
🧹 Nitpick comments (2)
.github/workflows/build-and-push-docker-image.yml (2)
168-172: Hardcoded organization-specific registry mirror may affect external consumers.The default fallback
https://docker-mirror.ase.in.tum.de:8765is organization-specific. If this workflow is intended for broader reuse, external consumers without access to this mirror may experience connection failures or degraded performance.Consider making the mirror opt-in (no default) or documenting this behavior prominently.
💡 Suggested change to make mirror opt-in
buildkitd-config-inline: | + ${{ inputs.registry-mirror && format('[registry."docker.io"] + mirrors = ["{0}"] + http = {1} + insecure = {1}', inputs.registry-mirror, inputs.registry-insecure) || '' }} - [registry."docker.io"] - mirrors = ["${{ inputs.registry-mirror || 'https://docker-mirror.ase.in.tum.de:8765' }}"] - http = ${{ inputs.registry-insecure }} - insecure = ${{ inputs.registry-insecure }}
287-326: Tag configuration logic is correct but could be more robust.The processing correctly handles both simple comma-separated tags and advanced
type=...configurations. However, directly interpolating${{ inputs.tags }}into the shell script (lines 300, 302) could cause issues if tags contain shell metacharacters.💡 Safer approach using environment variable
- name: Prepare tag configuration id: tag_config + env: + INPUT_TAGS: ${{ inputs.tags }} run: | # ... - if [ -n "${{ inputs.tags }}" ]; then - echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + if [ -n "$INPUT_TAGS" ]; then + echo "$INPUT_TAGS" | while IFS= read -r line || [[ -n "$line" ]]; do
- Remove registry-mirror and registry-insecure inputs (Harbor is deprecated) - Add use-cluster-cache boolean input for self-hosted runners - Configure buildkit with mirrors for both docker.io and ghcr.io - Cluster cache uses internal registry-mirror services - Standard mode has no mirror configuration (uses direct pulls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
295-333: Prevent command injection viainputs.tags.
echo "${{ inputs.tags }}"allows command substitution inside the input. Use an env var and a here-string instead.🔒 Proposed fix
- name: Prepare tag configuration id: tag_config + env: + TAGS_INPUT: ${{ inputs.tags }} run: | # Use TAGS_CONFIG environment variable to store tags configuration echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV @@ - if [ -n "${{ inputs.tags }}" ]; then - # Process input line by line - echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + if [ -n "$TAGS_INPUT" ]; then + # Process input line by line + while IFS= read -r line || [[ -n "$line" ]]; do # Trim whitespace line=$(echo "$line" | xargs) @@ - done + done <<< "$TAGS_INPUT" fi
Automatically injects NPM_REGISTRY=http://verdaccio.verdaccio.svc.cluster.local:4873 when use-cluster-cache is enabled. This allows Dockerfiles to use Verdaccio for npm/yarn package caching across builds.
This approach requires no Dockerfile changes - npm/yarn automatically read these config files. Works identically on standard and self-hosted runners.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
305-310: Use the selected registry for authentication in the merge job.The merge job pushes to
${{ inputs.registry }}(lines 361, 371, 375) but the login step at lines 306–311 hardcodesghcr.io,github.repository_owner, andGITHUB_TOKEN. This will fail for non-GHCR registries. Align the login step with the registry and credentials pattern already used in the build job (lines 203–208).✅ Suggested adjustment
- - name: Login to GHCR + - name: Log in to registry uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
…vars for mirrors - Remove build-amd64/build-arm64 flags and dynamic matrix setup job - Always build both AMD64 and ARM64 (matches main branch behavior) - Replace use-cluster-cache with explicit input variables: - docker-hub-mirror: Mirror URL for Docker Hub - ghcr-mirror: Mirror URL for ghcr.io - npm-registry: NPM/Yarn registry URL (injects .npmrc/.yarnrc) - No hardcoded registry URLs - callers pass their own values - No Dockerfile changes needed for npm caching
Injects apt proxy config file into build context when apt-proxy URL provided. Dockerfiles need to COPY .docker-build-config/01proxy /etc/apt/apt.conf.d/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
269-274: Merge job hardcodesghcr.ioinstead of usinginputs.registry.The build job correctly uses
inputs.registryfor login (line 203), but the merge job hardcodesghcr.io. If a user configures a different registry (e.g., Docker Hub or a private registry), the merge job will fail to authenticate and push the manifest.Additionally, the credentials are inconsistent: build job uses
secrets.registry-user/secrets.registry-passwordwith fallbacks, while merge job usesgithub.repository_owner/secrets.GITHUB_TOKEN.🐛 Proposed fix to use configurable registry and consistent credentials
- name: Login to GHCR uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
🤖 Fix all issues with AI agents
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 185-198: The heredoc in the "Inject apt proxy config" step writes
indented lines into "${CONTEXT_DIR}/.docker-build-config/01proxy" which
preserves YAML indentation and corrupts apt config; update the step that writes
to .docker-build-config/01proxy (using CONTEXT_DIR and PROXY_URL) to emit
unindented content — either replace the indented heredoc with a printf/echo
approach that writes exact lines (e.g., printf 'Acquire::http::Proxy
"%s";\nAcquire::https::Proxy "DIRECT";\n' "$PROXY_URL" >
"${CONTEXT_DIR}/.docker-build-config/01proxy") or switch to a left-aligned
heredoc (or use <<-EOF with tabs and ensure no spaces) so the file contains no
leading spaces.
🧹 Nitpick comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
280-317: Tag processing logic is sound but has an edge-case vulnerability.The tag processing correctly handles both simple comma-separated tags and advanced
type=configurations. However, ifinputs.tagscontains the literal stringEOF, it would prematurely terminate the heredoc delimiter and corruptGITHUB_ENV.Consider using a more unique delimiter or validating input.
♻️ Optional: Use a unique heredoc delimiter
- name: Prepare tag configuration id: tag_config run: | # Use TAGS_CONFIG environment variable to store tags configuration - echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV + echo "TAGS_CONFIG<<TAGS_CONFIG_DELIMITER" >> $GITHUB_ENV # Default tagging configuration ... - echo "EOF" >> $GITHUB_ENV + echo "TAGS_CONFIG_DELIMITER" >> $GITHUB_ENV
| - name: Inject apt proxy config | ||
| if: ${{ inputs.apt-proxy != '' }} | ||
| run: | | ||
| CONTEXT_DIR="${{ inputs.docker-context }}" | ||
| PROXY_URL="${{ inputs.apt-proxy }}" | ||
|
|
||
| mkdir -p "${CONTEXT_DIR}/.docker-build-config" | ||
| cat > "${CONTEXT_DIR}/.docker-build-config/01proxy" << EOF | ||
| Acquire::http::Proxy "${PROXY_URL}"; | ||
| Acquire::https::Proxy "DIRECT"; | ||
| EOF | ||
|
|
||
| echo "Injected apt proxy config pointing to: ${PROXY_URL}" | ||
| echo "Note: Dockerfile must COPY .docker-build-config/01proxy /etc/apt/apt.conf.d/ before apt commands" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heredoc preserves YAML indentation, corrupting apt config.
The << EOF heredoc preserves leading whitespace. Since the content lines are indented for YAML readability, the resulting 01proxy file will contain leading spaces, potentially causing apt to fail to parse the config correctly.
🐛 Proposed fix using printf or unindented heredoc
- name: Inject apt proxy config
if: ${{ inputs.apt-proxy != '' }}
run: |
CONTEXT_DIR="${{ inputs.docker-context }}"
PROXY_URL="${{ inputs.apt-proxy }}"
mkdir -p "${CONTEXT_DIR}/.docker-build-config"
- cat > "${CONTEXT_DIR}/.docker-build-config/01proxy" << EOF
- Acquire::http::Proxy "${PROXY_URL}";
- Acquire::https::Proxy "DIRECT";
- EOF
+ cat > "${CONTEXT_DIR}/.docker-build-config/01proxy" <<EOF
+Acquire::http::Proxy "${PROXY_URL}";
+Acquire::https::Proxy "DIRECT";
+EOF
echo "Injected apt proxy config pointing to: ${PROXY_URL}"
echo "Note: Dockerfile must COPY .docker-build-config/01proxy /etc/apt/apt.conf.d/ before apt commands"📝 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.
| - name: Inject apt proxy config | |
| if: ${{ inputs.apt-proxy != '' }} | |
| run: | | |
| CONTEXT_DIR="${{ inputs.docker-context }}" | |
| PROXY_URL="${{ inputs.apt-proxy }}" | |
| mkdir -p "${CONTEXT_DIR}/.docker-build-config" | |
| cat > "${CONTEXT_DIR}/.docker-build-config/01proxy" << EOF | |
| Acquire::http::Proxy "${PROXY_URL}"; | |
| Acquire::https::Proxy "DIRECT"; | |
| EOF | |
| echo "Injected apt proxy config pointing to: ${PROXY_URL}" | |
| echo "Note: Dockerfile must COPY .docker-build-config/01proxy /etc/apt/apt.conf.d/ before apt commands" | |
| - name: Inject apt proxy config | |
| if: ${{ inputs.apt-proxy != '' }} | |
| run: | | |
| CONTEXT_DIR="${{ inputs.docker-context }}" | |
| PROXY_URL="${{ inputs.apt-proxy }}" | |
| mkdir -p "${CONTEXT_DIR}/.docker-build-config" | |
| printf '%s\n' \ | |
| "Acquire::http::Proxy \"${PROXY_URL}\";" \ | |
| "Acquire::https::Proxy \"DIRECT\";" > "${CONTEXT_DIR}/.docker-build-config/01proxy" | |
| echo "Injected apt proxy config pointing to: ${PROXY_URL}" | |
| echo "Note: Dockerfile must COPY .docker-build-config/01proxy /etc/apt/apt.conf.d/ before apt commands" |
🤖 Prompt for AI Agents
In @.github/workflows/build-and-push-docker-image.yml around lines 185 - 198,
The heredoc in the "Inject apt proxy config" step writes indented lines into
"${CONTEXT_DIR}/.docker-build-config/01proxy" which preserves YAML indentation
and corrupts apt config; update the step that writes to
.docker-build-config/01proxy (using CONTEXT_DIR and PROXY_URL) to emit
unindented content — either replace the indented heredoc with a printf/echo
approach that writes exact lines (e.g., printf 'Acquire::http::Proxy
"%s";\nAcquire::https::Proxy "DIRECT";\n' "$PROXY_URL" >
"${CONTEXT_DIR}/.docker-build-config/01proxy") or switch to a left-aligned
heredoc (or use <<-EOF with tabs and ensure no spaces) so the file contains no
leading spaces.
BuildKit requires http and insecure flags to be configured on the mirror host section, not the upstream registry section. The mirrors array should contain just the host:port without protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-push-docker-image.yml (1)
307-312: Registry login hardcodesghcr.ioinstead of usinginputs.registry.The merge job's login step hardcodes
registry: ghcr.io, but the workflow accepts a configurableinputs.registry(defaulting toghcr.io). If a user specifies a different registry (e.g., Docker Hub or a private registry), the merge job will fail to authenticate correctly.🐛 Proposed fix to use configurable registry
- name: Login to GHCR uses: docker/login-action@v3 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}
🤖 Fix all issues with AI agents
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 172-192: The workflow step "Inject npm/yarn registry config"
currently unconditionally overwrites ${CONTEXT_DIR}/.npmrc and
${CONTEXT_DIR}/.yarnrc; change it to preserve existing files by checking for
existence and merging/appending the registry setting instead of using overwrite:
if the file does not exist create it, if it exists check for an existing
registry entry for ${REGISTRY_URL} and append the appropriate line only if
missing (for .npmrc use "registry=${REGISTRY_URL}", for .yarnrc use "registry
\"${REGISTRY_URL}\""); keep the current conditional creation for .yarnrc.yml and
consider making a backup (e.g., copy to .npmrc.bak/.yarnrc.bak) before modifying
to avoid data loss.
🧹 Nitpick comments (3)
.github/workflows/build-and-push-docker-image.yml (3)
124-164: Minor edge case: duplicate mirror host configuration.If both
docker-hub-mirrorandghcr-mirrorresolve to the same mirror host (e.g., both usehttp://registry-mirror:5000), the second[registry."${MIRROR_HOST}"]TOML section will overwrite the first. This is unlikely in practice but could cause unexpected behavior.♻️ Optional improvement to deduplicate mirror host entries
# Output config + # Deduplicate any repeated registry blocks echo "config<<EOF" >> $GITHUB_OUTPUT - echo "$CONFIG" >> $GITHUB_OUTPUT + echo "$CONFIG" | awk '!seen[$0]++' >> $GITHUB_OUTPUT echo "EOF" >> $GITHUB_OUTPUT
222-249: Minor: inconsistent indentation on line 238.Line 238 has an extra leading space before
if, which is inconsistent with the rest of the script. While not breaking, it affects readability.♻️ Fix indentation
# Process cache-from CACHE_FROM="${{ inputs.cache-from }}" - if [[ -n "$CACHE_FROM" ]]; then + if [[ -n "$CACHE_FROM" ]]; then
288-293: Consider documenting theubuntu-latestrunner choice for merge job.The merge job uses
ubuntu-latestinstead of the configurable runners. This is a reasonable choice since the merge job only manipulates manifests and doesn't need platform-specific capabilities, but it may confuse users expecting all jobs to use their custom runners. A comment explaining this would improve clarity.♻️ Add clarifying comment
merge: + # Uses ubuntu-latest as merge only creates manifests (no platform-specific work) runs-on: ubuntu-latest
| - name: Inject npm/yarn registry config | ||
| if: ${{ inputs.npm-registry != '' }} | ||
| run: | | ||
| # Inject registry config files into build context | ||
| # These are automatically read by npm/yarn - no Dockerfile changes needed | ||
| CONTEXT_DIR="${{ inputs.docker-context }}" | ||
| REGISTRY_URL="${{ inputs.npm-registry }}" | ||
|
|
||
| # .npmrc for npm and yarn classic | ||
| echo "registry=${REGISTRY_URL}" > "${CONTEXT_DIR}/.npmrc" | ||
|
|
||
| # .yarnrc for yarn classic (v1) | ||
| echo "registry \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc" | ||
|
|
||
| # .yarnrc.yml for yarn berry (v2+) - only if it doesn't exist | ||
| # (existing .yarnrc.yml likely has project-specific config) | ||
| if [[ ! -f "${CONTEXT_DIR}/.yarnrc.yml" ]]; then | ||
| echo "npmRegistryServer: \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc.yml" | ||
| fi | ||
|
|
||
| echo "Injected registry config pointing to: ${REGISTRY_URL}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overwriting existing .npmrc/.yarnrc may break builds.
The step unconditionally overwrites .npmrc and .yarnrc in the build context. If the project has existing registry config with auth tokens, scoped registries, or other settings, those will be lost. Consider checking for existing files or appending to them.
🐛 Proposed fix to preserve existing config
# .npmrc for npm and yarn classic
- echo "registry=${REGISTRY_URL}" > "${CONTEXT_DIR}/.npmrc"
+ if [[ -f "${CONTEXT_DIR}/.npmrc" ]]; then
+ # Prepend registry to existing config
+ echo "registry=${REGISTRY_URL}" | cat - "${CONTEXT_DIR}/.npmrc" > "${CONTEXT_DIR}/.npmrc.tmp"
+ mv "${CONTEXT_DIR}/.npmrc.tmp" "${CONTEXT_DIR}/.npmrc"
+ else
+ echo "registry=${REGISTRY_URL}" > "${CONTEXT_DIR}/.npmrc"
+ fi
# .yarnrc for yarn classic (v1)
- echo "registry \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc"
+ if [[ -f "${CONTEXT_DIR}/.yarnrc" ]]; then
+ echo "registry \"${REGISTRY_URL}\"" | cat - "${CONTEXT_DIR}/.yarnrc" > "${CONTEXT_DIR}/.yarnrc.tmp"
+ mv "${CONTEXT_DIR}/.yarnrc.tmp" "${CONTEXT_DIR}/.yarnrc"
+ else
+ echo "registry \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc"
+ fi📝 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.
| - name: Inject npm/yarn registry config | |
| if: ${{ inputs.npm-registry != '' }} | |
| run: | | |
| # Inject registry config files into build context | |
| # These are automatically read by npm/yarn - no Dockerfile changes needed | |
| CONTEXT_DIR="${{ inputs.docker-context }}" | |
| REGISTRY_URL="${{ inputs.npm-registry }}" | |
| # .npmrc for npm and yarn classic | |
| echo "registry=${REGISTRY_URL}" > "${CONTEXT_DIR}/.npmrc" | |
| # .yarnrc for yarn classic (v1) | |
| echo "registry \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc" | |
| # .yarnrc.yml for yarn berry (v2+) - only if it doesn't exist | |
| # (existing .yarnrc.yml likely has project-specific config) | |
| if [[ ! -f "${CONTEXT_DIR}/.yarnrc.yml" ]]; then | |
| echo "npmRegistryServer: \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc.yml" | |
| fi | |
| echo "Injected registry config pointing to: ${REGISTRY_URL}" | |
| - name: Inject npm/yarn registry config | |
| if: ${{ inputs.npm-registry != '' }} | |
| run: | | |
| # Inject registry config files into build context | |
| # These are automatically read by npm/yarn - no Dockerfile changes needed | |
| CONTEXT_DIR="${{ inputs.docker-context }}" | |
| REGISTRY_URL="${{ inputs.npm-registry }}" | |
| # .npmrc for npm and yarn classic | |
| if [[ -f "${CONTEXT_DIR}/.npmrc" ]]; then | |
| # Prepend registry to existing config | |
| echo "registry=${REGISTRY_URL}" | cat - "${CONTEXT_DIR}/.npmrc" > "${CONTEXT_DIR}/.npmrc.tmp" | |
| mv "${CONTEXT_DIR}/.npmrc.tmp" "${CONTEXT_DIR}/.npmrc" | |
| else | |
| echo "registry=${REGISTRY_URL}" > "${CONTEXT_DIR}/.npmrc" | |
| fi | |
| # .yarnrc for yarn classic (v1) | |
| if [[ -f "${CONTEXT_DIR}/.yarnrc" ]]; then | |
| echo "registry \"${REGISTRY_URL}\"" | cat - "${CONTEXT_DIR}/.yarnrc" > "${CONTEXT_DIR}/.yarnrc.tmp" | |
| mv "${CONTEXT_DIR}/.yarnrc.tmp" "${CONTEXT_DIR}/.yarnrc" | |
| else | |
| echo "registry \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc" | |
| fi | |
| # .yarnrc.yml for yarn berry (v2+) - only if it doesn't exist | |
| # (existing .yarnrc.yml likely has project-specific config) | |
| if [[ ! -f "${CONTEXT_DIR}/.yarnrc.yml" ]]; then | |
| echo "npmRegistryServer: \"${REGISTRY_URL}\"" > "${CONTEXT_DIR}/.yarnrc.yml" | |
| fi | |
| echo "Injected registry config pointing to: ${REGISTRY_URL}" |
🤖 Prompt for AI Agents
In @.github/workflows/build-and-push-docker-image.yml around lines 172 - 192,
The workflow step "Inject npm/yarn registry config" currently unconditionally
overwrites ${CONTEXT_DIR}/.npmrc and ${CONTEXT_DIR}/.yarnrc; change it to
preserve existing files by checking for existence and merging/appending the
registry setting instead of using overwrite: if the file does not exist create
it, if it exists check for an existing registry entry for ${REGISTRY_URL} and
append the appropriate line only if missing (for .npmrc use
"registry=${REGISTRY_URL}", for .yarnrc use "registry \"${REGISTRY_URL}\"");
keep the current conditional creation for .yarnrc.yml and consider making a
backup (e.g., copy to .npmrc.bak/.yarnrc.bak) before modifying to avoid data
loss.
Summary
Extends the reusable Docker build workflow with support for self-hosted ARC runners, flexible platform selection, registry caching, and Docker build secrets.
New Inputs
build-amd64truebuild-arm64truerunner-amd64ubuntu-24.04runner-arm64ubuntu-24.04-armcache-from''cache-to''docker-secrets''--mount=type=secret)Features
cache-fromandcache-tofor faster builds--mount=type=secretBackward Compatibility
All new inputs have sensible defaults matching previous behavior:
Existing callers continue to work without changes.
Usage Examples
Self-hosted ARC runners (amd64 only)
With registry caching
With Docker build secrets
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.