Skip to content

ci: add the upgrade-smoke job (a release must install on top of the running one) - #4

Merged
ks98 merged 7 commits into
mainfrom
ci/upgrade-smoke
Sep 22, 2026
Merged

ci: add the upgrade-smoke job (a release must install on top of the running one)#4
ks98 merged 7 commits into
mainfrom
ci/upgrade-smoke

Conversation

@ks98

@ks98 ks98 commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

ci.yml and release.yml get a new job upgrade-smoke, taken from the hub
template (linuxmusterDEV/templates/{ci,release}.yml) and filled with this
package's real paths. In release.yml it is also a needs: of the release
job, so a tag cannot produce a release that fails the upgrade path.

ci.yml no longer runs on every branch push (push.branches: [main], tags
unchanged): pull requests already cover branches, so until now every commit ran
the whole pipeline twice.

debian/changelog gains the 7.3.2 entry describing the new guard.

Why

Kevin's core requirement: a new version must still install cleanly on servers
that already run an older one. Until now CI only ever proved that the package
installs on a clean Ubuntu 24.04 — the interesting failure (config, secrets,
certificates or the instance record lost or clobbered on upgrade) was never
exercised by any pipeline.

How it works

  1. gh release list picks the newest non-draft, non-prerelease tag — the
    version a school is actually running — and downloads its .deb
    (3 retries). The prev_tag dispatch input overrides it for a deliberate
    jump across an older line. With no release at all the job skips itself.
  2. It installs that release and then creates the state docs/install.md
    produces on a real server: the three secret files (join.authfile,
    ldap-bind.secret, radius.secret, owner lmnradius, mode 0600), the EAP
    CA (lmnradius ca init, pure crypto — no DC, no Docker), an instance record
    (lmnradius create --name ci …) and its EAP server certificate
    (lmnradius cert issue ci).
    create is || true: the container cannot come up without a DC, but
    Reconciler.apply() stores the record and renders instance.d/ before
    Docker is touched, which is exactly the state under test. The instance uses
    ldap://, not ldaps://, because from 7.3.1 on an LDAPS instance must pin
    a CA with --ldap-ca — a flag the older releases this job can upgrade from
    do not have.
  3. It upgrades to the freshly built package with
    apt-get install --reinstall --allow-downgrades. Without those flags the
    version-equal case right after a release would report "already the newest
    version" and test nothing; with them dpkg unpacks over the installed version
    and sets $2 in the postinst, so the upgrade branches really run.
  4. Assertions after the upgrade:
    • config.yml (the API token!), secrets/*, certs/** and
      instance.d/** byte-identical (sha256sum -c);
    • permissions and ownership unchanged (find -printf '%m %U:%G %p' diff):
      secrets/ and certs/ stay 0700, secrets and private keys 0600;
    • the instance record compared field by field, not by checksum — 7.3.1
      legitimately added ldap_ca to the model and the postinst's update-all
      rewrites image, so a blanket sha256 would be a false alarm. The list of
      admin-set keys lives in the job; whoever adds such a field extends it in
      the same PR;
    • service still enabled and active, control-plane API answering;
    • the venv's importlib.metadata version equals the dpkg version;
    • no leftovers under /opt: nothing dpkg does not own, only the new
      lmnradius-<version>.dist-info, only cpython-312 bytecode, nothing
      world-writable;
    • dpkg --audit silent, apt-get -f install -s clean, dpkg --verify
      silent — guarded by a check that md5sums actually exists, because up to
      7.3.0 the package shipped none and dpkg --verify was a no-op.

Proof that the upgrade path really ran (not a silent no-op)

Two independent assertions in the job itself:

  • grep -qE 'Unpacking linuxmuster-radius \([^)]+\) over \([^)]+\)' on the apt
    output;
  • the service MainPID must change. prerm deliberately leaves the service
    running on upgrade, and the only thing that restarts it is systemctl try-restart inside the postinst's if [ -n "${2:-}" ] branch — which dpkg
    enters only when it passes an old version.

Making sure the upgrade path is not skipped

update_all() skips every instance that is already on the target image, so if
the fixture instance inherited the package default, the postinst's update path
would quietly do nothing on a version-adjacent upgrade and all assertions below
would pass without it. The fixture therefore pins the data-plane image of the
previous package line — what an instance created by an earlier .deb really
runs — the job asserts that it differs from DEFAULT_IMAGE, and it asserts that
the journal shows update-all reaching the instance. Its outcome stays
unasserted on purpose: without a DC the container cannot become healthy, the
postinst rolls it back and ignores the result.

The released version's CLI is driven from /opt/linuxmuster-radius/venv/bin
rather than /usr/bin: v0.1.0..v0.1.3 shipped without that symlink, and an old
prev_tag must not fall over a missing one. That the new version puts it back
is asserted after the upgrade.

How tested

  • This PR's own CI run (7.3.1 → 7.3.2), all five jobs green:
    https://github.com/faircomp/linuxmuster-radius/actions/runs/35761821314 
    Unpacking linuxmuster-radius (7.3.2) over (7.3.1) ...
postinst upgrade branch ran: MainPID 2849 -> 3281
audit:update start name=ci from=...@sha256:2dd07e23... to=...@sha256:7385d3a6...
audit:update unhealthy name=ci -> rollback to ...@sha256:2dd07e23...
    All eleven admin-owned files OK under sha256sum -c, permissions diff
    empty, instance record diff empty.
  • A dispatch run with prev_tag=v0.1.6 — the jump across the pre-conventions
    line — also green:
    https://github.com/faircomp/linuxmuster-radius/actions/runs/35762111067 
    Unpacking linuxmuster-radius (7.3.2) over (0.1.6) ...
postinst upgrade branch ran: MainPID 2842 -> 3275
    There the printed raw record diff shows exactly the legitimate change a
    blanket checksum would have failed on: 
     ldap_bind_secret: ldap-bind.secret
+ldap_ca: null
 ldap_server: ldap://dc.linuxmuster.lan
    while the field-by-field comparison stays empty.
  • No lab VM and no DC involved — the whole job runs on ubuntu-24.04.

What this cannot prove

Nothing about WLAN authentication: no DC, no winbind trust, no eapol_test.
The data-plane container is intentionally never asserted on (it cannot become
healthy without a domain). What is guarded here is the package and data level:
that an upgrade does not lose or clobber configuration, secrets, certificates
or the instance record.

Kevin Stenzel added 7 commits September 22, 2026 19:17
Kevin Stenzel
ci(workflows): guard the upgrade path with an upgrade-smoke job
a39b442 
A release must still install cleanly on top of the version a school is
already running. The new job installs the newest published release (or the
tag given as the prev_tag dispatch input), creates the state an admin has on
a running server -- the three secret files, the EAP CA, an instance record
and its server certificate -- upgrades to the freshly built package and then
proves that nothing was lost: config.yml, the secrets, the certificates and
the rendered instance.d configs byte-identical, permissions and ownership
unchanged, every admin-set field of the instance record intact, the service
still enabled and active, the venv metadata equal to the dpkg version and no
leftovers of the old version under /opt.

The instance record is compared field by field instead of by checksum on
purpose: 7.3.1 legitimately added ldap_ca to the model and the postinst's
update-all rewrites image, so a blanket sha256 would be a false alarm.

apt installs the built package with --reinstall --allow-downgrades so the
version-equal case after a release still unpacks over the installed version
and really runs the postinst upgrade branch; the job asserts both the dpkg
"over (<old>)" line and the service restart that only that branch performs.

ci.yml no longer runs on every branch push: pull requests cover branches, so
each commit was checked twice.
Kevin Stenzel
ci(upgrade-smoke): exclude symlinks from the world-writable check
5dfbd24 
The venv ships four symlinks (bin/python, bin/python3, bin/python3.12,
lib64) and a symlink is always lrwxrwxrwx -- chmod cannot change that, so
the postinst's `chmod -R go-w` leaves them as they are and the check
counted four false positives. Only files and directories are checked now.
Kevin Stenzel
ci(upgrade-smoke): print what the software rewrote in the instance re…
72cf555 
…cord

The field-by-field comparison deliberately ignores keys the software owns,
which means a newly added field escapes it unnoticed. Print the raw diff of
the instance record next to it (never fatal): across a version jump it shows
exactly which keys the software rewrote -- today ldap_ca, added in 7.3.1,
and image, rewritten and rolled back by the postinst's update-all -- so the
next added field gets into the key list in the same PR.
Kevin Stenzel
ci(upgrade-smoke): really exercise the postinst update path
d2c9ad7 
Three hardenings, two of them from the squid rollout:

The fixture instance now pins the data-plane image of the previous package
line instead of letting the record inherit the default. update_all() skips
every instance that is already on the target image, so with the default
pinned the postinst's update path silently did nothing on a version-adjacent
upgrade and every assertion passed without it having run. The job asserts
that the fixture image differs from DEFAULT_IMAGE, and that the journal
shows update-all reaching the instance -- its outcome stays unasserted, the
container cannot become healthy without a DC.

The released version's CLI is driven from the venv rather than /usr/bin:
v0.1.0..v0.1.3 shipped without that symlink, and an old prev_tag must not
fall over a missing one. That the new version puts it back is now asserted
after the upgrade.
Kevin Stenzel
ci(upgrade-smoke): harden the manifest and the skip path, prove the u…
762b8e6 
…pgrade twice

Review findings on this branch plus the two template changes made after it.

The checksum manifest could run empty and still pass: without -r, xargs runs
sha256sum once with no arguments, it hashes stdin, and `sha256sum -c` then
reports "-: OK" with exit 0. The manifest now uses `xargs -r` and is pinned to
exactly the eleven files the fixture creates, so a renamed path or a fixture
that silently did not run fails instead of passing green.

Skipping is now only legitimate when the repo has no release at all: the tag
selection falls back to the newest prerelease, and it refuses to skip when
releases exist. upgrade-smoke gates the release job, so a silent skip would
have published a tag whose upgrade path was never tested.

The upgrade is proven twice: apt's "Unpacking ... over (...)" line and dpkg's
own log entry, which does not depend on apt's wording.

The update-all assertion reads the journal from the start of the upgrade
instead of the whole boot, so a future fixture step logging the same line
cannot satisfy it.
Kevin Stenzel
ci(upgrade-smoke): keep the checkout off sys.path in every python call
c55f74f 
This job checks the repository out, so the working directory is on sys.path
for every python it starts. A module could then be imported from the source
tree instead of from the installed package, and the version and DEFAULT_IMAGE
assertions would test the checkout rather than the upgrade. Every python in
the job now runs with -P.

The paths happen to be safe today -- lmnradius lives under controlplane/, not
at the repository root -- so this closes the hazard, not an observed failure.
Kevin Stenzel
ci(upgrade-smoke): pass the dispatch input through env, not by interp…
55a86e1 
…olation

A ${{ }} expression inside a run block is substituted before the shell sees
it, so the value of prev_tag became part of the script text. The step now
carries it as PREV_TAG under env: and the script reads tag="${PREV_TAG:-}".
Follows the hub template.
@ks98
ks98 merged commit 6777b10 into main Sep 22, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ks98