From 57f234c8daa2f9fc2f0b5b583dca216a7b1ff344 Mon Sep 17 00:00:00 2001 From: Timothy Stewart Date: Sat, 1 Aug 2026 00:35:05 -0500 Subject: [PATCH] fix(ci): make Vagrant box cache version-aware - key immutable caches from a pinned box manifest instead of scenario YAML - skip producer restoration and downloads on exact cache hits - validate lock drift, exact versions, architectures, and missing boxes --- .github/download-boxes.sh | 89 +++++++++++++++++++++++++------- .github/test-download-boxes.sh | 92 ++++++++++++++++++++++++++++++++++ .github/vagrant-boxes.lock | 4 ++ .github/workflows/cache.yml | 15 +++--- .github/workflows/test.yml | 2 +- .pre-commit-config.yaml | 6 +++ 6 files changed, 180 insertions(+), 28 deletions(-) create mode 100755 .github/test-download-boxes.sh create mode 100644 .github/vagrant-boxes.lock diff --git a/.github/download-boxes.sh b/.github/download-boxes.sh index 1e81afa1..48dd29b4 100755 --- a/.github/download-boxes.sh +++ b/.github/download-boxes.sh @@ -1,40 +1,91 @@ #!/bin/bash # download-boxes.sh -# Check all molecule.yml files for required Vagrant boxes and download the ones that are not -# already present on the system. +# Validate the pinned Vagrant box set and download exact versions that are not +# already present in VAGRANT_HOME. set -euo pipefail GIT_ROOT=$(git rev-parse --show-toplevel) PROVIDER=virtualbox +LOCK_FILE="${VAGRANT_BOX_LOCK_FILE:-${GIT_ROOT}/.github/vagrant-boxes.lock}" -# Define the path to the molecule.yml files MOLECULE_YML_PATH=("${GIT_ROOT}"/molecule/*/molecule.yml) -# Extract and sort unique boxes from all molecule.yml files -all_boxes=$(for file in "${MOLECULE_YML_PATH[@]}"; do +# Extract the unique boxes referenced by the scenarios. +declared_boxes=$(for file in "${MOLECULE_YML_PATH[@]}"; do yq -r '.platforms[].box' "$file" done | sort -u) -echo all_boxes: "$all_boxes" +if [[ ! -r "$LOCK_FILE" ]]; then + printf 'Vagrant box lock file is missing or unreadable: %s\n' "$LOCK_FILE" >&2 + exit 1 +fi -# Read the boxes that are currently present on the system (for the current provider) +lock_entries=$(awk ' + /^[[:space:]]*#/ || NF == 0 { next } + NF != 3 { + printf "Invalid lock entry on line %d: expected box, version, architecture\n", NR > "/dev/stderr" + invalid = 1 + next + } + { print $1 " " $2 " " $3 } + END { exit invalid } +' "$LOCK_FILE") + +duplicate_boxes=$(printf '%s\n' "$lock_entries" | awk '{ print $1 }' | sort | uniq -d) +if [[ -n "$duplicate_boxes" ]]; then + printf 'Duplicate Vagrant box lock entries:\n%s\n' "$duplicate_boxes" >&2 + exit 1 +fi + +locked_boxes=$(printf '%s\n' "$lock_entries" | sort) +locked_names=$(printf '%s\n' "$locked_boxes" | awk '{ print $1 }') +missing_locks=$(comm -23 <(printf '%s\n' "$declared_boxes") <(printf '%s\n' "$locked_names")) +unused_locks=$(comm -13 <(printf '%s\n' "$declared_boxes") <(printf '%s\n' "$locked_names")) + +if [[ -n "$missing_locks" || -n "$unused_locks" ]]; then + if [[ -n "$missing_locks" ]]; then + printf 'Scenario boxes missing from the lock file:\n%s\n' "$missing_locks" >&2 + fi + if [[ -n "$unused_locks" ]]; then + printf 'Lock entries not referenced by a scenario:\n%s\n' "$unused_locks" >&2 + fi + exit 1 +fi + +printf 'Pinned Vagrant boxes:\n%s\n' "$locked_boxes" + +# Read exact box, provider, version, and architecture tuples already present. present_boxes=$( - (vagrant box list | - grep "${PROVIDER}" | # Filter by boxes available for the current provider - awk '{print $1;}' | # The box name is the first word in each line - sort | - uniq) || - echo "" # In case any of these commands fails, just use an empty list + vagrant box list --machine-readable | + awk -F, -v expected_provider="$PROVIDER" ' + $3 == "box-name" { name = $4; next } + $3 == "box-provider" { provider = $4; next } + $3 == "box-version" { version = $4; next } + $3 == "box-architecture" { + architecture = $4 + if (provider == expected_provider) { + print name " " version " " architecture + } + name = provider = version = architecture = "" + } + ' | + sort -u ) -# The boxes that we need to download are the ones present in $all_boxes, but not $present_boxes. -download_boxes=$(comm -2 -3 <(echo "${all_boxes}") <(echo "${present_boxes}")) +download_boxes=$(comm -23 \ + <(printf '%s\n' "$locked_boxes") \ + <(printf '%s\n' "$present_boxes")) -# Actually download the necessary boxes -if [ -n "${download_boxes}" ]; then - echo "${download_boxes}" | while IFS= read -r box; do - vagrant box add --provider "${PROVIDER}" "${box}" +if [[ -n "$download_boxes" ]]; then + printf '%s\n' "$download_boxes" | while read -r box version architecture; do + vagrant box add \ + --provider "$PROVIDER" \ + --box-version "$version" \ + --architecture "$architecture" \ + "$box" done +else + printf 'All pinned Vagrant boxes are already present.\n' fi diff --git a/.github/test-download-boxes.sh b/.github/test-download-boxes.sh new file mode 100755 index 00000000..f0365506 --- /dev/null +++ b/.github/test-download-boxes.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash + +# The single-quoted expressions below are written into fake executables and +# intentionally expand only when those executables run. +# shellcheck disable=SC2016 + +set -Eeuo pipefail + +repo_root=$(git rev-parse --show-toplevel) +test_root=$(mktemp -d) +fake_bin="$test_root/bin" +fake_log="$test_root/vagrant.log" +output="$test_root/output.txt" +mkdir -p "$fake_bin" +trap 'rm -rf "$test_root"' EXIT + +printf '%s\n' \ + '#!/usr/bin/env bash' \ + 'set -Eeuo pipefail' \ + 'printf "%s\n" generic/debian12 generic/rocky9 generic/ubuntu2204' \ + >"$fake_bin/yq" + +printf '%s\n' \ + '#!/usr/bin/env bash' \ + 'set -Eeuo pipefail' \ + 'emit_box() {' \ + ' printf "0,,box-name,%s\n" "$1"' \ + ' printf "0,,box-provider,virtualbox\n"' \ + ' printf "0,,box-version,4.3.12\n"' \ + ' printf "0,,box-architecture,amd64\n"' \ + '}' \ + 'if [[ "${1:-}" == box && "${2:-}" == list ]]; then' \ + ' emit_box generic/debian12' \ + ' emit_box generic/rocky9' \ + ' if [[ "${FAKE_PRESENT_MODE:-all}" == all ]]; then' \ + ' emit_box generic/ubuntu2204' \ + ' fi' \ + 'elif [[ "${1:-}" == box && "${2:-}" == add ]]; then' \ + ' printf "%s\n" "$*" >>"${FAKE_VAGRANT_LOG:?}"' \ + 'else' \ + ' printf "Unexpected vagrant arguments: %s\n" "$*" >&2' \ + ' exit 1' \ + 'fi' \ + >"$fake_bin/vagrant" +chmod +x "$fake_bin/yq" "$fake_bin/vagrant" + +PATH="$fake_bin:$PATH" \ + FAKE_VAGRANT_LOG="$fake_log" \ + "$repo_root/.github/download-boxes.sh" >"$output" +grep -Fq 'All pinned Vagrant boxes are already present.' "$output" +[[ ! -e "$fake_log" ]] + +PATH="$fake_bin:$PATH" \ + FAKE_PRESENT_MODE=partial \ + FAKE_VAGRANT_LOG="$fake_log" \ + "$repo_root/.github/download-boxes.sh" >"$output" +grep -Fxq \ + 'box add --provider virtualbox --box-version 4.3.12 --architecture amd64 generic/ubuntu2204' \ + "$fake_log" + +incomplete_lock="$test_root/incomplete.lock" +printf '%s\n' \ + 'generic/debian12 4.3.12 amd64' \ + 'generic/rocky9 4.3.12 amd64' \ + >"$incomplete_lock" +if PATH="$fake_bin:$PATH" \ + VAGRANT_BOX_LOCK_FILE="$incomplete_lock" \ + FAKE_VAGRANT_LOG="$fake_log" \ + "$repo_root/.github/download-boxes.sh" >"$output" 2>&1; then + printf 'Download script accepted a lock missing a scenario box.\n' >&2 + exit 1 +fi +grep -Fq 'Scenario boxes missing from the lock file:' "$output" +grep -Fq 'generic/ubuntu2204' "$output" + +duplicate_lock="$test_root/duplicate.lock" +printf '%s\n' \ + 'generic/debian12 4.3.12 amd64' \ + 'generic/debian12 4.3.11 amd64' \ + 'generic/rocky9 4.3.12 amd64' \ + 'generic/ubuntu2204 4.3.12 amd64' \ + >"$duplicate_lock" +if PATH="$fake_bin:$PATH" \ + VAGRANT_BOX_LOCK_FILE="$duplicate_lock" \ + FAKE_VAGRANT_LOG="$fake_log" \ + "$repo_root/.github/download-boxes.sh" >"$output" 2>&1; then + printf 'Download script accepted duplicate box lock entries.\n' >&2 + exit 1 +fi +grep -Fq 'Duplicate Vagrant box lock entries:' "$output" + +printf 'Vagrant box download tests passed.\n' diff --git a/.github/vagrant-boxes.lock b/.github/vagrant-boxes.lock new file mode 100644 index 00000000..76b433b4 --- /dev/null +++ b/.github/vagrant-boxes.lock @@ -0,0 +1,4 @@ +# box version architecture +generic/debian12 4.3.12 amd64 +generic/rocky9 4.3.12 amd64 +generic/ubuntu2204 4.3.12 amd64 diff --git a/.github/workflows/cache.yml b/.github/workflows/cache.yml index 520607d8..fc2cedb9 100644 --- a/.github/workflows/cache.yml +++ b/.github/workflows/cache.yml @@ -40,18 +40,17 @@ jobs: id: cache-vagrant uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # 6.1.0 with: - lookup-only: true # if it exists, we don't need to restore and can skip the next step + # This producer only needs to know whether the immutable cache exists. + # Molecule jobs restore it after this job completes. + lookup-only: true path: | .vagrant-home/boxes - key: vagrant-boxes-${{ runner.os }}-${{ runner.arch }}-virtualbox-7.2-vagrant-2.4-${{ hashFiles('**/molecule.yml') }} # yamllint disable-line rule:line-length - restore-keys: | - vagrant-boxes-${{ runner.os }}-${{ runner.arch }}-virtualbox-7.2-vagrant-2.4- + key: vagrant-boxes-${{ runner.os }}-${{ runner.arch }}-virtualbox-7.2-vagrant-2.4-${{ hashFiles('.github/vagrant-boxes.lock') }} # yamllint disable-line rule:line-length - name: Download Vagrant boxes for all scenarios - # To save some cache space, all scenarios share the same cache key. - # On the other hand, this means that the cache contents should be - # the same across all scenarios. This step ensures that. - if: steps.cache-vagrant.outputs.cache-hit != 'true' # only run if false since this is just a cache step + # An exact hit skips both cache restoration and upstream downloads. + # A lock change builds and saves one clean, version-pinned cache. + if: steps.cache-vagrant.outputs.cache-hit != 'true' run: | ./.github/download-boxes.sh vagrant box list diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 04c8c486..068f98e9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,7 +57,7 @@ jobs: uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # 6.1.0 with: path: .vagrant-home/boxes - key: vagrant-boxes-${{ runner.os }}-${{ runner.arch }}-virtualbox-7.2-vagrant-2.4-${{ hashFiles('**/molecule.yml') }} # yamllint disable-line rule:line-length + key: vagrant-boxes-${{ runner.os }}-${{ runner.arch }}-virtualbox-7.2-vagrant-2.4-${{ hashFiles('.github/vagrant-boxes.lock') }} # yamllint disable-line rule:line-length fail-on-cache-miss: true - name: Install dependencies diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5087152..8fde5469 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,3 +44,9 @@ repos: language: system pass_filenames: false files: ^\.github/scripts/(cleanup-runner-resources|test-cleanup-runner-resources)\.sh$ + - id: download-vagrant-boxes-test + name: Vagrant box download test + entry: .github/test-download-boxes.sh + language: system + pass_filenames: false + files: ^\.github/(download-boxes|test-download-boxes)\.sh$|^\.github/vagrant-boxes\.lock$