Skip to content

erofs: add a privileged CI job that mounts a layer - #2414

Merged
smoser merged 3 commits into
chainguard-dev:mainfrom
smoser:erofs-ci-privileged-mount
Aug 19, 2026
Merged

smoser merged 3 commits into
chainguard-dev:mainfrom
smoser:erofs-ci-privileged-mount

Conversation

@smoser

@smoser smoser commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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.erofs appears in
pkg/build/erofs_test.go as an optional second opinion, but nothing anywhere
has ever handed an apko-produced EROFS layer to the kernel driver.

The pre-descope tree in #2249 didn't either — grepping .github/ on
feat/apko-erofs-full for "erofs" returns nothing. So this is new work rather
than a restore.

What it does

hack/test-erofs.sh 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

  1. assert the layer mediaType is application/vnd.erofs, and that the blob's
    filename matches sha256 of its own bytes — the layer is stored
    uncompressed, so digest == diffID, and a compression step creeping into
    this path fails here;
  2. fsck.erofs -d3 and dump.erofs — the C implementation's opinion;
  3. mount -t erofs -o ro — the kernel's opinion;
  4. compare apko erofs ls against 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-base it covers 395
entries
, 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.erofs checks in pkg/build/erofs_test.go.

The comparison is verified to have teeth: fed a listing with the sticky bit
rewritten to drwxrwxrwx, the job fails with

-drwxrwxrwt 0/0 tmp
+drwxrwxrwx 0/0 tmp
'apko erofs ls' disagrees with the kernel about the layer contents

That is the class of bug the go-erofs bump in #2412 was about — Mkdir
dropping setuid/setgid/sticky on write and FileInfo.Mode() misreporting them
on read.

Job details

erofs is a module rather than built in, and the runner image doesn't ship
every module, so the workflow tries modprobe first and only downloads
linux-modules-extra-$(uname -r) if the module is genuinely absent. It fails
in 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.

sudo on ubuntu-latest is already established practice here:
go-tests.yaml installs erofs-utils the same way.

Verification

Ran end-to-end in a privileged ubuntu:24.04 container against a real kernel
mount, not just dry-read: fsck.erofs reports "No errors found", 395 entries
agree, umount is clean. Plus the negative test above.

shellcheck -e SC2129 clean (matching the actionlint config), actionlint
clean, zizmor --persona pedantic --config .github/zizmor.yml reports no
findings.

Not in this PR

The rest of section 1 of #2408: restoring apko erofs mount/umount with
state-file containment, partial-unmount recovery, a read-only default and a
Driver test seam. That work is deliberately second — this job only exercises
what is already on main, so it can land green now, and once it is here the
restored mount has somewhere to be tested. It will extend this job rather
than add another.

Refs #2408

🤖 Generated with Claude Code

smoser and others added 3 commits August 19, 2026 16:53
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>
@smoser

smoser commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Note for reviewers on the apko erofs ls ↔ kernel comparison: splitting the
listing with awk '{print $1, $2, $6}' is why the script has to reject paths
containing whitespace up front. That's interim.

The plan is a follow-up PR moving that comparison into a privileged Go test,
which buys two things the shell version can't have:

  • a structured comparison (filepath.WalkDir + Lstat against the reader)
    instead of column arithmetic on formatted output, with per-entry diffs;
  • a purpose-built fixture. Coverage of special bits here is accidental —
    wolfi-base happens to give us a sticky /tmp and five char devices, and
    no setuid or setgid at all. A Go fixture can place setuid, setgid, sticky,
    hardlinks and devices deliberately and assert the kernel agrees on each,
    which is exactly the class of bug behind Fix setuid/setgid/sticky bits dropped on read and in Mkdir erofs/go-erofs#41 and the bump in
    Bump go-erofs import to current main. #2412.

This job keeps the part that is genuinely shell-shaped either way: the CLI
contract of apko build --format erofs out/ producing a mountable OCI layout,
following the recipe docs/erofs.md publishes. internal/cli has no tests
that drive cobra in-process, so a library-level Go test would not cover the
output dir-vs-tar branch at internal/cli/build.go:163.

Gating a root-only Go test is new machinery for this repo — nothing here is
root-gated today — so it wants go test -c plus sudo rather than
sudo go test, and the walk needs root too, not just the mount.

@smoser
smoser merged commit 302ff3c into chainguard-dev:main Aug 19, 2026
26 checks passed
smoser added a commit to smoser/apko that referenced this pull request Aug 19, 2026
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.
smoser added a commit to smoser/apko that referenced this pull request Sep 1, 2026
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
smoser added a commit that referenced this pull request Sep 2, 2026
#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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants