Self-hosted APT repo on apt.learningequality.org#15054
Conversation
github-actions
Bot
added
DEV: dev-ops
Build Artifacts
Smoke test screenshot |
9cb3c25 to
1176de4
Compare
rtibbles
left a comment
rtibbles
left a comment
There was a problem hiding this comment.
CI passing. One blocking issue on the publish read-modify-write; everything else checks out (ACs each have a test, the committed keyring key matches the Pi trust key so URL-only source rewrites still verify).
- blocking —
platforms/apt-repo/publish.sh:38: transientgcloud storage lsfailure is indistinguishable from an empty prefix and can trigger the-c -dup-leg to wipe the live repo.
| # repo, so only a genuinely empty prefix may skip the down-leg. | ||
| case "$KOLIBRI_APT_BUCKET" in | ||
| gs://*) | ||
| if gcloud storage ls "$KOLIBRI_APT_BUCKET/**" >/dev/null 2>&1; then |
There was a problem hiding this comment.
blocking: This is the reviewer-guidance 1 scenario, and the answer is yes — it can wipe the repo. gcloud storage ls "$KOLIBRI_APT_BUCKET/**" >/dev/null 2>&1 exits non-zero for a genuinely empty prefix and for any transient network/throttle/auth failure, and both fall through to "empty prefix — first publish, starting fresh". The local tree then holds only the new deb, and the -c -d up-leg at line 86 deletes every prior package/pool/dists/db object. The comment at 32-35 states the required invariant ("only a genuinely empty prefix may skip the down-leg") but the code doesn't enforce it — exit status alone can't distinguish the two.
Fix: only skip the down-leg on a positively-confirmed-empty listing. Inspect the ls result rather than trusting exit status (e.g. count listed objects on success; treat any error that isn't an unambiguous "no objects" as fatal and abort before the destructive up-leg), or gate the -d flag on a sentinel-object probe.
There was a problem hiding this comment.
Fixed in 1920f5c. The gs:// branch now inspects the listing instead of trusting exit status:
- clean exit → objects exist → sync down
- non-zero exit whose stderr unambiguously says
matched no objects→ genuinely empty → skip the down-leg - any other non-zero exit (network/throttle/auth) → print the error and
exit 1before reaching the-c -dup-leg
So a transient gcloud storage ls failure now aborts the publish rather than falling through to "starting fresh" and wiping the live repo. Added tests/test_publish_empty_prefix_guard.sh, which drives all three outcomes through a fake gcloud on PATH and asserts the transient case aborts with no rsync leg run (repo untouched) while a confirmed-empty prefix still publishes and reaches the -d up-leg.
There was a problem hiding this comment.
Still fixed — the empty-prefix guard now inspects the listing and aborts on any non-empty-but-failed ls rather than trusting exit status, with tests/test_publish_empty_prefix_guard.sh driving both cases through a fake gcloud. The history rewrite folded that fix into the original publish.sh commit, so it now reads as one commit rather than a follow-up.
rtibbles
left a comment
rtibbles
left a comment
There was a problem hiding this comment.
A couple more tweaks to make this most useful.
Once updated, please also tidy up the commit history.
| VERSION=$(dpkg-parsechangelog -S Version) | ||
| echo "VERSION=${VERSION}" >> "$GITHUB_OUTPUT" | ||
| echo "Debian version: ${VERSION}" | ||
| build_deb_binary: |
There was a problem hiding this comment.
This is already built in the main release flow, right? I feel like just adding an input to pass in the already built debian file to this workflow and invoking it from the release flow like that is preferable to rebuilding the deb file again.
As a general rule of thumb, the platform release actions should be about releasing things that have already been built - not about building the artifacts.
There was a problem hiding this comment.
Done — build_deb_binary is gone. platform-debian-server-release.yml now takes a deb-artifact-name input, and release_kolibri.yml's server_release job passes ${{ needs.server_deb.outputs.deb-file-name }} (it already needs: server_deb, so the artifact is there). The release flow only releases now; nothing in it builds a deb.
One knock-on: the GCS auth/bucket secrets had to be declared and passed explicitly down the chain rather than relying on secrets: inherit in the nested call — a called workflow's secrets context only holds what it was itself given, so inherit from platform-debian-server-release.yml would only have forwarded the three Launchpad/GPG secrets it receives.
| @@ -0,0 +1,46 @@ | |||
| name: Test the self-hosted APT repo tooling | |||
There was a problem hiding this comment.
I am unsure about proliferating test yml files like this. I know this isn't the first, but I wonder if it would be better to have a single workflow for all tests. This can stay like this for now, until we do that follow up.
There was a problem hiding this comment.
Agreed — leaving it as its own file for now, per your call, and it can fold into a single test workflow whenever that consolidation happens. No change made here.
| @@ -0,0 +1,72 @@ | |||
| name: Publish to the self-hosted APT repo | |||
| on: | |||
| workflow_call: | |||
There was a problem hiding this comment.
We should add workflow_dispatch here as well, firstly for manual testing of the workflow, and also to populate from an existing release. We should use our usual pattern of accepting a URL to the asset in the workflow_dispatch and then downloading from that URL.
There was a problem hiding this comment.
Added, following the platform-pi-build_img.yml pattern: workflow_dispatch takes a required deb-url, workflow_call takes an optional deb-artifact-name or deb-url, and a validation step rejects both-or-neither. The URL path is a plain curl into dist/, so publish.sh dist/*.deb is unchanged.
platform-debian-server-release.yml gained a matching optional deb-url dispatch input and passes whichever it got through, so a manual server release can populate the repo from an existing release asset too (the apt-publish job is skipped when neither is given).
1920f5c to
066feb1
Compare
|
Rebased onto
Verified the tree is byte-identical to the pre-rewrite branch apart from the two intentional review changes (the release/publish workflow rewiring and the README paragraph describing it). |
The self-hosted APT repo replaces the per-installer github.io Pages repos with a single reprepro repository living on the release GCS bucket's apt/ path. publish.sh does the read-modify-write: sync the whole tree down, includedeb the new package(s), re-export the signed indices and pubkey.asc, then sync back up with -c -d so prior packages from earlier releases persist. The empty-prefix probe only skips the down-leg on a positively confirmed empty listing — a transient gcloud failure aborts rather than falling through to the destructive up-leg.
Ships the deb822 source file and the repo signing key so a fresh install needs no hand-written sources.list line. The key is a byte-identical copy of the trust key the Raspberry Pi image already installs, asserted by the test.
The reusable publish workflow only ever releases an already-built .deb: the release flow hands it the artifact built by release_kolibri.yml's server_deb job, and a workflow_dispatch takes a URL to an existing release asset so the repo can be populated or the workflow exercised by hand. The GCS auth and bucket secrets are declared and passed explicitly down the call chain, since a nested `secrets: inherit` only forwards what the calling reusable workflow was itself given.
kolibri-server's postinst sources the shared snippet and rewrites any learningequality.github.io source file to apt.learningequality.org, so the installed base self-migrates with no user action. It is a no-op — and safe under the postinst's set -e — when no such source is present.
Pushes the cutover .deb into the soon-to-be-archived gh-pages repos so clients still pointed at them pick up the migrating package. Run once, for the cutover release only; a repeat run is a logged no-op rather than a failure.
The E2E test stands up an old and a new reprepro repo behind local HTTP servers and drives a container through the whole cutover: install from the old source, postinst rewrites it, then upgrade served from the new host.
The tests SKIP when their tooling is missing so they stay usable on a bare dev box; APT_REPO_TESTS_STRICT=1 turns that SKIP into a failure in CI, so a missing runner package can't make the suite pass vacuously.
The new host serves one unified repo at its root, so the two per-installer github.io source files collapse into a single .sources entry.
066feb1 to
3cd80a4
Compare
2 participants
Summary
Move the self-hosted Debian/Ubuntu APT repo off non-redirectable
github.ioPages so the installed base self-migrates with no user action.repreprorepo atapt.learningequality.org, backed by the release GCS bucket'sapt/path.kolibri-archive-keyringpackage for fresh installs.kolibri-serverpostinstsnippet rewrites any existinggithub.iosource to the new host on upgrade.kolibri-serverrelease flow only publishes: it takes an already-built.deb(release-run artifact name, or a URL on manual dispatch) and hands it to the publish workflow..sourcesfile pointing at the new host — the unified repo serves both packages, so the separatekolibri-serversource file goes away.References
Implements #14965.
Ops prerequisites deferred to #13720:
Acceptance criteria
All met and verified by the apt-repo test suite:
apt/path —test_publish_roundtrip.sh.release_kolibri.ymlGCS auth —platform-apt-repo-publish.yml.test_publish_roundtrip.sh.pubkey.asc—test_publish_roundtrip.sh.test_publish_roundtrip.sh.kolibri-archive-keyring.debinstalls its source file and signing key —test_keyring_install.sh.apt updateworks after install with no manual source line —test_keyring_install.sh.postinstrewrites an existinggithub.iosource on Debian 13 —test_migrate_apt_source.shCase A,e2e_cutover.sh.postinstis a no-op when nogithub.iosource is present —test_migrate_apt_source.shCases B and D..debto the oldgh-pagesbranch(es) —test_seed_old_pages.sh.test_seed_old_pages.sh.README.rstand the script's banner.e2e_cutover.sh.e2e_cutover.sh.test_publish_empty_prefix_guard.sh.Reviewer guidance
platforms/apt-repo/publish.sh:40— the sync-down guard aborts instead of treating an ambiguousgcloud storage lsfailure as "empty prefix" (which would let thersync -c -dup-leg wipe the live repo); driven by a fakegcloudintest_publish_empty_prefix_guard.sh. Confirm thematched no objectsstring match is the right signal for a genuinely-empty prefix.platforms/apt-repo/conf/distributions.in:5—Architecturesomitsarmhf, soArchitecture: alldebs are invisible to 32-bit Pi clients; confirm the old repos don't serve them or addarmhfbefore cutover..github/workflows/platform-apt-repo-publish.yml— confirm the reusedGPG_SIGNING_KEYprivate half matches the committedplatforms/raspberry-pi/files/learningequality.asctrust key..github/workflows/platform-apt-repo-test.yml— new workflow that runs the apt-repo shell tests on each relevant PR. The tests SKIP when their tooling is missing so they stay usable on a bare dev box;APT_REPO_TESTS_STRICT=1turns that SKIP into a failure in CI.platforms/raspberry-pi/base.Pifile— droppinglearningequality-kolibri-server.sourcesreverses the Migrate kolibri-server into the Python monorepo #14964 review decision to keep it, on the grounds that the single new source now coverskolibri-servertoo. Confirm that reading before merge.AI usage
Used Claude Code to implement the reviewed plan. Verified with the repo's shell test suite, the containerized keyring and end-to-end cutover tests, and prek.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Deviations from the issue spec
kolibri-serversource file — not in the issue, but the Pi carries the oldgithub.ioURLs and would otherwise keep provisioning fresh images onto a repo we are cutting over.gcloud storage lsfailure reads as "empty prefix" and thersync -c -dup-leg wipes the live repo.🟡 Waiting for feedback
Last updated: 2026-07-24 22:18 UTC