From ab0b4d7d0fb513c8592e164c57ce8d88536fc97a Mon Sep 17 00:00:00 2001 From: ergosteur <1992147+ergosteur@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:10:51 +0000 Subject: [PATCH] Make update-image-pins.sh shellcheck-clean shellcheck flagged three things, one of which was worth acting on beyond silencing it. echo was emitting a script fragment containing a literal \n meant for the generated code, not for this shell. Bash's echo leaves it alone by default, but the handling of backslashes is implementation-defined and xpg_echo would have expanded it and corrupted the fragment. printf '%s\n' instead. The discover_* functions were dispatched as "discover_$distro", which reads as dead code to shellcheck and to anyone grepping for a caller. An explicit case dispatch replaces it, which is what SC2317 was really complaining about. The two remaining SC2016 hits are intentional single quotes, one a sed address matching a literal $DISTRO_CHOICE and one emitting code, and are annotated rather than fixed. Also reject an unknown --distro. It previously filtered every distro out and reported "all pins are already current" with exit 0, so a typo looked like a clean run. Co-Authored-By: Claude Opus 5 (1M context) --- update-image-pins.sh | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/update-image-pins.sh b/update-image-pins.sh index 8accca0..61da2a5 100755 --- a/update-image-pins.sh +++ b/update-image-pins.sh @@ -34,6 +34,9 @@ TARGET="$SCRIPT_DIR/build-cloudinit-template.sh" WRITE=false ONLY=() +# Arch is absent deliberately: it is rolling and has no pin to check. +DISTROS=(alpine ubuntu debian fedora almalinux) + while [[ $# -gt 0 ]]; do case $1 in -h|--help) usage; exit 0 ;; @@ -50,6 +53,15 @@ for bin in curl sed grep sort; do command -v "$bin" >/dev/null || { echo "Required command '$bin' not found." >&2; exit 1; } done +# Reject a misspelled --distro rather than filtering everything out and +# reporting "all pins are current", which reads as success. +for d in ${ONLY[@]+"${ONLY[@]}"}; do + [[ " ${DISTROS[*]} " == *" $d "* ]] && continue + echo "Error: unknown distro '$d'." >&2 + echo "Choose one of: ${DISTROS[*]} (arch is rolling and has no pin)" >&2 + exit 1 +done + [[ -f "$TARGET" ]] || { echo "Error: template script not found at $TARGET" >&2; exit 1; } [[ -w "$TARGET" ]] || [[ "$WRITE" == false ]] || { echo "Error: $TARGET is not writable." >&2; exit 1; } @@ -87,6 +99,7 @@ current_pin() { image_urls_from() { local script=$1 pins cases out pins=$(sed -n '/^# --- Distribution version pins/,/^# Arch is rolling/p' "$script") + # shellcheck disable=SC2016 # a literal $DISTRO_CHOICE in the sed address cases=$(sed -n '/^case \$DISTRO_CHOICE in$/,/^esac$/p' "$script") # Both anchors must have matched something recognisable. Without this an # unmatched anchor yields an empty fragment that still runs, producing URLs @@ -101,7 +114,10 @@ image_urls_from() { printf '%s\n' "$pins" echo 'for DISTRO_CHOICE in alpine ubuntu debian fedora almalinux arch; do' printf '%s\n' "$cases" - echo 'printf "%s %s\n" "$DISTRO_CHOICE" "$IMAGE_URL"; done' + # printf, not echo: the \n belongs to the emitted fragment, and + # echo's handling of backslashes is implementation-defined. + # shellcheck disable=SC2016 # emitting code, expansion happens there + printf '%s\n' 'printf "%s %s\n" "$DISTRO_CHOICE" "$IMAGE_URL"; done' } | bash 2>/dev/null ) || true # Six distros in, six URLs out; anything less means the template no longer @@ -189,9 +205,21 @@ discover_alpine() { printf 'ALPINE_MAJOR=v%s\nALPINE_PATCH=%s\nALPINE_PREFIX=%s\n' "${patch%.*}" "$patch" "$prefix" } +# Dispatch explicitly rather than calling "discover_$distro". The indirect form +# reads as dead code to both shellcheck and to anyone grepping for a caller. +discover() { + case $1 in + alpine) discover_alpine ;; + ubuntu) discover_ubuntu ;; + debian) discover_debian ;; + fedora) discover_fedora ;; + almalinux) discover_almalinux ;; + *) return 1 ;; + esac +} + # --- Collect proposals ------------------------------------------------------ -DISTROS=(alpine ubuntu debian fedora almalinux) declare -A PROPOSED=() CHANGED=() FAILED=() @@ -199,7 +227,7 @@ FAILED=() echo "Checking upstream for current stable releases..." for distro in "${DISTROS[@]}"; do wanted "$distro" || continue - if ! result=$("discover_$distro"); then + if ! result=$(discover "$distro"); then echo " $distro: lookup FAILED" >&2 FAILED+=("$distro") continue