Repository navigation
Conversation
Each region ran a dedicated ALB for electric that served a few thousand requests a day, costing about $16-20/mo per region for its hourly charge alone. Ingest now composes its ALB and listener explicitly; electric attaches a host rule (priority 10) and its own SNI certificate to that listener, while ingest keeps the catch-all (priority 50000). Both services move to new target groups, since an ALB target group can only belong to one load balancer: the first deploy rolls ingest and gives electric its usual ~60s gap. Electric's ALB security group goes away; the shared ALB admits only Cloudflare, which is how electric-sync reaches it.
Maple review🟢 Confidence 4/5 · likely safe to merge Moves both regional ingress paths onto ingest's single ALB: ingest owns the load balancer and listener, electric attaches a host rule and its own SNI certificate. The wiring reads correctly and the doc drift is the only defect I could confirm; the alchemy assumptions are unverifiable here.
Findings🔵 Note · F1 · Runbook this file points to still documents electric's own ALB and groupmaintainability · The header comment still points at 🤖 Prompt to fix this finding with an AI agentWhat was checked
Observability coverage: 2 of 2 changes observable
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughIngest now creates and exposes an ALB listener and security-group ID. When an Electric domain is configured, Electric uses that listener for hostname-based routing and publishes a proxied CNAME for the hostname. ChangesShared ALB routing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant Stack as Maple stack
participant Ingest as Ingest ALB listener
participant Electric as Electric ECS service
participant DNS
Stack->>Ingest: Create listener and ingest target-group rule
Ingest-->>Stack: Return listener and ALB security-group ID
Stack->>Electric: Pass listener, security group, and hostname
Ingest->>Electric: Forward hostname-matched traffic
Stack->>DNS: Publish proxied CNAME for hostname
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The shared ingress configuration has no established merge-blocking issue. Proceed with the planned deployment and post-deployment health checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change narrows Electric’s network exposure and preserves its configured application secret and database authority. It also couples both services to one regional ingress boundary. Recovery from an interrupted load-balancer and DNS migration remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| yield* publishProxiedCname({ | ||
| id: "electric-public-cname", | ||
| hostname, | ||
| serviceUrl: service.url, | ||
| }) |
There was a problem hiding this comment.
🟡 Electric DNS outruns its TLS certificate
During ALB replacement, publishProxiedCname can repoint Electric before ListenerCertificate attaches its certificate. Both depend on listener, but neither the service nor DNS record depends on ListenerCertificate. Electric requests then fail TLS until attachment finishes.
Learn more
Alchemy orders resources through the Outputs referenced in their properties, not the order in which the factory yields them. The Electric certificate attachment and the ECS service both reference the shared listener, but the CNAME references only the service URL. On the initial ALB migration, the service can become ready and update DNS while the listener still offers only ingest's default certificate. Cloudflare connects to the Electric hostname over TLS and rejects the wrong origin certificate until the attachment completes.
Example: On a production redeploy, the new ALB serves the Electric target group at 12:00:00 and the Electric CNAME changes at 12:00:01. If the SNI certificate attaches at 12:00:15, requests to electric.maple.dev fail during those 14 seconds instead of continuing to reach the old ALB.
Recommended fix: Carry an Output from AWS.ELBv2.ListenerCertificate into the DNS record's dependency chain so the CNAME cannot reconcile before SNI attachment. Keep the certificate on the listener before switching the public name; validate with an initial ALB replacement plan.
Was this helpful? React with 👍 or 👎 to provide feedback.
What
Electric stops owning an ALB. In each region, ingest's ALB now serves both:
apps/ingest/alchemy.run.tscomposes the ALB and HTTPS listener itself (previously owned implicitly by the ECS service viapublic: true). Ingest's rule is the catch-all at priority 50000.apps/electric/alchemy.run.tsattaches aHost: electric[.eu].maple.devrule at priority 10 plus its own certificate viaELBv2.ListenerCertificate(SNI). Its ALB security group is removed; the task group now admits only the shared ALB's group.alchemy.run.tspasses the listener and ALB group through, and only creates electric whendomains.electricis set (prd only, which always has it).The listener uses
certificateArn, notcertificates: the declarative list would strip electric'sListenerCertificateon every ingest reconcile.Why
Over 3 days the US electric ALB served ~29k requests and the EU one ~2k, versus ~45M on the US ingest ALB. Each dedicated ALB costs $16-20/mo in hourly charges alone, so this saves roughly $36/mo across both regions (out of a ~$500/mo AWS bill).
Deploy notes
publishProxiedCnamerepointsingest.maple.dev/ingest.eu.maple.devat the new ALB in the same deploy; the old one is reaped afterwards. Clients only ever see the proxied hostname.forward(new logical ids) instead of reusing the owned-ALB ones. Expect one rolling deploy of ingest (same path as the Sep 21 EC2 rollout) and electric's usual ~60s singleton gap.0.0.0.0/0; the shared one admits only Cloudflare ranges. electric-sync useshttps://electric.maple.devthrough the proxy, so it is unaffected.DependencyViolationwhile the old task ENIs detach. Re-running the deploy finishes it.Verification
tsc -p tsconfig.alchemy.jsonpasses; oxfmt clean.curl https://electric.maple.dev/v1/health(and the EU host), a shape through electric-sync, and an OTLP POST to both ingest hosts.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit