Add a test suite for the entrypoint

[tigerblue77]

Twenty-six cases, fifty assertions, about a second, and nothing to install past bash and the coreutils a runner already has. No Dell hardware, no iDRAC, no Docker daemon, no network. Three files plus a workflow.

It is green against your code, not mine. Run against run.sh at d5be4b8, all twenty-six pass. Most of what it pins is what you wrote: the credential handling from 8edc5dc and the validation and failure checks from d5be4b8 — an empty secret file, a directory given as one, a variable and its _FILE both set, a username beginning with a dash from a file as well as from the environment, the trailing newline not becoming part of the credential.

Why it is worth carrying

Not the entrypoint's line count. It is that the two defects you fixed in d5be4b8 had read as correct since 2020, and the only way to see what they actually did was to run the script against mocked getent, adduser, chpasswd and init and look at where it ended up. That is a test harness. In the repository rather than in somebody's scratch directory, it costs three files and the next person does not have to rebuild it.

It asserts outcomes, and that was learned the hard way

An earlier draft failed twice against your implementation, and both were the tests' fault rather than your code's. One asserted that a dash-leading username reached getent after an end-of-options marker; one asserted the exact sentence Failed to set the password. Both were true of one way of solving the problem and false of yours, which solves it at least as well — refusing the name outright is simpler than passing it through, and the wording of a message is yours. A test that fails when somebody fixes the same thing differently is a test that punishes the fix, so both now assert the outcome: the container stops, nothing was misread as an option, and the message names which half it was in.

What it does not do

It does not test the Dockerfile. Whether that produces a working image is answered by building it, which build-pr.yml already does, and no unit test stands in for that usefully.

One compromise, named rather than buried

run.sh writes to absolute paths and ends by exec'ing init, so the harness copies it and rewrites those paths into a temporary directory. The suite therefore tests the script's logic, not the image's filesystem layout — that is the Dockerfile's business and the build's to prove. tests/README.md says so in those words.

Three things the runner enforces rather than trusts

Case names must be unique, because every case file is sourced into one shell and a duplicate would silently replace a definition and report a pass for a body that never ran. A case that asserts nothing fails, because passing by doing nothing is worse than failing. And assertions record and carry on rather than stopping the case, so one run reports every failure it found.

The workflow is separate from build-pr.yml on purpose: that one answers "does the image build", in minutes, with a daemon. This one answers "does run.sh still do what it is meant to", in a second, with nothing.

Authored by me with Assisted-by: Claude, per your answer on #31.

[ShaneMcC]

For the moment, I'm going to close this as not-planned.

Generally the entrypoint is stable and unchanged (and unlikely to change much going forward, this weekend as an exception due to the _FILE changes).

I think it's still small enough and easy enough for a person to read and understand and be confident it does what is expected (given a reasonably-expected set of inputs), without needing a custom test harness and suite that is way more lines of code than what we're testing. (I still think the fixes in d5be4b8053175705dbbe6ad284cfc4f6480a4646 aren't something anyone has hit in real use so far as it wouldn't fit a regular username pattern)

Overall I don't dislike the test harness, and if this was a large and more complicated entrypoint, or we were changing it more frequently then I can see the suite and harness adding value, but right now I don't think it adds enough value. But it's good to have it here in a PR in case I change my mind in future and need somewhere to start.

I do however think that the path-rewriting bit is a potential footgun waiting to happen. I think using a chroot, docker (or docker-in-docker) is the only safe way to do it - and I don't really like that in the case where it fails to rewrite for some reason, eg due to simple changes in the script (such as changing the file order, or flags), it then clobbers real files.

At the moment the entrypoint gets tested by running it. It's not as nice as a test suite, but it's worked for 6 years so far!

[tigerblue77]

Fair enough on all of it — closing as not-planned is the right read when the thing under test is eighty lines that have held for six years. Thanks for taking the time to say why rather than just clicking close.

The path-rewriting objection was the one worth having, and you were right that it is a footgun rather than a theoretical one. sed reports nothing for a pattern that matches nothing, and the expression covering the cleanup matched one exact literal, so reordering the two arguments of rm -Rf /tmp/* /var/tmp/* — or writing -rf — would have left that line untouched in a copy the harness then executes. Against the machine running the suite, not the sandbox.

I have pushed d941ec3 to the branch so whatever you come back to, if you ever do, is not sitting on that. The copy is now checked before anything runs it: every rewrite has to have landed, and no rm in it may name a path outside the sandbox — which also catches a destructive line the three expressions do not know about yet. A failed check takes the copy away and ends the run 0 passed, 26 FAILED. Verified against the current entrypoint (26/26), against a copy with those arguments reordered, and against one carrying an unrelated rm -rf /var/cache/omsa/*.

It does not make the harness a sandbox, and you are right that a chroot or a container is the only actually safe way to do this. That is written above the function, so the next person to look does not have to rediscover it.

---

Generated by Claude Code