From e155ca6a676288ab8d5db649b7dabafafa4670fb Mon Sep 17 00:00:00 2001 From: Tommy Keswick Date: Tue, 1 Sep 2026 17:01:20 -0700 Subject: [PATCH 1/2] Add an S3 publish action Uploads built assets to an S3 bucket and invalidates the matching CloudFront paths, authenticating with OIDC so no long-lived AWS keys exist anywhere. Replaces the per-repository publish_to_s3.bash and invalidate_cdn.bash scripts, which needed an uncommitted media.env to exist on someone's machine. The invalidation stays in this action rather than becoming a separate one. It is cleanup after an upload, not a step anyone would run on its own. Two guards, both covering ways the old scripts could do damage quietly: - It refuses to publish an empty or missing source directory. A build that "succeeds" into nothing would otherwise leave the CDN serving stale objects with no signal that anything went wrong. - The invalidation is scoped to the project's own prefix. The previous script invalidated /*, which charged for and discarded every other project's cached objects in the same distribution. Content types are set explicitly per extension rather than left to the AWS CLI's guess, which omits the charset and has changed between CLI versions. publish_to_s3.bash set text/javascript and text/css with charset=utf-8, and the live objects carry those values; replacing it should not quietly change them. It uses `cp`, not `sync --delete`. The CL-web-components prefix holds a fonts/ directory that exists in no repository -- three woff files that page.tmpl and the footer component load. A delete-enabled sync would remove them and nothing would put them back. --acl public-read is kept and the reason recorded: the media bucket is in legacy ObjectWriter mode with no bucket policy, so public read comes entirely from per-object ACLs. Removing the need for it means adding a bucket policy first, verifying the CDN still serves, then setting BucketOwnerEnforced -- in that order, or every project on the distribution goes down. The calling job must declare `permissions: id-token: write`; a composite action cannot. The role's trust policy must name the calling repository's subject claim, which is repo-specific. Both are documented, because both cost time to diagnose from the error message alone. CI tests the argument handling and the guards but holds no AWS credentials. This repository should not have any. Co-Authored-By: Claude Opus 5 --- .github/actions/publish-to-s3/action.yml | 113 +++++++++++++++++ .github/workflows/ci.yml | 26 ++++ README.md | 38 ++++++ bin/publish-to-s3.sh | 147 +++++++++++++++++++++++ 4 files changed, 324 insertions(+) create mode 100644 .github/actions/publish-to-s3/action.yml create mode 100755 bin/publish-to-s3.sh diff --git a/.github/actions/publish-to-s3/action.yml b/.github/actions/publish-to-s3/action.yml new file mode 100644 index 0000000..b823dac --- /dev/null +++ b/.github/actions/publish-to-s3/action.yml @@ -0,0 +1,113 @@ +name: Publish assets to S3 +description: >- + Authenticate to AWS with OIDC, upload built assets to S3, and invalidate the + matching CloudFront paths so the change is visible. No long-lived credentials + are involved. + + The calling job must declare `permissions: id-token: write`, which a composite + action cannot do on its own. + +inputs: + role-to-assume: + description: >- + ARN of the role to assume via OIDC. The role's trust policy must name the + calling repository's subject claim -- it is repo-specific, so a role that + works for one repository will not work for another without an entry. + required: true + aws-region: + description: AWS region. + required: false + default: us-west-2 + bucket: + description: S3 bucket name. + required: true + prefix: + description: Key prefix within the bucket, without leading or trailing slash. + required: true + sources: + description: >- + Directories to upload, one per line. A directory named `dist` uploads to + the prefix root; anything else uploads to a subdirectory of that name. + required: false + default: | + dist + css + distribution-id: + description: CloudFront distribution to invalidate. Omit to skip invalidation. + required: false + default: "" + acl: + description: >- + Object ACL. Defaults to public-read because the media bucket is in legacy + ObjectWriter mode with no bucket policy, so public read comes entirely + from per-object ACLs. Set to "none" once a bucket policy exists. + required: false + default: public-read + public-base-url: + description: >- + Public URL the prefix is served from, with a trailing slash. When set, + the job summary links every published file. S3 website endpoints have no + directory listing, so naming the files is the only way to link them. + required: false + default: "" + dry-run: + description: Show what would be uploaded and invalidated, and change nothing. + required: false + default: "false" + +runs: + using: composite + steps: + - uses: aws-actions/configure-aws-credentials@v6 + with: + role-to-assume: ${{ inputs.role-to-assume }} + aws-region: ${{ inputs.aws-region }} + + - name: Publish + shell: bash + env: + BUCKET: ${{ inputs.bucket }} + PREFIX: ${{ inputs.prefix }} + SOURCES: ${{ inputs.sources }} + DISTRIBUTION: ${{ inputs.distribution-id }} + ACL: ${{ inputs.acl }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + ARGS=(--bucket "$BUCKET" --prefix "$PREFIX" --acl "$ACL") + ARGS+=(--manifest "$RUNNER_TEMP/published.txt") + [ -n "$DISTRIBUTION" ] && ARGS+=(--distribution "$DISTRIBUTION") + [ "$DRY_RUN" = "true" ] && ARGS+=(--dry-run) + while IFS= read -r line; do + [ -n "$line" ] && ARGS+=(--source "$line") + done <<< "$SOURCES" + "${{ github.action_path }}/../../../bin/publish-to-s3.sh" "${ARGS[@]}" + + - name: Summarize + shell: bash + env: + BUCKET: ${{ inputs.bucket }} + PREFIX: ${{ inputs.prefix }} + BASE_URL: ${{ inputs.public-base-url }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + MANIFEST="$RUNNER_TEMP/published.txt" + COUNT="$(grep -c . "$MANIFEST" 2>/dev/null || echo 0)" + { + echo "### S3 publish${DRY_RUN:+ (dry run)}" + echo + echo "| | |" + echo "|---|---|" + echo "| bucket | \`$BUCKET\` |" + echo "| prefix | \`$PREFIX\` |" + echo "| files | $COUNT |" + echo "| dry run | $DRY_RUN |" + echo + if [ -n "$BASE_URL" ] && [ "$COUNT" -gt 0 ]; then + echo "#### Published files" + echo + sort "$MANIFEST" | head -n 40 | while IFS= read -r key; do + [ -n "$key" ] && echo "- [\`$key\`](${BASE_URL%/}/$key)" + done + [ "$COUNT" -gt 40 ] && echo "- …and $((COUNT - 40)) more" + fi + } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 046905b..15860ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -191,6 +191,32 @@ jobs: || { echo "::error::unhelpful error: $(cat /tmp/err)"; exit 1; } echo "ok" + publish-to-s3-guards: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + # No AWS credentials here on purpose: this repository should not hold + # any. What is testable without them is the argument handling and the + # guards, which are where the damage would be. + - name: Required arguments are enforced + run: | + fail() { echo "::error::$1"; exit 1; } + bin/publish-to-s3.sh 2>/dev/null && fail "should require --bucket" + bin/publish-to-s3.sh --bucket b 2>/dev/null && fail "should require --prefix" + bin/publish-to-s3.sh --bucket b --prefix p 2>/dev/null && fail "should require --source" + echo "ok" + + - name: An empty or missing source is refused + run: | + fail() { echo "::error::$1"; exit 1; } + mkdir -p /tmp/empty-dist + bin/publish-to-s3.sh --bucket b --prefix p --source /tmp/empty-dist 2>/dev/null \ + && fail "publishing an empty directory should be an error" + bin/publish-to-s3.sh --bucket b --prefix p --source /tmp/nope 2>/dev/null \ + && fail "a missing directory should be an error" + echo "ok" + # deploy-site is the seam every generator meets, so its guards matter more # than any single builder's. deploy-guards: diff --git a/README.md b/README.md index 020f065..bec21e0 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ decision instead of a rewrite. | `build-sphinx` | Sphinx, using the project's own documentation requirements | | `index-site` | Pagefind search index over built HTML — works with any generator | | `deploy-site` | Checks the build produced a real site, uploads it for Pages | +| `publish-to-s3` | Uploads built assets to S3 over OIDC and invalidates CloudFront | Adding a generator means one new `build-*` action; nothing downstream changes. @@ -190,6 +191,43 @@ rather than at `sphinx-build` with a confusing error. Skip this for generators with their own search. +### `publish-to-s3` + +Authenticates with OIDC, so no long-lived AWS keys exist anywhere. The calling +job must declare `permissions: id-token: write` — a composite action cannot. + +The role's trust policy has to name the **calling repository's** subject claim. +That claim is repo-specific, so a role that works for one repository will not +work for another until an entry is added. + +| Input | Default | | +| --- | --- | --- | +| `role-to-assume` | — | ARN of the role to assume via OIDC | +| `aws-region` | `us-west-2` | | +| `bucket` | — | S3 bucket name | +| `prefix` | — | Key prefix within the bucket | +| `sources` | `dist`, `css` | Directories to upload, one per line. `dist` uploads to the prefix root | +| `distribution-id` | — | CloudFront distribution to invalidate; omit to skip | +| `acl` | `public-read` | Set to `none` once the bucket has a policy instead of per-object ACLs | +| `public-base-url` | — | Public URL the prefix is served from. When set, the job summary links every published file | +| `dry-run` | `false` | Show what would happen and change nothing | + +Run it with `dry-run: true` first. It refuses to publish an empty or missing +source directory, because a build that "succeeds" into nothing would otherwise +leave the CDN serving stale objects with no signal. + +Content types are set explicitly per extension rather than left to the CLI's +guess, which omits the charset and can differ between CLI versions — otherwise +what the CDN serves could change for files nobody edited. + +S3 website endpoints serve no directory listing — a URL ending in `/` returns +404 rather than an index — so the summary names each published file instead of +linking a parent directory. Set `public-base-url` to turn those into links. + +It uses `aws s3 cp`, **not `sync --delete`**. Buckets often hold objects the +publishing repository does not manage, and a delete-enabled sync would remove +them. Check what else lives under the prefix before changing that. + ### `deploy-site` | Input | Default | | diff --git a/bin/publish-to-s3.sh b/bin/publish-to-s3.sh new file mode 100755 index 0000000..23fe648 --- /dev/null +++ b/bin/publish-to-s3.sh @@ -0,0 +1,147 @@ +#!/usr/bin/env bash +# +# Upload built assets to an S3 bucket behind CloudFront, then invalidate the +# paths that changed. +# +# Credentials are not handled here. The caller is expected to have configured +# them already -- in CI that means OIDC via aws-actions/configure-aws-credentials, +# so no long-lived keys exist anywhere. Locally it means whatever profile you +# normally use. +# +# publish-to-s3.sh --bucket my-bucket --prefix cl-webcomponents \ +# --source dist --source css --distribution E123 --dry-run +# +# Always run --dry-run first. It prints what would be uploaded and what would +# be invalidated, and touches nothing. + +set -euo pipefail + +BUCKET="" +PREFIX="" +DISTRIBUTION="" +DRY_RUN="false" +ACL="public-read" +MANIFEST="" +declare -a SOURCES=() + +usage() { + cat <<'USAGE' +publish-to-s3.sh -- sync built assets to S3 and invalidate CloudFront + +Options: + --bucket NAME S3 bucket (required) + --prefix PATH key prefix within the bucket (required) + --source DIR directory to upload, repeatable (required) + --distribution ID CloudFront distribution to invalidate (optional) + --acl ACL object ACL, or "none" to omit (default: public-read) + --manifest FILE write the published keys, one per line, for a caller + that wants to report or link them + --dry-run show what would happen, change nothing + -h, --help this text +USAGE +} + +while [ $# -gt 0 ]; do + case "$1" in + --bucket) BUCKET="$2"; shift 2 ;; + --prefix) PREFIX="$2"; shift 2 ;; + --source) SOURCES+=("$2"); shift 2 ;; + --distribution) DISTRIBUTION="$2"; shift 2 ;; + --acl) ACL="$2"; shift 2 ;; + --manifest) MANIFEST="$2"; shift 2 ;; + --dry-run) DRY_RUN="true"; shift ;; + -h|--help) usage; exit 0 ;; + *) echo "publish-to-s3: unknown option: $1" >&2; usage >&2; exit 2 ;; + esac +done + +[ -n "$BUCKET" ] || { echo "publish-to-s3: --bucket is required" >&2; exit 2; } +[ -n "$PREFIX" ] || { echo "publish-to-s3: --prefix is required" >&2; exit 2; } +[ ${#SOURCES[@]} -gt 0 ] || { echo "publish-to-s3: at least one --source is required" >&2; exit 2; } +command -v aws >/dev/null || { echo "publish-to-s3: the AWS CLI is not installed" >&2; exit 1; } + +# Refuse to publish nothing. An empty build that "succeeds" would otherwise +# leave the CDN serving whatever was there before, with no signal. +for dir in "${SOURCES[@]}"; do + [ -d "$dir" ] || { echo "publish-to-s3: no such directory: $dir" >&2; exit 1; } + [ -n "$(find "$dir" -type f -print -quit)" ] || { + echo "publish-to-s3: $dir is empty" >&2; exit 1; } +done + +PREFIX="${PREFIX%/}" +declare -a EXTRA=() +[ "$DRY_RUN" = "true" ] && EXTRA+=("--dryrun") +# The bucket is in legacy ObjectWriter mode with no bucket policy: public read +# comes entirely from per-object ACLs. Dropping this makes uploads invisible. +# Removing the need for it means adding a bucket policy first -- see the +# media bucket notes before changing this. +[ "$ACL" != "none" ] && EXTRA+=("--acl" "$ACL") + +# Content types are set explicitly rather than left to the CLI's guess. The +# guess omits the charset and can differ by CLI version, which would silently +# change what the CDN serves for files nobody edited. +content_type_for() { + case "$1" in + js) echo "text/javascript; charset=utf-8" ;; + css) echo "text/css; charset=utf-8" ;; + json) echo "application/json; charset=utf-8" ;; + jsonld) echo "application/ld+json; charset=utf-8" ;; + svg) echo "image/svg+xml; charset=utf-8" ;; + woff) echo "font/woff" ;; + woff2) echo "font/woff2" ;; + map) echo "application/json; charset=utf-8" ;; + *) echo "" ;; + esac +} + +# NOTE: this uses `cp`, not `sync --delete`. The bucket holds objects this +# repository does not manage -- fonts/ among them -- and a delete-enabled sync +# would remove them. Do not "improve" this to a sync without checking what else +# lives under the prefix. +[ -n "$MANIFEST" ] && : > "$MANIFEST" + +for dir in "${SOURCES[@]}"; do + dest="s3://$BUCKET/$PREFIX/$(basename "$dir")/" + # dist/ holds the bundles themselves, which belong at the prefix root + [ "$(basename "$dir")" = "dist" ] && dest="s3://$BUCKET/$PREFIX/" + echo "==> $dir -> $dest" + + # One pass per extension we have a content type for, then a final pass for + # everything else. The final pass is a no-op when nothing is left over. + declare -a SKIP=() + while IFS= read -r ext; do + [ -n "$ext" ] || continue + ct="$(content_type_for "$ext")" + [ -n "$ct" ] || continue + SKIP+=("--exclude" "*.$ext") + aws s3 cp "$dir/" "$dest" --recursive \ + --exclude "*" --include "*.$ext" --content-type "$ct" \ + ${EXTRA[@]+"${EXTRA[@]}"} + done <<< "$(find "$dir" -type f -name '*.*' | sed 's|.*/||; s|.*\.||' | sort -u)" + + aws s3 cp "$dir/" "$dest" --recursive \ + ${SKIP[@]+"${SKIP[@]}"} ${EXTRA[@]+"${EXTRA[@]}"} + + # Keys published, relative to the prefix. + if [ -n "$MANIFEST" ]; then + sub="" + [ "$(basename "$dir")" = "dist" ] || sub="$(basename "$dir")/" + ( cd "$dir" && find . -type f | sed "s|^\./|$sub|" ) >> "$MANIFEST" + fi +done + +if [ -n "$DISTRIBUTION" ]; then + # Scoped to this project's prefix. Invalidating /* would charge for and + # discard every other site's cached objects in the same distribution. + PATHS="/$PREFIX/*" + if [ "$DRY_RUN" = "true" ]; then + echo "==> would invalidate $PATHS on $DISTRIBUTION" + else + echo "==> invalidating $PATHS on $DISTRIBUTION" + aws cloudfront create-invalidation \ + --distribution-id "$DISTRIBUTION" --paths "$PATHS" \ + --query 'Invalidation.{Id:Id,Status:Status}' --output text + fi +fi + +echo "publish-to-s3: ${DRY_RUN:+dry run }done" From 112a60a8a8d49b0a7ef2b33f0f1636c459fdbf1a Mon Sep 17 00:00:00 2001 From: Tommy Keswick Date: Tue, 1 Sep 2026 18:18:49 -0700 Subject: [PATCH 2/2] Harden publish-to-s3, and start a changelog Five things, all surfacing from the first dry run of publish-to-s3 against a real consumer. Fixed: the summary step failed after a successful publish whenever 40 or fewer files were uploaded -- every realistic upload. It ended on a test that returns non-zero when false, and composite action steps run with `bash -eo pipefail`, so the step failed after the work had already succeeded. CI did not catch it because the guards job exercises the script and this was in the action; the consumer's dry run is what found it. `prefix` is now optional. Empty means the bucket root, which is the shape a project gets when it has a bucket to itself. Paths no longer collapse into a double slash, stray leading and trailing slashes are normalised, and the resulting whole-distribution invalidation is reported rather than silent. The `acl` default is now `none`, following AWS's recommendation to disable ACLs and grant access by bucket policy, which is also what new buckets get. That is a breaking change: a bucket in legacy ObjectWriter mode with no policy must now ask for public-read. It is safe to make now only because nothing consumes this action yet. The risk in that flip is a silent failure -- an object uploaded without an ACL the bucket needs uploads fine and is unreadable -- so setting public-base-url now also fetches one published file afterwards and fails on a non-200. Both mistakes are loud in either direction: a missing ACL fails the fetch, and setting one on a bucket with BucketOwnerEnforced already failed with AccessControlListNotSupported. CHANGELOG.md, following Keep a Changelog 1.1.0. Consumers reference a moving major tag, so when @v1 advances they have had no way to learn what moved short of reading commits. CONTRIBUTING.md asks for an entry with any change that reaches a consumer, and documents the release procedure: entries accumulate under [Unreleased], releasing renames that heading and tags the commit that does so, one commit and one tag per release rather than per change. ADR-0007 records why publishing touches only what it published -- copy rather than sync with delete, no ACL by default, explicit content types, invalidation scoped to the prefix. Each looks like an oversight to a reader assuming a dedicated bucket, and each is destructive if "corrected". It is scoped to the action rather than to any deployment: operational facts about a particular bucket belong in the repository that publishes to it. Co-Authored-By: Claude Opus 5 --- .github/actions/publish-to-s3/action.yml | 29 +++-- .github/workflows/ci.yml | 17 +++ CHANGELOG.md | 75 +++++++++++++ CONTRIBUTING.md | 34 ++++++ README.md | 23 +++- bin/publish-to-s3.sh | 63 ++++++++--- ...hout-destroying-what-you-did-not-create.md | 102 ++++++++++++++++++ 7 files changed, 313 insertions(+), 30 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md diff --git a/.github/actions/publish-to-s3/action.yml b/.github/actions/publish-to-s3/action.yml index b823dac..0081446 100644 --- a/.github/actions/publish-to-s3/action.yml +++ b/.github/actions/publish-to-s3/action.yml @@ -22,8 +22,12 @@ inputs: description: S3 bucket name. required: true prefix: - description: Key prefix within the bucket, without leading or trailing slash. - required: true + description: >- + Key prefix within the bucket. Leave empty to publish to the bucket root, + which is the shape a project gets when it has a bucket to itself. With no + prefix the invalidation covers the whole distribution. + required: false + default: "" sources: description: >- Directories to upload, one per line. A directory named `dist` uploads to @@ -38,16 +42,19 @@ inputs: default: "" acl: description: >- - Object ACL. Defaults to public-read because the media bucket is in legacy - ObjectWriter mode with no bucket policy, so public read comes entirely - from per-object ACLs. Set to "none" once a bucket policy exists. + Object ACL, or "none" to set none. AWS recommends ACLs disabled and + access granted by bucket policy, which is the default here and what new + buckets get. A bucket still in legacy ObjectWriter mode with no policy + needs "public-read", or its objects upload fine and are unreadable -- + set public-base-url as well and the action will catch that. required: false - default: public-read + default: none public-base-url: description: >- Public URL the prefix is served from, with a trailing slash. When set, - the job summary links every published file. S3 website endpoints have no - directory listing, so naming the files is the only way to link them. + the job summary links every published file, and after a real publish one + of them is fetched to confirm it is readable. S3 website endpoints have + no directory listing, so naming the files is the only way to link them. required: false default: "" dry-run: @@ -72,9 +79,11 @@ runs: DISTRIBUTION: ${{ inputs.distribution-id }} ACL: ${{ inputs.acl }} DRY_RUN: ${{ inputs.dry-run }} + BASE_URL: ${{ inputs.public-base-url }} run: | ARGS=(--bucket "$BUCKET" --prefix "$PREFIX" --acl "$ACL") ARGS+=(--manifest "$RUNNER_TEMP/published.txt") + [ -n "$BASE_URL" ] && ARGS+=(--verify-url "$BASE_URL") [ -n "$DISTRIBUTION" ] && ARGS+=(--distribution "$DISTRIBUTION") [ "$DRY_RUN" = "true" ] && ARGS+=(--dry-run) while IFS= read -r line; do @@ -108,6 +117,8 @@ runs: sort "$MANIFEST" | head -n 40 | while IFS= read -r key; do [ -n "$key" ] && echo "- [\`$key\`](${BASE_URL%/}/$key)" done - [ "$COUNT" -gt 40 ] && echo "- …and $((COUNT - 40)) more" + if [ "$COUNT" -gt 40 ]; then + echo "- …and $((COUNT - 40)) more" + fi fi } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15860ec..6c6833f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -207,6 +207,23 @@ jobs: bin/publish-to-s3.sh --bucket b --prefix p 2>/dev/null && fail "should require --source" echo "ok" + - name: An empty prefix publishes to the bucket root + run: | + fail() { echo "::error::$1"; exit 1; } + mkdir -p /tmp/pfx/dist && touch /tmp/pfx/dist/a.js + printf '#!/bin/sh\nexit 0\n' > /tmp/aws && chmod +x /tmp/aws + out="$(PATH=/tmp:$PATH bin/publish-to-s3.sh --bucket b --source /tmp/pfx/dist \ + --distribution E1 --dry-run 2>&1)" + echo "$out" | grep -q 's3://b/$' || fail "no prefix should target the bucket root, got: $out" + echo "$out" | grep -q 'invalidate /\* ' || fail "no prefix should invalidate /*" + echo "$out" | grep -q 'no prefix, so this invalidates the whole distribution' \ + || fail "should say so when invalidating everything" + # and a prefix with stray slashes is normalized + out="$(PATH=/tmp:$PATH bin/publish-to-s3.sh --bucket b --prefix /wrapped/ \ + --source /tmp/pfx/dist --distribution E1 --dry-run 2>&1)" + echo "$out" | grep -q 's3://b/wrapped/$' || fail "stray slashes not normalized: $out" + echo ok + - name: An empty or missing source is refused run: | fail() { echo "::error::$1"; exit 1; } diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..b219cff --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,75 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +Consuming repositories reference a **moving major tag** such as `@v1`, which +advances with every backward-compatible change. This file is how you find out +what moved. See +[ADR-0006](docs/decisions/0006-version-with-moving-major-tags.md). + +## [Unreleased] + +### Added + +- `CHANGELOG.md`, following + [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +- [ADR-0007](docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md), + recording why publishing touches only what it published: copy rather than + sync with delete, no ACL by default, explicit content types, and invalidation + scoped to the prefix. Each looks like an oversight to a reader assuming a + dedicated bucket, and each is destructive if "corrected". + +### Changed + +- `publish-to-s3`: `prefix` is now optional. An empty prefix publishes to the + bucket root and invalidates the whole distribution, which is correct for a + bucket belonging to one project. The script reports when that happens. +- `publish-to-s3`: **the `acl` default is now `none`**, following AWS's + recommendation to disable ACLs and grant access by bucket policy. A bucket + in legacy `ObjectWriter` mode with no policy must now set + `acl: public-read` explicitly. +- `publish-to-s3`: when `public-base-url` is set, one published file is + fetched after a real publish and a non-200 fails the job. An object uploaded + without an ACL a legacy bucket needs otherwise succeeds and is silently + unreadable. + +### Fixed + +- `publish-to-s3` no longer fails after a successful publish when 40 or fewer + files are uploaded. The summary step ended on a test that returns non-zero + when false, and composite action steps run with `bash -eo pipefail`. + +## [1.1.0] - 2026-09-01 + +### Added + +- `publish-to-s3` action: uploads built assets to S3 and invalidates the + matching CloudFront paths, authenticating with OIDC so no long-lived AWS + keys are required. Replaces per-repository `publish_to_s3.bash` and + `invalidate_cdn.bash` scripts. +- `public-base-url` input on `publish-to-s3`, which turns the job summary into + links to each published file. S3 website endpoints serve no directory + listing, so the files have to be named individually. + +## [1.0.0] - 2026-08-31 + +### Added + +- `build-pandoc`, `build-zensical` and `build-sphinx` actions, one per + documentation generator. Only the build step differs between them; see + [ADR-0003](docs/decisions/0003-one-build-action-per-generator.md). +- `index-site` action, building a Pagefind index over already-built HTML, so + it works with any generator. +- `deploy-site` action, which refuses to publish a build containing no HTML or + no `index.html` before uploading it as a Pages artifact. +- `docs-pandoc.yml` reusable workflow, wrapping the common build, index and + deploy sequence. +- The shared Caltech Pandoc theme in `pandoc/`, overridable per project. +- Architecture decision records in `docs/decisions/`. + +[Unreleased]: https://github.com/caltechlibrary/workflows/compare/v1.1.0...HEAD +[1.1.0]: https://github.com/caltechlibrary/workflows/compare/v1.0.0...v1.1.0 +[1.0.0]: https://github.com/caltechlibrary/workflows/releases/tag/v1.0.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 30e7012..d596cab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -169,3 +169,37 @@ rejected, which is the part the code cannot tell you. Adding a generator does not need an ADR. Changing the contract every generator satisfies does. + +Every change that reaches a consumer gets a `CHANGELOG.md` entry under +`## [Unreleased]`, following +[Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Consumers reference a +moving major tag, so the changelog is the only way they learn what moved. + +## Releasing + +Entries accumulate under `## [Unreleased]` as pull requests merge. Releasing is +a separate, deliberate step — one commit and one tag per release, not per +change. + +1. Rename `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, and add a fresh empty + `## [Unreleased]` above it. +2. Update the link references at the bottom of `CHANGELOG.md`. +3. Commit that on its own: `Release X.Y.Z`. +4. Tag that commit: `git tag -a vX.Y.Z -m "vX.Y.Z — "`. +5. Move the major tag: `git tag -f vX` and force-push it. +6. Push the commit and both tags. + +Tagging the release commit means the tag points at a changelog that describes +itself, rather than one still saying "Unreleased". + +Choosing the number, per +[ADR-0006](docs/decisions/0006-version-with-moving-major-tags.md): + +| Change | | +| --- | --- | +| An input renamed or removed, an action path changed, **a default changed** | major | +| A new action or input, backward compatible | minor | +| A fix with no interface change | patch | + +Changing a default is breaking even though nothing in the interface moved: +existing callers get different behavior without editing anything. diff --git a/README.md b/README.md index bec21e0..38b8089 100644 --- a/README.md +++ b/README.md @@ -205,11 +205,11 @@ work for another until an entry is added. | `role-to-assume` | — | ARN of the role to assume via OIDC | | `aws-region` | `us-west-2` | | | `bucket` | — | S3 bucket name | -| `prefix` | — | Key prefix within the bucket | +| `prefix` | — | Key prefix within the bucket. Leave empty to publish to the bucket root | | `sources` | `dist`, `css` | Directories to upload, one per line. `dist` uploads to the prefix root | | `distribution-id` | — | CloudFront distribution to invalidate; omit to skip | -| `acl` | `public-read` | Set to `none` once the bucket has a policy instead of per-object ACLs | -| `public-base-url` | — | Public URL the prefix is served from. When set, the job summary links every published file | +| `acl` | `none` | AWS recommends ACLs disabled. Set to `public-read` for a legacy bucket with no policy | +| `public-base-url` | — | Public URL the prefix is served from. When set, the summary links every published file and one is fetched afterwards to confirm it is readable | | `dry-run` | `false` | Show what would happen and change nothing | Run it with `dry-run: true` first. It refuses to publish an empty or missing @@ -220,13 +220,24 @@ Content types are set explicitly per extension rather than left to the CLI's guess, which omits the charset and can differ between CLI versions — otherwise what the CDN serves could change for files nobody edited. +The `acl` default follows AWS's recommendation: ACLs disabled, access granted +by bucket policy. A bucket still in legacy `ObjectWriter` mode with no policy +needs `acl: public-read` — without it, objects upload successfully and are +unreadable. Setting `public-base-url` catches that: after a real publish one +file is fetched, and a 403 fails the job. + +With no `prefix`, files go to the bucket root and the invalidation covers the +whole distribution — correct when the bucket belongs to one project, wrong when +it is shared. The script says so when it happens. + S3 website endpoints serve no directory listing — a URL ending in `/` returns 404 rather than an index — so the summary names each published file instead of linking a parent directory. Set `public-base-url` to turn those into links. It uses `aws s3 cp`, **not `sync --delete`**. Buckets often hold objects the publishing repository does not manage, and a delete-enabled sync would remove -them. Check what else lives under the prefix before changing that. +them with nothing to put them back. See +[ADR-0007](docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md). ### `deploy-site` @@ -282,6 +293,10 @@ documentation requirements for `build-sphinx.sh`, and ## Versioning +Changes are recorded in [CHANGELOG.md](CHANGELOG.md), following +[Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Because `@v1` moves, +that file is how you find out what moved. + Reference a **major tag**: ```yaml diff --git a/bin/publish-to-s3.sh b/bin/publish-to-s3.sh index 23fe648..0c636f2 100755 --- a/bin/publish-to-s3.sh +++ b/bin/publish-to-s3.sh @@ -8,7 +8,7 @@ # so no long-lived keys exist anywhere. Locally it means whatever profile you # normally use. # -# publish-to-s3.sh --bucket my-bucket --prefix cl-webcomponents \ +# publish-to-s3.sh --bucket my-bucket --prefix my-project \ # --source dist --source css --distribution E123 --dry-run # # Always run --dry-run first. It prints what would be uploaded and what would @@ -20,8 +20,9 @@ BUCKET="" PREFIX="" DISTRIBUTION="" DRY_RUN="false" -ACL="public-read" +ACL="none" MANIFEST="" +VERIFY_URL="" declare -a SOURCES=() usage() { @@ -30,10 +31,13 @@ publish-to-s3.sh -- sync built assets to S3 and invalidate CloudFront Options: --bucket NAME S3 bucket (required) - --prefix PATH key prefix within the bucket (required) + --prefix PATH key prefix within the bucket; omit to + publish to the bucket root --source DIR directory to upload, repeatable (required) --distribution ID CloudFront distribution to invalidate (optional) - --acl ACL object ACL, or "none" to omit (default: public-read) + --acl ACL object ACL, or "none" (default: none) + --verify-url URL after publishing, fetch one published file from here + and fail if it is not readable --manifest FILE write the published keys, one per line, for a caller that wants to report or link them --dry-run show what would happen, change nothing @@ -49,6 +53,7 @@ while [ $# -gt 0 ]; do --distribution) DISTRIBUTION="$2"; shift 2 ;; --acl) ACL="$2"; shift 2 ;; --manifest) MANIFEST="$2"; shift 2 ;; + --verify-url) VERIFY_URL="$2"; shift 2 ;; --dry-run) DRY_RUN="true"; shift ;; -h|--help) usage; exit 0 ;; *) echo "publish-to-s3: unknown option: $1" >&2; usage >&2; exit 2 ;; @@ -56,7 +61,6 @@ while [ $# -gt 0 ]; do done [ -n "$BUCKET" ] || { echo "publish-to-s3: --bucket is required" >&2; exit 2; } -[ -n "$PREFIX" ] || { echo "publish-to-s3: --prefix is required" >&2; exit 2; } [ ${#SOURCES[@]} -gt 0 ] || { echo "publish-to-s3: at least one --source is required" >&2; exit 2; } command -v aws >/dev/null || { echo "publish-to-s3: the AWS CLI is not installed" >&2; exit 1; } @@ -68,13 +72,16 @@ for dir in "${SOURCES[@]}"; do echo "publish-to-s3: $dir is empty" >&2; exit 1; } done -PREFIX="${PREFIX%/}" +# An empty prefix means the bucket root, which is the shape a project gets +# when it has a bucket to itself. +PREFIX="${PREFIX#/}"; PREFIX="${PREFIX%/}" +BASE="s3://$BUCKET" +[ -n "$PREFIX" ] && BASE="$BASE/$PREFIX" declare -a EXTRA=() [ "$DRY_RUN" = "true" ] && EXTRA+=("--dryrun") -# The bucket is in legacy ObjectWriter mode with no bucket policy: public read -# comes entirely from per-object ACLs. Dropping this makes uploads invisible. -# Removing the need for it means adding a bucket policy first -- see the -# media bucket notes before changing this. +# AWS recommends ACLs disabled and access granted by bucket policy, so no ACL +# is the default. A bucket still in legacy ObjectWriter mode with no policy +# needs --acl public-read, or its objects upload fine and are unreadable. [ "$ACL" != "none" ] && EXTRA+=("--acl" "$ACL") # Content types are set explicitly rather than left to the CLI's guess. The @@ -94,16 +101,15 @@ content_type_for() { esac } -# NOTE: this uses `cp`, not `sync --delete`. The bucket holds objects this -# repository does not manage -- fonts/ among them -- and a delete-enabled sync -# would remove them. Do not "improve" this to a sync without checking what else -# lives under the prefix. +# NOTE: `cp`, not `sync --delete`. A bucket commonly holds objects no +# repository produces, and a delete-enabled sync removes them with nothing to +# put them back. See ADR-0007. [ -n "$MANIFEST" ] && : > "$MANIFEST" for dir in "${SOURCES[@]}"; do - dest="s3://$BUCKET/$PREFIX/$(basename "$dir")/" + dest="$BASE/$(basename "$dir")/" # dist/ holds the bundles themselves, which belong at the prefix root - [ "$(basename "$dir")" = "dist" ] && dest="s3://$BUCKET/$PREFIX/" + [ "$(basename "$dir")" = "dist" ] && dest="$BASE/" echo "==> $dir -> $dest" # One pass per extension we have a content type for, then a final pass for @@ -133,7 +139,12 @@ done if [ -n "$DISTRIBUTION" ]; then # Scoped to this project's prefix. Invalidating /* would charge for and # discard every other site's cached objects in the same distribution. - PATHS="/$PREFIX/*" + if [ -n "$PREFIX" ]; then + PATHS="/$PREFIX/*" + else + PATHS="/*" + echo "note: no prefix, so this invalidates the whole distribution" >&2 + fi if [ "$DRY_RUN" = "true" ]; then echo "==> would invalidate $PATHS on $DISTRIBUTION" else @@ -144,4 +155,22 @@ if [ -n "$DISTRIBUTION" ]; then fi fi +# An object uploaded without the ACL a legacy bucket needs succeeds and is +# then unreadable. Fetching one published file turns that into a failure here +# rather than a report from someone whose page stopped working. +if [ -n "$VERIFY_URL" ] && [ "$DRY_RUN" != "true" ] && [ -n "$MANIFEST" ]; then + key="$(head -n 1 "$MANIFEST")" + if [ -n "$key" ]; then + url="${VERIFY_URL%/}/$key" + code="$(curl -s -o /dev/null -w '%{http_code}' -L "$url" || echo 000)" + if [ "$code" = "200" ]; then + echo "verified: $url is readable" + else + echo "publish-to-s3: published, but $url returned $code" >&2 + echo " if this is 403, the bucket likely needs --acl public-read" >&2 + exit 1 + fi + fi +fi + echo "publish-to-s3: ${DRY_RUN:+dry run }done" diff --git a/docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md b/docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md new file mode 100644 index 0000000..6fd7bad --- /dev/null +++ b/docs/decisions/0007-publish-without-destroying-what-you-did-not-create.md @@ -0,0 +1,102 @@ +# 7. Publish without destroying what you did not create + +- Status: accepted +- Date: 2026-09-01 + +## Context and Problem Statement + +`publish-to-s3` uploads a build to a bucket this repository knows nothing +about. It does not know what else lives there, who else writes to it, how its +access is granted, or whether the distribution in front of it serves anything +else. + +Four of its choices look like oversights under that ignorance, and a reader +who assumes a bucket dedicated to one project would tidy every one of them +away: + +- it copies rather than syncing, so nothing is ever deleted +- it defaults to setting no object ACL +- it sets content types explicitly instead of letting the AWS CLI detect them +- it invalidates one prefix rather than the whole distribution + +## Decision + +Publishing touches only what it published. Anything broader is opt-in. + +### Copy, never sync with delete + +`aws s3 cp --recursive`, not `aws s3 sync --delete`. + +A bucket commonly holds objects no repository produces — uploaded by hand, +left by an older process, or written by another project under another prefix. +A delete-enabled sync removes them, and nothing puts them back, because +nothing knows they existed. + +Switching to `sync` is defensible only after enumerating what is under the +prefix and confirming every object is reproducible from a build. + +### Default to no ACL + +AWS has recommended disabling ACLs and granting access by bucket policy since +2023, and new buckets are created that way. The default follows the +recommendation rather than accommodating the older arrangement. + +A bucket still in legacy `ObjectWriter` mode with no policy needs +`acl: public-read`. Omitting it there is a silent failure: the upload succeeds +and the objects are unreadable. That is why `public-base-url` also triggers a +check — one published file is fetched afterwards, and a non-200 fails the job. + +So both mistakes are loud. Omitting a needed ACL fails the fetch; setting one +on a bucket with `BucketOwnerEnforced` fails with +`AccessControlListNotSupported`. + +### Set content types explicitly + +The AWS CLI infers a content type from the file extension. Its inference omits +the charset and has changed between CLI versions, so leaving it to the guess +means what a site serves can change for files nobody edited, at a time nobody +chose — including as a side effect of a runner image update. + +### Invalidate the prefix, not the distribution + +`/$PREFIX/*`, never `/*` unless the prefix is empty. + +A distribution may serve several projects. A wildcard invalidation discards +every other project's cached objects, so their next requests all miss to the +origin, and CloudFront bills per invalidation path beyond the free tier. + +An empty prefix is the exception: it means the bucket root, which is what a +project gets when the bucket is its own, and `/*` is then correct. The script +reports when that happens rather than doing it quietly. + +## Scope + +These are properties of the action, not of any bucket. Operational facts about +a particular bucket — its ownership mode, what unmanaged content it holds, the +order of a migration off ACLs — belong in the repository that publishes to it. +Recording them here would tie a generic action to one deployment and go stale +without anyone noticing. + +## Consequences + +Good: + +- The action is safe to point at a bucket nobody has audited, which is the + normal case for adoption. +- Each choice has a stated failure mode, so a reader can tell a defensive + decision from a leftover. + +Bad, and accepted: + +- More conservative than a greenfield action would be. Someone publishing to a + dedicated bucket with a policy and no unmanaged content carries defaults they + do not need — though `acl`, `prefix` and `distribution-id` are all inputs, so + the cost is configuration rather than a fork. +- Never deleting means a file dropped from a build stays published until + someone removes it by hand. That is the deliberate trade: stale is + recoverable, deleted is not. + +## More Information + +- `bin/publish-to-s3.sh` — the choices in the code +- [CHANGELOG.md](../../CHANGELOG.md) — when the ACL default changed, and why