LPX-604: headless mode — skip ALB / listeners / Route53 for apps without a public face#29
Merged
stevethomas merged 14 commits intoMay 19, 2026
Conversation
…lic face Detect headless apps from manifest shape (no `domain`, no `apex`, no tenant declares either) and skip every step that touches a public- facing AWS resource: ALB, target group, HTTP/HTTPS listeners, listener rule, hosted zone, SSL cert, Route 53 record set. The ECS service still runs without a load balancer association. Use cases: queue worker pools, scheduler-only apps, internal services with no public surface. Surface: - `Manifest::isHeadless()` — true when neither `domain`/`apex` is set on the solo manifest, and (for multitenant) every tenant entry lacks both keys. - `Contracts/ExecutesWebStep` — new marker for ALB-side steps. Applied to SyncLoadBalancerStep, SyncTargetGroupStep, SyncHttpListenerStep, SyncHttpsListenerStep, SyncListenerRuleStep. - `ExecutesDomainStep` finally wired into the gate (was an unused marker, flagged in the PR #26 review). Applied to SyncSoloRecordSetStep alongside its existing ExecutesSoloStep. - `ChecksIfCommandsShouldBeRunning` skips both markers under headless. - `SyncEcsServiceStep` create/update payloads drop `loadBalancers` and `healthCheckGracePeriodSeconds` when headless. Extracted `needsUpdate()` / `createPayload()` / `updatePayload()` as pure helpers — also closes the test-coverage gap the PR #25 review flagged for this step. - `Manifest::tenants()` no longer TypeErrors on a headless tenant entry — apex normalisation gracefully resolves to null instead of bombing on `str_starts_with(null, ...)`. 16 new unit tests pin the headless detector matrix, the SyncEcsServiceStep drift predicate (incl. carry-over coverage gap from PR #25 review), the headless conditional payload shape, and the tenants() normaliser's headless tolerance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three step-skip conditions (solo-in-multitenant, multitenancy-in-solo, public-step-in-headless) sat as two separate blocks. Folding into one compound `if` matches the shape of the check (all are "skip when manifest mode disagrees with the marker") and makes the skip surface trivially extensible for the next marker we add. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two markers for "stuff that only matters with a public face" was YAGNI — no real step is web-but-not-domain or domain-but-not-web. Headless mode skips the lot uniformly. * ExecutesWebStep is now the sole public-facing marker. * ExecutesDomainStep contract deleted. * SyncHostedZoneStep, SyncSslCertificateStep, SyncSoloRecordSetStep re-tagged with ExecutesWebStep. * ChecksIfCommandsShouldBeRunning skip-clause halves in size. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous compound-skip block lumped three different concerns into one OR-chain. Tenancy-mode mismatch (solo step in mt app, mt step in solo app) and "is this a public-facing step in a headless app" are orthogonal — readability wins from separation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Before any AWS-touching command runs handle(), verify the manifest's aws.account-id matches sts:GetCallerIdentity for the resolved profile. Catches the foot-gun where YOLO_<ENV>_AWS_PROFILE points at the wrong account and the manifest's ID gets used silently for ARN interpolation. One STS call per command run (~50ms). Skips when the manifest doesn't declare an account (legacy / test fixtures). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ensureManifestAccountMatchesProfile()` returned bool + called error() itself, while every other manifest-validation failure in the codebase (Manifest::apex, Helpers::validate*, EnsureEnvIsConfiguredCorrectlyStep, SyncListenerRuleStep::nextAvailablePriority, the SG-rule integrity guards) throws IntegrityCheckException. Aligning with the prevailing pattern: void return, throw on either STS lookup failure or account mismatch, Symfony's exception renderer handles the styled output for free. DX wins: single responsibility (guard either succeeds or throws), the caller drops the `if (! ...) return 1` shim, stack trace available with -vvv for debugging. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…countId helper
Two pieces of feedback:
1. Throwing IntegrityCheckException from Command::execute() broke the
prevailing pattern at that layer — every other early gate (missing
manifest, wrong environment, missing AWS_PROFILE) uses
`error() + return 1`. The IntegrityCheckException throws all live
deeper, in Steps and Manifest helpers. Reverted to bool return so
the four early gates look identical.
2. Was reaching past Aws::accountId() to call Manifest::get() directly.
Plus the STS side had no helper. Added Aws::profileAccountId()
(named for the .env profile rather than 'caller', since the value
is whatever YOLO_<ENV>_AWS_PROFILE resolves to) so both sides go
through Aws::*.
Presence check now Manifest::has('aws.account-id') rather than the
truthy-check on Aws::accountId() — the latter has a `: string` return
type that TypeErrors when the key is absent, and widening to `?string`
would cascade into every existing caller using sprintf/concat.
InitCommand carve-out at Command::execute() short-circuits before
registerAwsServices(), so new apps don't hit the guard before they've
written a manifest.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "skip when not declared" early-return was a silent foot-gun — same one the whole guard exists to prevent. A manifest missing aws.account-id would happily run against whatever YOLO_<ENV>_AWS_PROFILE resolves to, with no safety net. Now bails with a clear "must declare aws.account-id under environments.<env>" message. InitCommand short-circuits before this guard, so new apps run `yolo init` without tripping it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Split the manifest-vs-AWS reconciliation into two early gates in Command::execute(): * ensureManifestIntegrity() runs *before* registerAwsServices(). Bails when any of `name`, `aws.region`, `aws.account-id` is missing. `aws.region` is the load-bearing one — RegistersAws feeds it into every SDK client at construction time, so a null region collapses into a deep AWS SDK error. `name` drives every keyedResourceName() and silently produces invalid AWS resource names (trailing hyphen) if absent. account-id presence is now also in this group. * ensureAccountMatchesProfile() runs *after* registerAwsServices() and STS-verifies the manifest's account-id against YOLO_<ENV>_AWS_PROFILE. Renamed from ensureManifestAccountMatches- Profile (the manifest part is implied now that integrity runs upstream). Test split: CommandManifestIntegrityTest (no AWS mocks needed) and CommandAccountGuardTest (STS mock). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ensureManifestIntegrity() reads as a series of named gates short- circuited via && rather than a foreach loop with implicit semantics. Each extracted method tells you what it's checking — the loop was clever but obscured the per-key checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manifest::has() now falls back to top-level when an env-scoped lookup misses. The dedicated ensureNameDeclared() method + its bespoke error message were the only special-case for top-level keys; collapsing both down keeps the integrity check a flat list of `Manifest::has` lookups. Error message dropped the "under environments.<env>" qualifier — works uniformly for top-level `name` and env-scoped `aws.*` keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rate Widening Manifest::has() to fall back to top-level was solving one caller's problem by relaxing the contract of a widely-used helper — opens subtle ambiguity if a future env-scoped key ever shares a name with a top-level one. Reverted; ensureNameDeclared() handles the top-level `name` lookup directly via Manifest::current(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
routedHosts() comment block restated the ternary, and the tenants normaliser comment took three lines to caption a one-line null coalesce. Both code paths read for themselves. Kept comments elsewhere that explain non-obvious *why* — un-memoised SG cache (UsesEc2), AwsException translation (UsesEcs), narrow service drift detection (SyncEcsServiceStep), headless tenant raw read (Manifest::isHeadless). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* `// prefer the apex key when specified` — restated what `get(key, default)` already does. * `// apex resolves to null for headless tenants` — the `?? ($config['domain'] ?? null)` chain already shows it. Kept the one-line `// Read raw — tenants() normaliser TypeErrors on a headless tenant.` in isHeadless() — that's a real why-not-use-tenants() signal that disappears without it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevethomas
deleted the
steve/lpx-604-headless-mode-skip-alb-listeners-route53-for-apps-without-a
branch
stevethomas
added a commit
that referenced
this pull request
May 20, 2026
LPX-604 (#29) added headless-mode gating for solo DNS via the ExecutesWebStep interface, but missed the multitenancy mirror. SyncMultitenancyRecordSetStep dereferences \$this->config['apex'] and ['domain'] unconditionally, so a headless multitenant deploy would undefined-key-crash before reaching ECS. Symmetric fix: implement ExecutesWebStep on the tenant step too, so Manifest::isHeadless() drives the skip via the existing ChecksIfCommandsShouldBeRunning::shouldBeRunning() trait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevethomas
added a commit
that referenced
this pull request
May 20, 2026
* feat(LPX-606): yolo deploy always builds; --app-version names the build Restores build-in-deploy that v1's DeployCommand rewrite in #26 lost. Drops the alpha-era assumption that `--app-version=<tag>` meant "deploy this previously-built artefact" — that's rollback's job, not deploy's (future LPX-608). `--app-version` is now an input to the build: the tag to stamp on the fresh image. Absent → BuildCommand generates a timestamp. Present (e.g. a GitHub release name) → BuildCommand uses it verbatim. Build always runs. BuildCommand writes --app-version back onto the shared InputInterface, so deploy's downstream steps pick up the freshly-baked tag via `$this->option('app-version')`. Also drops the `Manifest::has('tasks.web')` guard. The interface mechanism (ExecutesWebStep + Manifest::isHeadless) already gates DNS steps for headless apps. The guard was catching a different scenario — "yolo.yml has no Fargate spec at all" — which is best left to surface as ServiceNotFoundException via the natural AWS error path. README's `yolo init && yolo build && yolo sync production && yolo deploy production` was nonsensical (`yolo build` errored on the missing env arg; explicit build was redundant). Updated to: yolo init && yolo sync production && yolo deploy production Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(LPX-606): SyncMultitenancyRecordSetStep skips on headless LPX-604 (#29) added headless-mode gating for solo DNS via the ExecutesWebStep interface, but missed the multitenancy mirror. SyncMultitenancyRecordSetStep dereferences \$this->config['apex'] and ['domain'] unconditionally, so a headless multitenant deploy would undefined-key-crash before reaching ECS. Symmetric fix: implement ExecutesWebStep on the tenant step too, so Manifest::isHeadless() drives the skip via the existing ChecksIfCommandsShouldBeRunning::shouldBeRunning() trait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey, I made a thing! 🥳
LPX-604
What problems are you solving?
Three additions on top of the original headless work — bundled because each lives in
Command::execute()or its immediate neighbours, and review boundaries get blurry otherwise.Headless mode
Apps with no
domain, noapex, and no tenant declaring either now deploy without an ALB / target group / listeners / hosted zone / SSL cert / Route 53 record set. The ECS service runs without a load balancer association.Manifest::isHeadless()— true when solo + no domain/apex, or multitenant + every tenant headless.Contracts/ExecutesWebStep— marker on every step that touches public-facing infrastructure.SyncLoadBalancerStep,SyncTargetGroupStep,SyncHttpListenerStep,SyncHttpsListenerStep,SyncListenerRuleStep,SyncHostedZoneStep,SyncSslCertificateStep,SyncSoloRecordSetStep.ChecksIfCommandsShouldBeRunningskipsExecutesWebStepinstances when headless.SyncEcsServiceStepdropsloadBalancersandhealthCheckGracePeriodSecondsfrom the create/update payloads when headless. Drift detection still coversdesiredCount.needsUpdate()/createPayload()/updatePayload()extracted as pure helpers.Manifest::tenants()no longer TypeErrors on a headless tenant entry.Manifest integrity
Four early gates in
Command::execute()that bail with a clear message:namedeclared (top-level — drives every keyed AWS resource name)aws.regiondeclared (load-bearing for every SDK client at construction)aws.account-iddeclaredaws.account-idmatchessts:GetCallerIdentityfor the resolvedYOLO_<ENV>_AWS_PROFILEAws::profileAccountId()added next toAws::accountId()for symmetry.Cherry-picked guard
The AWS account-mismatch guard (originally
steve/yolo-aws-account-mismatch-guard) is part of this PR — single STS call beforehandle(), fails fast when the manifest's expected account doesn't match the resolved profile.InitCommandshort-circuits beforeregisterAwsServices(), so new apps don't trip onyolo init.Is there anything the reviewer needs to know to deploy this?
Manifest::isHeadless() === true. Integrity checks were always implicit — the failures they now catch were previously deep AWS SDK errors.name,aws.region,aws.account-id. Coding Labs marketing site, Convict Records, LP-staging-eventual all already do; no migration needed. A hand-edited manifest missing any of these would now bail at startup instead of erroring midway through a sync.tasks.webmanifest shape. The container is namedwebin the task def even when it never binds an HTTP port. Propertasks.queue/tasks.schedulertask types are tracked separately (LPX-580+).🤖 Generated with Claude Code