Skip to content

feat :Testing workflow to push images#405

Open
arekay-nv wants to merge 1 commit into
mainfrom
arekay/push_image_workflow
Open

feat :Testing workflow to push images#405
arekay-nv wants to merge 1 commit into
mainfrom
arekay/push_image_workflow

Conversation

@arekay-nv

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds a reproducible way to build and publish the endpoints client image to
ghcr.io/mlcommons/endpoints:

  • scripts/push_docker_image.sh — builds scripts/Dockerfile.dev at a chosen
    git ref via docker buildx and pushes it, tagged with both the short commit SHA
    and the ref name so consumers can pin either.
  • .github/workflows/publish-image.yml — a manually-triggered
    (workflow_dispatch) workflow that runs the script, so a publish can be kicked
    off from the Actions tab against any selected branch/tag/SHA without a local
    Docker setup.

The workflow reuses the script (single source of truth for the tag scheme) rather
than duplicating build logic.

Behavior & design

Aspect Choice
Default platform linux/amd64 (what benchmark clients run on); multi-arch is one flag/input away
Multi-arch --multi-arch / linux/amd64,linux/arm64 publishes a manifest list usable on x86 and arm
Tags :<short-sha> + :<sanitized-ref>
Builder self-provisions a docker-container buildx builder (the default docker driver can neither --push nor build multi-platform)
Auth script logs in via GHCR_USER/GHCR_TOKEN if set, else assumes docker login; workflow uses github.actor + the automatic GITHUB_TOKEN
Safety refuses to publish a :$SHA tag from a dirty tree (--allow-dirty to override); optional ref checkout is restored on exit
PROVISION_DSR1 passthrough build-arg (default = Dockerfile default 1); set 0 for a lean client-only image

CI prerequisites (one-time, outside this PR)

  • Workflow must land on the default branch before the "Run workflow" button appears.
  • permissions: packages: write is set in the workflow; the org must allow the
    GITHUB_TOKEN to write packages, and the endpoints package must grant this repo
    Write access (Package → Manage Actions access). Usually automatic since the
    package lives under this repo's org.

Note: GITHUB_TOKEN authenticates as github-actions[bot] (scoped to this repo);
username: github.actor only sets the login name. For a push attributed to a
specific account, supply that account's PAT as a secret instead.

Validation

  • Multi-arch dry build (--multi-arch --no-push): both linux/amd64 (emulated)
    and linux/arm64 (native) compiled all stages, exit 0.
  • Script is clean under macOS bash 3.2.57 and bash -n; the empty-array
    expansion and tag sanitizer were unit-checked; the dirty-tree guard was exercised
    across push/dry/allow-dirty/clean/checked-out cases.
  • Reviewed via a two-model council (Codex + Claude); all findings fixed: detached-HEAD
    ref resolution, Docker-tag sanitization beyond /, bash-3.2 empty-array crash, and
    the dirty-tree publish guard. Workflow security posture confirmed (inputs passed via
    env: not inline ${{ }}, actions pinned to SHAs, minimal contents:read +
    packages:write).

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor/cleanup

Related issues

Testing

  • Tests added/updated
  • All tests pass locally
  • Manual testing completed

Checklist

  • Code follows project style
  • Pre-commit hooks pass
  • Documentation updated (if needed)

Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
@arekay-nv arekay-nv requested a review from a team July 9, 2026 16:20
@github-actions github-actions Bot requested a review from nvzhihanj July 9, 2026 16:20
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new bash script, scripts/push_docker_image.sh, designed to build and push the endpoints client Docker image to a registry with support for multi-platform builds, custom git refs, and registry authentication. The review feedback highlights two important shell scripting improvements: using printf instead of echo to safely pipe the registry login token, and properly quoting the BUILD_ARGS array expansion to prevent word splitting.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

LOGIN_TOKEN="${GHCR_TOKEN:-${GITHUB_TOKEN:-}}"
if [[ "$PUSH" == "1" && -n "$LOGIN_TOKEN" ]]; then
echo ">> Logging in to ${REGISTRY_HOST} as ${LOGIN_USER:-<token>}"
echo "$LOGIN_TOKEN" | docker login "$REGISTRY_HOST" -u "${LOGIN_USER:-x-access-token}" --password-stdin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using echo to pipe sensitive tokens or arbitrary strings can lead to unexpected behavior if the token starts with a hyphen (e.g., -n or -e), as echo may interpret it as an option. It is safer and more robust to use printf '%s\n' instead.

Suggested change
echo "$LOGIN_TOKEN" | docker login "$REGISTRY_HOST" -u "${LOGIN_USER:-x-access-token}" --password-stdin
printf '%s\n' "$LOGIN_TOKEN" | docker login "$REGISTRY_HOST" -u "${LOGIN_USER:-x-access-token}" --password-stdin

-f "$DOCKERFILE" \
-t "${IMAGE}:${SHORT_SHA}" \
-t "${IMAGE}:${REF_TAG}" \
${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"} \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The array expansion ${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"} is missing outer double quotes. Without outer double quotes, any array elements containing spaces or glob characters will undergo word splitting and pathname expansion when expanded. To safely preserve spaces within array elements while maintaining compatibility with set -u in older Bash versions, wrap the entire expansion in double quotes.

Suggested change
${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"} \
"${BUILD_ARGS[@]+"${BUILD_ARGS[@]}"}" \

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant