Conversation
|
@Apfelwurm There are test failures. Can you please fix them? |
858c2c4 to
7c20609
Compare
|
Sorry for that, did not expect tests for the Dockerfile in the application test suite :D Changed/Added the ct ones with AI, so they check everything we have in there in regards to the startup as of right now. |
|
Thanks for picking this up again! This is the first attempt that actually handles the migration, wich is what killed the earlier tries (see #214, #217 and #281). I built the image and played around with it a bit. Fresh volumes work, and so does upgrading from root-owned volumes. tini is PID 1 running as node, and the browser launches fine. Nice. A few things I ran into though:
Something like this should cover all three: #!/bin/sh
set -e
if [ "$(id -u)" = "0" ]; then
find /db /conf \! -user node -exec chown node:node {} + || echo "WARN: could not fix ownership of /db or /conf" >&2
export HOME=/home/node
exec setpriv --reuid=node --regid=node --init-groups /usr/bin/tini -g -- "$@"
fi
exec /usr/bin/tini -g -- "$@"With that, hardened setups can just run with I'd also keep the entrypoint for good instead of switching to Two small things:
Once thats in, I'll take it through |
|
Thanks @orangecoding for your valid thoughts and additions :) Let me know if there is anything else to do :) |
|
@Apfelwurm One thing left: the foreign="$(find /db /conf \! -writable 2>/dev/null | head -n 5)"The error text and the test need a tiny tweak for it then. Small nit while your at it: the After that I'll take it into |
|
done :) |
What does this PR do?
This PR is intended to migrate fredy to a rootless docker image, since it is pretty sad that this is still the default.
Since the last attempts seem to have died because of migration problems, i added a small docker entrypoint for now, which takes care of chowning everything nessecary to the node user, before then starting tini via setpriv on the node user.
I have tested running the master locally, created a job, stopped it and ran my image and the instance and the job seems to work just fine.
When this is rolled out for a while, the entrypoint script could be removed again and be replaced with a single
USER nodeline before the old entrypoint.Related issue
There seemed to be a few older ones: #217 #214 and #207 , but it seems they have been rolled back.
AI disclosure (required)
Fredy accepts AI assisted contributions, but they have to be declared. An
undeclared AI PR will be closed. Tick exactly one box:
ai:none- I wrote this myself. No AI generated code, text or commit messages.ai:assisted- AI helped with parts of it (autocomplete, refactoring, tests, docs). I reviewed and understand every line.ai:generated- AI produced most or all of this PR. I reviewed it, but it is largely machine written.If you ticked
ai:assistedorai:generated, the three answers below aremandatory. Keep them on the same line as the label.
Which AI: GPT-5
How much: Generating the setpriv command and generating the test additions
Why: to make sure passing of the CMD line works flawlessly with the right escaping and build the tests quicker
Checklist
yarn test:offlinepasses (oryarn testif the change touches a live provider)yarn lintandyarn format:checkpass