Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
297 lines (216 loc) · 26.8 KB

CONTRIBUTING.md

File metadata and controls

297 lines (216 loc) · 26.8 KB
Raw

Contributing

This document covers the development and maintenance workflows for working on Atomic Image Builder itself. End-user installation and usage live in docs/installing.md and docs/using.md, which README.md links to. Release procedure, what the automation does, and the traps worth knowing are in MAINTAINER.md.

New to the code? ARCHITECTURE.md maps atomic_image_builder.py — the repo layout, the five-stage runtime model, what each region of the file holds, and where to start for a given kind of change.

Working here with a coding agent? .github/copilot-instructions.md is the canonical brief for one: the checks, and the handful of things that are easy to get wrong in this repo specifically. It points at the documents below rather than restating them, and every other agent-facing file in the repo points at it.

Submitting a change

This is a normal public GitHub repo — there's no separate CLA or contributor setup:

  1. Fork the repo and create a branch off main.
  2. Make your change. There's no fixed branch-naming convention for contributions; maintainer_docs/MAINTAINER.md's release/vX.Y.Z pattern is the maintainer's own release process, not something contributors need to follow.
  3. Run the checks below locally — Tests, Coverage, and Linting — before opening a pull request. CI runs the same checks (unittest, the 90% coverage gate, ruff, shellcheck, actionlint, and hadolint) and has to pass before a PR can be reviewed.
  4. Open the PR against main and describe what changed and why. Keep it scoped to one thing; a PR that mixes an unrelated cleanup with the actual fix is harder to review and to revert if something goes wrong. docs/review-rubric.md is what review will look for, in order, and every item in it has a real example behind it. .github/pull_request_template.md prefills the description with those questions and a checklist of the local checks — it's a prompt, not a gate, so delete whatever doesn't apply to your change.

main is protected by the protect main ruleset: every change, the maintainer's included, reaches main as a pull request that passed test, and nothing can bypass that. The ruleset requires no approval, so review here is a practice rather than a gate. docs/branch-protection.md explains each rule and how to check what GitHub is enforcing.

Tests

Install the pinned tooling once — this covers both the Coverage and Linting sections below, and matches the exact versions ci.yml installs so a local run agrees with CI instead of disagreeing in either direction:

pip install coverage==7.16.0 ruff==0.16.5

One test also needs just — CI pins v1.57.0 by sha256, the same way it pins hadolint and actionlint. It runs the generated Justfile's spawn-vm recipe with command stubs, because that recipe forwards arguments to another recipe and neither just --fmt --check nor a dry run notices a wrong binding; only running it does (see #239). Without just installed that test skips, so a local run can pass over the one defect it exists to catch. Nothing is built and no VM starts: both the nested just and systemd-vmspawn are stubbed.

Ten more tests drive the real gum binary (Esc and Ctrl-J handling, table width, spinner stderr), and one runs hadolint against an unparseable Containerfile line. CI pins gum v0.17.0 and hadolint v2.14.0 by sha256 in ci.yml, nightly-compliance.yml and ai-fix.yml, so these run there; without the binaries they skip locally.

Run the test suite with unittest:

python3 -m unittest discover -s tests

Coverage

docs/quality.md is the index of every quality signal in this project and what each one is worth; start there if you want the overview rather than the mechanics.

There are five separate coverage measurements, and they are not interchangeable.

Unit coverage measures the source tree. .coveragerc holds the settings so a local run reports the same numbers CI does, and CI gates on it at 90%:

python3 -m coverage run -m unittest discover -s tests
python3 -m coverage report

The 90 is not written into ci.yml. It lives in .coverage-thresholds.json, which the workflow reads at gate time and which a test compares against every --fail-under= and every quoted percentage in these docs — so changing the gate is one edit, and a doc left saying the old number fails the suite rather than misleading someone quietly. That file also lists the other four measurements as advisory, which is the part most easily forgotten: they are uploaded, summarized, and deliberately ungated, and the reasons are below.

End-to-end coverage measures what a real run of the built container image executes. container/Containerfile.coverage layers coverage.py onto the built image and swaps the atomic-image-builder launcher for a shim that runs the packaged script under it, so container/entrypoint.sh and the packaged path stay in the measured run. .coveragerc.e2e maps the in-image path (/opt/atomic-image-builder) back onto the checkout. To reproduce a CI run locally:

podman build -t aib-local -f Containerfile .
podman build -t aib-local-cov -f container/Containerfile.coverage --build-arg BASE_IMAGE=aib-local .
tests/e2e/coverage_scenarios.sh
python3 -m coverage combine --rcfile=.coveragerc.e2e --keep e2e-coverage/data
python3 -m coverage report --rcfile=.coveragerc.e2e

tests/e2e/ holds the suites CI runs against the built image: smoke.sh asserts the behaviour of every non-interactive path, and coverage_scenarios.sh runs the same paths through the coverage shim to produce the data above. Both default to the aib-local / aib-local-cov tags those two builds produce, and take an image tag as their first argument, which is how CI points them at its own. tests/e2e/README.md covers what is reachable end to end and what adding a scenario involves.

End-to-end coverage is deliberately low and is not gated: the guided wizard needs a TTY, so the only end-to-end reachable paths are --version/-V, --help/-h, the unrecognised-argument exit, and the preflight failure. It exists so coverage gaps can be classified honestly rather than inferred from the unit run alone.

Shell-entrypoint coverage measures executable lines in contrib/aib and container/entrypoint.sh. The same behavioral suites used by CI run under Bashcov, which follows the child Bash processes they launch. .simplecov restricts the report to those two user-facing scripts so the test harnesses and temporary command stubs do not inflate the denominator. To reproduce it locally with the version used by CI:

gem install bashcov -v 4.0.0 --no-document
bashcov --root "$PWD" --command-name contrib -- tests/test_contrib_aib.sh
bashcov --root "$PWD" --command-name entrypoint -- tests/test_entrypoint.sh

The report is written to shell-coverage/ as HTML and JSON. Warnings that a temporary stub was deleted before reporting are expected: the harnesses deliberately remove their fake gh, podman, rpm-ostree, and atomic-image-builder commands after every scenario, and .simplecov excludes them from the report anyway. This tier is advisory and has no minimum. Bashcov reports line coverage, not branch coverage, so condition outcomes remain proven by the behavioral assertions rather than a branch percentage.

A multi-line array literal also reads as partly uncovered even when the statement it belongs to ran: Bashcov attributes a hit only to the line a statement starts on, so the continuation lines of a multi-line podman_args+=(...) block read as unhit however thoroughly the statement is asserted on. contrib/aib had one such block, the rpm-ostree status mount, and it is now written as one push per line so the reported number matches what the tests actually prove — see #118. Keep new pushes single-line for the same reason, and if a partial percentage shows up here anyway, check whether the missing lines are continuation lines of an already-asserted statement before reading them as an untested path.

Maintenance-audit coverage measures what a real run of maintenance_audit.py and homebrew_formula.py executes. Both are unit-tested only against mocked network calls — FakeResponse, patched urlopen and subprocess — and the weekly .github/workflows/maintenance-audit.yml job is the only place either one runs against the live GitHub API and a real tarball download. .coveragerc.maintenance-audit holds the settings. To reproduce a run locally (GH_TOKEN needs to be set for the audit's API calls):

python3 -m coverage run --rcfile=.coveragerc.maintenance-audit maintenance_audit.py --check-action-updates
python3 -m coverage run --rcfile=.coveragerc.maintenance-audit -a homebrew_formula.py --check
python3 -m coverage report --rcfile=.coveragerc.maintenance-audit

Like the end-to-end measurement it is advisory and not gated. Its value is the inverse of the unit number: a line that only a real network error reaches — a rate limit, an unreachable host, an unexpected API response shape — stays missing here even on a clean pass, which the unit suite's mocks cannot tell you on their own. A path that is 100% covered by mocks and never touched by a live run is exactly what this exists to make visible.

It follows that a low percentage here is the expected reading, not a defect, and is not a coverage gap to be closed. Driving this number up would mean making a live run against GitHub's real API actually hit a rate limit, a DNS failure and a malformed response — manufacturing the failures the tier exists to observe, which would destroy its only signal. Error-handling paths are proven instead in the unit tier, against a real socket rather than a patched urlopen: tests/_local_http_server.py serves a fixed response on loopback, and closed_port_url() returns a port nothing is listening on, so urllib.request's own connect, read and error-parsing code runs for real. See test_github_api_json_real_rate_limit_response_from_local_server and its neighbours.

So: read this artifact to learn which error paths a live run never touches, and check that each one is covered by a loopback test. Do not file the percentage itself as a finding — see #123, which did, and #124, which added the loopback tests that are the right answer to it.

Homebrew-release coverage measures what a real run of homebrew_formula.py --update executes when .github/workflows/update-homebrew-formula.yml points the formula at a published release. This is the narrowest and most easily misread of the five. Every test in tests/test_homebrew_formula.py that exercises update() patches fetch_sha256, so the unit suite reports this module at 100% without a single real tarball download ever having been measured; a release is the only place --update downloads one and rewrites the formula for real. It reuses .coveragerc.maintenance-audit, since that config already scopes exactly these maintainer scripts for the same "real run against live GitHub" purpose. To reproduce locally (--update rewrites Formula/, so do it on a throwaway branch):

python3 -m coverage run --rcfile=.coveragerc.maintenance-audit homebrew_formula.py --update <tag>
python3 -m coverage run --rcfile=.coveragerc.maintenance-audit -a homebrew_formula.py --check
python3 -m coverage report --rcfile=.coveragerc.maintenance-audit

Because that config also names maintenance_audit, which this job never runs, coverage prints a module-not-imported warning and then omits the absent module from the report. Both are expected.

Like the maintenance-audit measurement it is advisory and not gated, and for a sharper version of the same reason: it only runs when a release is published, so there is no push that could be blocked on it.

All five measurements are uploaded as workflow artifacts — coverage-unit, coverage-shell, and coverage-e2e from .github/workflows/ci.yml, coverage-maintenance-audit from the weekly audit, and coverage-homebrew-release from the release workflow. The Python artifacts contain coverage.xml, an HTML report, and raw coverage data files (one per e2e scenario); the shell artifact contains Bashcov's HTML, JSON, and result-set files. Download the two Python CI artifacts and merge them into a single report from any clone:

gh run download <run-id>
python3 -m coverage combine --rcfile=.coveragerc.e2e coverage-unit/data coverage-e2e/data
python3 -m coverage report --rcfile=.coveragerc.e2e

That works off the runner only because the raw data is path-portable, which took two things: relative_files = True in all three configs, so the unit data names files relative to the checkout rather than by the runner's workspace path, and the [paths] mapping in .coveragerc.e2e, which rewrites the in-image /opt/atomic-image-builder onto the checkout. Removing either one makes the artifacts combinable only in the workspace that produced them.

The maintenance-audit and homebrew-release data files live under data/ for the same reason, so either can be folded into that combine as well — these jobs all run on different schedules, so pass whichever pair of downloads you actually want to compare. Every config sets branch = True; coverage refuses outright to merge branch data with statement data, so a config that dropped it would silently make its artifact uncombinable with the others.

A commit that changes nothing the image is built from will not produce a coverage-e2e artifact, since the container-build job is path-scoped. Run the CI workflow manually (workflow_dispatch) to force one.

Because artifacts expire after 30 days, coverage_badge.py publishes the unit number somewhere durable: given a percentage, date, and SHA, it writes a shields.io endpoint-badge payload and appends a trend row (date, SHA, percentage) to a CSV. After the unit gate passes on a push to main, the publish-coverage job commits those two files to the dedicated coverage-data branch. Pull requests cannot publish, and the separate job is the only part of CI granted contents: write. The README badge reads the JSON endpoint and links to docs/coverage.md, which leads on to the CSV history. See coverage_badge.py --help and tests/test_coverage_badge.py for the artifact format.

Linting

Four linters, one per language CI ships. All four gate the test job.

ruff check                                    # Python -- pip install command is in Tests, above
shellcheck -x contrib/aib container/entrypoint.sh tests/test_contrib_aib.sh tests/test_entrypoint.sh tests/e2e/*.sh
actionlint                                    # GitHub Actions -- every workflow under .github/workflows
hadolint Containerfile container/Containerfile.coverage

Markdown tables are kept aligned for a fixed-width reader, since most of this repo's documentation is read in a terminal, a pager or a diff rather than a browser. A test fails when one drifts; python3 format_markdown_tables.py fixes every tracked file, and takes paths to fix just some.

ruff.toml holds the rule selection and the py310 target, so ruff check with no arguments — the same invocation CI uses — checks the same things locally that it does in CI. Note the undotted name: ruff reads .ruff.toml in preference to ruff.toml, so adding the dotted variant would take over silently and leave edits to this one doing nothing. A test asserts only one of the two exists.

ruff is pinned in the install command at the top of Tests, which keeps it in sync with the exact version ci.yml installs — an unpinned local ruff can enable or disable different rules release to release and disagree with CI in either direction. shellcheck comes from the runner image; there is no local pin to match, only whatever version your system package manager gives you. Its argument list is the whole of it — a shell file left off is a file nothing lints — so keep the command above byte-identical to the one ci.yml's Run shellcheck step runs. -x is load-bearing rather than decorative: tests/e2e/smoke.sh and tests/e2e/coverage_scenarios.sh each carry a # shellcheck source=tests/e2e/lib.sh directive, and shellcheck follows it only when told it may read external sources.

actionlint takes no file arguments on purpose: given none it lints every workflow under .github/workflows, so unlike the shellcheck command above there is no file list that can fall out of step with the directory. CI pins v1.7.12 by sha256, for the same ACTION_PINS reason hadolint is pinned. Workflows are the one thing here that cannot be exercised locally — a bad expression, a misspelt runner label or an unparseable path filter is otherwise discovered only by pushing. The workflows this tool generates into other people's repositories get the same check, from test_generated_workflows_pass_actionlint, which skips when actionlint is not installed. That test disables actionlint's shellcheck integration: every shellcheck finding in a generated project sits inside a run: block copied verbatim from template_snapshots/, which this repo must not reformat, so gating on it would mean gating on a fix only upstream can make. hadolint is a static binary; CI pins v2.14.0 by sha256 rather than using hadolint/hadolint-action, because maintenance_audit.py requires every uses: in this repo's workflows to appear in ACTION_PINS — the table shipped to generated repos — and a linter those repos never run does not belong in it. To match CI locally, install the same release from hadolint/hadolint.

One hadolint finding needed a real fix rather than a suppression: DL4006 on the cosign checksum, which was echo "<sha> <file>" | sha256sum -c -. Under /bin/sh a pipeline reports only its last command's status. Note that the usual remedy — a SHELL instruction setting pipefail — is a trap here: buildah honours SHELL in Docker format but ignores it (with a warning) in OCI format, which a plain local podman build defaults to, so the behaviour would depend on how the image was built. The checksum goes through a file instead, which removes the pipe. Keep it that way; do not reintroduce a pipe in a RUN expecting pipefail to catch it.

Maintainer Audit

This repo includes a small maintenance audit for the bundled template snapshot and workflow action pins.

Run the local-only checks without touching the network:

python3 maintenance_audit.py --skip-upstream

Run the full audit, including upstream HEAD drift checks against both bundled template snapshots (ublue-os/image-template and blue-build/template):

python3 maintenance_audit.py

Add the action-pin checks to get pin-refresh signals for the GitHub Actions written into generated repos:

python3 maintenance_audit.py --check-action-updates

That flag runs two complementary checks, because a SHA pin can go stale in two different ways:

  • Trailing tags compares each pin's label against the newest upstream semver tag, at the label's own precision. Catches an exact label like v4.4.0 when v4.6.0 exists.
  • Pin freshness resolves the tag or branch a pin names and checks it still points at the pinned SHA. Catches what the first check cannot: a pin labelled v7 stays "current" against v7.0.1 by precision rules while its frozen SHA is v7.0.0, and a pin labelled main is skipped by the first check entirely.

Both report as advisories: they print, but they do not fail the run. A pin that names a branch drifts on every upstream commit, so failing on it would leave the weekly audit permanently red. Genuine inconsistencies — a snapshot pin missing from the tables, a SHA that disagrees with them, upstream template drift — still fail.

This matters more than it looks. Generated repos ship .github/dependabot.yml, so a stale pin here becomes a Dependabot PR in someone's brand-new repo within a minute of creating it.

The same flag also asks sigstore/cosign for its latest release and advises when it is newer than COSIGN_COMPATIBILITY_FLOOR. That one version is written into four places — the Containerfile RPM URL and its sha256, cosign-release in .github/workflows/publish-image.yml, COSIGN_COMPATIBILITY_FLOOR in atomic_image_builder.py, and cosign-release in template_snapshots/containerfile/.github/workflows/build.yml — so a bump changes all four together and re-pins the sha256 (see #737).

The same flag also downloads the aib attached to the latest release and compares it to contrib/aib, after checking that the aib.sha256 beside it exists and matches — the recommended install verifies the pair, so a release the install cannot complete from fails too. The digest comparison fails the run when they differ: the recommended install fetches the wrapper from the release, so a wrapper change merged without a release leaves every new user running the old one while the docs describe the new one (see #356). Nothing outside the repo moved and only a release clears it, which is what makes it a failure rather than an advisory. maintainer_docs/MAINTAINER.md's Cutting a release says what to do.

The audit runs weekly and on demand through .github/workflows/maintenance-audit.yml, with --check-action-updates enabled and the output written to the run summary.

Releases and the Homebrew Formula

Formula/atomic-image-builder.rb is a tap-ready Homebrew formula living in this repo, so no separate homebrew-* repository is needed — brew tap user/name <url> accepts any git URL.

The formula records a release tarball and its sha256, and neither can be known until the tag exists. So the order is fixed:

The formula installs the command as aib-tool, which is TOOL_COMMAND in atomic_image_builder.py. That constant is deliberately separate from TOOL_SLUG: TOOL_SLUG feeds STATE_FILE (.atomic-image-builder.json), which is written into every managed repo and is how the tool recognises repos it created, so renaming it would orphan all of them. TOOL_COMMAND only has to match what the installers put on PATH, and is aib-tool rather than aib because contrib/aib already claims aib.

  1. Bump VERSION in atomic_image_builder.py. It is the single source for the tool's --version, the published image's version tag, and the release tag, so they should all agree. Merge it.
  2. Tag and publish the release on GitHub.

That is the whole manual process, apart from opening and merging one pull request. .github/workflows/update-homebrew-formula.yml takes it from there on release: published — it checks out main (the release event checks out the tag, and the formula lives on main), runs --update for the released tag, verifies the result with --check, and pushes the change (a new url and sha256) to a formula/<tag> branch. Its run summary and a reminder issue titled "Open the Homebrew formula PR for " both carry a link that opens the pull request with its title filled in. Click it, create the pull request, and merge it once test passes. Users get the new version on their next brew update && brew upgrade after that.

It never pushes to main, which the ruleset refuses anyway. It needs no secrets: the branch push and the issue use the workflow's own GITHUB_TOKEN. It does not open the pull request itself, because Actions is not permitted to create pull requests in this repository, and a pull request opened with GITHUB_TOKEN would start no CI, so test would never report. A person opening it is what makes test run. docs/branch-protection.md has the details.

To fix it up by hand — for a release published before that workflow existed, or after a failed run — either dispatch the workflow with the tag, or do it locally:

python3 homebrew_formula.py --update v0.9.0

Because the formula can only be updated after a release exists, a failure there leaves it pointing at the previous one, which installs the wrong version silently. So the weekly maintenance audit independently runs:

python3 homebrew_formula.py --check

which re-downloads the recorded URL and confirms the recorded sha256 still matches it, reporting as an advisory rather than a failure. It also catches the placeholder digest that ships in the formula before a release has ever been cut.

Container Image

The tool is also published as a container image (see docs/installing.md for end-user usage). Containerfile at the repo root defines the image, built from atomic_image_builder.py and the bundled template_snapshots/. .github/workflows/publish-image.yml builds and pushes it to ghcr.io/danathar/atomic-image-builder — tagged with the short commit SHA on every merge to main, when a GitHub release is published, or on manual dispatch. Only a build of main tags the image latest, and only a build of a release tag tags it with the tool's VERSION: a release cut from an older commit must not drag latest backwards, and a merge after a release must not re-point that release's version tag. Each ref has a concurrency group of its own: merges to main share one and settle on the newest commit, and a release build gets another, so a merge landing while it waits cannot cancel it. Separate groups cannot race for either tag, since each has one kind of writer. Publishing on merge is deliberate: it previously required a release or a manual dispatch, no releases were ever cut, and latest sat seven weeks behind main. Everything the tool bakes in — the script, its ACTION_PINS table, and the bundled template snapshots — reaches users only through this image, so a stale image silently strands every fix made to any of them. .github/workflows/ci.yml's container-build job builds (but does not push) the Containerfile on any push or PR that touches Containerfile, container/, atomic_image_builder.py, template_snapshots/, or tests/e2e/ (a change to the suites that prove the image must run them). It then runs tests/e2e/smoke.sh over every non-interactive path the packaged entrypoint has, and tests/e2e/coverage_scenarios.sh to collect end-to-end coverage from the same paths (see Coverage). contrib/aib is the host-side wrapper and is not baked into the image, so it is deliberately not a trigger. To build and test the image locally:

podman build -t aib-local -f Containerfile .
tests/e2e/smoke.sh