diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index 0bf688ed2..a645c769e 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -25,9 +25,13 @@ inputs: required: false default: "rustfs-deps" cache-save-if: - description: "Condition for saving cache" + description: >- + Whether to save the cache. The fail-safe default is 'false': a caller that + wants to populate a cache must opt in explicitly, so a forgotten input + costs a cold cache (minutes) rather than silently consuming the + repository-wide 10GB Actions cache quota and evicting other lanes. required: false - default: "true" + default: "false" install-cross-tools: description: "Install cross-compilation tools" required: false diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index a7a5c914f..408c8684f 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -75,10 +75,27 @@ jobs: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - - name: Setup Rust environment - uses: ./.github/actions/setup + # cargo-deny compiles nothing, so the full setup composite (apt packages, + # protoc, flatc, nextest, rustfmt/clippy) was pure overhead here. It does + # still need a real cargo: `cargo deny check` runs `cargo metadata`, and + # Cargo.toml pins datafusion and s3s as git dependencies, which must be + # materialised into ~/.cargo/git — a cold clone is hundreds of MB, so the + # cache stays. + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + + # Was relying on the composite's default, which used to be "true": every + # PR touching Cargo.toml/Cargo.lock saved a second, PR-scoped copy of this + # cache and pushed the main-scoped lanes out of the 10GB quota. The + # default is now "false", but state it explicitly — see + # scripts/security/check_cache_save_if.sh. + - name: Setup Rust cache + uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 with: - cache-shared-key: rustfs-cargo-deny + cache-all-crates: true + cache-on-failure: true + shared-key: rustfs-cargo-deny + save-if: ${{ github.ref == 'refs/heads/main' }} - name: Install cargo-deny uses: taiki-e/install-action@bffeee26d4db9be238a4ea78d8826604ebcb594d # v2 @@ -100,6 +117,9 @@ jobs: - name: Report unpinned GitHub Actions run: ./scripts/security/check_workflow_pins.sh --enforce + - name: Check setup cache-save-if is explicit + run: ./scripts/security/check_cache_save_if.sh + - name: Check preview release workflow policy run: ./scripts/security/check_preview_release_workflow.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cfb6cd337..696532a90 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -254,7 +254,15 @@ jobs: target: ${{ matrix.target }} cache-shared-key: build-${{ matrix.target }} github-token: ${{ secrets.GITHUB_TOKEN }} - cache-save-if: ${{ github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/') }} + # main only. A cache saved on refs/tags/X is scoped to that tag: no + # other tag, no main run and no PR can restore it, so every release + # cycle wrote up to 12 entries of 1-2GB (preview tag plus final tag, + # six legs each) that nobody could read, evicting the hot lanes from + # the repo-wide 10GB quota. Tag builds still restore the main-scoped + # cache, since default-branch caches are readable from every ref. + # The one real cost: re-running a failed leg of the same tag no longer + # finds that tag's own warm cache and falls back to main's. + cache-save-if: ${{ github.ref == 'refs/heads/main' }} install-cross-tools: ${{ matrix.cross }} - name: Download static console assets diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 450ef6ac1..d663451e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -169,7 +169,7 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-test + cache-shared-key: ci-dev github-token: ${{ secrets.GITHUB_TOKEN }} cache-save-if: ${{ github.ref == 'refs/heads/main' }} @@ -183,6 +183,19 @@ jobs: echo "started_at=$(date --utc --iso-8601=seconds)" } > artifacts/test-and-lint/run-metadata.txt + # This job is the sole writer of the ci-dev cache, so what it builds is + # what every reader restores. Its own steps are not a superset of theirs: + # clippy only emits metadata, the nextest pass excludes e2e_test, and + # nothing here enables e2e-test-hooks (a different feature set means a + # different -Cmetadata). Without these two builds the readers restore a + # cache that is missing exactly the dependencies they need. Runs on main + # only, so it costs nothing on the PR critical path. + - name: Warm shared cache superset (main only) + if: github.ref == 'refs/heads/main' + run: | + cargo build --tests -p e2e_test + cargo build -p rustfs --bins --features e2e-test-hooks + # Clippy runs before the test pass: lint failures are the most common # CI-only breakage and should surface in minutes, not after 20+ minutes # of tests. @@ -362,9 +375,9 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-ilm-serial + cache-shared-key: ci-dev github-token: ${{ secrets.GITHUB_TOKEN }} - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-save-if: 'false' # test_transition_and_restore_flows was re-enabled by rustfs/backlog#1303: # its "missing xl.meta on disk2" was a test-util bug (open_disk hardcoded @@ -400,10 +413,17 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-test-rio-v2 + cache-shared-key: ci-feat-rio github-token: ${{ secrets.GITHUB_TOKEN }} cache-save-if: ${{ github.ref == 'refs/heads/main' }} + # Sole writer of ci-feat-rio. The reader (build-rustfs-debug-binary-rio-v2) + # builds with rio-v2,e2e-test-hooks, which this job never enables, so warm + # that combination here or the reader restores a cache without it. + - name: Warm shared cache superset (main only) + if: github.ref == 'refs/heads/main' + run: cargo build -p rustfs --bins --features rio-v2,e2e-test-hooks + - name: Run rio-v2 clippy lints run: cargo clippy -p rustfs -p rustfs-ecstore --all-targets --features rio-v2 -- -D warnings @@ -442,9 +462,17 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-test-${{ matrix.features.name }} + cache-shared-key: ci-feat-proto github-token: ${{ secrets.GITHUB_TOKEN }} - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-save-if: ${{ github.ref == 'refs/heads/main' && matrix.features.name == 'swift' }} + + # The swift leg is the sole writer of ci-feat-proto, shared with the sftp + # leg. Build the other leg's feature set too so the saved target/ holds + # the union; running them in sequence is safer than `--features + # swift,sftp`, which is a combination no lane actually compiles. + - name: Warm shared cache superset (main only) + if: github.ref == 'refs/heads/main' && matrix.features.name == 'swift' + run: cargo clippy -p rustfs -p rustfs-protocols --all-targets --features sftp -- -D warnings - name: Run clippy with ${{ matrix.features.name }} run: | @@ -470,8 +498,8 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-rustfs-debug-binary - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-shared-key: ci-dev + cache-save-if: 'false' github-token: ${{ secrets.GITHUB_TOKEN }} - name: Build debug binary @@ -501,8 +529,8 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-rustfs-debug-binary-rio-v2 - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-shared-key: ci-feat-rio + cache-save-if: 'false' github-token: ${{ secrets.GITHUB_TOKEN }} - name: Build debug binary with rio-v2 @@ -541,6 +569,13 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable + # Keeps its own key rather than joining ci-dev. rust-cache's key is + # built from runner.os/arch plus rustc and lockfile fingerprints — it + # does NOT include the runner label or image. ubuntu-latest and + # sm-standard-4 are therefore indistinguishable to it, so sharing a key + # would let two different system images overwrite each other's + # artifacts, and would make a 2-core hosted runner unpack ci-dev's ~3GB + # instead of this lane's ~1.3GB. cache-shared-key: ci-uring github-token: ${{ secrets.GITHUB_TOKEN }} cache-save-if: ${{ github.ref == 'refs/heads/main' }} @@ -591,9 +626,9 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-e2e + cache-shared-key: ci-dev github-token: ${{ secrets.GITHUB_TOKEN }} - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-save-if: 'false' # Download after the cache restore so the freshly built binary from the # build job always wins over anything restored into target/debug. @@ -672,9 +707,9 @@ jobs: uses: ./.github/actions/setup with: rust-version: stable - cache-shared-key: ci-e2e + cache-shared-key: ci-dev github-token: ${{ secrets.GITHUB_TOKEN }} - cache-save-if: ${{ github.ref == 'refs/heads/main' }} + cache-save-if: 'false' # Download after the cache restore so the freshly built binary from the # build job always wins over anything restored into target/debug. diff --git a/scripts/security/check_cache_save_if.sh b/scripts/security/check_cache_save_if.sh new file mode 100755 index 000000000..3e63bdc8a --- /dev/null +++ b/scripts/security/check_cache_save_if.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Every `uses: ./.github/actions/setup` must state `cache-save-if` explicitly. +# +# The composite action's default is "false" (fail-safe: a forgotten input costs +# a cold cache, not a slice of the repository-wide 10GB Actions cache quota). +# But a silent default in either direction is how audit.yml ended up saving a +# ~843MB PR-scoped entry on every dependency change, evicting the main-scoped +# lanes that the expensive jobs restore from. Requiring the input to be spelled +# out keeps the decision visible in review. +# +# Usage: scripts/security/check_cache_save_if.sh +set -euo pipefail + +cd "$(dirname "$0")/../.." + +status=0 + +for file in .github/workflows/*.yml .github/workflows/*.yaml; do + [ -e "$file" ] || continue + + # Walk the file, and for every `uses: ./.github/actions/setup` scan the rest + # of that step for cache-save-if. `uses:` and `with:` are siblings at the + # same indentation, so the step ends only at a line indented *less* than the + # `uses:` line (the next `- name:` item, or a dedent out of the steps list). + awk -v file="$file" ' + function flush() { + if (pending && !found) { + printf "%s:%d: `uses: ./.github/actions/setup` without an explicit `cache-save-if:`\n", file, uses_line > "/dev/stderr" + bad++ + } + pending = 0; found = 0 + } + { + line = $0 + sub(/[[:space:]]+$/, "", line) + if (line ~ /^[[:space:]]*#/ || line ~ /^[[:space:]]*$/) next + + match(line, /^[[:space:]]*/) + indent = RLENGTH + + if (pending && indent < uses_indent) flush() + + if (line ~ /uses:[[:space:]]*\.\/\.github\/actions\/setup[[:space:]]*$/) { + flush() + pending = 1; found = 0 + uses_indent = indent + uses_line = NR + next + } + + if (pending && line ~ /^[[:space:]]*cache-save-if:/) found = 1 + } + END { flush(); exit (bad > 0) } + ' "$file" || status=1 +done + +if [ "$status" -ne 0 ]; then + echo "" >&2 + echo "Add an explicit cache-save-if to each call above, e.g." >&2 + echo " cache-save-if: \${{ github.ref == 'refs/heads/main' }} # this lane owns the key" >&2 + echo " cache-save-if: 'false' # this lane only reads it" >&2 + echo "Exactly one lane per shared-key may save; see rustfs/backlog#1600." >&2 + exit 1 +fi + +echo "OK: every ./.github/actions/setup call states cache-save-if explicitly"