fix(build): bound retries for Bicep downloads

build/scripts/install-bicep.sh

@@ -55,22 +55,52 @@ cleanup() {
 # cross-host redirects, so the token is not sent to the download CDN. The array is
 # seeded with the User-Agent so it is never empty -- expanding an empty array
 # under 'set -u' is an error on bash 3.2 (macOS).
+# Takes a URL and output file; prints the final URL only after success.
 gh_curl() {
+    local url="$1" output="$2"
+    shift 2
+    local result status http_code remaining timeout
+    local deadline=$((SECONDS + 300)) attempt=0 delay=1
     local headers=(-H "User-Agent: ${REPO##*/}-installer")
     if [ -n "${GITHUB_TOKEN:-}" ]; then
         headers+=(-H "Authorization: Bearer ${GITHUB_TOKEN}")
     fi
-    # --retry rides out transient failures (timeouts and HTTP 408/429/5xx such as
-    # the 504 gateway timeouts GitHub's release CDN returns intermittently) with
-    # exponential backoff. --retry-all-errors extends that same backoff to
-    # failures curl's default retry logic does not classify as transient,
-    # notably curl 56 ("Connection died"), the dropped/reset mid-download
-    # connections seen intermittently against the release CDN (see #12661).
-    # Combined with -f (used by every caller below), --retry-all-errors also
-    # retries HTTP 404s (e.g. a wrong/nonexistent version) instead of failing
-    # immediately, so a bad version now takes the full retry budget (~31s) to
-    # fail rather than failing fast.
-    curl --proto '=https' --tlsv1.2 --retry 5 --retry-connrefused --retry-all-errors "${headers[@]}" "$@"
+    # Keep one budget for HTTP failures and dropped connections. curl's
+    # --retry-all-errors also retries permanent HTTP and certificate errors.
+    while true; do
+        remaining=$((deadline - SECONDS))
+        if ((remaining <= 0)); then
+            log "GitHub request exceeded the 300-second retry budget"
+            return 28
+        fi
+        timeout=60
+        if ((remaining < timeout)); then timeout="$remaining"; fi
+        status=0
+        result="$(curl --proto '=https' --tlsv1.2 -fsSL \
+            --connect-timeout 15 --max-time "$timeout" \
+            "${headers[@]}" "$@" -o "$output" \
+            -w '%{http_code} %{url_effective}' "$url")" || status=$?
+        if ((status == 0)); then
+            printf '%s\n' "${result#* }"
+            return 0
+        fi
+        http_code="${result%% *}"
+        case "${status}:${http_code}" in
+            22:408 | 22:429 | 22:500 | 22:502 | 22:503 | 22:504) ;;
+            5:* | 6:* | 7:* | 18:* | 28:* | 52:* | 55:* | 56:*) ;;
+            *) return "$status" ;;
+        esac
+        remaining=$((deadline - SECONDS))
+        if ((attempt >= 10 || remaining <= delay)); then
+            log "GitHub request exhausted its retry budget"
+            return "$status"
+        fi
+        log "request failed (curl ${status}, HTTP ${http_code}); retry in ${delay}s"
+        sleep "$delay"
+        attempt=$((attempt + 1))
+        delay=$((delay * 2))
+        if ((delay > 60)); then delay=60; fi
+    done
 }
 
 detect_os() {
@@ -93,7 +123,7 @@ detect_arch() {
 # Avoids the GitHub API (no token, no rate limit).
 resolve_latest_version() {
     local effective_url
-    effective_url="$(gh_curl -fsSLI -o /dev/null -w '%{url_effective}' "${RELEASES_URL}/latest")" \
+    effective_url="$(gh_curl "${RELEASES_URL}/latest" /dev/null --head)" \
         || fail "could not resolve the latest bicep version"
     printf '%s\n' "${effective_url##*/tag/}"
 }
@@ -174,13 +204,15 @@ main() {
     WORKDIR="$(mktemp -d)"
 
     log "downloading ${asset} ${version}..."
-    gh_curl -fsSL "${RELEASES_URL}/download/${version}/${asset}" -o "${WORKDIR}/bicep" \
+    gh_curl "${RELEASES_URL}/download/${version}/${asset}" \
+        "${WORKDIR}/bicep" >/dev/null \
         || fail "could not download ${asset} ${version}"
 
     # Azure/bicep does not publish checksums, so verification only happens when a
     # pinned checksum is supplied (the common case via build/tools.yaml).
     if [ -n "$checksum" ]; then
-        verify_checksum "$checksum" "${WORKDIR}/bicep"
+        verify_checksum "$checksum" "${WORKDIR}/bicep" \
+            || fail "checksum verification failed for ${asset} ${version}"
     else
         log "WARNING: no checksum supplied for ${platform}; installing without verification."
     fi
@@ -200,5 +232,7 @@ main() {
     fi
 }
 
-trap cleanup EXIT
-main "$@"
+if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
+    trap cleanup EXIT
+    main "$@"
+fi

build/scripts/install-bicep_test.sh

@@ -0,0 +1,188 @@
+#!/usr/bin/env bash
+
+# Check the Bicep download policy with a fake clock and curl responses.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+readonly SCRIPT_DIR
+# shellcheck source=build/scripts/install-bicep.sh
+source "${SCRIPT_DIR}/install-bicep.sh"
+
+# Unsetting SECONDS removes Bash's clock behavior for deterministic tests.
+unset SECONDS
+SECONDS=0
+
+TEST_DIR="$(mktemp -d)"
+readonly TEST_DIR
+trap 'cleanup; rm -rf "${TEST_DIR}"' EXIT
+
+fail_test() {
+    echo "FAIL: $*" >&2
+    exit 1
+}
+
+count_lines() {
+    local _line count=0
+    while IFS= read -r _line; do
+        count=$((count + 1))
+    done < "$1"
+    printf '%s\n' "$count"
+}
+
+curl() {
+    local output="" timeout="" connect_timeout="" format=""
+    local status http_code url="${RELEASES_URL}/tag/v0.46.1"
+    while (($#)); do
+        case "$1" in
+            -o) output="$2"; shift ;;
+            -w) format="$2"; shift ;;
+            --max-time) timeout="$2"; shift ;;
+            --connect-timeout) connect_timeout="$2"; shift ;;
+            --head) echo head >> "${TEST_DIR}/heads" ;;
+            --retry*) fail_test "curl must not have a second retry policy" ;;
+        esac
+        shift
+    done
+    [[ "$connect_timeout" == 15 ]] || fail_test "connection timeout"
+    [[ "$format" == '%{http_code} %{url_effective}' ]] \
+        || fail_test "response metadata"
+    [[ "$timeout" -gt 0 && "$timeout" -le 60 ]] \
+        || fail_test "attempt timeout"
+    echo "$timeout" >> "${TEST_DIR}/timeouts"
+    local count
+    count="$(count_lines "${TEST_DIR}/timeouts")"
+    status="${RESPONSES[count - 1]:-${RESPONSES[${#RESPONSES[@]} - 1]}}"
+    http_code="${status#*:}"
+    status="${status%%:*}"
+    if [[ "$output" != /dev/null ]]; then
+        if [[ "$status" == 0 ]]; then
+            printf '%s' "$PAYLOAD" > "$output"
+        else
+            printf partial > "$output"
+        fi
+    fi
+    printf '%s %s' "$http_code" "$url"
+    return "$status"
+}
+
+sleep() {
+    echo "$1" >> "${TEST_DIR}/delays"
+    SECONDS=$((SECONDS + $1))
+}
+
+reset_case() {
+    : > "${TEST_DIR}/timeouts"
+    : > "${TEST_DIR}/delays"
+    : > "${TEST_DIR}/heads"
+    RESPONSES=("$@")
+    PAYLOAD="verified bicep fixture"
+    SECONDS=0
+}
+
+check_request() {
+    local expected="$1" actual=0
+    gh_curl "${RELEASES_URL}/download/v0.46.1/bicep-linux-x64" \
+        "${TEST_DIR}/download" > "${TEST_DIR}/url" \
+        2> "${TEST_DIR}/log" || actual=$?
+    [[ "$actual" == "$expected" ]] \
+        || fail_test "expected curl status ${expected}, got ${actual}"
+}
+
+for response in 22:408 22:429 22:500 22:502 22:503 22:504 \
+    5:000 6:000 7:000 18:200 28:000 52:000 55:200 56:200; do
+    reset_case "$response" 0:200
+    check_request 0
+    [[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 2 ]] \
+        || fail_test "did not retry ${response}"
+    [[ "$(< "${TEST_DIR}/download")" == "$PAYLOAD" ]] \
+        || fail_test "partial download was retained"
+done
+
+for response in 22:400 22:401 22:403 22:404 22:501 23:200 60:000; do
+    reset_case "$response" 0:200
+    check_request "${response%%:*}"
+    [[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 1 ]] \
+        || fail_test "retried permanent error ${response}"
+done
+
+# Six failed requests exhausted the old five-retry policy.
+reset_case 22:500 22:500 22:500 22:500 22:500 22:500 0:200
+check_request 0
+[[ "$(paste -sd, "${TEST_DIR}/delays")" == 1,2,4,8,16,32 ]] \
+    || fail_test "unexpected backoff"
+
+reset_case 22:500
+check_request 22
+[[ "$SECONDS" -eq 243 ]] || fail_test "unexpected retry deadline"
+[[ "$(tail -1 "${TEST_DIR}/timeouts")" -eq 57 ]] \
+    || fail_test "last attempt did not use remaining budget"
+[[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 10 ]] \
+    || fail_test "unbounded HTTP retries"
+
+reset_case 56:200
+check_request 56
+[[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 10 ]] \
+    || fail_test "unbounded dropped-connection retries"
+
+reset_case 22:500 56:200 22:404 0:200
+check_request 22
+[[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 3 ]] \
+    || fail_test "permanent error after transient errors"
+
+# A request must not start after the shared deadline.
+sleep() { SECONDS=$((SECONDS + 300)); }
+reset_case 22:500
+check_request 28
+[[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 1 ]] \
+    || fail_test "request started after deadline"
+
+# A clock that does not advance still stops at the retry-count limit.
+sleep() { echo "$1" >> "${TEST_DIR}/delays"; }
+reset_case 22:500
+check_request 22
+[[ "$(count_lines "${TEST_DIR}/timeouts")" -eq 11 ]] \
+    || fail_test "retry-count limit"
+
+reset_case 0:200
+[[ "$(resolve_latest_version)" == v0.46.1 ]] \
+    || fail_test "latest version resolution"
+[[ "$(count_lines "${TEST_DIR}/heads")" -eq 1 ]] \
+    || fail_test "latest version must use HEAD"
+
+# Exercise checksum validation and preserve an existing binary on failure.
+uname() {
+    case "$1" in
+        -s) echo Linux ;;
+        -m) echo x86_64 ;;
+    esac
+}
+BICEP_VERSION=v0.46.1
+BICEP_OS=linux
+BICEP_ARCH=arm64
+GITHUB_TOKEN=""
+GITHUB_PATH="${TEST_DIR}/github-path"
+export BICEP_VERSION BICEP_OS BICEP_ARCH GITHUB_TOKEN GITHUB_PATH
+reset_case 0:200
+if command -v sha256sum >/dev/null 2>&1; then
+    BICEP_CHECKSUM_LINUX_ARM64="$(printf '%s' "$PAYLOAD" | sha256sum)"
+else
+    BICEP_CHECKSUM_LINUX_ARM64="$(printf '%s' "$PAYLOAD" | shasum -a 256)"
+fi
+BICEP_CHECKSUM_LINUX_ARM64="${BICEP_CHECKSUM_LINUX_ARM64%% *}"
+export BICEP_CHECKSUM_LINUX_ARM64
+main "${TEST_DIR}/install" > "${TEST_DIR}/main-log" 2>&1
+[[ "$(< "${TEST_DIR}/install/bicep")" == "$PAYLOAD" ]] \
+    || fail_test "verified binary was not installed"
+[[ ! -e "$GITHUB_PATH" ]] || fail_test "cross-platform PATH export"
+cleanup
+
+reset_case 0:200
+PAYLOAD="corrupt fixture"
+if (trap cleanup EXIT; main "${TEST_DIR}/install") \
+    > "${TEST_DIR}/main-log" 2>&1; then
+    fail_test "checksum mismatch was accepted"
+fi
+[[ "$(< "${TEST_DIR}/install/bicep")" == "verified bicep fixture" ]] \
+    || fail_test "checksum failure replaced the existing binary"
+
+echo "PASS: Bicep download policy and checksum checks"

build/test.mk

@@ -60,6 +60,12 @@ test: test-get-envtools test-helm test-manage-radius-installation test-release-p
 test-manage-radius-installation: ## Tests Radius installation lifecycle reconciliation
 	@bash ./.github/scripts/manage-radius-installation_test.sh
 
+.PHONY: test-install-bicep
+test-install-bicep: ## Tests bounded Bicep downloads without network access
+	@bash ./build/scripts/install-bicep_test.sh
+
+test: test-install-bicep
+
 .PHONY: test-cluster-diagnostics
 test-cluster-diagnostics: ## Tests workflow diagnostics without a Kubernetes cluster
 	@bash ./.github/scripts/collect-cluster-diagnostics_test.sh

docs/contributing/contributing-code/contributing-code-shell-and-make/README.md

@@ -23,6 +23,8 @@ The canonical versions, release sources, platform assets, and SHA-256 checksums
 
 Bicep remains on a manually tested pin (`update: false`) because upgrades must preserve ACR authentication, local registry trust, and extension compatibility. Its `versionFiles` entry synchronizes the version in `pkg/cli/bicep/tools/download_tools.go`, keeping `rad bicep download`, `make install-bicep`, and Bicep container staging on the same release. Local registries require [explicit hostname trust](../contributing-code-tests/running-functional-tests.md#local-bicep-registry-trust); do not disable registry or certificate validation to test an upgrade.
 
+The Bicep installer retries temporary download failures with increasing delays, up to 10 retries within a 300-second budget per request. Each attempt has a 15-second connection timeout and a total timeout of at most 60 seconds, reduced to the remaining budget. HTTP 408, 429, 500, 502, 503, and 504, connection failures, and incomplete transfers can be retried. Permanent errors, such as HTTP 403 or 404 and invalid certificates, fail at once. The installer checks the pinned checksum before it replaces the binary. Run `make test-install-bicep` to check this policy without network access.
+
 ## Updating resource types and recipe packs
 
 [`deploy/manifest/defaults.yaml`](../../../../deploy/manifest/defaults.yaml) pins the resource-types-contrib commits that Radius ships. `make update-resource-types`, `make update-recipe-packs`, and `make update-resource-types-and-recipe-packs` move the pins; `make sync-resource-types` regenerates the vendored files from the current pins without changing them.