Add Route53 custom domain for App Runner demo

[danielnaab]

Summary

• Adds Route53 hosted zone and custom domain configuration for 10x-forms.labs.flexion.us pointing to the demo App Runner service
• Adds optional customDomain parameter to SandboxStack so DNS is only provisioned for stacks that need it
• Configures App Runner custom domain association with automatic certificate validation records
• Outputs NS records needed for delegation from the labs.flexion.us parent zone

Resources created (demo stack only)

• Route53Zone for 10x-forms.labs.flexion.us
• ApprunnerCustomDomainAssociation linking domain to App Runner service
• Route53Record entries for certificate validation CNAMEs (x3)
• Route53Record CNAME pointing domain to App Runner service URL
• TerraformOutput with NS records for parent zone delegation

Post-apply

After cdktf deploy, the output will include the 4 NS records that need to be configured as an NS record set for 10x-forms in the labs.flexion.us hosted zone (managed in another repo).

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5b8b4b1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Code Review: Add Route53 custom domain for App Runner demo

Overview

This PR adds optional DNS provisioning to SandboxStack, wiring up a Route53 hosted zone and App Runner custom domain association for 10x-forms.labs.flexion.us. The approach is clean — the customDomain optional config parameter means non-demo stacks are unaffected. The PR summary also helpfully documents the required post-apply manual step.

---

Issues & Suggestions

Hard-coded loop count for certificate validation records

for (let i = 0; i < 3; i++) {

This assumes App Runner will always return exactly 3 certificate validation CNAME records. In practice, the count depends on the domain and the issuing CA. If the number ever differs, the deployment will either create dangling records or — worse — silently miss a required validation record, causing certificate issuance to fail without an obvious error.

Suggestion: Add a comment explaining why 3 is expected here (i.e., it's the observed behavior for App Runner with a single subdomain), or reference the AWS docs/source that guarantees this count. If the count is not guaranteed by the API contract, consider using a count-style dynamic approach or at least a named constant with a note:

// App Runner issues certificates via ACM and provides exactly 3 CNAME validation
// records for a custom subdomain association. See AWS docs: <link>
const CERT_VALIDATION_RECORD_COUNT = 3;
for (let i = 0; i < CERT_VALIDATION_RECORD_COUNT; i++) {

Raw Terraform escape-hatch strings bypass CDKTF type safety

name: `\${${customDomainAssociation.fqn}.certificate_validation_records[${i}].name}`,
type: `\${${customDomainAssociation.fqn}.certificate_validation_records[${i}].type}`,
records: [
  `\${${customDomainAssociation.fqn}.certificate_validation_records[${i}].value}`,
],

These escape-hatch strings (${...}) are a known CDKTF pattern for dynamic attribute access, but they're fragile — if the attribute path changes (e.g., provider upgrade renames certificate_validation_records), the TypeScript compiler won't catch the breakage.

Suggestion: Consider Fn.lookupNested or storing the association output explicitly if the provider supports it. If the escape-hatch is unavoidable, a short comment noting it's intentional (and why) would help future readers.

CNAME for subdomain apex

new Route53Record(this, `${id}-apprunner-alias`, {
  ...
  name: domainName,   // '10x-forms.labs.flexion.us'
  type: 'CNAME',

A CNAME at the zone apex is technically invalid per RFC 1912 and will be rejected by Route53. However, since 10x-forms.labs.flexion.us is a subdomain (not the zone apex of flexion.us), this is actually fine — the apex of this hosted zone is 10x-forms.labs.flexion.us and a CNAME there would conflict with the SOA/NS records.

Potential bug: Route53 will reject a CNAME at the zone apex. If name equals the zone name, this will fail. Verify that App Runner's serviceUrl is intended to be used as a CNAME target here and that Route53 accepts this configuration. If 10x-forms.labs.flexion.us is truly the zone apex, consider using an ALIAS record (A record pointing to an ALB or CloudFront) instead. App Runner does not natively support Route53 ALIAS records, so a CloudFront distribution in front of App Runner (or accepting the CNAME limitation) may be needed.

Note: Route53 does support CNAME at a zone apex via its "ALIAS" A/AAAA records feature, but a plain CNAME type will be rejected. You may need to test this specifically.

Missing prevent_destroy lifecycle on the hosted zone

The App Runner service has a createBeforeDestroy lifecycle rule, but the Route53Zone has none. If cdktf destroy is run (or a terraform plan replaces the zone), all DNS records will be deleted, causing downtime and requiring new NS records to be submitted to the parent zone.

Suggestion: Consider adding a lifecycle { prevent_destroy = true } to the Route53Zone to guard against accidental deletion:

const zone = new Route53Zone(this, `${id}-zone`, {
  name: domainName,
  tags: { ... },
  lifecycle: {
    preventDestroy: true,
  },
});

Minor: enableWwwSubdomain intent not documented

ApprunnerCustomDomainAssociation defaults enableWwwSubdomain to false. Since this is a subdomain (10x-forms.labs.flexion.us), a www. prefix is likely not desired, but being explicit makes the intent clear:

new ApprunnerCustomDomainAssociation(this, `${id}-custom-domain`, {
  domainName: domainName,
  serviceArn: appRunnerService.arn,
  enableWwwSubdomain: false,
});

---

What's Done Well

• The customDomain?: string optional parameter keeps non-demo stacks completely unaffected — good encapsulation.
• Tags are applied consistently to the hosted zone (Name + Environment).
• The TerraformOutput for NS records is a thoughtful touch — it surfaces the delegation info needed after apply without requiring manual inspection.
• The PR description clearly documents the required post-apply step (NS record delegation), which is critical operational context.
• The appRunnerService variable capture is clean and minimal — no refactoring beyond what's needed.

---

Summary

The CNAME-at-zone-apex risk is the most important thing to verify before merging, as it could cause a silent deployment failure. The hard-coded 3 validation records is the second concern — worth a comment at minimum. The preventDestroy lifecycle guard is a low-effort safety net worth adding. Everything else is minor.

[claude]

Code Review: Add Route53 custom domain for App Runner demo

Overview

This PR adds Route53 DNS and custom domain configuration for the demo App Runner service at 10x-forms.labs.flexion.us. The changes are well-scoped and minimal. The approach of making customDomain optional is good design — existing stacks are unaffected.

---

Issues / Bugs

1. Hardcoded certificate validation record count (sandbox-stack.ts, line ~448)

for (let i = 0; i < 3; i++) {

The AWS App Runner ApprunnerCustomDomainAssociation resource documents that certificate_validation_records is a set of records — the count is not guaranteed to be exactly 3. Depending on the domain and certificate, AWS may return fewer or more. Iterating with a fixed upper bound of 3 can result in Terraform errors on apply if the actual count differs.

A safer approach using CDKTF's TerraformIterator or Fn.toset:

// Use dynamic blocks via TerraformIterator if CDKTF supports it for this resource,
// or document that this assumption holds for single-hostname subdomains

At minimum, add a comment documenting the assumption and a link to the AWS docs confirming the count.

2. Raw Terraform attribute interpolation is fragile (sandbox-stack.ts, lines ~450–458)

name: `\${${customDomainAssociation.fqn}.certificate_validation_records[${i}].name}`,

This bypasses CDKTF's type-safe attribute resolution in favour of raw string interpolation. If the FQN or attribute path ever changes (e.g., after a provider upgrade), this will silently produce invalid Terraform config that only fails at cdktf deploy time, not at synthesis. This pattern is sometimes necessary in CDKTF when typed accessors aren't available, but it warrants a comment explaining why the typed accessor can't be used here.

---

Potential Issues

3. No dependsOn between validation records and the domain association

Terraform infers dependencies from interpolated references, so the template string approach should work — but because it's raw string interpolation rather than a proper CDKTF reference, the dependency may not be registered. Consider adding explicit dependsOn: [customDomainAssociation] on the validation Route53Record resources to be safe.

4. ApprunnerCustomDomainAssociation missing lifecycle config

The ApprunnerService has lifecycle: { createBeforeDestroy: true }, but ApprunnerCustomDomainAssociation has none. If the service is replaced, the association may not be cleanly destroyed first, potentially leaving a dangling domain association in AWS. Consider adding lifecycle: { createBeforeDestroy: false } (or explicit replaceTriggeredBy) to ensure correct teardown order.

5. CNAME on subdomain root — verify App Runner expectation

App Runner typically expects an ALIAS or CNAME pointing to its service URL. Using CNAME is fine here since 10x-forms.labs.flexion.us is not an apex domain. This is correct ✓ — just confirming it's intentional.

---

Nix / Dev Environment (shell.nix, .envrc)

6. Node version may diverge from .nvmrc

shell.nix hardcodes nodejs_22 and corepack_22. The project's .nvmrc specifies the canonical Node version. If .nvmrc ever changes, shell.nix won't track it. Consider adding a comment linking the two, or aligning by reading from .nvmrc if the Nix expression supports it.

7. shell.nix missing cdktf / infra toolchain?

The shell includes terraform but not cdktf (the npm CLI). Since cdktf is available via corepack/pnpm, this is probably fine — but worth confirming the shell is self-sufficient for the infra workflow.

---

Code Quality

• The optional customDomain? parameter is clean and non-breaking. ✓
• Tags (Name, Environment) are consistent with existing resources. ✓
• The TerraformOutput for NS records is a nice operational touch — makes post-apply steps clear. ✓
• The PR description's "Post-apply" section is excellent documentation. ✓

---

Summary

Severity	Issue
Medium	Hardcoded i < 3 loop for validation records
Medium	Raw Terraform string interpolation bypasses type safety
Low	Missing explicit dependsOn on validation records
Low	No lifecycle config on ApprunnerCustomDomainAssociation
Nit	Node version in shell.nix may drift from .nvmrc

The core logic is sound and the change is appropriately scoped. The main items to address before merging are the hardcoded validation record count and documenting (or replacing) the raw interpolation pattern.

[claude]

Code Review: Add Route53 custom domain for App Runner demo

Overview

This PR adds optional custom domain/DNS support to SandboxStack, wires it up for the demo environment (10x-forms.labs.flexion.us), and adds Nix development environment files. The design is clean and the opt-in approach via customDomain? is the right call.

---

Issues

🔴 CNAME at zone apex is a DNS violation

File: infra/cdktf/src/lib/aws/sandbox-stack.ts, Route53Record block

The Route53Record creates a CNAME for domainName (10x-forms.labs.flexion.us), which is the same name as the zone itself — the zone apex. Per RFC 1034, a CNAME record cannot coexist with other records at the zone apex (SOA/NS records are already there). Most DNS resolvers and Route53 itself will reject or silently misbehave with this configuration.

Fix: Route53 supports Alias (A/AAAA) records at the zone apex for AWS services. If App Runner's serviceUrl is a hostname (not an IP), you'll need to check whether App Runner supports Route53 ALIAS records directly. If not, the correct target is customDomainAssociation.dnsTarget used as a CNAME on a non-apex subdomain (e.g., www.10x-forms.labs.flexion.us), or you can omit this record entirely and rely on the App Runner custom domain association's own DNS target output for manual configuration.

// If an alias record is supported:
new Route53Record(this, `${id}-apprunner-alias`, {
  zoneId: zone.zoneId,
  name: domainName,
  type: 'A',
  alias: {
    name: customDomainAssociation.dnsTarget,
    zoneId: '<apprunner-hosted-zone-id>',
    evaluateTargetHealth: true,
  },
});

🟡 serviceUrl vs dnsTarget inconsistency

The CNAME record uses appRunnerService.serviceUrl as the target, but customDomainAssociation.dnsTarget is output separately. App Runner's custom domain association documentation specifies dnsTarget as the intended CNAME target for the custom domain. These values may differ; using serviceUrl could cause the certificate validation to succeed but traffic routing to fail, or vice versa. Prefer customDomainAssociation.dnsTarget here.

🟡 Certificate validation records require a two-apply process

The cert validation CNAMEs are output but not created automatically. The PR description notes this as a post-apply manual step. This is a known limitation of App Runner's validation flow, but it could lead to automation gaps in CI/CD. Consider automating the creation of the cert validation records by iterating over customDomainAssociation.certificateValidationRecords:

// Dynamically create cert validation records
Fn.tolist(customDomainAssociation.certificateValidationRecords).forEach(...)
// Or use a for_each in the underlying HCL via cdktf's TerraformIterator

If the two-apply process is intentional and acceptable, add a comment in the code (or a README note) so future operators know why it exists.

---

Suggestions

🟢 Add prevent_destroy lifecycle to the hosted zone

If customDomain is ever removed from the config, Terraform will attempt to delete the Route53 zone. Any out-of-band records (e.g., manually added entries) would cause the destroy to fail or those records to be lost. A lifecycle guard is cheap insurance:

const zone = new Route53Zone(this, `${id}-zone`, {
  name: domainName,
  tags: { ... },
  lifecycle: { preventDestroy: true },
});

🟢 .envrc and shell.nix — consider scope

These files are placed at the repo root, which means they apply to the entire monorepo (not just /infra/cdktf). For a Nix environment that is only relevant to infrastructure work, consider placing them in infra/cdktf/ instead, or document in the PR/CLAUDE.md that this is an infra-only shell. The current placement could surprise contributors working on the application layer who use direnv.

🟢 shell.nix is missing cdktf

The shell provides terraform but not cdktf. Operators will still need to install CDKTF via npm/pnpm after entering the shell. Noted as a minor gap — acceptable if the intent is to rely on pnpm for CDKTF, but worth documenting.

---

What's done well

• The customDomain?: string opt-in parameter is the right design — no risk of breaking existing stacks.
• The TerraformOutputs include descriptive description fields, which is helpful for operators running cdktf deploy.
• Resource naming is consistent with the existing ${id}-* convention.
• The allowUnfreePredicate in shell.nix is correctly scoped to terraform only, rather than broadly enabling unfree packages.

---

Summary

The main blocker is the CNAME-at-zone-apex issue — this will likely fail or produce incorrect DNS behavior in production. The serviceUrl vs dnsTarget discrepancy is a secondary concern that should be addressed alongside it. The cert validation two-step is workable but worth automating or at minimum documenting in the codebase. Everything else is low-risk polish.

[claude]

Code Review: Add Route53 custom domain for App Runner demo (latest iteration)

This review is against the current commit (5b8b4b1). Previous Claude reviews addressed concerns from earlier iterations — the CNAME-at-zone-apex bug, the hardcoded i < 3 loop, raw Terraform string interpolation, and the missing preventDestroy lifecycle guard are all resolved in this version. The remaining issues are minor.

---

Remaining Issues

.envrc and shell.nix are scoped to the wrong directory

Both files are added at the repo root, but they only contain tooling relevant to infra/cdktf. Since direnv auto-loads .envrc on cd, every contributor entering the repo root — regardless of whether they're doing infra work — will have Nix triggered. This could:

• Surprise contributors working on the application layer (Astro, Node, etc.) who don't use or want Nix.
• Cause confusion if a team member has direnv installed but not Nix.

Suggestion: Move both files to infra/cdktf/ where they belong:

infra/cdktf/.envrc
infra/cdktf/shell.nix

shell.nix is missing cdktf

The shell provides terraform but cdktf is not listed. It is available via pnpm after entering the shell, so this is workable — but an operator who enters the Nix shell expecting a complete infra environment will find they still need to run pnpm install separately. Worth a comment in the file or the PR description.

Node version in shell.nix may drift from .nvmrc

shell.nix pins nodejs_22 (major version only). The repo's .nvmrc specifies v22.19.0. As Nix channels roll forward, the minor/patch version in the shell will drift from what the rest of the team uses. This is a nit — major-version alignment is usually sufficient for a dev shell — but a comment linking to .nvmrc would help future maintainers keep them in sync.

---

Informational: Two-step apply process

The cert validation CNAMEs are output but not created as Route53Record resources — they require a manual second pass. The code comment and PR description both document this clearly, which is good. The limitation is inherent to App Runner's custom domain validation flow (the validation record values aren't known until after the association resource is created), so this is acceptable as-is. No action needed.

---

What's Done Well

• All blockers from prior review rounds are resolved: no CNAME at zone apex, no hardcoded loop count, no raw string interpolation.
• Zone has lifecycle: { preventDestroy: true } — guards against accidental destruction.
• The comment on the ApprunnerCustomDomainAssociation block explains why no separate CNAME/ALIAS record is created — valuable for future readers.
• TerraformOutput descriptions are clear and operator-friendly.
• enableWwwSubdomain: false is explicit rather than relying on the default.
• allowUnfreePredicate in shell.nix is correctly scoped to terraform only.

---

Summary

The infra logic itself is solid. The one thing worth fixing before merge is the placement of .envrc and shell.nix — they should live in infra/cdktf/ to avoid surprising contributors who are not working on infrastructure.