Build and publish signed multi-arch loreserver images to GHCR - #180
Open
aleksanderllada wants to merge 10 commits into
Open
Build and publish signed multi-arch loreserver images to GHCR#180aleksanderllada wants to merge 10 commits into
aleksanderllada wants to merge 10 commits into
Conversation
## Summary Fills in the publish workflow: each architecture builds on its own native runner and is pushed to GHCR by digest, a merge job stitches the digests into one manifest list, and cosign signs that list keylessly through the job's OIDC token. No secrets are involved. Also drops the Graviton3+ target-cpu from the arm64 container build. The repo tunes aarch64-unknown-linux-gnu for Epic's own deployment, which emits instructions that fault on older arm64 parts and, because the build passes no --target, reaches the proc macros cargo runs on the build host too. A published image has to run anywhere armv8-a runs. ## Test Plan 1. actionlint passes on the workflow. 2. Dispatch from this branch and confirm the manifest list carries linux/amd64 and linux/arm64, and that cosign verify succeeds. 3. Pull the arm64 image on a non-Graviton arm64 host and confirm the server starts. Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
The first full run measured 145G on the runner root with 108G still free after both release builds, so deleting unrelated preinstalled toolchains buys nothing and only adds a step that can fail on its own. Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
## Summary The previous attempt set CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_RUSTFLAGS to drop the Graviton3+ target-cpu. That does not work: the variable is another source for the same config key, and cargo joins config arrays rather than replacing them, so -C target-cpu=neoverse-512tvb survived and the published arm64 image died with SIGILL on any older arm64 part. Use RUSTFLAGS instead, which is a mutually exclusive source that suppresses [build] and [target.*] rustflags outright, and repeat the flags the build actually needs. Also stop overriding the release strip setting. The workspace asks for line-tables-only debug info on purpose, and the override was discarding the line numbers that make a panic backtrace useful. Finally, run the freshly pushed image on its native runner. Nothing in the workflow executed the binary, which is exactly why a SIGILL shipped past a fully green run. ## Test Plan 1. actionlint passes. 2. The new smoke step fails the arm64 job when the Graviton flag is present and passes once it is gone. 3. Pull the arm64 image on Apple Silicon, which has no SVE, and confirm loreserver --version prints. Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
duncangrist
requested changes
Sep 1, 2026
duncangrist
left a comment
Contributor
There was a problem hiding this comment.
Can you ensure you run a /code-review in CC as my review brought up a few things.
Also, two CI jobs are failing currently.
## Summary Review of the publish workflow turned up six problems, three of them able to publish or sign the wrong thing. Tags and labels are now resolved once, in a new `meta` job that runs before anything is built. That addresses three of them together: - `latest` was gated only on the ref starting with refs/tags/v, so v1.0.0-rc.1 moved and signed `latest`, and a v-prefixed tag that is not semver at all left `latest` as the only tag the run published. The default `latest=auto` flavor applies it only to a tag that parses as a stable semver release. - A workflow_dispatch from a branch with a blank tag input resolved to no tags at all, and the job then died on `imagetools inspect ghcr.io/...:`, after both arch images had been pushed. A type=ref,event=branch entry gives that path a tag from the ref alone, as the input description already promised, and a gate in the meta job fails the run before any layer reaches GHCR if the list is still empty. - The build job ran its own metadata-action with no tags config, so the labels baked into the image carried the ref-derived version (v1.2.3, or a branch name) while the published tag was 1.2.3. And three smaller ones: - The tag input was interpolated straight into the tags list, where an embedded newline would have smuggled in a further directive. It is held to Docker's tag grammar before it gets there. - imagetools create never received the annotations, so the published index carried none. Building its argument list as an array also removes the word splitting the old command relied on, which would have broken on the spaces in image.description. - The cosign identity regexp was anchored on the repository alone, so it accepted a signature from any workflow here that can ask for an OIDC token. It is anchored on this workflow's path now, and defined once, since the summary hands the same expression to downstream users. Separately, the image was built with --release, which Cargo.toml labels a fast build suitable for local development: debug-assertions, and with them overflow-checks, stay on and there is no LTO. Acceptable for a local convenience image, not for the artifact the Helm chart points at. Build release-lto, "the actual release build", and strip the full DWARF it asks for back out, keeping the symbol table so backtraces still name frames. ## Test Plan 1. actionlint, with the repo config, reports no errors. 2. docker buildx build --check on the Dockerfile is clean. 3. The tag validator accepts edge, 1.2.3 and v1.0.0-rc.1, and rejects an embedded newline, a leading dash, a leading dot, a space, a slash and a 129-character input. 4. The imagetools argument construction, dry-run against a representative metadata-action JSON, keeps image.description as one argument. 5. Still to confirm on the first run: fat LTO links inside the memory and time a 4-core runner has, a prerelease tag does not move latest, and a branch dispatch publishes under the branch name. Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aleksanderllada
force-pushed
the
aarruda/publish-loreserver-image
branch
from
September 1, 2026 13:04
13fcea7 to
aaee068
Compare
## Summary
Lore is tuned for and deployed on Graviton3+, so a tuned arm64 image now
ships as well — but as an opt-in `-graviton` tag rather than as the default,
because `-C target-cpu=neoverse-512tvb` faults on any older arm64 part and
the default tag is what a community chart pulls.
X.Y.Z, X.Y, latest arm64 baseline armv8-a
X.Y.Z-graviton, latest-graviton arm64 tuned for Graviton3+
The Dockerfile now assembles RUSTFLAGS itself from an ARM64_TARGET_CPU build
arg, defaulting to empty. amd64 is baseline in both variants, so it is built
once and both manifest lists reference that single digest — the extra arm64
leg runs in parallel and leaves the critical path where it was.
OCI cannot express this: platform.variant covers v6/v7/v8, not
microarchitecture, so a client cannot be steered to the right arm64 build
automatically. Separate tags are the only honest mechanism.
## Also corrects the smoke test's comment
The comment claimed the step catches codegen aimed at the wrong CPU. It does
not. GitHub's arm64 runner is Neoverse-N2 and reports SVE, so it executes the
Graviton-tuned build without complaint — measured, not assumed. The step is
still worth keeping for a broken entrypoint or a missing shared library, and
the comment now says only that.
## Test Plan
1. actionlint passes.
2. Dispatch from a branch and confirm two manifest lists publish, each
carrying linux/amd64 and linux/arm64, each signed and verified.
3. Confirm both lists reference the same amd64 digest.
4. Run the default arm64 image on Apple Silicon, which has no SVE, and
confirm loreserver starts; confirm the -graviton arm64 image does not.
Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
release-lto sets debug = 2, and a fat-LTO link holds the whole dependency graph's DWARF in memory at once. On a 16 GB runner rustc is SIGKILLed part way through linking loreserver, so both the amd64 and the Graviton arm64 builds died. Build with -C debuginfo=0. strip --strip-debug discarded that DWARF a line later regardless, and tracing's file and line fields come from compile-time macros rather than debug info, so nothing observable is lost. Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
## Summary Four ways this workflow could have shipped the wrong thing without saying so. ### A failed graviton leg no longer withholds the default image `merge` declared `needs: [meta, build]`, which waits on the whole matrix *and* requires all of it to pass. So the opt-in graviton leg was a hard dependency of the portable default image — the one a community chart pulls — exactly inverting what this workflow is for. It also made the partial-manifest guard unreachable, since `merge` could never run with a leg missing. The smoke test makes this concrete rather than theoretical: it passes only because GitHub's arm64 pool is Neoverse-N2 and reports SVE. Rotate that pool to an N1 part and the graviton leg SIGILLs, taking the default release with it. `if: !cancelled() && needs.meta.result == 'success'` depends on the legs having finished rather than on all of them having passed. `meta` stays a hard requirement — a variant with no resolved tags has nothing to publish — and the leg-existence guard now decides which variant can actually ship. ### No release tag points at an unsigned digest `imagetools create` moved every tag, `latest` included, and `cosign sign` ran seven steps later. A Fulcio or OIDC failure in between left `latest` published and unverifiable, which undercuts the verifiable tag this workflow exists to produce. The index is now created under a `sha-<commit>` staging tag alone, signed, verified, and only then promoted onto the release tags. The staging tag never moves and nothing consumes it, so it is the one thing exposed to that window. Promotion depends on `imagetools create` copying a lone index source through byte for byte. That is proven first, by copying the index onto the staging tag it already occupies: a no-op while the digest is preserved, and if some later buildx stops preserving it, the tag that moves out from under the signature is the staging tag rather than `latest`. Each release tag is re-resolved afterwards as well. `primary-tag` and `graviton-primary-tag` go with it — the staging tag addresses the index now, so those outputs had no remaining reader. The gate step they came from stays, since an empty tag list still has to fail here. ### The -graviton image says so in its own labels The graviton leg was handed the default variant's labels, so its arm64 manifest reported `image.version=X.Y.Z` while the index annotation said `X.Y.Z-graviton`. `docker inspect` on a pulled image could not tell the two arm64 builds apart, which is the first thing anyone debugging a SIGILL on older arm64 hardware would reach for. amd64 is shared between both manifest lists and can only carry one set, which remains the default's. ### One place to retune Graviton `GRAVITON_TARGET_CPU` was never read; the matrix hard-coded the literal instead, so editing the env var changed nothing about what was built. The build arg now derives from that env var via the variant, and the matrix column is gone. `env` is unavailable in `strategy.matrix`, hence resolving it at the step. An untuned build is a perfectly successful one, so a graviton leg that resolved no target CPU would have published a baseline image under the `-graviton` tag with every step green. It now fails before building. ## Test Plan 1. actionlint passes. 2. Dispatch from a branch; confirm both manifest lists publish, each signed and verified, and that every release tag resolves to the signed digest. 3. Confirm the `sha-<commit>` tag exists on that same digest, and that the signature verifies against it. 4. Confirm `docker inspect` on the arm64 `-graviton` image reports a `-graviton` `org.opencontainers.image.version`, and the default does not. 5. Force the graviton build leg to fail; confirm the default variant still publishes and signs, and that the graviton merge fails on the leg guard rather than being skipped. 6. Confirm the build log for the graviton leg prints `tuning arm64 for neoverse-512tvb`, and that the baseline legs skip that step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
## Summary
Making the arm64 image portable by default contradicted three documents that
were not updated with it.
`docs/how-to/deploy-local-lore-server.md` is the primary Run with Docker
how-to, and it still told Apple Silicon and Windows readers to build and run
with `--platform linux/amd64` because the arm64 image targets Graviton3 SVE —
the restriction the baseline default removed, and the opposite of what
DOCKER.md now says. Following it meant a release Rust build under QEMU and an
emulated runtime to work around nothing. The path-chooser bullet carried the
same caveat.
`contrib/aws/README.md` deploys c8gd.8xlarge Graviton instances and used to
inherit `-C target-cpu=neoverse-512tvb` from `.cargo/config.toml`. The
Dockerfile assembles RUSTFLAGS itself now, so that command silently shipped an
untuned baseline binary to the one place tuning was meant for. It passes
`--build-arg ARM64_TARGET_CPU=neoverse-512tvb`.
## Also in DOCKER.md
The new Published images heading was inserted above an existing paragraph,
which left "generates self-signed TLS certificates for QUIC using
scripts/server/make-certs.sh" reading as a claim about the published GHCR
images. It was wrong twice over: that script has never existed in this
repository, and the Dockerfile generates no certificates. The server generates
an ephemeral self-signed one at startup, which is now stated under Running,
where someone whose client rejects the certificate will look for it. The
`docker.toml` bullet claimed to configure QUIC certificates for the same
reason and had the same problem; the file it describes sets two store paths.
The tag table was missing `X.Y-graviton`, which `meta-graviton` emits from the
same `{{major}}.{{minor}}` pattern the default row already documents, so
pinning a minor series on Graviton was undiscoverable. It also now describes
the `sha-<commit>` tag, which the two-phase signing change publishes alongside
each release and which would otherwise look like stray litter in the package
list.
## Test Plan
1. docs-lint.sh passes with vale, markdownlint and lychee installed.
2. Build the image on Apple Silicon following the how-to verbatim, with no
`--platform` flag, and confirm loreserver starts.
3. Build with the contrib/aws command and confirm the log line reports
`-C target-cpu=neoverse-512tvb`.
4. Confirm no document still references `scripts/server/make-certs.sh` as a
step in building the Docker image.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
Contributor
Author
|
@duncangrist done, and I've updated the workflow to build both portable |
## Summary
Comment-only. The explanations added while fixing this workflow had grown
longer than what they explain, so they are cut back to the reasoning that is
not recoverable from the code beside them: why `merge` waits on the legs
rather than on their success, why the index is staged before it is signed, why
the target CPU comes from the workflow env, and why the smoke test does not
prove what it looks like it proves.
101 comment lines down to 81. No expression, step, or shell line is touched:
diff <(git show HEAD:<file> | grep -vE '^\s*#') \
<(grep -vE '^\s*#' <file>)
is empty, so run 33642540774 still attests to this behaviour and the workflow
needs no re-run.
## Test Plan
1. YAML parses and actionlint passes.
2. The non-comment diff against the previous revision is empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
Contributor
|
CI sill failing. |
Rewriting the gate as a loop introduced `for json in "$JSON" ...`, a local
differing from the environment variable beside it only in case. That is what
SC2153 looks for, so actionlint failed the workflow:
SC2153:info:2:14: Possible misspelling: JSON may not be assigned.
Did you mean json?
Name the loop variable `tags_json`. Behaviour is unchanged.
Checked the rest of the file for the same pattern: no other script assigns a
local whose uppercase form matches an environment variable it reads, and
shellcheck now passes on all thirteen `run` blocks.
Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Aleksander Arruda <aleksander.arruda@xa.epicgames.com>
|
Imported as Lore CR-551. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fills in
publish-loreserver-image.yml, which landed earlier as a stub. Two variants publish, differing only in how arm64 is compiled:X.Y.Z,X.Y,latestarmv8-a— runs on any arm64 hostX.Y.Z-graviton,latest-gravitonThe portable build is the default so a community chart works everywhere, and the tuned build is opt-in for Graviton deployments.
linux/amd64is baseline in both, so it is built once and both manifest lists reference that single digest.Each variant builds on its own native runner (QEMU is far too slow for a release Rust build) and is pushed by digest; a merge job stitches the digests into manifest lists and signs each with keyless cosign. No secrets are involved:
GITHUB_TOKENauthenticates to GHCR and cosign signs against Fulcio via the job's OIDC token.Triggers on
v*tags, plus manual dispatch with an optional extra tag for proving the workflow from a branch.Why two tags rather than one
OCI cannot express this.
platform.variantcoversv6/v7/v8, not microarchitecture, and nothing in the pull path consults CPU features — Graviton3, Ampere Altra and Apple Silicon are all justlinux/arm64. A client cannot be steered to the right arm64 build automatically, so a separate tag is the only honest mechanism.Architecture selection itself is unaffected: one tag still serves both amd64 and arm64, which is why
-gravitonis also multi-arch and a mixed-architecture cluster can pin a single tag.Dockerfile changes worth reviewing
arm64 codegen.
.cargo/config.tomlpinsaarch64-unknown-linux-gnuto Graviton3+ with-C target-cpu=neoverse-512tvb. The Dockerfile now assemblesRUSTFLAGSitself from anARM64_TARGET_CPUbuild arg, empty by default. It has to beRUSTFLAGSand notCARGO_TARGET_<triple>_RUSTFLAGS: the latter is only another source for the same config key and cargo joins config arrays, so the Graviton flag survives and a supposedly-baseline build still faults.Debug info.
[profile.release-lto]setsdebug = 2, and a fat-LTO link holds the whole dependency graph's DWARF at once — rustc was SIGKILLed part way through linking on a 16 GB runner. Building with-C debuginfo=0fixes it and costs nothing observable, becausestrip --strip-debugdiscarded that DWARF a line later anyway andtracing's file/line fields come from compile-time macros. Side effect: the arm64 image drops from 100 MB to 51 MB.DOCKER.mddocuments both variants and the build arg.Test evidence
A full dispatch published both variants, each signed and verified in-run. The amd64 digest is shared, as intended:
Both arm64 images were then run on an Apple M3 Max, which has no SVE:
That is the intended behaviour of both variants, and it is the only test that actually distinguishes them. The default image also boots and serves — 10 QUIC listeners on
0.0.0.0:41337.A limitation to be aware of
The per-arch smoke step runs the freshly pushed image, which catches a broken entrypoint or a missing shared library. It does not catch codegen aimed at a CPU the eventual host lacks: GitHub's arm64 runner is Neoverse-N2 and reports SVE, so it executes the Graviton-tuned build without complaint. This was measured, not assumed, and it is why the Apple Silicon check above matters. A CI gate running the binary under
qemu-aarch64 -cpu cortex-a72would close it and is not yet implemented.Notes
permissions: {}, per-job least privilege with justification comments, and every action pinned to a full commit SHA.ci-testandci-test-gravitontags are leftovers from proving this and will be deleted.mainthrough the usual mirror path rather than by merging here.