diff --git a/CLAUDE.md b/CLAUDE.md index c4b24562..42209dd8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ Make sure your work includes a "maintenance budget" for code directly related to - Reducing duplicated or dead code - Adding new debug log lines and similar observability - Updating documentation pages or test suites -- Checking off completed issues in TODO.md +- Checking off completed items in the `## Done` list of the epic's `plan/` file Structure your commits for human review: diff --git a/plan/infra-review.md b/plan/infra-review.md index 6a92ac88..a3c5d88c 100644 --- a/plan/infra-review.md +++ b/plan/infra-review.md @@ -22,10 +22,10 @@ few hundred megabytes onto a disk that has been filling up, and nothing below turns on a check `validate` would have made. The security shape is good and is not what this epic is about. The IAM -scoping in particular is the tightest part of the directory, and the two -findings that block an apply are both *functional*: the deployment as -committed stands up an instance that cannot publish a DNS record and a zone -whose own name does not resolve. +scoping in particular is the tightest part of the directory. The findings +that held up an apply were both *functional* — an instance that could not +publish a DNS record, and a zone whose own name did not resolve — and both +are in `## Done` below. ## What is already right, so nobody re-derives it @@ -82,36 +82,6 @@ exist until the server has one (didbot-dns/src/lib.rs:136-148, didbot-tls/src/dns.rs:78), so it cannot be written ahead of time. route53.tf:1-19 says so and creates the zone only. -## The two findings that block an apply - -**Nothing sets the DNS record target, and the only value it can have is one -Route53 refuses.** `Provisioner::target` initialises to -`DEFAULT_RECORD_TARGET`, which is `RecordTarget::Loopback` -(provision.rs:48, provision.rs:1325). The override, -`Provisioner::with_record_target` (provision.rs:1450), has no callers -anywhere in the tree, and `didbot-dev`'s builder chain never calls it -(didbot-dev.rs:1312-1330) — there is no flag for it either. `Route53Dns` -cannot represent `Loopback`: `wire_record` returns `None` for it -(didbot-dns/src/route53.rs:874) and `publish` answers -`DnsError::UnsupportedTarget`, which -`publish_refuses_loopback_as_unsupported` (route53.rs:1422) pins. Step 7 of -provisioning propagates that error and unwinds the account -(provision.rs:3543-3547). So on this deployment every registration fails, and -`aws_eip.pds` (ec2.tf:91) — the address the whole stack exists to put agents -at — is passed to nothing. Closing this is a server-side change: a way to -tell the running process its own public address, and the Terraform to pass -it. - -**The zone apex has no address record and no owner.** route53.tf:1-19 says -"its own apex record" is the PDS's job at runtime. It is not: the only -`publish` call sites outside the DNS crate's own tests are the two agent -hostnames in provision.rs:3544 and provision.rs:3561, and the reconciler's. -Nothing publishes `` itself. That is the name the certificate -covers, the name `--port 443` serves and the name a stranger resolving a DID -arrives at, and after `terraform apply` plus the NS delegation it answers -nothing. Whichever side fixes the finding above should fix this one in the -same move, since both need the same fact — the instance's public address. - ## Findings that are an owner's call, not a fix **Both EBS volumes use the account's default `aws/ebs` key.** `encrypted = @@ -179,16 +149,14 @@ strips file capabilities, so the obvious hardening pass turns a working listener into an `EACCES` at startup. **`ExecStartPre=-/usr/bin/docker rm -f didbot-pds` -(user_data.sh.tftpl:93) must stay a pre-start and never become the stop +(user_data.sh.tftpl:108) must stay a pre-start and never become the stop path.** `docker rm -f` is a SIGKILL, which is the one thing the WAL's sync boundary is not covered against. The graceful path is `ExecStop=/usr/bin/docker -stop -t 20` (user_data.sh.tftpl:121) — and the twenty seconds are only real -once the server handles `SIGTERM`, which on `main` it does not: the only -handler installed is `tokio::signal::ctrl_c`, i.e. SIGINT -(didbot-serve/src/lib.rs:361). Until that lands, every production stop is a -default-disposition terminate with no shutdown path run at all. That work is -in flight elsewhere; the twenty-second grace here is sized for it and needs no -change. +stop -t 20` (user_data.sh.tftpl:198). `shutdown_signal` selects over `SIGINT` +and `SIGTERM` alike (didbot-serve/src/lib.rs:457-488) and then drains for +`SHUTDOWN_GRACE`, ten seconds — half the twenty here, so the drain finishes +before `docker stop` escalates to a SIGKILL. Raising `SHUTDOWN_GRACE` past +this number is what would break it. ## What an operator has to supply @@ -220,6 +188,19 @@ from inside the server or from CloudWatch. ## Done +- [x] **The DNS record target is set from the instance's Elastic IP.** + `--record-target` takes the address every hostname a run publishes + resolves to (didbot-pds.rs:405, :851), the builder applies it + (didbot-pds.rs:1447), and startup refuses a Route53 zone without one + (didbot-pds.rs:1010) so the failure is one message rather than one + unwound registration per call. `infra/pds` passes `aws_eip.pds` through + to it (ec2.tf:65, user_data.sh.tftpl:194). +- [x] **The zone apex has an address record and an owner.** + `aws_route53_record.apex` (route53.tf) is Terraform's, because Terraform + knows the address at apply time and the server never reaches `publish` + for `root_zone`. The owner half is + `Provisioner::ensure_server_account` (provision.rs:1752): the apex is an + account with a signing key, a repository and a registration record. - [x] **The unit's crash-loop brake actually engages.** `StartLimitIntervalSec` and `StartLimitBurst` sat in `[Service]`, where `systemd-analyze verify` reports the first as an unknown key it ignores.