Repository navigation
feat(aws-startup-advisor): heroku-to-aws — Route 53 weighted cutover (dns.tf) and a Rollback phase in the migration guide - #392
Conversation
…(dns.tf) and a real Rollback phase in MIGRATION_GUIDE.md
Two things the guide promised and never delivered:
1. Clarify asks Q10 "DNS strategy" and defaults to route53, but Generate
emitted zero Route 53 resources — the cutover section was two lines
("CNAME → <ALB_DNS_NAME>", "set TTL low").
2. MIGRATION_GUIDE.md had no rollback content at all (grep: 0 hits), and
"Decommission Heroku" appeared twice, once as an orphaned heading.
dns.tf (generate-terraform.md Step 9.5), emitted when dns_strategy ==
route53 AND the inventory has a `domain` resource:
- ACM certificate for every custom domain, DNS-validated in the same
apply; the ALB HTTPS listener (Step 6) and EB web environments (Step 6.5,
new aws:elbv2:listener:443 settings — EB had no 443 listener before) use
it instead of var.acm_certificate_arn.
- For each non-apex hostname, a weighted CNAME pair (Heroku target / AWS
target). `cutover_weight` 0→10→50→100 is the cutover; 0 is the rollback.
Apex hostnames get a single ALIAS created only at weight 100, with the
Route 53 limitation (no CNAME/ALIAS to an external host) explained.
- Variables with placeholder-rejecting validation: hosted_zone_id,
heroku_dns_targets (from `heroku domains`; discovery does not capture the
DNS target today — noted), cutover_weight. Output dns_cutover shows live
weights. Zone is a data source, never a resource.
MIGRATION_GUIDE.md (generate-docs.md):
- Phase 5 Cutover: pre-cutover checklist (TTL lowered 24h earlier, DNS
targets known, fresh export on full_cutover); Route 53 path as three
applies with what to watch at each step and the apex rule; external-DNS
path with per-record instructions and the ALIAS/ANAME note; write-freeze
step placed correctly for full_cutover.
- NEW Phase 6 Rollback: 72-hour window, trigger table tied to the Phase 4
checks (health path, 5xx, p95, functional, row counts), per-phase
rollback table with data-at-risk and RTO, "DNS back to Heroku" as one
apply (or the provider edit), and "Writes after cutover" — full_cutover
must accept the gap or reverse-sync before `heroku maintenance:off`;
data-first loses nothing because the DB already lives on AWS.
- Phase 7 Decommission: the duplicate heading removed; decommission gated
on the rollback window.
- Template variables (custom_domains, aws_dns_target_label,
health_check_path), conditional-section rules, TOC.
generate.md postconditions: MIGRATION_GUIDE.md must have Cutover and
Rollback (in order, Decommission after) with a DNS-revert action and a
post-cutover-writes statement; dns.tf must exist with its three variables
and the ACM validation when the route53 condition holds, and must not
exist otherwise.
Prose only; no schema/shared/vendored changes. Merge-tree clean against
aws#383. markdownlint, tools/validate.py, and check-skill-refs (from aws#387)
pass.
… around fences in the external-DNS block (MD031)
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Reviewed 185dbcb5c6e1a762e1d174f820c8a92f33b18b24 with OCR delegation, a repository review, and one fresh independent general review. The complete correction list is grouped into the eight inline comments:
- [P1] Use the DNS preference field that Clarify actually writes.
- [P1] Make the advertised database verification command read-only.
- [P1] Base rollback actions on the actual database handoff and retained access.
- [P1] Make DNS authority and existing records part of the cutover lifecycle.
- [P1] Preserve domain, application, zone, and web-endpoint associations.
- [P2] Handle EKS explicitly before choosing the Beanstalk fallback.
- [P2] Accept the documented Cedar Private Space DNS target format.
- [P1] Persist the rollback weight before later Terraform applies.
Validation: manifest validation, plugin parity, plugin specification checks, changed-file Markdown lint, and git diff --check passed. Both reviewers executed the generated PostgreSQL script with local command stubs; the parent also checked the exact cutover variable with the Terraform CLI. Captured CI checks passed. No live cloud, DNS, or database changes were made. The findings concern behavior not covered by those passing structural checks.
…ccept Cedar DNS targets, persist cutover_weight in tfvars - Clarify Q10 writes preferences.data.dns_strategy (clarify-assemble.md); generate-terraform.md, generate-docs.md and the generate.md gate read that field instead of the non-existent global.dns_strategy, and the manual-DNS reason branches on 'no custom domain' first. - heroku_dns_targets validation accepts <haiku>.<haiku>.herokuspace.com (Cedar Private Space DNS Target) while still rejecting placeholders. - Every Phase 5 / Phase 6 weight change edits cutover_weight in terraform.tfvars and runs a plain apply; terraform.tfvars.example ships cutover_weight = 0 uncommented so a -var override can no longer be silently reverted by the next apply.
… EKS exclusion, authoritative-zone lifecycle - dns.tf routes each hostname to its own heroku_app's EB/Fargate web endpoint via a generator-emitted local.aws_targets map and a hosted_zone_ids map (one zone per hostname); the scalar local.aws_dns_target, the single data.aws_route53_zone and the has_fargate_web -> Beanstalk fallback are gone. dns_cutover output and the guide tables carry hostname -> app -> target -> zone. - generate-docs.md Step 0 adds has_fargate_web (web process only), has_eks, dns_hostnames[] and eligible_hostnames[]; EKS designs never get dns.tf and the manual branch names the Service EXTERNAL-IP target and the missing TLS; the EB label now uses the real eb_environment_url output. - New Phase 1 'DNS preparation (Route 53)' runs before the first apply: authority check (dig NS vs get-hosted-zone), atomic conversion of an existing simple record into the weighted 'heroku' member plus terraform import, zone adoption for externally served zones, and heroku_apex_ips. - The apex is a weighted A (Heroku IPs) / weighted ALIAS (AWS) pair that exists at every cutover_weight, so rollback never deletes the apex or sends the user to a non-authoritative provider.
…s.sh --verify, database handoff/failback per migration approach, interim path open through the rollback window
…nvironment_url outputs; the guide names each hostname's own output
… no longer assume dns.tf exists for the manual DNS branch
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖] REQUEST_CHANGES after a complete startups-hybrid-v1 follow-up at 450aab80169c96cea2dc217fc7315397e471c0a9 (base 1de6d7a92f4bfc114bc74eae3587619716328402). OCR delegation, the parent repository review, one fresh isolated independent reviewer, candidate verification, and the convergence gate are complete.
The consolidated remaining correction list is:
- Complete database failback: drain every writer, keep AWS writers stopped through later applies, and fully reset the restore target (original failback concern remains).
- Make the new read-only verifier fail on query errors.
- Preserve authoritative Heroku DNS before Phase 1 teardown.
- Initialize Terraform before the new DNS imports.
- Complete the EKS HTTPS handoff on port 443.
- Omit PostgreSQL prerequisites/failback for designs without Postgres.
The verifier, import ordering, and EKS TLS gaps were introduced by the fixes. The DNS teardown and no-Postgres paths were present in the earlier reviewed source; I missed them then. Details and bounded acceptance criteria are in the original threads and the new inline comment. Four fully fixed original conversations were verified and resolved; other conversations stay open.
Validation: manifest, plugin parity, specification, changed-file Markdown lint and diff checks passed; 96 existing report tests passed. Exact Bash stubs and local-only Terraform probes exercised the failures. Current captured CI passed. No live cloud/database operations or source changes were performed. Approval remains user-only.
A failed row-count query was treated as a match, failback could restart writers or leave stale Heroku tables, DNS import ran before init, and a web-only pack still required PostgreSQL. Co-authored-by: Cursor <cursoragent@cursor.com>
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖] Reviewed c8048ac4313e3c209f599bd0b3edb7fc9c718ede with OCR delegation, a fresh independent general reviewer, the parent repository review, and a complete convergence-v1 evidence gate.
Five correction groups remain:
- P1 — Reverse handoff: finish cross-route shutdown/persistence, use the correct EKS paths, prevent compute changes in DNS rollback plans, and restore Heroku formation/jobs after verified failback. See the original failback thread.
- P1 — Forward full cutover: stop Heroku background writers before the final export and keep them stopped while AWS is primary. I missed this existing behavior in the previous review.
- P2 — DNS adoption: the new early
cd terraform/makes the later import block enter a nonexistent nested directory. Keep the init/import/apply sequence in the intended directory. - P2 — EKS TLS check: validate the custom certificate hostname while connecting to the new load balancer before DNS moves.
- P2 — Route regeneration: explicitly reconcile a retained
dns.tfwhen switching to external DNS/EKS; preserve user edits and retain the read-only gate. I previously missed this incomplete transition.
Verified corrected causes: read-only query failure handling, Phase 1 DNS preservation, initialization ordering, EKS port 443, and no-Postgres failback rendering. New errors in the repaired import/TLS sequences are called out separately. Original threads receive the detailed evidence; only the fully addressed verifier and no-Postgres conversations are eligible for resolution.
Validation: repository manifest/spec/parity checks and changed-file Markdown lint pass; git diff --check passes; independent embedded-script, Terraform, directory and TLS probes retained. The initial npm authentication failure was preserved and the public-registry retry passed. Nine current GitHub checks report success. No source edits, commits, pushes or live cloud/database changes were made. Approval remains for the user; no review is dismissed.
External-DNS failback could not persist writers_quiesced, the DNS import ran from the wrong directory, and a Beanstalk-targeted plan could restart writers. The guide now stops every writer before the export and restores the recorded formation only after the database copy checks out. Co-authored-by: Cursor <cursoragent@cursor.com>
Problem
Two gaps between what
heroku-to-awsasks and what it hands back:route53, but Generate emits no Route 53 at all. The guide's cutover section was two lines:{{app_domain}} → CNAME → {{ALB_DNS_NAME}}and "set TTL low (60s)". A founder who answered Route 53 got nothing for it, and Elastic Beanstalk web environments had no 443 listener, so a custom domain pointed at them served plain HTTP.MIGRATION_GUIDE.mdhad no rollback content (grep -i rollbackacross the three generate files: 0 hits), and "Decommission Heroku" appeared twice, once as an orphaned heading with no body.Both are from the evaluation's "artifacts that run" list; both are things a startup hits at 2 a.m. on cutover night.
Change
dns.tf—generate-terraform.mdStep 9.5Emitted when
dns_strategy == "route53"and the inventory has at least onedomainresource (so a run with no custom domain never gets an unusable file).aws:elbv2:listener:443settings) use it;var.acm_certificate_arnis dropped whendns.tfexists.cutover_weight.0is all-Heroku (the default),100is all-AWS, and lowering it is the rollback. Threeterraform apply -var cutover_weight=Ncalls replace hand-edited records.www.first.hosted_zone_id,heroku_dns_targets(map hostname →…herokudns.com; discovery recordshostnameandsni_endpointbut not the DNS target today — called out, with theheroku domainscommand),cutover_weight(0–100 integer). Outputdns_cutovershows live weights per hostname. The zone is a data source, never a resource — the user may already serve other records from it.MIGRATION_GUIDE.md—generate-docs.mdfull_cutover), then exactly one of two DNS subsections: the Route 53 path as 10 % → 50 % → 100 % with what to watch at each step, or the external-provider path with per-record instructions and the ALIAS/ANAME note. Theheroku maintenance:onwrite-freeze is placed where it belongs forfull_cutover(after 100 %, not before).full_cutoverthe AWS database has rows Heroku never saw, so the user must accept the gap or reverse-sync beforeheroku maintenance:off; on data-first nothing is lost because the database already lives on AWS.generate.mdpostconditionsMIGRATION_GUIDE.mdmust have Cutover and Rollback sections in order with Decommission after, and the Rollback section must name a DNS-revert action and a post-cutover-writes statement.terraform/dns.tfmust exist with its three variables and the ACM validation when the Route 53 condition holds, and must not exist otherwise.Scope
Three files in
skills/heroku-to-aws/references/phases/generate/, prose only. No schema, shared, or vendored changes; no change to any Clarify question. Merge-tree clean against #383 (which touchesgenerate.mdpreconditions, not postconditions).Tested
tools/validate.py, full-repo markdownlint: pass.check-skill-refs.pyfrom ci(aws-startup-advisor): reference-path lint — every file path a skill mentions must exist #387 run against this tree:PASS — 1111 reference(s) checked, 0 missing.migration_approachvalues; the conditional-section rules ingenerate-docs.mdsay which blocks render in each.Not in this PR
heroku domains --jsonhas it) soheroku_dns_targetscould be pre-filled. One small follow-up touchingdiscover-live.mdandschema-discover-heroku.md..ebextensionsor nginx config; the ALB path already redirects).rollback.shwrapper. With Route 53 the rollback is oneterraform apply; a script would add a layer without adding safety.