Record the known framework limitations in a TODO
Mode flapping under umask 002, broad NOPASSWD sudo, runner self-updates, per-secret vault reads and fact caching, each with its cost and fix. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
be0b90b879
commit
d28494d21f
@@ -74,6 +74,7 @@ scripts/check.sh # the full static gate — what CI runs
|
||||
skills/adding-a-stack/ # step-by-step guide to adding a service
|
||||
docs/ # bootstrap, secrets, CI setup, connectivity, deploys
|
||||
AGENTS.md # the conventions and the role contract, in full
|
||||
TODO.md # known defects and deferred decisions
|
||||
```
|
||||
|
||||
## Getting started
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
# TODO
|
||||
|
||||
Known defects and deferred decisions in the framework, each with what it costs
|
||||
to leave alone and what fixing it would take. Nothing here blocks a deploy.
|
||||
Items are listed worst first.
|
||||
|
||||
## 1. The payload sync fights the role over `stack_dest`'s mode
|
||||
|
||||
`roles/compose_stack/tasks/sync.yml` creates `{{ stack_dest }}` with an explicit
|
||||
`mode: "0755"`, and then `ansible.posix.synchronize` runs with `-a`, which
|
||||
propagates the *source* directory's permissions onto it. A control node with
|
||||
`umask 002` produces a `0775` payload directory, so every run alternates: the
|
||||
sync sets `0775`, the `file` task sets it back to `0755`, and both report a
|
||||
change.
|
||||
|
||||
**Cost.** Cosmetic: the mode ends up correct either way. But it makes rule 8 of
|
||||
`AGENTS.md` false ("a second run must report zero changes") for anyone deploying
|
||||
from a `umask 002` workstation, which is most Debian and Ubuntu desktops. A rule
|
||||
that is known-false is worse than no rule.
|
||||
|
||||
**Fix.** Either drop the explicit `mode: "0755"` from *Create the stack
|
||||
directory* and let the sync own it, or stop the sync carrying permissions at all
|
||||
with `--no-perms --omit-dir-times` in `rsync_opts`. Pinning them outright with
|
||||
`--chmod=D755,F644` is the most explicit, but would strip the executable bit
|
||||
from payload scripts such as `webapp`'s `app/migrate.sh`, so it needs an
|
||||
exception for those.
|
||||
|
||||
## 2. `NOPASSWD:ALL` is broader than the role needs
|
||||
|
||||
`bootstrap.yml -e deploy_sudo=true` writes `deploy ALL=(ALL) NOPASSWD:ALL`. The
|
||||
role escalates for exactly one thing: `ansible.builtin.file` setting `owner` or
|
||||
`group` on a `stack_dirs` entry. Anyone who can dispatch a deploy — or who
|
||||
obtains the deploy key — can do anything on the host.
|
||||
|
||||
**Cost.** It widens what a compromised CI secret is worth.
|
||||
|
||||
**Fix.** Scope the sudoers file to the commands `ansible.builtin.file` actually
|
||||
invokes. Worth measuring against a real run before writing the rule, because a
|
||||
too-narrow sudoers file fails at deploy time rather than at bootstrap time.
|
||||
|
||||
## 3. A self-hosted runner cannot be updated from CI
|
||||
|
||||
Not a stack in this repository, but the first thing most adopters add. A deploy
|
||||
that recreates the runner kills the job doing the deploy, so a runner stack has
|
||||
to stay out of `site.yml` and the deploy form and refuse to run under CI (see
|
||||
the table in `skills/adding-a-stack/SKILL.md`). Updating it is then a
|
||||
workstation job that nothing prompts anyone to run.
|
||||
|
||||
**Approaches considered.**
|
||||
|
||||
- **A deferred self-recreate.** Sync and pull inside the job, then start
|
||||
`sleep 30; docker compose up -d` on the host with `async`/`poll: 0` and exit
|
||||
green. Smallest change, but green proves only that the pull succeeded, not
|
||||
that the new runner came up.
|
||||
- **A second, maintenance runner (preferred).** The runners update each other:
|
||||
a separate stack with its own label that nothing else targets, and a workflow
|
||||
on that label that deploys the main runner. Each playbook's refusal becomes
|
||||
conditional on an environment variable saying which runner it is on. Give the
|
||||
runners a `shutdown_timeout` and a matching `stop_grace_period`, so in-flight
|
||||
jobs finish before one is replaced.
|
||||
- **A systemd path unit on the host** that CI triggers by touching a file. Puts
|
||||
the update logic outside Ansible and needs root to set up.
|
||||
|
||||
## 4. One vault request per secret, even when they share a path
|
||||
|
||||
`roles/compose_stack/tasks/secrets.yml` issues one `uri` call per `stack_secrets`
|
||||
entry. `webapp`'s two variables read two fields of the same KV path and cost two
|
||||
round trips.
|
||||
|
||||
**Cost.** Negligible — the read is over loopback on the host.
|
||||
|
||||
**Probably won't fix.** The 1:1 mapping between entries and `results` is what
|
||||
keeps the assert loops short and readable, and that legibility is load-bearing
|
||||
in a file whose whole job is to fail safely without printing a value. Grouping
|
||||
by path would mean carrying an index. Revisit only if a stack appears with
|
||||
enough secrets to make the round trips visible.
|
||||
|
||||
## 5. Fact caching would persist secret-bearing variables
|
||||
|
||||
Nothing configures `fact_caching` today, so the registered results of the vault
|
||||
reads live only in memory for the duration of a play, and `secrets.yml` clears
|
||||
them once the `.env` is written.
|
||||
|
||||
**Cost.** None, currently.
|
||||
|
||||
**Watch for.** If fact caching is ever turned on, registered variables become
|
||||
candidates for being written to disk on the control node. `no_log` does not help
|
||||
with that. Check `secrets.yml` before enabling it.
|
||||
Reference in New Issue
Block a user