erofs: add a privileged CI job that mounts a layer - #2414
Conversation
Every EROFS check that exists today runs unprivileged and reads the
image back with go-erofs -- the same library that wrote it. A
misunderstanding shared by our writer and our reader is invisible to
that arrangement. Nothing anywhere has ever handed an apko-produced
EROFS layer to the kernel driver.
Add hack/test-erofs.sh and an EROFS workflow that runs it. The script
follows the recipe docs/erofs.md already publishes: build into an OCI
layout with --format=erofs, resolve the layer blob through index.json
and the manifest, then
- assert the layer mediaType is application/vnd.erofs and that the
blob's filename matches sha256 of its own bytes, since the layer is
stored uncompressed and digest == diffID. A compression step
creeping into this path would fail here.
- fsck.erofs -d3 and dump.erofs, the C implementation's opinion.
- mount -t erofs -o ro, the kernel's opinion.
- compare `apko erofs ls` against the mounted tree -- path, mode
string, uid/gid and symlink target for every entry.
That last comparison is the point of the job: it is the only thing that
can catch our reader and the kernel disagreeing. On examples/wolfi-base
it covers 395 entries, including a sticky directory, five char devices,
237 symlinks and several non-root uid/gid pairs. Verified to fail as
intended by feeding it a listing with the sticky bit dropped. setuid
and setgid are not present in that fixture; the writer's handling of
them is covered by the optional fsck.erofs checks in
pkg/build/erofs_test.go.
erofs is a module rather than built in, and the runner image does not
ship every module, so the workflow tries modprobe first and only
downloads linux-modules-extra when the module is genuinely absent. It
fails there rather than inside the script, where an absent driver would
read as a mount bug. The script itself installs nothing, so it stays
usable locally.
Landing this before the mount/umount restore is deliberate: it only
exercises what is already on main, and once it is here the restored
`apko erofs mount` has somewhere to be tested.
Refs chainguard-dev#2408
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparison walked the mountpoint with a plain find, which cannot descend into the mode-0700 directories the image intentionally contains -- root, usr/man, var/adm, var/mail and four more, each owned by a different uid. `apko erofs ls` reads the image directly and sees all of them, so find reported a smaller tree and the job failed on eight "Permission denied" lines rather than on anything about apko. Run find under sudo, like the mount either side of it. This passed locally because that run was root in a container, where the 0700 directories are readable. Re-verified as an unprivileged user with passwordless sudo, which is what the runner is: 395 entries agree. Refs chainguard-dev#2408 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job's egress allowlist named the Ubuntu archives on port 80 only, copying go-tests.yaml. That holds until apt falls back: when azure.archive.ubuntu.com:80 timed out, apt tried archive.ubuntu.com and security.ubuntu.com over https, harden-runner blocked both, and the step spent seven minutes failing to fetch a 111 kB package. Allow :443 alongside :80 for all three mirrors, so a fallback that would otherwise work is not blocked. The original timeout looks like runner-side flakiness -- an earlier run on this branch installed erofs-utils in seconds -- but there is no reason for the allowlist to be the thing that turns it into a failure. Refs chainguard-dev#2408 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Note for reviewers on the The plan is a follow-up PR moving that comparison into a privileged Go test,
This job keeps the part that is genuinely shell-shaped either way: the CLI Gating a root-only Go test is new machinery for this repo — nothing here is |
The privileged CI job added in chainguard-dev#2414 exercised mount(8) directly, so nothing anywhere ran the orchestration in pkg/erofsmount against a real kernel -- the unit tests drive it through a fake driver, which cannot catch a wrong mount(8) invocation or a layout that overlayfs rejects. Extend the script with four sections after the existing comparison: - Read-only image mount: assert the single-layer short-circuit (the layer straight at merged, no layers/ or upper/), check the state file's mode, dest, writable and mounts, and diff the mounted tree against the `apko erofs ls` listing already computed above. - `--rw`: assert the overlay layout (layers/00 plus merged) and the LIFO order recorded in the state file, write a file through the mount, and check umount left it in upper/ rather than deleting it. - A raw blob, which carries no state file, so umount takes the single-mountpoint fall-back. - A tampered state file naming a decoy tmpfs outside DEST: umount must fail *and* leave the decoy mounted. Asserting on the message alone would pass against code that ran the umount anyway. Cleanup no longer tracks one mountpoint. It reads /proc/self/mounts and unmounts everything under the workdir, deepest first, which covers the mounts apko makes and a mount left behind by a failure mid-test; the rm -rf now runs under sudo since apko created part of the tree as root. The two normalization pipelines become functions so the mounted-tree comparison can be reused, and the mount sections run apko through sudo via an absolute path.
The privileged job from chainguard-dev#2414 only ever built and mounted a single layer, because that was all apko could produce. It now builds the same config a second time with `layering`, and checks the split the only way a unit test cannot: against a real kernel. Added after the existing single-layer comparison: - Layer roles and mediaTypes across the manifest, and digest == the sha256 of each blob, so a compression step cannot creep in unnoticed. - fsck.erofs on every layer, and a kernel mount of every layer, stacked as read-only overlayfs lowerdirs in OCI order reversed. - `apko erofs ls` on the layered OCI directory, diffed against the overlay mount. Stack's merge is only reachable for apko's own output now that splitting is back, and this is the first thing to compare it with what the kernel assembles. - Every non-final layer must hold a regular file other than the partial installed db. That is exactly the shape the routing bug produced: ancestor directories and a db, no package files. - The merged tree, diffed against a tar build of the same config unpacked in layer order. The tar split is the reference for what splitting must preserve; a directory that reaches no layer shows up here and nowhere else. Both builds resolve from one `apko lock` output so a package published between them cannot make them differ. Cleanup tracks a list of mountpoints rather than a single one and comes down in reverse, so a failure part-way through the stack does not wedge the job. Refs chainguard-dev#2408
#2418) Section 2 of #2408: brings back the EROFS layer split, descoped in e5778f1 so #2249 could land as a single-layer writer plus `ls`, with the five bugs review found fixed on top. #2415 has since merged, and this is rebased on it. The code is still disjoint — that PR is `pkg/erofsmount` and the CLI, this one is `pkg/build` — they meet only in `hack/test-erofs.sh`. ### 1. Restore, unchanged `pkg/build/erofs_layers.go` and its test come back exactly as they were, and the `layering` + `format: erofs` rejection leaves `ImageConfiguration.Validate` and `buildLayers`. This commit deliberately restores the known-broken behavior; the four after it are the fix, and that delta is the point of the PR. ### 2. Package routing — the bug that inverted the whole thing The type assertion asked `info.Sys()` for a `Package()` method. tarfs, which is what a real build walks, returns a fresh `*tar.Header` from `Sys()` and hangs `Package()` off the FileInfo itself — the receiver `splitLayers` asserts on. So it never succeeded outside a fixture: every file landed in the top layer, each group layer held only ancestor directories plus a partial installed db (that case keys on the path string, so it still fired), and per-layer scanners read a db naming packages whose files were not there. Assert on `info`, and turn a `packageToWriter` miss into an error rather than a silent fall back to `top`. `TestSplitErofsLayers` could not catch this: it drove the split through `apkfs.NewMemFS()`, which implements `Package()` nowhere, so the fixture had zero package-owned files by construction and passed identically with and without routing. It is replaced by `erofs_layers_test.go`, which builds its fixture with `pkg/tarfs` and installs files through `WriteHeader` with an `*apk.Package`, the way an apk install does. Its routing assertions fail against the previous code. ### 3. Directory-only subtrees Directories were recorded during the walk and materialized only by `emitAncestors`, which runs for non-directory entries. A directory whose subtree holds no file was therefore never written anywhere: `/tmp`, `/run`, `/var/empty`, every empty dir and every mount point were absent from the merged view. Each directory is now emitted into its owning writer as it is walked, which is what both siblings already do. ### 4. Temp files and fds No error return out of `splitErofsLayers` closed or removed the per-layer temp files, and each go-erofs `Writer` also holds an unlinked spool fd that only `Close` releases. The CLI happened to be bounded by its `MkdirTemp`/`RemoveAll` wrapper; a library caller accumulated both until process exit. The per-writer state moves into an `erofsGroupWriter` type with `finish`/`discard`, and a deferred sweep discards every writer unless the function reaches its return. Still no upstream abort API, so `discard` writes an image it then removes. ### 5. Build time `newErofsGroupWriter` passed `WithBuildTime` only for a non-zero build time — the exact case `erofsBuildTime`'s comment describes, where go-erofs stamps `time.Now()` from `Close` and a library caller gets different layer digests every build. It now uses `erofsBuildTime`, same as `writeErofs`. apko's own CLI was never affected. ### 6. Drive it from `hack/test-erofs.sh` The privileged job from #2414, extended by #2415, only built and mounted a single layer, because that was all apko could produce. The script now builds the same config again with `layering` and adds, after the existing comparisons: - layer roles and mediaTypes across the manifest, and `digest` == the sha256 of each blob; - `fsck.erofs` on every layer, and a kernel mount of every layer stacked as read-only overlayfs lowerdirs; - `apko erofs ls` on the layered OCI directory diffed against that overlay mount — `Stack`'s merge only becomes reachable for apko's own output now that splitting is back, and this is the first thing to compare it with what the kernel assembles; - every non-final layer must hold a regular file other than the partial installed db, which is exactly the shape the routing bug produced; - the merged tree diffed against a **tar** build of the same config unpacked in layer order. The tar split is the reference for what splitting must preserve, and a directory that reaches no layer shows up here and nowhere else. Both builds resolve from one `apko lock` output so a package published between them cannot make them differ. The unpack uses `--numeric-owner`; without it GNU tar resolves each header's uname/gname against the runner's `/etc/passwd` and invents ids for `lp`, `mail`, `news` and `uucp`. The layered section runs last, after #2415's `apko erofs mount` / `apko erofs umount` checks, and reuses their helpers — `fail`, `assert_mounted`, `normalize_ls`, `tree_listing`. Their `cleanup` already unmounts everything under the workdir deepest-first, so the layer and overlay mounts need no separate bookkeeping. The one existing line this touches is the whitespace-in-paths guard, lifted into a `check_ls_whitespace` function so the layered listing gets it too. ### Not in this PR Whiteout support in `Stack` — the last item in section 2 — is really the layer-horizon fix from section 5, and is reader-side and independently testable. Left for its own PR. ### Verification `gofmt -l` clean, `golangci-lint run -n` reports 0 issues, `go build ./...` and `GOOS=darwin go build ./...` both succeed, and `SOURCE_DATE_EPOCH=0 go test ./...` passes. `shellcheck` is clean on the script. Each fix was checked to fail without its change: the four new tests are red against the restore commit and green after their own commit. Locally, a layered `wolfi-base` build produces five layers whose merged `apko erofs ls` listing is identical to the single-layer build's, and whose non-top layers are 0.7–7.4 MB rather than the directories-only skeletons the routing bug produced. Refs #2408 🤖 Generated with [Claude Code](https://claude.com/claude-code)
First of the section 1 items from #2408, and the one that makes the rest of
them testable.
Why
Every EROFS check that exists today runs unprivileged and reads the image back
with go-erofs — the same library that wrote it. That arrangement cannot see a
misunderstanding shared by our writer and our reader.
fsck.erofsappears inpkg/build/erofs_test.goas an optional second opinion, but nothing anywherehas ever handed an apko-produced EROFS layer to the kernel driver.
The pre-descope tree in #2249 didn't either — grepping
.github/onfeat/apko-erofs-fullfor "erofs" returns nothing. So this is new work ratherthan a restore.
What it does
hack/test-erofs.shfollows the recipedocs/erofs.mdalready publishes:build into an OCI layout with
--format=erofs, resolve the layer blob throughindex.jsonand the manifest, thenapplication/vnd.erofs, and that the blob'sfilename matches sha256 of its own bytes — the layer is stored
uncompressed, so
digest == diffID, and a compression step creeping intothis path fails here;
fsck.erofs -d3anddump.erofs— the C implementation's opinion;mount -t erofs -o ro— the kernel's opinion;apko erofs lsagainst the mounted tree: path, mode string,uid/gid and symlink target, for every entry.
Step 4 is the point of the job. On
examples/wolfi-baseit covers 395entries, including a sticky directory (
tmp), five char devices with rdev,237 symlinks and several non-root uid/gid pairs. setuid and setgid aren't
present in that fixture; the writer's handling of those stays covered by the
optional
fsck.erofschecks inpkg/build/erofs_test.go.The comparison is verified to have teeth: fed a listing with the sticky bit
rewritten to
drwxrwxrwx, the job fails withThat is the class of bug the go-erofs bump in #2412 was about —
Mkdirdropping setuid/setgid/sticky on write and
FileInfo.Mode()misreporting themon read.
Job details
erofsis a module rather than built in, and the runner image doesn't shipevery module, so the workflow tries
modprobefirst and only downloadslinux-modules-extra-$(uname -r)if the module is genuinely absent. It failsin that step rather than inside the script, where an absent driver would read
as a mount bug. There is no silent skip — an unavailable driver is a red job,
not a quiet pass.
The script installs nothing itself, so it stays usable locally. Package
installation lives in the workflow.
sudoonubuntu-latestis already established practice here:go-tests.yamlinstallserofs-utilsthe same way.Verification
Ran end-to-end in a privileged
ubuntu:24.04container against a real kernelmount, not just dry-read:
fsck.erofsreports "No errors found", 395 entriesagree, umount is clean. Plus the negative test above.
shellcheck -e SC2129clean (matching the actionlint config),actionlintclean,
zizmor --persona pedantic --config .github/zizmor.ymlreports nofindings.
Not in this PR
The rest of section 1 of #2408: restoring
apko erofs mount/umountwithstate-file containment, partial-unmount recovery, a read-only default and a
Drivertest seam. That work is deliberately second — this job only exerciseswhat is already on
main, so it can land green now, and once it is here therestored
mounthas somewhere to be tested. It will extend this job ratherthan add another.
Refs #2408
🤖 Generated with Claude Code