From 2489736c9f5f428ba73e0e2c2246dacc12a85541 Mon Sep 17 00:00:00 2001 From: Nikolas Garza <90273783+nmgarza5@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:58:04 +0000 Subject: [PATCH] fix(ci): pin release lines to main's base image digests and post one failure notice per failure (#15889) --- .github/scripts/run-cve-alerts.sh | 26 +++- .github/workflows/cve-alerts-nightly.yml | 2 +- .github/workflows/cve-alerts.yml | 157 +++++++++++++------- tools/ods/cmd/update_base_digests.go | 168 ++++++++++++++++------ tools/ods/cmd/update_base_digests_test.go | 94 ++++++++++-- 5 files changed, 336 insertions(+), 111 deletions(-) diff --git a/.github/scripts/run-cve-alerts.sh b/.github/scripts/run-cve-alerts.sh index 23a9da87e1..5abfe7741e 100755 --- a/.github/scripts/run-cve-alerts.sh +++ b/.github/scripts/run-cve-alerts.sh @@ -8,6 +8,9 @@ # Run reads are retried: the API can 404 a run just after its dispatch or 500 # at any time, and `gh run watch` exits alike for a failed run and a failed read. # +# A failed run that posted its own Slack notice passes here, so the nightly +# posts only for a run nobody reported: never started, lost, or unannounced. +# # usage: run-cve-alerts.sh (GH_TOKEN and GH_REPO set) set -euo pipefail @@ -19,19 +22,19 @@ poll_seconds=60 read_retry_seconds=600 read_retry_interval=10 -# Prints " " for a run, retrying failed API reads. -run_state() { - local run="$1" +# Prints the result of a jq query over a run's fields, retrying failed API reads. +read_run() { + local run="$1" fields="$2" query="$3" local deadline=$((SECONDS + read_retry_seconds)) - local state - until state="$(gh run view "${run}" --json status,conclusion --jq '"\(.status) \(.conclusion)"')"; do + local out + until out="$(gh run view "${run}" --json "${fields}" --jq "${query}")"; do if [ "${SECONDS}" -ge "${deadline}" ]; then echo "::error::Reading run ${run} kept failing for ${read_retry_seconds}s" >&2 return 1 fi sleep "${read_retry_interval}" done - echo "${state}" + echo "${out}" } for attempt in $(seq 1 "${attempts}"); do @@ -43,7 +46,7 @@ for attempt in $(seq 1 "${attempts}"); do fi echo "Watching run ${run} on ${ref} (attempt ${attempt} of ${attempts})" while true; do - state="$(run_state "${run}")" + state="$(read_run "${run}" status,conclusion '"\(.status) \(.conclusion)"')" read -r status conclusion <<< "${state}" if [ "${status}" = "completed" ]; then break @@ -57,6 +60,15 @@ for attempt in $(seq 1 "${attempts}"); do cancelled) echo "Run ${run} was cancelled while waiting its turn" ;; + failure) + notified="$(read_run "${run}" jobs 'any(.jobs[]; .name == "notify-slack-on-failure" and .conclusion == "success")')" + if [ "${notified}" = true ]; then + echo "::warning::CVE Alerts on ${ref} failed and posted its own notice, see run ${run}" + exit 0 + fi + echo "::error::CVE Alerts on ${ref} failed without posting a notice, see run ${run}" + exit 1 + ;; *) echo "::error::CVE Alerts on ${ref} ended ${conclusion}, see run ${run}" exit 1 diff --git a/.github/workflows/cve-alerts-nightly.yml b/.github/workflows/cve-alerts-nightly.yml index 30991467de..c3a7de275b 100644 --- a/.github/workflows/cve-alerts-nightly.yml +++ b/.github/workflows/cve-alerts-nightly.yml @@ -113,4 +113,4 @@ jobs: with: webhook-url: ${{ secrets.CVE_REVIEWS_WEBHOOK }} title: "🚨 CVE Alerts nightly failed" - details: "*A CVE Alerts run failed or never started on one of the branches.* Its alerts retry on the next nightly. Check the run logs." + details: "*A CVE Alerts run never started, could not be watched, or failed without posting its own notice.* Its alerts retry on the next nightly. Check the run logs." diff --git a/.github/workflows/cve-alerts.yml b/.github/workflows/cve-alerts.yml index 1cc60cb972..e9aec3a5fc 100644 --- a/.github/workflows/cve-alerts.yml +++ b/.github/workflows/cve-alerts.yml @@ -11,9 +11,9 @@ ### whether Onyx is affected, and publish commits the checked result onto ### Dependabot's PR or a fixed branch, and a fix on main, on either, is ### backported to the four newest release lines in the same run. A release run -### cherry-picks a fix main already merged (a bump, or a base image digest -### refresh), or else fixes on main and backports. cve-alerts-nightly.yml runs -### main, then each release line. +### cherry-picks a bump main already merged, pins main's base image digests for +### an OS package, or else fixes on main and backports. cve-alerts-nightly.yml +### runs main, then each release line. ### Every alert posts to Slack tagging a reviewer from cve-reviewers.json. name: CVE Alerts @@ -315,7 +315,7 @@ jobs: steps: # On main, pushing onto Dependabot's open PR for the package keeps one PR # per package (Dependabot stops rebasing once someone else commits). On a - # release branch, a fix already merged on main is cherry-picked instead. + # release branch, a fix already merged on main is applied instead. - name: Pick the base branch id: base env: @@ -339,11 +339,19 @@ jobs: echo '{"ref": "main", "pr": null}' > "${RUNNER_TEMP}/triage/base.json" fi elif [[ "${ECOSYSTEM}" == *:* ]]; then - # An OS package ships in a base image, so main's fix is each - # affected family's last merged digest refresh, as long as this - # branch still pins older digests than main. A package in both the - # node and python images needs both refreshes. + # An OS package ships in a base image, so main's fix is each affected + # family's last merged digest refresh, while this branch pins other digests + # than main. Publish pins main's digests rather than picking the refresh, + # which rewrites the same lines weekly and conflicts on a line that missed one. families="$(jq -r '[.manifests[]? | if contains("node") then "node" else "python" end] | unique | join(" ")' <<< "${ALERT}")" + families="${families:-python}" + # Until the distribution ships a fixed build for every advisory, no + # refresh is the whole fix, so the alert is reported unfixed instead. + unfixed="$(jq -r '[.advisories[] | select((.fixed_in // "") == "") | .id] | join(" ")' <<< "${ALERT}")" + if [ -n "${unfixed}" ]; then + echo "::warning::No fixed version for ${unfixed}, so main's digest refresh is no fix" + families="" + fi digests() { for f in ${files}; do # errexit does not reach a command substitution, so fail by hand. @@ -353,7 +361,7 @@ jobs: } main_fixes='[]' missing="" - for family in ${families:-python}; do + for family in ${families}; do main_fix="$(gh pr list --state merged --base main --head "auto/base-image-digests/${family}" \ --json number,title,mergeCommit,mergedAt --limit 10 | jq -c 'sort_by(.mergedAt) | last // empty')" if [ -z "${main_fix}" ]; then @@ -369,22 +377,23 @@ jobs: branch_digests="$(digests "${GITHUB_REF_NAME}")" main_digests="$(digests main)" if [ "${branch_digests}" = "${main_digests}" ]; then - echo "::warning::This branch already pins main's ${family} digests, nothing to cherry-pick" + echo "::warning::This branch already pins main's ${family} digests, nothing to pin" continue fi - main_fixes="$(jq -c --argjson fix "${main_fix}" '. + [{pr: $fix.number, sha: $fix.mergeCommit.oid, title: $fix.title}]' <<< "${main_fixes}")" + main_fixes="$(jq -c --argjson fix "${main_fix}" --arg family "${family}" '. + [{pr: $fix.number, sha: $fix.mergeCommit.oid, title: $fix.title, family: $family}]' <<< "${main_fixes}")" done - # A family main has not refreshed leaves the alert unfixed, so no - # partial pick gets presented as the fix; the next run tries again. + # A family main has not refreshed leaves the alert unfixed, so no partial + # pin gets presented as the fix. The next run tries again. if [ -n "${missing}" ]; then - echo "::warning::Main has no merged digest refresh for${missing}, nothing to cherry-pick yet" + echo "::warning::Main has no merged digest refresh for${missing}, nothing to pin yet" main_fixes='[]' fi ref=main if [ "${main_fixes}" != "[]" ]; then ref="${GITHUB_REF_NAME}" fi - jq -n --arg ref "${ref}" --argjson fixes "${main_fixes}" '{ref: $ref, pr: null, main_fix: $fixes}' \ + jq -n --arg ref "${ref}" --argjson fixes "${main_fixes}" \ + '{ref: $ref, pr: null, main_fix: $fixes, digest_families: ([$fixes[].family] | join(" "))}' \ > "${RUNNER_TEMP}/triage/base.json" else main_fix="$(gh pr list --state merged --base main \ @@ -483,12 +492,12 @@ jobs: export OSV_DIR="${RUNNER_TEMP}/claude/osv" export NOTE_FILE="${RUNNER_TEMP}/claude/note.md" export RESULT_FILE="${RUNNER_TEMP}/claude/result.json" - # A fix already merged on main is cherry-picked by the workflow, so - # Claude only judges the advisory. + # A fix already merged on main is applied by the workflow, so Claude + # only judges the advisory. MAIN_FIX="" fix="$(jq -r '[.main_fix[]? | "PR #\(.pr) (\(.title))"] | join(" and ")' "${RUNNER_TEMP}/triage/base.json")" if [ -n "${fix}" ]; then - MAIN_FIX="Main already has the fix in ${fix}, and the workflow cherry-picks it onto this branch. Skip step 1: change no file and leave fixed_version empty." + MAIN_FIX="Main already has the fix in ${fix}, and the workflow applies it to this branch. Skip step 1: change no file and leave fixed_version empty." fi export MAIN_FIX # shellcheck disable=SC2016 # envsubst takes the variable names literally. @@ -650,13 +659,18 @@ jobs: base="${RUNNER_TEMP}/triage/base.json" pr="" main_fix="" + families="" fix_base="${GITHUB_REF_NAME}" if [ -s "${base}" ]; then pr="$(jq -r '.pr // ""' "${base}")" main_fix="$(jq -r '[.main_fix[]?.sha] | join(" ")' "${base}")" + families="$(jq -r '.digest_families // ""' "${base}")" fix_base="$(jq -r .ref "${base}")" fi - echo "main-fix=${main_fix}" >> "$GITHUB_OUTPUT" + { + echo "main-fix=${main_fix}" + echo "digest-families=${families}" + } >> "$GITHUB_OUTPUT" releases="" if [ -n "${pr}" ]; then branch="${fix_base}" @@ -746,8 +760,9 @@ jobs: # A cherry-picked Actions bump touches .github/workflows. permission-workflows: write - # A fix already merged on main is cherry-picked with ods, which opens the - # PR itself. ods comes from main's tools/ods, like the scans. + # ods cherry-picks a bump already merged on main, opening the PR itself, and + # pins main's base image digests. ods comes from main's tools/ods, like the + # scans. - name: Checkout ods source if: steps.branch.outputs.main-fix != '' || steps.branch.outputs.releases != '' uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # ratchet:actions/checkout@v6 @@ -763,9 +778,17 @@ jobs: go-version-file: ods-src/tools/ods/go.mod cache-dependency-path: ods-src/tools/ods/go.sum + - name: Build ods + if: steps.branch.outputs.main-fix != '' || steps.branch.outputs.releases != '' + working-directory: ods-src/tools/ods + run: go build -o "${RUNNER_TEMP}/ods" . + + # A conflict is reported in the alert as a pick to do by hand, so it does + # not fail the run. - name: Cherry-pick the fix from main - if: steps.branch.outputs.main-fix != '' + if: steps.branch.outputs.main-fix != '' && steps.branch.outputs.digest-families == '' id: cp + continue-on-error: true working-directory: repo env: GH_TOKEN: ${{ steps.app-token.outputs.token }} @@ -775,7 +798,6 @@ jobs: REVIEWER: ${{ steps.alert.outputs.reviewer }} PACKAGE: ${{ matrix.alert.package }} run: | - (cd "${GITHUB_WORKSPACE}/ods-src/tools/ods" && go build -o "${RUNNER_TEMP}/ods" .) bot_user_id="$(gh api "/users/${APP_SLUG}[bot]" --jq .id)" git config user.name "${APP_SLUG}[bot]" git config user.email "${bot_user_id}+${APP_SLUG}[bot]@users.noreply.github.com" @@ -826,16 +848,43 @@ jobs: gh pr edit "${url}" --body-file "${RUNNER_TEMP}/cp-body.md" --add-reviewer "${REVIEWER}" echo "url=${url}" >> "$GITHUB_OUTPUT" + - name: Checkout main for its digests + if: steps.branch.outputs.digest-families != '' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # ratchet:actions/checkout@v6 + with: + persist-credentials: false + ref: main + path: main + + # Main's pins are the cache the release checkout is rewritten from: a tag + # main still pins lands on main's digest, a tag main dropped keeps its own, + # and no registry is asked, as this job has no login. + - name: Pin main's base image digests + if: steps.branch.outputs.digest-families != '' + id: digests + working-directory: repo + env: + FAMILIES: ${{ steps.branch.outputs.digest-families }} + run: | + (cd "${GITHUB_WORKSPACE}/main" && "${RUNNER_TEMP}/ods" update-base-digests --pins-file "${RUNNER_TEMP}/main-digests.json") + for family in ${FAMILIES}; do + "${RUNNER_TEMP}/ods" update-base-digests --write --family "${family}" --cache-file "${RUNNER_TEMP}/main-digests.json" --cached-only + done + changed=false + git diff --quiet || changed=true + echo "changed=${changed}" >> "$GITHUB_OUTPUT" + # Dependabot's PR gets the reviewer and the note even when it already # holds the whole fix. - name: Commit, push, and open or update the PR - if: steps.branch.outputs.main-fix == '' && (steps.changes.outputs.changed == 'true' || steps.branch.outputs.pr != '') + if: (steps.branch.outputs.main-fix == '' && (steps.changes.outputs.changed == 'true' || steps.branch.outputs.pr != '')) || steps.digests.outputs.changed == 'true' id: pr working-directory: repo env: GH_TOKEN: ${{ steps.app-token.outputs.token }} APP_SLUG: ${{ steps.app-token.outputs.app-slug }} - CHANGED: ${{ steps.changes.outputs.changed }} + CHANGED: ${{ steps.changes.outputs.changed == 'true' || steps.digests.outputs.changed == 'true' }} + FAMILIES: ${{ steps.branch.outputs.digest-families }} PACKAGE: ${{ matrix.alert.package }} ISSUE: ${{ matrix.alert.issue }} REVIEWER: ${{ steps.alert.outputs.reviewer }} @@ -844,6 +893,18 @@ jobs: BASE: ${{ steps.branch.outputs.base }} run: | version="$(jq -r '.fixed_version // ""' "${RUNNER_TEMP}/triage/result.json" 2>/dev/null || true)" + advisories="$(jq -r '[.advisories[] | "[\(.id)](\(.url))"] | join(", ")' "${RUNNER_TEMP}/alert.json")" + title="fix(deps): pin ${PACKAGE} to ${version}" + # The backports read the version back from " to everywhere". + commit="${title} everywhere" + summary="Fixes \`${PACKAGE}\` for ${advisories}. Updated by the [CVE Alerts run](${RUN_URL})." + if [ -n "${FAMILIES}" ]; then + title="fix(deps): pin main's ${FAMILIES// / and } base image digests for ${PACKAGE}" + commit="${title}" + summary="Pins main's ${FAMILIES// / and } base image digests, which carry the fix for ${advisories}. Opened by the [CVE Alerts run](${RUN_URL})." + elif [ -n "${version}" ]; then + summary="Pins \`${PACKAGE}\` to \`${version}\` in every manifest and lockfile for ${advisories}. Opened by the [CVE Alerts run](${RUN_URL})." + fi # Push as the App through gh, which reads GH_TOKEN at push time. gh auth setup-git if [ "${CHANGED}" = "true" ]; then @@ -851,16 +912,11 @@ jobs: git config user.name "${APP_SLUG}[bot]" git config user.email "${bot_user_id}+${APP_SLUG}[bot]@users.noreply.github.com" git add -A - git commit -m "fix(deps): pin ${PACKAGE} to ${version} everywhere" + git commit -m "${commit}" fi - advisories="$(jq -r '[.advisories[] | "[\(.id)](\(.url))"] | join(", ")' "${RUNNER_TEMP}/alert.json")" { - if [ -n "${version}" ]; then - echo "Pins \`${PACKAGE}\` to \`${version}\` in every manifest and lockfile for ${advisories}. Opened by the [CVE Alerts run](${RUN_URL})." - else - echo "Fixes \`${PACKAGE}\` for ${advisories}. Updated by the [CVE Alerts run](${RUN_URL})." - fi + echo "${summary}" echo echo "Closes #${ISSUE}" echo @@ -889,12 +945,12 @@ jobs: git push --force origin "HEAD:refs/heads/${BRANCH}" existing="$(gh pr list --state open --head "${BRANCH}" --json url --jq '.[0].url')" if [ -n "${existing}" ]; then - # A new advisory may need a higher pin, so the title follows the version. - gh pr edit "${existing}" --title "fix(deps): pin ${PACKAGE} to ${version}" --body-file "${RUNNER_TEMP}/pr-body.md" --add-reviewer "${REVIEWER}" + # A new advisory may need a higher pin, so the title is refreshed. + gh pr edit "${existing}" --title "${title}" --body-file "${RUNNER_TEMP}/pr-body.md" --add-reviewer "${REVIEWER}" url="${existing}" else url="$(gh pr create --base "${BASE}" --head "${BRANCH}" \ - --title "fix(deps): pin ${PACKAGE} to ${version}" \ + --title "${title}" \ --body-file "${RUNNER_TEMP}/pr-body.md" \ --reviewer "${REVIEWER}")" fi @@ -903,7 +959,9 @@ jobs: # One PR per release line: the four newest for a fix opened on main, or # from the line that found the advisory up for a release run. A line # with an open PR for this exact pin is skipped and a conflict is aborted, - # so a retry opens no second PR and the newer lines still get tried. + # so a retry opens no second PR and the newer lines still get tried. A + # conflict is reported in the alert and keeps it pending, so it does not + # fail the run. - name: Backport the fix to the release lines if: steps.pr.outcome == 'success' && steps.branch.outputs.releases != '' id: backport @@ -917,7 +975,6 @@ jobs: BRANCH: ${{ steps.branch.outputs.name }} APP_SLUG: ${{ steps.app-token.outputs.app-slug }} run: | - (cd "${GITHUB_WORKSPACE}/ods-src/tools/ods" && go build -o "${RUNNER_TEMP}/ods" .) # The picks commit as the App; the fix step sets this only when it # committed something itself. bot_user_id="$(gh api "/users/${APP_SLUG}[bot]" --jq .id)" @@ -979,21 +1036,14 @@ jobs: fi urls="${urls} ${url}" done - # The step succeeds so both outputs survive; the failed lines fail - # the job in the next step. + if [ -n "${failed}" ]; then + echo "::warning::The backports onto release lines${failed} failed, likely on a conflict. The alert stays pending." + fi { echo "urls=${urls# }" echo "failed=${failed# }" } >> "$GITHUB_OUTPUT" - - name: Fail on a backport conflict - if: steps.backport.outputs.failed != '' - env: - FAILED: ${{ steps.backport.outputs.failed }} - run: | - echo "::error::The backports onto release lines ${FAILED} failed, likely on a conflict. The alert stays pending." - exit 1 - # Post even after a failed step. The pending label stays until a fix PR # is posted to Slack, so a later run retries. - name: Fill in a failed triage @@ -1057,9 +1107,11 @@ jobs: BACKPORT_FAILED: ${{ steps.backport.outputs.failed }} BACKPORT_OUTCOME: ${{ steps.backport.outcome }} CP_OUTCOME: ${{ steps.cp.outcome }} + DIGESTS_OUTCOME: ${{ steps.digests.outcome }} CHANGES_OUTCOME: ${{ steps.changes.outcome }} run: | - if [ "${ALERT_REASON}" = "retry" ] && [ -z "${PR_URL}" ]; then + # A failed pin is breakage, not a settled verdict, so it is always noted. + if [ "${ALERT_REASON}" = "retry" ] && [ -z "${PR_URL}" ] && [ "${DIGESTS_OUTCOME}" != "failure" ]; then last="$(gh issue view "${ISSUE}" --json comments --jq '[.comments[] | select(.author.login == "github-actions")] | last.body // ""')" if [ "${TRIAGE_OK}" != "true" ] || { [ -n "${last}" ] && ! grep -q "triage did not finish" <<< "${last}"; }; then exit 0 @@ -1090,6 +1142,8 @@ jobs: else if [ "${CP_OUTCOME}" = "failure" ]; then echo "The cherry-pick of main's fix failed, likely on a conflict. Pick it by hand, see the run." + elif [ "${DIGESTS_OUTCOME}" = "failure" ]; then + echo "Pinning main's base image digests on this branch failed, see the run." elif [ "${CHANGES_OUTCOME}" = "failure" ]; then echo "Claude's change failed validation (patch apply, manifest check, or lockfile install), so nothing was applied. The next run retries, see the run." else @@ -1162,6 +1216,7 @@ jobs: BACKPORT_OUTCOME: ${{ steps.backport.outcome }} CHANGED: ${{ steps.changes.outputs.changed }} CP_OUTCOME: ${{ steps.cp.outcome }} + DIGESTS_OUTCOME: ${{ steps.digests.outcome }} CHANGES_OUTCOME: ${{ steps.changes.outcome }} TRIAGE_OK: ${{ steps.triage.outputs.ok }} run: | @@ -1212,6 +1267,8 @@ jobs: fi elif [ "${CP_OUTCOME}" = "failure" ]; then echo "*Fix PR:* the cherry-pick of main's fix failed, likely on a conflict. Pick it by hand, see the run." + elif [ "${DIGESTS_OUTCOME}" = "failure" ]; then + echo "*Fix PR:* pinning main's base image digests on this branch failed, see the run." elif [ "${CHANGES_OUTCOME}" = "failure" ]; then echo "*Fix PR:* Claude's change failed validation (patch apply, manifest check, or lockfile install), so nothing was applied. The next run retries, see the run." elif [ "${TRIAGE_OK}" != "true" ]; then @@ -1291,7 +1348,7 @@ jobs: # A failed triage, note, assignment, or post keeps the label, so the next # run retries the whole alert. - name: Clear the pending label - if: ${{ !cancelled() && steps.note.outcome == 'success' && steps.assign.outcome == 'success' && steps.post.outcome == 'success' && steps.triage.outputs.ok == 'true' && steps.changes.outcome == 'success' && steps.cp.outcome != 'failure' && steps.pr.outcome != 'failure' && steps.backport.outcome != 'failure' && steps.backport.outputs.failed == '' }} + if: ${{ !cancelled() && steps.note.outcome == 'success' && steps.assign.outcome == 'success' && steps.post.outcome == 'success' && steps.triage.outputs.ok == 'true' && steps.changes.outcome == 'success' && steps.cp.outcome != 'failure' && steps.digests.outcome != 'failure' && steps.pr.outcome != 'failure' && steps.backport.outcome != 'failure' && steps.backport.outputs.failed == '' }} env: ISSUE: ${{ matrix.alert.issue }} run: gh issue edit "${ISSUE}" --remove-label cve-alert-pending @@ -1310,9 +1367,11 @@ jobs: persist-credentials: false sparse-checkout: .github/actions/slack-notify + # A dropped post fails this job, so the nightly reports the run instead. - name: Notify Slack about the failure uses: ./.github/actions/slack-notify with: webhook-url: ${{ secrets.CVE_REVIEWS_WEBHOOK }} + fail-on-error: "true" title: "🚨 CVE Alerts workflow failed" details: "*The CVE Alerts run failed.* A scan, the issue sync, or an alert's triage or publish broke. Pending alerts retry on the next run. Check the run logs." diff --git a/tools/ods/cmd/update_base_digests.go b/tools/ods/cmd/update_base_digests.go index 0b7523d768..64c9887ad7 100644 --- a/tools/ods/cmd/update_base_digests.go +++ b/tools/ods/cmd/update_base_digests.go @@ -14,15 +14,20 @@ import ( "github.com/onyx-dot-app/onyx/tools/ods/internal/paths" ) +// UpdateBaseDigestsOptions holds options for the update-base-digests command. +type UpdateBaseDigestsOptions struct { + Write bool + SummaryFile string + Family string + ListStaleFamilies bool + CacheFile string + CachedOnly bool + PinsFile string +} + // NewUpdateBaseDigestsCommand creates the update-base-digests command. func NewUpdateBaseDigestsCommand() *cobra.Command { - var ( - write bool - summaryFile string - family string - listStaleFamilies bool - cacheFile string - ) + opts := &UpdateBaseDigestsOptions{} cmd := &cobra.Command{ Use: "update-base-digests", @@ -48,9 +53,12 @@ Examples: ods update-base-digests # report every reference ods update-base-digests --list-stale-families # families needing a bump ods update-base-digests --write --family python \ - --cache-file digests.json # rewrite one family`, + --cache-file digests.json # rewrite one family + ods update-base-digests --pins-file digests.json # dump this checkout's pins as a cache + ods update-base-digests --write --family python \ + --cache-file digests.json --cached-only # pin to another checkout's digests`, Run: func(cmd *cobra.Command, args []string) { - output, err := runUpdateBaseDigests(write, summaryFile, family, listStaleFamilies, cacheFile) + output, err := runUpdateBaseDigests(opts) fmt.Print(output) if err != nil { log.Fatal(err) @@ -58,18 +66,20 @@ Examples: }, } - cmd.Flags().BoolVar(&write, "write", false, "rewrite the files; without it the command only reports") - cmd.Flags().StringVar(&summaryFile, "summary-file", "", "write a markdown summary of the changes to this path") - cmd.Flags().StringVar(&family, "family", "", "only act on one base family, for example python or node") - cmd.Flags().BoolVar(&listStaleFamilies, "list-stale-families", false, "print the families that have a stale digest, one per line, and exit") - cmd.Flags().StringVar(&cacheFile, "cache-file", "", "read resolved digests from this path, or write them there if absent") + cmd.Flags().BoolVar(&opts.Write, "write", false, "rewrite the files; without it the command only reports") + cmd.Flags().StringVar(&opts.SummaryFile, "summary-file", "", "write a markdown summary of the changes to this path") + cmd.Flags().StringVar(&opts.Family, "family", "", "only act on one base family, for example python or node") + cmd.Flags().BoolVar(&opts.ListStaleFamilies, "list-stale-families", false, "print the families that have a stale digest, one per line, and exit") + cmd.Flags().StringVar(&opts.CacheFile, "cache-file", "", "read resolved digests from this path, or write them there if absent") + cmd.Flags().BoolVar(&opts.CachedOnly, "cached-only", false, "touch only the references --cache-file covers and ask no registry; the rest are kept as pinned") + cmd.Flags().StringVar(&opts.PinsFile, "pins-file", "", "write the digests this checkout pins as a cache file and exit, so another checkout can be pinned to them with --cache-file --cached-only") return cmd } // runUpdateBaseDigests returns what Run prints, and an error whose text is the // message Run logs before exiting. -func runUpdateBaseDigests(write bool, summaryFile, family string, listStaleFamilies bool, cacheFile string) (string, error) { +func runUpdateBaseDigests(opts *UpdateBaseDigestsOptions) (string, error) { var out strings.Builder root, err := paths.GitRoot() @@ -90,31 +100,65 @@ func runUpdateBaseDigests(write bool, summaryFile, family string, listStaleFamil return out.String(), fatalErrorf("No pinned image references found.") } - resolved, err := resolveWithCache(refs, cacheFile) - if err != nil { - return out.String(), fatalErrorf("Could not resolve every tag:\n%v", err) + if opts.PinsFile != "" { + if err := writePins(refs, opts.PinsFile); err != nil { + return out.String(), fatalErrorf("Could not write the pins: %v", err) + } + fmt.Fprintf(&out, "Wrote %d pinned reference(s) to %s.\n", len(refs), opts.PinsFile) + return out.String(), nil } - stale := basedigest.Stale(refs, resolved) + // Filter before resolving, so one family's rewrite never asks a registry + // about another family's tags. + if opts.Family != "" { + refs = basedigest.FilterFamily(refs, opts.Family) + if len(refs) == 0 { + return out.String(), fatalErrorf("No references in family %q.", opts.Family) + } + } - if listStaleFamilies { + var resolved map[string]string + if opts.CachedOnly { + if opts.CacheFile == "" { + return out.String(), fatalErrorf("--cached-only needs --cache-file.") + } + // An absent cache would read as empty and quietly touch nothing. + if _, err := os.Stat(opts.CacheFile); err != nil { + return out.String(), fatalErrorf("Could not read the cache: %v", err) + } + resolved, err = readCache(opts.CacheFile) + if err != nil { + return out.String(), fatalErrorf("Could not read the cache: %v", err) + } + } else { + resolved, err = resolveWithCache(refs, opts.CacheFile) + if err != nil { + return out.String(), fatalErrorf("Could not resolve every tag:\n%v", err) + } + } + + // A reference the cache leaves out is kept as pinned, so a tag the other + // checkout dropped never changes and nothing gets a blank digest. + var covered []basedigest.Ref + for _, ref := range refs { + if _, ok := resolved[ref.Query()]; ok { + covered = append(covered, ref) + } + } + stale := basedigest.Stale(covered, resolved) + + if opts.ListStaleFamilies { for _, name := range basedigest.Families(stale) { out.WriteString(name + "\n") } return out.String(), nil } - if family != "" { - refs = basedigest.FilterFamily(refs, family) - stale = basedigest.FilterFamily(stale, family) - if len(refs) == 0 { - return out.String(), fatalErrorf("No references in family %q.", family) - } - } - for _, ref := range refs { state := "current" - if ref.Digest != resolved[ref.Query()] { + if digest, ok := resolved[ref.Query()]; !ok { + state = "kept" + } else if ref.Digest != digest { state = "update" } fmt.Fprintf(&out, "%7s %-45s %s:%d\n", state, ref.Display(), ref.Path, ref.Line) @@ -124,7 +168,7 @@ func runUpdateBaseDigests(write bool, summaryFile, family string, listStaleFamil if len(stale) == 0 { out.WriteString("\nAll pinned digests are current.\n") - } else if write { + } else if opts.Write { if err := basedigest.Rewrite(root, stale, resolved); err != nil { return out.String(), fatalErrorf("Could not rewrite the digests: %v", err) } @@ -133,14 +177,55 @@ func runUpdateBaseDigests(write bool, summaryFile, family string, listStaleFamil fmt.Fprintf(&out, "\n%d reference(s) are stale. Re-run with --write to apply.\n", len(stale)) } - if summaryFile != "" { - if err := os.WriteFile(summaryFile, []byte(summary), 0o644); err != nil { + if opts.SummaryFile != "" { + if err := os.WriteFile(opts.SummaryFile, []byte(summary), 0o644); err != nil { return out.String(), fatalErrorf("Could not write the summary: %v", err) } } return out.String(), nil } +// writePins saves each ref's pinned digest in --cache-file form, so another +// checkout can be rewritten to this one's digests. Two pins of one tag that +// disagree have drifted, and the file would silently keep the last. +func writePins(refs []basedigest.Ref, pinsFile string) error { + pins := map[string]string{} + for _, ref := range refs { + if other, ok := pins[ref.Query()]; ok && other != ref.Digest { + return fmt.Errorf("%s is pinned to two digests, %s and %s", ref.Display(), other, ref.Digest) + } + pins[ref.Query()] = ref.Digest + } + return writeCache(pinsFile, pins) +} + +// readCache loads a --cache-file, empty when the file does not exist yet. +func readCache(path string) (map[string]string, error) { + cached := map[string]string{} + data, err := os.ReadFile(path) + switch { + case err == nil: + if err := json.Unmarshal(data, &cached); err != nil { + return nil, fmt.Errorf("read cache %s: %w", path, err) + } + case !os.IsNotExist(err): + return nil, fmt.Errorf("read cache %s: %w", path, err) + } + return cached, nil +} + +// writeCache saves digests by query as the JSON --cache-file reads. +func writeCache(path string, digests map[string]string) error { + data, err := json.MarshalIndent(digests, "", " ") + if err != nil { + return fmt.Errorf("encode cache: %w", err) + } + if err := os.WriteFile(path, append(data, '\n'), 0o644); err != nil { + return fmt.Errorf("write cache %s: %w", path, err) + } + return nil +} + // resolveWithCache resolves every distinct tag, reusing cacheFile when it exists. // // The cache lets a caller resolve once and then rewrite one family at a time @@ -153,14 +238,9 @@ func runUpdateBaseDigests(write bool, summaryFile, family string, listStaleFamil func resolveWithCache(refs []basedigest.Ref, cacheFile string) (map[string]string, error) { cached := map[string]string{} if cacheFile != "" { - data, err := os.ReadFile(cacheFile) - switch { - case err == nil: - if err := json.Unmarshal(data, &cached); err != nil { - return nil, fmt.Errorf("read cache %s: %w", cacheFile, err) - } - case !os.IsNotExist(err): - return nil, fmt.Errorf("read cache %s: %w", cacheFile, err) + var err error + if cached, err = readCache(cacheFile); err != nil { + return nil, err } } @@ -181,12 +261,8 @@ func resolveWithCache(refs []basedigest.Ref, cacheFile string) (map[string]strin maps.Copy(resolved, cached) if cacheFile != "" { - data, err := json.MarshalIndent(resolved, "", " ") - if err != nil { - return nil, fmt.Errorf("encode cache: %w", err) - } - if err := os.WriteFile(cacheFile, append(data, '\n'), 0o644); err != nil { - return nil, fmt.Errorf("write cache %s: %w", cacheFile, err) + if err := writeCache(cacheFile, resolved); err != nil { + return nil, err } } return resolved, nil diff --git a/tools/ods/cmd/update_base_digests_test.go b/tools/ods/cmd/update_base_digests_test.go index 5c6665eaa0..af971f7035 100644 --- a/tools/ods/cmd/update_base_digests_test.go +++ b/tools/ods/cmd/update_base_digests_test.go @@ -64,7 +64,7 @@ func TestRunUpdateBaseDigests_reportsWithoutWriting(t *testing.T) { } summaryFile := filepath.Join(t.TempDir(), "summary.md") - output, err := runUpdateBaseDigests(false, summaryFile, "", false, cacheFile) + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{SummaryFile: summaryFile, CacheFile: cacheFile}) if err != nil { t.Fatalf("runUpdateBaseDigests failed: %v", err) } @@ -94,7 +94,7 @@ func TestRunUpdateBaseDigests_writesOneFamily(t *testing.T) { shotWorkflowPython: shotDigest("c"), }) - output, err := runUpdateBaseDigests(true, "", "python", false, cacheFile) + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, Family: "python", CacheFile: cacheFile}) if err != nil { t.Fatalf("runUpdateBaseDigests failed: %v", err) } @@ -120,7 +120,7 @@ func TestRunUpdateBaseDigests_listsStaleFamilies(t *testing.T) { shotWorkflowPython: shotDigest("d"), }) - output, err := runUpdateBaseDigests(true, "", "", true, cacheFile) + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, ListStaleFamilies: true, CacheFile: cacheFile}) if err != nil { t.Fatalf("runUpdateBaseDigests failed: %v", err) } @@ -130,6 +130,84 @@ func TestRunUpdateBaseDigests_listsStaleFamilies(t *testing.T) { } } +func TestRunUpdateBaseDigests_writesPinsAsACache(t *testing.T) { + shotDigestRepo(t) + pinsFile := filepath.Join(t.TempDir(), "pins.json") + + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{PinsFile: pinsFile}) + if err != nil { + t.Fatalf("runUpdateBaseDigests failed: %v", err) + } + + if want := "Wrote 3 pinned reference(s) to " + pinsFile + ".\n"; output != want { + t.Fatalf("expected output %q, got %q", want, output) + } + var pins map[string]string + if err := json.Unmarshal([]byte(readShotFile(t, pinsFile)), &pins); err != nil { + t.Fatal(err) + } + want := map[string]string{ + shotDockerfilePython: shotDigest("a"), + shotDockerfileNode: shotDigest("b"), + shotWorkflowPython: shotDigest("c"), + } + if fmt.Sprint(pins) != fmt.Sprint(want) { + t.Fatalf("expected pins %v, got %v", want, pins) + } + + // One checkout's pins rewrite another with --cached-only: no registry is + // asked, and a tag the pins lack keeps its own digest, in any family. + other := shotGitRepo(t, map[string]string{ + "Dockerfile": "FROM ${BASE_IMAGE_REGISTRY}/library/python:3.13-slim@" + shotDigest("0") + "\n" + + "FROM python:3.12-slim@" + shotDigest("1") + "\n" + + "FROM golang:1.26.5-alpine@" + shotDigest("2") + "\n", + }) + output, err = runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, Family: "python", CacheFile: pinsFile, CachedOnly: true}) + if err != nil { + t.Fatalf("runUpdateBaseDigests failed: %v", err) + } + wantOutput := shotReportLine("update", "library/python:3.13-slim", "Dockerfile:1") + + shotReportLine("kept", "python:3.12-slim", "Dockerfile:2") + + "\nUpdated 1 reference(s).\n" + if output != wantOutput { + t.Fatalf("expected output:\n%s\ngot:\n%s", wantOutput, output) + } + wantDockerfile := "FROM ${BASE_IMAGE_REGISTRY}/library/python:3.13-slim@" + shotDigest("a") + "\n" + + "FROM python:3.12-slim@" + shotDigest("1") + "\n" + + "FROM golang:1.26.5-alpine@" + shotDigest("2") + "\n" + if got := readShotFile(t, filepath.Join(other, "Dockerfile")); got != wantDockerfile { + t.Fatalf("expected only the covered python pin to change, got:\n%s", got) + } +} + +func TestRunUpdateBaseDigests_cachedOnlyNeedsACache(t *testing.T) { + shotDigestRepo(t) + + _, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, CachedOnly: true}) + if err == nil || err.Error() != "--cached-only needs --cache-file." { + t.Fatalf("expected the missing cache flag to be refused, got %v", err) + } + + missing := filepath.Join(t.TempDir(), "missing.json") + _, err = runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, CachedOnly: true, CacheFile: missing}) + if err == nil || !strings.HasPrefix(err.Error(), "Could not read the cache: ") || !strings.Contains(err.Error(), missing) { + t.Fatalf("expected the absent cache file to be refused, got %v", err) + } +} + +func TestRunUpdateBaseDigests_refusesDriftedPins(t *testing.T) { + shotGitRepo(t, map[string]string{ + "Dockerfile": "FROM node:22@" + shotDigest("a") + "\n", + "Dockerfile.model": "FROM node:22@" + shotDigest("b") + "\n", + }) + + _, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{PinsFile: filepath.Join(t.TempDir(), "pins.json")}) + want := "Could not write the pins: node:22 is pinned to two digests, " + shotDigest("a") + " and " + shotDigest("b") + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got %v", want, err) + } +} + func TestRunUpdateBaseDigests_reportsAllCurrent(t *testing.T) { _, cacheFile := shotDigestRepo(t) shotWriteCache(t, cacheFile, map[string]string{ @@ -139,7 +217,7 @@ func TestRunUpdateBaseDigests_reportsAllCurrent(t *testing.T) { }) summaryFile := filepath.Join(t.TempDir(), "summary.md") - output, err := runUpdateBaseDigests(true, summaryFile, "node", false, cacheFile) + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: true, SummaryFile: summaryFile, Family: "node", CacheFile: cacheFile}) if err != nil { t.Fatalf("runUpdateBaseDigests failed: %v", err) } @@ -214,7 +292,7 @@ func TestRunUpdateBaseDigests_reportsFailures(t *testing.T) { write, summaryFile, family := tt.setup(t, root, cacheFile) want := strings.ReplaceAll(tt.want, "", cacheFile) - output, err := runUpdateBaseDigests(write, summaryFile, family, false, cacheFile) + output, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{Write: write, SummaryFile: summaryFile, Family: family, CacheFile: cacheFile}) if err == nil || !strings.HasPrefix(err.Error(), want) { t.Fatalf("expected %q, got %v", want, err) @@ -229,7 +307,7 @@ func TestRunUpdateBaseDigests_reportsFailures(t *testing.T) { func TestRunUpdateBaseDigests_needsPinnedReferences(t *testing.T) { shotGitRepo(t, map[string]string{"Dockerfile": "FROM python:3.13\n"}) - _, err := runUpdateBaseDigests(false, "", "", false, "") + _, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{}) if err == nil || err.Error() != "No pinned image references found." { t.Fatalf("expected the no-references error, got %v", err) @@ -241,7 +319,7 @@ func TestRunUpdateBaseDigests_needsARepository(t *testing.T) { t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(dir)) t.Chdir(dir) - _, err := runUpdateBaseDigests(false, "", "", false, "") + _, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{}) if err == nil || !strings.HasPrefix(err.Error(), "Could not find the repository root: ") { t.Fatalf("expected the repository root error, got %v", err) @@ -260,7 +338,7 @@ func TestRunUpdateBaseDigests_reportsListFailure(t *testing.T) { } t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) - _, err := runUpdateBaseDigests(false, "", "", false, "") + _, err := runUpdateBaseDigests(&UpdateBaseDigestsOptions{}) if err == nil || err.Error() != "Could not list the tracked files: git ls-files: exit status 5" { t.Fatalf("expected the list error, got %v", err)