fix: address reviewer findings round 2 (#1261 loop 2) (#1272)

* fix(docker): parameterize image with CCS_IMAGE env so smoke-test exercises new digest

docker/compose.yaml hardcoded image: ghcr.io/kaitranntt/ccs:latest, causing
network-contract.sh to always use the old published image regardless of what
IMAGE_OVERRIDE was passed. Switching to \${CCS_IMAGE:-ghcr.io/kaitranntt/ccs:latest}
lets CI pass CCS_IMAGE=<digest> so the smoke-test actually exercises the
just-built image. Default end-user behaviour is unchanged.

Resolves REV4 (reviewer loop 2, PR #1261).

* fix(test): image-size.sh fails loudly on manifest inspection failure

The --platform branch previously exited 0 when imagetools inspect returned
empty or zero bytes, silently passing the budget check. A broken image
could reach production undetected if buildx had any inspection issue.

Changed to exit 1 with a clear diagnostic message explaining possible causes
(old buildx, manifest format mismatch, image not yet pushed). No --allow-inspect-failure
escape hatch is provided; CI must fix the root cause.

Resolves REV5 (reviewer loop 2, PR #1261).

* test(docker): cover --platform branch in image-size-logic tests

Prior tests only exercised the local docker image inspect path. The
--platform branch (imagetools inspect) had zero coverage, meaning the
REV5 silent-pass regression would not have been caught by CI.

Added 5 mock-based test cases for the --platform branch:
- pass when platform-scoped compressed size < budget
- fail when platform-scoped compressed size > budget
- fail (exit 1) when imagetools inspect errors (REV5 guard)
- fail (exit 1) when reported size is "0" (REV5 guard)
- fail (exit 1) when size output is empty (REV5 guard)

Mocks follow the existing pattern (override docker in PATH with a temp
wrapper), extended to handle buildx imagetools inspect subcommand.

Resolves REV6 (reviewer loop 2, PR #1261).

* fix(test): compose-parity image_name() handles \${VAR:-default} syntax

After parameterizing compose.yaml's image field with \${CCS_IMAGE:-...},
the image_name() sed pipeline cut at the first colon in the shell variable
syntax (:-) instead of the tag separator, returning '\${CCS_IMAGE' and
failing the kaitranntt/ccs grep.

Added a sed step to unwrap \${VAR:-default} by extracting just the default
value before stripping the tag. Existing plain image: name:tag references
are unaffected.
This commit is contained in:
Kai (Tam Nhu) Tran
2026-05-17 05:01:15 -04:00
committed by GitHub
parent 78004746be
commit adf5a836fc
4 changed files with 93 additions and 6 deletions
+1 -1
View File
@@ -14,7 +14,7 @@
services:
ccs:
image: ghcr.io/kaitranntt/ccs:latest
image: ${CCS_IMAGE:-ghcr.io/kaitranntt/ccs:latest}
restart: unless-stopped
ports:
- "3000:3000"
+7 -2
View File
@@ -25,14 +25,19 @@ ok() { printf '[OK] %s\n' "$*"; }
fail() { printf '[X] %s\n' "$*" >&2; fail=1; }
# ---------------------------------------------------------------------------
# Helper: extract image name (repo path without tag) for a service
# Helper: extract image name (repo path without tag) for a service.
# Handles both plain image references and ${VAR:-default} shell variable
# syntax (e.g. image: ${CCS_IMAGE:-ghcr.io/kaitranntt/ccs:latest}).
# ---------------------------------------------------------------------------
image_name() {
local file="$1" service="$2"
# Match lines like: image: ghcr.io/owner/repo:tag or image: name:tag
# Match lines like:
# image: ghcr.io/owner/repo:tag
# image: ${CCS_IMAGE:-ghcr.io/owner/repo:tag}
grep -A 50 "^ ${service}:" "$file" \
| grep -m1 '^\s*image:' \
| sed 's/.*image:\s*//' \
| sed 's/\${[^:-]*:-\([^}]*\)}/\1/' \
| sed 's/:.*//' \
| tr -d ' '
}
+81
View File
@@ -120,6 +120,87 @@ else
(( FAIL++ )) || true
fi
# ------------------------------------------------------------------
# --platform branch tests (multi-arch via imagetools)
# ------------------------------------------------------------------
# Mock docker that handles `docker buildx imagetools inspect` by echoing
# a fixed layer-size string (space-separated byte counts as imagetools does).
# The mock ignores the --format flag and just prints precomputed sizes.
# ------------------------------------------------------------------
make_mock_docker_platform() {
local layer_output="$1" # space-separated byte values, e.g. "100000000 50000000"
cat > "${MOCK_DIR}/docker" <<'MOCK_EOF'
#!/usr/bin/env bash
# Mock docker for --platform branch tests
if [[ "$1" == "buildx" && "$2" == "imagetools" && "$3" == "inspect" ]]; then
MOCK_EOF
# Inject the layer_output value into the mock script
printf ' echo "%s"\n' "$layer_output" >> "${MOCK_DIR}/docker"
cat >> "${MOCK_DIR}/docker" <<'MOCK_EOF'
exit 0
fi
# image inspect / pull / other sub-commands: succeed silently
exit 0
MOCK_EOF
chmod +x "${MOCK_DIR}/docker"
}
make_mock_docker_platform_fail() {
# imagetools inspect returns nothing (simulates buildx incompatibility)
cat > "${MOCK_DIR}/docker" <<'MOCK_EOF'
#!/usr/bin/env bash
if [[ "$1" == "buildx" && "$2" == "imagetools" && "$3" == "inspect" ]]; then
exit 1
fi
exit 0
MOCK_EOF
chmod +x "${MOCK_DIR}/docker"
}
run_platform_test() {
local name="$1"
local expected_exit="$2"
local max_bytes="$3"
# mock docker already set by caller
local actual_exit=0
PATH="${MOCK_DIR}:${PATH}" bash "$SCRIPT" "mock-image:tag" "$max_bytes" \
--platform linux/amd64 > /dev/null 2>&1 || actual_exit=$?
if [[ "$actual_exit" -eq "$expected_exit" ]]; then
echo "[OK] ${name}"
(( PASS++ )) || true
else
echo "[X] ${name}: expected exit ${expected_exit}, got ${actual_exit}"
(( FAIL++ )) || true
fi
}
echo ""
echo "Running --platform branch tests..."
echo ""
# --platform pass: two layers summing to 150 MB, budget 200 MB
make_mock_docker_platform "100000000 57671680"
run_platform_test "--platform: pass when platform-scoped size < budget" 0 "209715200"
# --platform fail: two layers summing to 250 MB, budget 200 MB
make_mock_docker_platform "150000000 112000000"
run_platform_test "--platform: fail when platform-scoped size > budget" 1 "209715200"
# --platform inspect failure → must exit 1 (REV5 regression guard)
make_mock_docker_platform_fail
run_platform_test "--platform: fail loudly when imagetools inspect fails (REV5 guard)" 1 "209715200"
# --platform returns "0" → must exit 1 (REV5 guard for zero-byte output)
make_mock_docker_platform "0"
run_platform_test "--platform: fail loudly when reported size is 0 (REV5 guard)" 1 "209715200"
# --platform returns empty string → must exit 1 (REV5 guard for empty output)
make_mock_docker_platform ""
run_platform_test "--platform: fail loudly when size output is empty (REV5 guard)" 1 "209715200"
# ------------------------------------------------------------------
# Summary
# ------------------------------------------------------------------
+4 -3
View File
@@ -80,9 +80,10 @@ if [[ -n "$PLATFORM" ]]; then
fi
if [[ -z "$ACTUAL_BYTES" || "$ACTUAL_BYTES" == "0" ]]; then
echo "[!] Could not determine size for ${IMAGE} platform=${PLATFORM} — skipping budget check" >&2
echo " (imagetools may not support format templates on this buildx version)" >&2
exit 0
echo "[X] Could not determine size for ${IMAGE} platform=${PLATFORM}" >&2
echo " Possible causes: buildx version too old, manifest format unsupported, image not pushed yet" >&2
echo " Refusing to silently pass — fix the inspection or push the image first" >&2
exit 1
fi
else
# Local-image path: use docker image inspect (host architecture only)