From b888f09eb1fe79090963bd254824ecc672455897 Mon Sep 17 00:00:00 2001 From: Ryan McGuire Date: Sun, 9 Aug 2026 12:05:23 -0400 Subject: [PATCH] Readable job names, latest tracks main, and an AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Job names in the Gitea UI rendered as the raw matrix expression ("${{ fromJSON(needs.select.outputs.images) }}"). Gitea resolves a job's matrix when it PARSES the workflow, before `needs` outputs exist, so matrix.image was interpolated against an unresolved matrix. There is no way to win with a dynamic matrix: jobparser.nameWithMatrix interpolates a name containing "${{ }}", and appends "()" to one that doesn't — either way the raw expression shows. See go-gitea/gitea#28207. So drop the matrix. build.yaml now runs one job, "Build changed images", that loops over the selected images and emits ::group:: markers, giving a collapsible section per image. Also: every step has an explicit static name, because Gitea does not interpolate expressions in names either ("Log in to ${{ env.REGISTRY }}" rendered literally). Tag scheme, per review: `latest` now tracks main rather than the newest release, and `edge` is gone — it's an Alpine/Traefik convention, not a broad standard, and `main-` already covers "a specific commit". `latest` has exactly one owner so a release tag and a main build can't race to define it. Release tags remain immutable `:vX.Y.Z`/`:vX.Y`/`:vX` for pinning. This also means the ToolHive manifest's `:latest` resolves as soon as this lands on main, with no release tag needed first. hack/docker-tags.sh folded into hack/build-images.sh, which is now the whole pipeline — tag, build, smoke-test, push — shared by `make build` and CI. It uses plain `docker build` instead of buildx, dropping setup-buildx-action: it loads into the local store so the test runs pre-publish, and emits a plain manifest with no attestations for Gitea's registry. Verified in a simulated runner (repo in a docker volume, socket mounted, real runner image): select + build + group markers, all four tag modes, and the multi-image loop with a scratch second image. AGENTS.md records the conventions and, importantly, the three Gitea gotchas that all look fine locally: no bind-mounting the workspace into a sibling container, no dynamic matrix, no expressions in names. Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/build.yaml | 56 +++++-------- .gitea/workflows/lint.yaml | 4 +- AGENTS.md | 156 ++++++++++++++++++++++++++++++++++++ Makefile | 11 +-- README.md | 33 ++++---- hack/build-images.sh | 121 ++++++++++++++++++++++++++++ hack/docker-tags.sh | 54 ------------- hack/select-images.sh | 31 ++++--- 8 files changed, 336 insertions(+), 130 deletions(-) create mode 100644 AGENTS.md create mode 100755 hack/build-images.sh delete mode 100755 hack/docker-tags.sh diff --git a/.gitea/workflows/build.yaml b/.gitea/workflows/build.yaml index 9d2a26f..2cb2d49 100644 --- a/.gitea/workflows/build.yaml +++ b/.gitea/workflows/build.yaml @@ -32,61 +32,41 @@ jobs: images: ${{ steps.select.outputs.images }} any: ${{ steps.select.outputs.any }} steps: - - uses: actions/checkout@v7 + - name: Check out repository + uses: actions/checkout@v7 with: fetch-depth: 0 # need history to diff against the base commit - - id: select + - name: Work out which images changed + id: select run: ./hack/select-images.sh >> "$GITHUB_OUTPUT" env: DISPATCH_IMAGE: ${{ github.event.inputs.image }} BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + # One job that loops, rather than a matrix job per image: Gitea resolves a matrix + # when it parses the workflow, before `needs` outputs exist, so a dynamic matrix + # produces a single job named after the raw expression. Looping keeps the job name + # readable and gives one collapsible log section per image. build: - name: ${{ matrix.image }} + name: Build changed images needs: select if: needs.select.outputs.any == 'true' runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - image: ${{ fromJSON(needs.select.outputs.images) }} steps: - - uses: actions/checkout@v7 + - name: Check out repository + uses: actions/checkout@v7 - - id: meta - run: ./hack/docker-tags.sh '${{ matrix.image }}' >> "$GITHUB_OUTPUT" - env: - PR_NUMBER: ${{ github.event.pull_request.number }} - - - uses: docker/setup-buildx-action@v4 - - # Loaded rather than pushed, so the smoke test runs before anything is - # published. provenance is off because Gitea's registry rejects buildkit - # attestation manifests. - - name: Build - uses: docker/build-push-action@v7 - with: - context: images/${{ matrix.image }} - load: true - tags: ${{ steps.meta.outputs.tags }} - provenance: false - build-args: | - VERSION=${{ steps.meta.outputs.version }} - REVISION=${{ github.sha }} - CREATED=${{ steps.meta.outputs.created }} - - - name: Smoke test - run: make test IMAGE='${{ matrix.image }}' REF='${{ steps.meta.outputs.primary }}' - - - name: Log in to ${{ env.REGISTRY }} - if: steps.meta.outputs.push == 'true' + # Never on a pull request, which is exactly when nothing gets published. + - name: Log in to the Gitea container registry + if: github.event_name != 'pull_request' uses: docker/login-action@v4 with: registry: ${{ env.REGISTRY }} username: ${{ env.DOCKER_USER }} password: ${{ secrets.API_TOKEN }} - - name: Push - if: steps.meta.outputs.push == 'true' - run: printf '%s\n' '${{ steps.meta.outputs.tags }}' | xargs -r -n1 -t docker push + - name: Build, smoke-test and publish + run: ./hack/build-images.sh ${{ needs.select.outputs.images }} + env: + PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/.gitea/workflows/lint.yaml b/.gitea/workflows/lint.yaml index eaee22a..a25eba0 100644 --- a/.gitea/workflows/lint.yaml +++ b/.gitea/workflows/lint.yaml @@ -8,9 +8,11 @@ on: jobs: lint: + name: Lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - name: Check out repository + uses: actions/checkout@v7 # Same targets you run locally with `make check lint`. - name: Repository layout diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..bace386 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,156 @@ +# AGENTS.md + +Guidance for coding agents when working in this repository. + +## Overview + +A base image registry. Each image is self-contained in its own directory under +`images/` and is published to the **public** `rmcguire` namespace on Gitea at +`gitea.libretechconsulting.com/rmcguire/`. Anonymous pulls work, so +consumers need no `imagePullSecret`. + +``` +images//Dockerfile the image +images//README.md required — `make check` fails without it +images//test.sh optional smoke test; must be executable to run +template/ skeleton to copy when adding an image +hack/ the three scripts CI and the Makefile share +Makefile single entry point for local work and CI +.gitea/workflows/ lint.yaml, build.yaml +``` + +## Core principle: do not pin, rebuild instead + +Base image tags, apk package versions and tool releases are deliberately +unpinned. Rebuilding is how upstream updates land; the **published image tag** is +what pins things for consumers. Do not add `=version` to `apk add`, and do not +pin `FROM` to a digest. + +Corollary: prefer an Alpine package over an upstream download when one exists — +one install mechanism beats two. Only fetch from upstream when Alpine does not +package the tool at all (`yq` is the sole example today). Where a major version +must be held back, constrain the major only and still track latest within it. + +## Adding an image + +```sh +cp -r template images/my-image +$EDITOR images/my-image/Dockerfile images/my-image/README.md images/my-image/test.sh +make build IMAGE=my-image # builds and smoke-tests +make check lint +``` + +No workflow changes are ever needed — `hack/select-images.sh` discovers any +directory under `images/` containing a `Dockerfile`. + +Dockerfile conventions, all visible in `images/node-agent/Dockerfile`: + +- Keep the `ARG VERSION/REVISION/CREATED` + `LABEL` block **last**, so changing + build metadata does not invalidate the layers above it. +- `# hadolint ignore=DL3018` above `apk add`, since versions are unpinned on + purpose. State the reason in a comment. +- Set `SHELL ["/bin/bash", "-o", "pipefail", "-c"]` before any `RUN` containing a + pipe (only after bash is installed). +- For a tool fetched from upstream, run it in the same layer (`yq --version`). + That is the check that matters: it catches a truncated download or an HTML + error page, which is the realistic failure mode. + +## Tagging and releases + +Tagging is derived from the git ref by `hack/build-images.sh`, identically for +every image in a run: + +| Ref | Tags | Published | +| --- | --- | --- | +| push to `main` | `:latest`, `:main-` | yes | +| tag `/vX.Y.Z` | `:vX.Y.Z`, `:vX.Y`, `:vX` | yes | +| pull request | `:pr-` | no | +| anything else / local | `:dev` | no | + +`latest` has exactly one owner — `main` — so a release tag and a `main` build can +never race to define it. Release tags are per image: `git tag node-agent/v1.0.0` +builds and publishes that image alone. + +## Smoke tests + +`images//test.sh` takes an image ref and must exit non-zero on failure. Two +conventions worth keeping, both in `images/node-agent/test.sh`: + +- **Assert the deployment contract, not just tool presence.** node-agent runs + the checks under `--user 1000:1000 --read-only --tmpfs /tmp --cap-drop ALL`, + mirroring the securityContext it is actually deployed with, so CI catches + read-only or non-root breakage. +- **Do not `set -e` inside the container script.** Use a `fail` counter so every + check reports, then exit with it. + +## Gitea Actions gotchas + +These cost real debugging time. All three are Gitea behaviours, not bugs in this +repo, and all three will look fine locally. + +**Never bind-mount the workspace into a sibling container.** The job itself runs +in a container and the checkout lives in a docker volume, not on the host. A +`docker run -v "$PWD":/repo` started through the host's docker socket resolves +that path against the **host**, where it does not exist — Docker silently creates +an empty directory, and the tool reports every file as missing. Stream files in +on stdin instead; `hack/lint.sh` does this for hadolint and shellcheck. Note +`docker build` and `docker run` without `-v` are unaffected, because the build +context is streamed over the API. + +**Do not use a dynamic `fromJSON(needs...)` matrix.** Gitea resolves a job's +matrix when it *parses* the workflow, before `needs` outputs exist, so you get a +single job whose name is the raw uninterpolated expression. See +`nameWithMatrix` in Gitea's `modules/actions/jobparser` and +[go-gitea/gitea#28207](https://github.com/go-gitea/gitea/issues/28207). That is +why `build.yaml` loops over images inside one job and emits `::group::` markers +per image instead. + +**Do not put `${{ }}` in a job or step `name:`.** Gitea does not interpolate +expression contexts in names — `name: Log in to ${{ env.REGISTRY }}` renders +literally. Give every step an explicit static `name:`, or it displays as the raw +`run:` command. + +## Verifying + +```sh +make all # check + lint + build + smoke-test; what CI does +make help # all targets +``` + +To reproduce the CI container environment locally — worth it before touching +anything in `hack/` or the workflows, since bind-mount and path assumptions only +break there: + +```sh +docker volume create ci-sim +docker run --rm -v ci-sim:/w -v "$PWD":/src:ro alpine sh -c 'cp -a /src/. /w/' +docker run --rm -v ci-sim:/workspace/rmcguire/images \ + -v /var/run/docker.sock:/var/run/docker.sock \ + -w /workspace/rmcguire/images -e GITHUB_ACTIONS=true \ + --entrypoint bash gitea/runner-images:ubuntu-latest -c 'make check lint' +docker volume rm ci-sim +``` + +The runner is a local Docker container (`gitea-gitea_runner-1`), config at +`~/work/docker/gitea/data/act_runner/config/config.yaml`. Its cached +`gitea/runner-images:ubuntu-latest` can go stale; current action majors all +declare `using: node24`, so refresh it with +`docker pull gitea/runner-images:ubuntu-latest` if JS actions misbehave. + +When bumping action versions, check the real latest major rather than copying +from a sibling repo — several were a major behind. + +## Downstream consumers + +`node-agent` backs the shell MCP server in the **`50W/kube-manifests`** repo. When +its toolset changes, update all three of these or agents will not know what they +can run: + +- `toolhive/mcpserver-shell.yaml` — the `WHAT'S IN THIS CONTAINER` comment block. +- `toolhive/mcptoolconfig-shell.yaml` — `toolsOverride.run_process.description`, + the tool description models actually read. +- `ai/kagent/remotemcpserver-shell.yaml` — `spec.description`. + +That pod runs with no Kubernetes credentials, HTTPS-only egress, a read-only root +filesystem and all capabilities dropped, which is what `node-agent`'s smoke test +mirrors. Do not add a pull secret there — the registry is public. diff --git a/Makefile b/Makefile index f3c6b52..ecdfdd3 100644 --- a/Makefile +++ b/Makefile @@ -43,13 +43,10 @@ shellcheck: ## Lint every shell script lint: hadolint shellcheck ## Run all linters -build: ## Build IMAGE, or every image when IMAGE is unset - @for i in $(TARGETS); do \ - echo "==> building $$i"; \ - docker build -t $(REGISTRY)/$(NAMESPACE)/$$i:dev images/$$i || exit 1; \ - done +build: ## Build and smoke-test IMAGE, or every image when IMAGE is unset + @hack/build-images.sh $(TARGETS) -test: ## Smoke-test IMAGE, or every image when IMAGE is unset +test: ## Smoke-test an already-built IMAGE, or every image when IMAGE is unset @for i in $(TARGETS); do \ if [ -x images/$$i/test.sh ]; then \ images/$$i/test.sh "$(if $(REF),$(REF),$(REGISTRY)/$(NAMESPACE)/$$i:dev)" || exit 1; \ @@ -58,4 +55,4 @@ test: ## Smoke-test IMAGE, or every image when IMAGE is unset fi; \ done -all: check lint build test ## Everything CI does, locally +all: check lint build ## Everything CI does, locally (build smoke-tests too) diff --git a/README.md b/README.md index c605ccc..27460cd 100644 --- a/README.md +++ b/README.md @@ -23,12 +23,15 @@ The registry is public, so no pull secret is required. ## Tags -| Tag | Points at | -| --- | --- | -| `vX.Y.Z` / `vX.Y` / `vX` | a released build | -| `latest` | the most recent release | -| `edge` | the current tip of `main` | -| `main-` | one specific commit on `main` | +| Tag | Points at | Set by | +| --- | --- | --- | +| `latest` | the current tip of `main` | push to `main` | +| `main-` | one specific commit on `main` | push to `main` | +| `vX.Y.Z` / `vX.Y` / `vX` | an immutable released build | tag `/vX.Y.Z` | +| `pr-` | a pull request build — built, never published | pull request | + +`latest` has exactly one owner — `main` — so a release tag and a `main` build can +never race to define it. Pin to `vX.Y.Z` when you want a build that will not move. Releases are cut per image by pushing a tag named `/vX.Y.Z`, which builds and publishes that image alone: @@ -51,23 +54,25 @@ cp -r template images/my-image here (`FROM gitea.libretechconsulting.com/rmcguire/node-agent:latest`). 2. Fill in `images/my-image/README.md`. 3. Extend `images/my-image/test.sh` to assert whatever the image promises. -4. `make build test IMAGE=my-image` +4. `make build IMAGE=my-image` CI needs no changes — it discovers every directory under `images/` that contains -a `Dockerfile`. +a `Dockerfile`. See [AGENTS.md](AGENTS.md) for the full conventions. ## Local development ```sh -make help # list targets -make all # everything CI does -make build test IMAGE=node-agent # one image +make help # list targets +make all # everything CI does +make build IMAGE=node-agent # build and smoke-test one image ``` ## CI -Two Gitea workflows, both of which just call the `make` targets above: +Two Gitea workflows, both of which just call the `make` and `hack/` entry points +above, so a green `make all` locally means a green pipeline: - **`lint.yaml`** — repository layout, `hadolint`, `shellcheck`. -- **`build.yaml`** — builds only the images whose files changed, smoke-tests each - one before anything is published, then pushes on `main` or a release tag. +- **`build.yaml`** — works out which images changed, then builds, smoke-tests and + (on `main` or a release tag) publishes each one. Nothing is pushed until its + smoke test passes. diff --git a/hack/build-images.sh b/hack/build-images.sh new file mode 100755 index 0000000..c57d67d --- /dev/null +++ b/hack/build-images.sh @@ -0,0 +1,121 @@ +#!/usr/bin/env bash +# Build, smoke-test and — in CI, on a publishing ref — push images. +# +# Usage: build-images.sh ... +# +# How an image gets tagged is a property of the git ref, not of the image, so one +# run treats every image it builds the same way: +# +# tag /vX.Y.Z -> :vX.Y.Z :vX.Y :vX published release +# push to main -> :latest :main- latest tracks main +# pull request -> :pr- built, never pushed +# anywhere else -> :dev built, never pushed (local default) +# +# `latest` has exactly one owner (main), so a release tag and a main build can never +# race to define it. +# +# Uses plain `docker build` rather than buildx with a container driver: it loads +# straight into the local image store so the smoke test can run before anything is +# published, and it produces a plain manifest with no attestations, which Gitea's +# registry rejects. +set -euo pipefail +cd "$(dirname "$0")/.." + +REGISTRY=${REGISTRY:-gitea.libretechconsulting.com} +NAMESPACE=${NAMESPACE:-rmcguire} + +[ $# -gt 0 ] || { + echo "usage: ${0##*/} ..." >&2 + exit 2 +} + +sha=$(git rev-parse --short=7 HEAD) +revision=$(git rev-parse HEAD) +created=$(date -u +%Y-%m-%dT%H:%M:%SZ) + +push=false +if [ "${GITHUB_REF_TYPE:-}" = tag ]; then + mode="release" + version=${GITHUB_REF_NAME##*/} + [[ $version =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]] || { + echo "tag '${GITHUB_REF_NAME}' is not of the form /vX.Y.Z" >&2 + exit 1 + } + push=true +elif [ "${GITHUB_EVENT_NAME:-}" = pull_request ]; then + mode="pr" + version="pr-${PR_NUMBER:-0}" +elif [ "${GITHUB_REF_NAME:-}" = main ]; then + mode="main" + version="main-$sha" + push=true +else + mode="local" + version=dev +fi + +tags_for() { + local repo="$REGISTRY/$NAMESPACE/$1" n + case $mode in + release) + n=${version#v} + printf '%s\n' "$repo:$version" "$repo:v${n%.*}" "$repo:v${n%%.*}" + ;; + main) printf '%s\n' "$repo:latest" "$repo:$version" ;; + *) printf '%s\n' "$repo:$version" ;; + esac +} + +# Collapsible sections in the Actions log; plain headers when run locally. +group() { + if [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::group::$*" + else + echo "==> $*" + fi +} +endgroup() { + if [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::endgroup::" + fi +} + +echo "mode=$mode version=$version push=$push images=$*" + +for image in "$@"; do + [ -f "images/$image/Dockerfile" ] || { + echo "no such image: images/$image/Dockerfile does not exist" >&2 + exit 1 + } + + mapfile -t tags < <(tags_for "$image") + tag_args=() + for tag in "${tags[@]}"; do tag_args+=(--tag "$tag"); done + + group "build $image" + docker build "${tag_args[@]}" \ + --build-arg "VERSION=$version" \ + --build-arg "REVISION=$revision" \ + --build-arg "CREATED=$created" \ + "images/$image" + endgroup + + if [ -x "images/$image/test.sh" ]; then + group "test $image" + "images/$image/test.sh" "${tags[0]}" + endgroup + else + echo "note: $image has no test.sh, nothing to smoke-test" + fi + + if [ "$push" = true ]; then + group "push $image" + for tag in "${tags[@]}"; do + echo "pushing $tag" + docker push "$tag" + done + endgroup + fi +done + +echo "done: $* ($mode)" diff --git a/hack/docker-tags.sh b/hack/docker-tags.sh deleted file mode 100755 index efadc49..0000000 --- a/hack/docker-tags.sh +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env bash -# Work out how one image should be tagged, and whether it should be published. -# Prints GITHUB_OUTPUT lines: -# -# tags= every ref to build, one per line -# primary= the ref the smoke test runs against -# version= org.opencontainers.image.version -# created= org.opencontainers.image.created -# push=true|false whether these refs get published -# -# Tag scheme: -# tag /vX.Y.Z -> :vX.Y.Z :vX.Y :vX :latest published release -# push to main -> :edge :main- tip of main -# pull request -> :pr- built, never published -# anything else -> :dev- built, never published -set -euo pipefail - -image=${1:?usage: docker-tags.sh } -repo="${REGISTRY:?REGISTRY is not set}/${NAMESPACE:?NAMESPACE is not set}/${image}" - -sha=$(git rev-parse --short=7 HEAD) -created=$(date -u +%Y-%m-%dT%H:%M:%SZ) -push=false - -if [ "${GITHUB_REF_TYPE:-}" = tag ]; then - version=${GITHUB_REF_NAME##*/} - [[ $version =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]] || { - echo "tag '${GITHUB_REF_NAME}' is not of the form /vX.Y.Z" >&2 - exit 1 - } - n=${version#v} - tags=("$repo:$version" "$repo:v${n%.*}" "$repo:v${n%%.*}" "$repo:latest") - push=true -elif [ "${GITHUB_EVENT_NAME:-}" = pull_request ]; then - version="pr-${PR_NUMBER:-0}" - tags=("$repo:$version") -elif [ "${GITHUB_REF_NAME:-}" = main ]; then - version="edge-$sha" - tags=("$repo:edge" "$repo:main-$sha") - push=true -else - version="dev-$sha" - tags=("$repo:$version") -fi - -printf 'version %s, push %s, tags: %s\n' "$version" "$push" "${tags[*]}" >&2 - -echo "tags<<__TAGS__" -printf '%s\n' "${tags[@]}" -echo "__TAGS__" -echo "primary=${tags[0]}" -echo "version=$version" -echo "created=$created" -echo "push=$push" diff --git a/hack/select-images.sh b/hack/select-images.sh index caa90d2..5d2a5dc 100755 --- a/hack/select-images.sh +++ b/hack/select-images.sh @@ -1,8 +1,15 @@ #!/usr/bin/env bash # Decide which images CI should build. Prints GITHUB_OUTPUT lines: # -# images=["node-agent",...] JSON array consumed by the build job's matrix -# any=true|false whether there is anything to build at all +# images=node-agent other-image space-separated, fed straight to build-images.sh +# any=true|false whether there is anything to build at all +# +# Space-separated rather than a JSON matrix on purpose. Gitea resolves a job's matrix +# when it parses the workflow — before `needs` outputs exist — so a dynamic +# `fromJSON(needs...)` matrix yields one job whose name is the raw, uninterpolated +# expression. See jobparser.nameWithMatrix in the Gitea source, and +# https://github.com/go-gitea/gitea/issues/28207. Looping inside one job gives an +# honest job name and collapsible per-image log sections instead. # # Rules, first match wins: # 1. workflow_dispatch naming one image -> that image @@ -27,21 +34,13 @@ all_images() { done } -# Names on stdin -> ["a","b"]. Built by hand so the runner needs no jq. -as_json() { - local out='' name - while IFS= read -r name; do - [ -n "$name" ] || continue - out="${out:+$out,}\"$name\"" - done - printf '[%s]' "$out" -} - emit() { - local names=$1 reason=$2 any=false - [ -n "$names" ] && any=true - echo "selected (${reason}): ${names:-}" >&2 - printf 'images=%s\n' "$(printf '%s\n' "$names" | as_json)" + local names=$1 reason=$2 any=false flat + # Collapse the newline-separated list onto one line for the workflow output. + flat=$(printf '%s\n' "$names" | tr '\n' ' ' | sed -e 's/ */ /g' -e 's/^ //' -e 's/ $//') + [ -n "$flat" ] && any=true + echo "selected (${reason}): ${flat:-}" >&2 + printf 'images=%s\n' "$flat" printf 'any=%s\n' "$any" exit 0 }