From b31eb4a3ddaeae1044d98050b6a436f6d9c08db6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 04:28:31 +0000 Subject: [PATCH 1/2] feat(devtools): Aurora Serverless v2 scale-to-zero + NAT-free Lambda connectivity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements ADR-033. Adds two independent, opt-in, default-off capabilities on database.postgres so a Frigg-owned Aurora can idle at ~$0. Scale-to-zero (minCapacity: 0): - Validator accepts minCapacity 0 OR [0.5,128]; (0,0.5) rejected. - Scaling config uses `?? 0.5` / `?? 4` (the old `|| 0.5` turned a requested 0 back into 0.5) and emits SecondsUntilAutoPause (default 300, range 300-86400) when minCapacity === 0. - Warns when minCapacity:0 is pinned to an engine older than 13.15/14.12/15.7/16.3. Connectivity mode (connectivity: 'vpc' default | 'public'): - 'public' places Aurora in public subnets (PubliclyAccessible), opens 5432 to allowedCidrs (default ['0.0.0.0/0']) via CidrIp — one ingress rule per CIDR — instead of the Lambda security group, and enforces TLS (sslmode=require). - The VpcBuilder decouples the Lambda VPC attachment + NAT from Aurora's networking: it still builds the VPC/public subnets/DB subnet group Aurora needs but skips the NAT Gateway and VPC endpoints and clears vpcConfig, so the composer leaves provider.vpc unset and the Lambda keeps default internet egress. Also threads the new fields through the legacy-config translation, documents them in app-definition types, and fixes the stale maxCapacity default JSDoc (1 -> 4). Default behavior is unchanged: 'vpc' connectivity + minCapacity 0.5 compose a byte-for-byte identical template. Adds template-shape unit tests (validator + generated CloudFormation) covering both capabilities and the no-regression path. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JDh45c1vm91ySYtvVv9Z65 --- ...scale-to-zero-and-nat-free-connectivity.md | 65 +++++ .../aurora-scale-to-zero-connectivity.test.js | 255 ++++++++++++++++ .../domains/database/aurora-builder.js | 271 +++++++++++++++--- .../domains/networking/vpc-builder.js | 69 ++++- .../domains/shared/types/app-definition.js | 20 +- 5 files changed, 635 insertions(+), 45 deletions(-) create mode 100644 docs/architecture-decisions/033-aurora-serverless-v2-scale-to-zero-and-nat-free-connectivity.md create mode 100644 packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js diff --git a/docs/architecture-decisions/033-aurora-serverless-v2-scale-to-zero-and-nat-free-connectivity.md b/docs/architecture-decisions/033-aurora-serverless-v2-scale-to-zero-and-nat-free-connectivity.md new file mode 100644 index 000000000..926e3b2e2 --- /dev/null +++ b/docs/architecture-decisions/033-aurora-serverless-v2-scale-to-zero-and-nat-free-connectivity.md @@ -0,0 +1,65 @@ +# ADR-033: Aurora Serverless v2 scale-to-zero + NAT-free Lambda connectivity + +**Status**: Proposed +**Date**: 2026-08-20 +**Deciders**: Sean Matthews + +## Context + +When Frigg provisions its own database (`database.postgres.enable: true`, `ownership: 'stack'`), it creates an Aurora Serverless v2 cluster and attaches the app's Lambdas to a VPC so they can reach it privately. Two costs make this untenable for demos and small production apps that should idle at ~$0: + +1. **Aurora never scales to zero.** `aurora-builder.js` sets `ServerlessV2ScalingConfiguration.MinCapacity` from `dbConfig.minCapacity || 0.5` and the validator rejects `minCapacity < 0.5`. So the cluster idles at **0.5 ACU (~$43/mo)** even with zero traffic. AWS added true Aurora Serverless v2 scale-to-zero (`MinCapacity: 0`, with auto-pause after inactivity) in **November 2024**; Frigg has not wired it. + +2. **The VPC forces a NAT Gateway.** Aurora must live in a VPC (an AWS constraint — a VPC itself is free). But Frigg attaches the Lambda to that VPC to reach Aurora, and a VPC-attached Lambda loses default internet egress. A Frigg app is an *integration* app: it calls external SaaS APIs (Gong, Fireflies, Salesforce, …). Reaching them from an in-VPC Lambda requires a **NAT Gateway (~$32/mo + data)**, always on. + +Together that is **~$75/mo just to idle** a Frigg-owned database — which pushes every cost-sensitive deploy to an external DB (Neon/Atlas) instead of using Frigg's own Aurora support. + +There is a well-known topology that removes both costs: put Aurora in **public** subnets with a public endpoint, keep the **Lambda outside the VPC** (so it retains normal internet egress and needs no NAT), and let the Lambda connect to Aurora over its public endpoint with TLS and a security-group allowlist. Frigg already has a `database.postgres.publiclyAccessible` flag, but it still force-attaches the Lambda to the VPC and points the Aurora ingress rule at the Lambda's VPC security group — so the NAT cost remains and the public endpoint is unreachable from the (now VPC-less) intent. This ADR closes that gap. + +## Decision + +Introduce two independent, opt-in capabilities on `database.postgres`. Each is off by default; existing app definitions are byte-for-byte unaffected. + +### 1. Scale-to-zero (`minCapacity: 0`) + +- The validator accepts `minCapacity` of **`0`** (scale-to-zero) **or** a value in **`[0.5, 128]`**. Values in `(0, 0.5)` remain invalid. +- The scaling config reads `MinCapacity: dbConfig.minCapacity ?? 0.5` (nullish coalescing — the current `|| 0.5` silently turns a requested `0` back into `0.5`, the bug this fixes). +- When `minCapacity === 0`, emit `ServerlessV2ScalingConfiguration.SecondsUntilAutoPause` from a new optional `dbConfig.secondsUntilAutoPause` (default **300**, AWS-valid range **300–86400**). Below that idle window the cluster pauses to 0 ACU. +- Scale-to-zero requires a supported engine version (Aurora PostgreSQL 13.15+/14.12+/15.7+/16.3+). Frigg's default `engineVersion` (15.13) qualifies; document the constraint and warn if a user pins an older version with `minCapacity: 0`. + +### 2. Connectivity mode (`connectivity: 'vpc' | 'public'`) + +A new `database.postgres.connectivity` selector (default **`'vpc'`** = today's behavior): + +- **`'vpc'`** (default, unchanged): Aurora in private subnets, Lambda attached to the VPC, ingress from the Lambda security group. Requires a NAT (or VPC endpoints) for the Lambda's external egress. +- **`'public'`** (NAT-free): + - Aurora is placed in **public** subnets with `PubliclyAccessible: true` (implies the existing `publiclyAccessible` behavior). + - The app's **Lambdas are NOT attached to the VPC** — the composer does not set `provider.vpc`, so they keep default internet egress. **No NAT Gateway and no VPC endpoints are provisioned.** + - The Aurora ingress rule opens **5432 to `database.postgres.allowedCidrs`** (a new option; default **`['0.0.0.0/0']`**) via `CidrIp`, instead of `SourceSecurityGroupId` pointing at the Lambda SG (which no longer exists on the Lambda side). A VPC-less Lambda has dynamic egress IPs, so a demo typically needs `0.0.0.0/0`; production should narrow it. + - **TLS is required.** The generated `DATABASE_URL` / connection params must carry `sslmode=require` (or stricter). Public Postgres without TLS is not an allowed configuration. + +The two combine: `connectivity: 'public'` + `minCapacity: 0` yields a **Frigg-owned Aurora that idles at $0 with no NAT** — the goal. + +## Security posture + +`connectivity: 'public'` exposes the database endpoint to the internet. This is an explicit, opt-in trade and the builder must make it loud: + +- Emit a validation **warning** whenever `connectivity: 'public'` is set, and a stronger one when `allowedCidrs` includes `0.0.0.0/0`. +- Require TLS (above). Credentials stay in Secrets Manager with rotation, never in the definition. +- Recommend narrowing `allowedCidrs` to known egress ranges where the deployment can (e.g. a fixed NAT/proxy, office IPs, or a CI runner range). Document that a VPC-less Lambda cannot be pinned to a stable IP without extra infra, which is why the demo default is open. +- The default stays `'vpc'`: nobody gets a public database unless they ask for one. + +## Consequences + +- **Cold-resume latency.** After auto-pause, the first query pays a resume penalty (~seconds to low tens of seconds). Acceptable for demos and low-traffic apps; document it so it is not mistaken for a hang. +- **No behavior change by default.** `'vpc'` connectivity and `minCapacity` defaulting to 0.5 mean every existing definition composes an identical template. New behavior is strictly additive and opt-in. +- **Two supported "$0 idle" paths, clearly separated.** External serverless DB (Neon/Atlas via `DATABASE_URL`) remains the zero-framework path and is cheapest for tiny apps (Atlas M0 is free even while active). This ADR makes *Frigg-owned* Aurora a viable $0-idle option for teams that want Frigg to own the whole stack. + +## Scope of the implementing change + +- `packages/devtools/infrastructure/domains/database/aurora-builder.js` — validator, scaling config (`?? 0.5`, `SecondsUntilAutoPause`), public-mode subnet/PubliclyAccessible selection, and SG ingress via `CidrIp`. +- `packages/devtools/infrastructure/domains/networking/vpc-builder.js` + `infrastructure-composer.js` — in `public` connectivity, provision the public subnets/subnet-group Aurora needs but **do not** emit the Lambda `vpcConfig` and **do not** create a NAT Gateway. +- `packages/devtools/infrastructure/domains/shared/types/app-definition.js` — document `minCapacity: 0`, `secondsUntilAutoPause`, `connectivity`, `allowedCidrs`. +- Tests asserting the generated template: `MinCapacity: 0` + `SecondsUntilAutoPause` present; public mode → no `provider.vpc`/function VPC config, no `AWS::EC2::NatGateway` resource, Aurora ingress `CidrIp`, `PubliclyAccessible: true` in public subnets; and default (`vpc`, no `minCapacity`) composes unchanged. + +**Note:** end-to-end AWS deployment validation is out of scope for the implementing PR's automated tests (it requires a live account); the PR validates template *shape* via unit tests and documents the manual deploy check. diff --git a/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js b/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js new file mode 100644 index 000000000..0488ac29b --- /dev/null +++ b/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js @@ -0,0 +1,255 @@ +/** + * ADR-033: Aurora Serverless v2 scale-to-zero + NAT-free public connectivity + * + * These tests assert on the GENERATED CloudFormation template (via the full + * composer) plus the AuroraBuilder validator. They are deterministic and require + * neither a live AWS account nor Prisma client generation: + * - Validator cases call AuroraBuilder.validate() directly (pure, no I/O). + * - Template cases use vpc.management='create-new' + database.postgres + * management='managed', which resolve to STACK ownership without depending on + * any discovered AWS resource, so cloud discovery returning empty is fine. + * + * Both new capabilities are opt-in and default-off; the final describe block + * guards the no-regression promise: a default (vpc, no minCapacity) definition + * composes the same template as before. + */ + +const { composeServerlessDefinition } = require('../infrastructure-composer'); +const { AuroraBuilder } = require('../domains/database/aurora-builder'); + +// Shared: an app definition that creates a fresh VPC + a Frigg-owned Aurora +// cluster in-stack, with overridable postgres config. +function makeApp(postgresOverrides = {}) { + return { + name: 'adr033-app', + provider: 'aws', + region: 'us-east-1', + integrations: [], + vpc: { enable: true, management: 'create-new' }, + database: { + postgres: { + enable: true, + management: 'managed', + ...postgresOverrides, + }, + }, + }; +} + +function findResources(template, predicate) { + return Object.entries(template.resources.Resources).filter(([, r]) => predicate(r)); +} + +describe('ADR-033: Aurora scale-to-zero + connectivity', () => { + beforeAll(() => { + process.env.AWS_REGION = 'us-east-1'; + // Intentionally NOT setting FRIGG_SKIP_AWS_DISCOVERY — the builders must + // execute. create-new/managed resolve to STACK without needing discovery. + }); + + afterAll(() => { + delete process.env.AWS_REGION; + }); + + // --------------------------------------------------------------------- + // Validator + // --------------------------------------------------------------------- + describe('validator (AuroraBuilder.validate)', () => { + const build = new AuroraBuilder(); + // ValidationResult exposes hasErrors(); "valid" means no errors. + const validateResult = (postgres) => + build.validate({ database: { postgres: { enable: true, ...postgres } } }); + const isValid = (postgres) => !validateResult(postgres).hasErrors(); + + test('accepts minCapacity: 0 (scale-to-zero)', () => { + expect(isValid({ minCapacity: 0 })).toBe(true); + }); + + test('rejects minCapacity: 0.3 (inside the forbidden (0, 0.5) band)', () => { + const r = validateResult({ minCapacity: 0.3 }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/minCapacity must be 0 \(scale-to-zero\) or between 0\.5 and 128/); + }); + + test('accepts minCapacity: 0.5 and minCapacity: 64', () => { + expect(isValid({ minCapacity: 0.5 })).toBe(true); + expect(isValid({ minCapacity: 64 })).toBe(true); + }); + + test('rejects secondsUntilAutoPause: 100 (below 300)', () => { + const r = validateResult({ minCapacity: 0, secondsUntilAutoPause: 100 }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/secondsUntilAutoPause must be an integer between 300 and 86400/); + }); + + test('accepts secondsUntilAutoPause: 3600', () => { + expect(isValid({ minCapacity: 0, secondsUntilAutoPause: 3600 })).toBe(true); + }); + + test("accepts connectivity: 'public'", () => { + expect(isValid({ connectivity: 'public' })).toBe(true); + }); + + test("rejects connectivity: 'nope'", () => { + const r = validateResult({ connectivity: 'nope' }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/Invalid database\.postgres\.connectivity/); + }); + + test('rejects non-array / non-CIDR allowedCidrs, accepts valid CIDRs', () => { + expect(isValid({ allowedCidrs: 'nope' })).toBe(false); + expect(isValid({ allowedCidrs: ['not-a-cidr'] })).toBe(false); + expect(isValid({ allowedCidrs: ['10.0.0.0/8', '203.0.113.5/32'] })).toBe(true); + }); + + test('warns (does not fail) when minCapacity:0 with an older pinned engine version', () => { + const r = validateResult({ minCapacity: 0, engineVersion: '15.4' }); + expect(r.hasErrors()).toBe(false); // warning, not error + expect(r.warnings.join(' ')).toMatch(/may not support .*scale-to-zero/); + }); + + test('does not warn about engine when minCapacity:0 on a capable version', () => { + const r = validateResult({ minCapacity: 0, engineVersion: '15.13' }); + expect(r.warnings.join(' ')).not.toMatch(/may not support .*scale-to-zero/); + }); + }); + + // --------------------------------------------------------------------- + // Scale-to-zero (template shape) + // --------------------------------------------------------------------- + describe('scale-to-zero (minCapacity: 0)', () => { + test('MinCapacity is exactly 0 and SecondsUntilAutoPause defaults to 300', async () => { + const t = await composeServerlessDefinition(makeApp({ minCapacity: 0 })); + const scaling = t.resources.Resources.FriggAuroraCluster.Properties.ServerlessV2ScalingConfiguration; + + // Mutation guard: with the old `|| 0.5` bug this would be 0.5, not 0. + expect(scaling.MinCapacity).toBe(0); + expect(scaling.MinCapacity).not.toBe(0.5); + expect(scaling.SecondsUntilAutoPause).toBe(300); + }); + + test('SecondsUntilAutoPause honors a custom value', async () => { + const t = await composeServerlessDefinition(makeApp({ minCapacity: 0, secondsUntilAutoPause: 1800 })); + const scaling = t.resources.Resources.FriggAuroraCluster.Properties.ServerlessV2ScalingConfiguration; + expect(scaling.MinCapacity).toBe(0); + expect(scaling.SecondsUntilAutoPause).toBe(1800); + }); + + test('MaxCapacity defaults to 4 and is preserved with scale-to-zero', async () => { + const t = await composeServerlessDefinition(makeApp({ minCapacity: 0 })); + const scaling = t.resources.Resources.FriggAuroraCluster.Properties.ServerlessV2ScalingConfiguration; + expect(scaling.MaxCapacity).toBe(4); + }); + }); + + // --------------------------------------------------------------------- + // Public connectivity (template shape) + // --------------------------------------------------------------------- + describe("connectivity: 'public'", () => { + test('Aurora ingress uses CidrIp — one rule per allowedCidr — not SourceSecurityGroupId', async () => { + const t = await composeServerlessDefinition( + makeApp({ connectivity: 'public', allowedCidrs: ['10.1.0.0/16', '203.0.113.7/32'] }) + ); + const ingress = findResources( + t, + (r) => r.Type === 'AWS::EC2::SecurityGroupIngress' && r.Properties.FromPort === 5432 + ); + expect(ingress).toHaveLength(2); + const cidrs = ingress.map(([, r]) => r.Properties.CidrIp).sort(); + expect(cidrs).toEqual(['10.1.0.0/16', '203.0.113.7/32']); + ingress.forEach(([, r]) => { + expect(r.Properties.CidrIp).toBeDefined(); + expect(r.Properties.SourceSecurityGroupId).toBeUndefined(); + }); + }); + + test('allowedCidrs defaults to 0.0.0.0/0 when omitted', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + const ingress = findResources( + t, + (r) => r.Type === 'AWS::EC2::SecurityGroupIngress' && r.Properties.FromPort === 5432 + ); + expect(ingress).toHaveLength(1); + expect(ingress[0][1].Properties.CidrIp).toBe('0.0.0.0/0'); + }); + + test('Aurora instance is PubliclyAccessible and cluster sits in public subnets', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + expect(t.resources.Resources.FriggAuroraInstance.Properties.PubliclyAccessible).toBe(true); + expect(t.resources.Resources.FriggDBSubnetGroup.Properties.SubnetIds).toEqual([ + { Ref: 'FriggPublicSubnet' }, + { Ref: 'FriggPublicSubnet2' }, + ]); + }); + + test('NO NAT Gateway resource is emitted', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + const nats = findResources(t, (r) => r.Type === 'AWS::EC2::NatGateway'); + expect(nats).toHaveLength(0); + }); + + test('Lambda is NOT attached to the VPC (provider.vpc unset)', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + expect(t.provider.vpc).toBeUndefined(); + }); + + test('DATABASE_URL enforces TLS (sslmode=require)', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + const url = t.provider.environment.DATABASE_URL; + expect(url['Fn::Sub'][0]).toContain('sslmode=require'); + }); + + test('combines with scale-to-zero: $0-idle NAT-free Aurora', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public', minCapacity: 0 })); + const scaling = t.resources.Resources.FriggAuroraCluster.Properties.ServerlessV2ScalingConfiguration; + expect(scaling.MinCapacity).toBe(0); + expect(scaling.SecondsUntilAutoPause).toBe(300); + expect(findResources(t, (r) => r.Type === 'AWS::EC2::NatGateway')).toHaveLength(0); + expect(t.provider.vpc).toBeUndefined(); + }); + }); + + // --------------------------------------------------------------------- + // No-regression: default (vpc) connectivity, no new fields + // --------------------------------------------------------------------- + describe("default connectivity: 'vpc' (no regression)", () => { + test('MinCapacity defaults to 0.5 with no SecondsUntilAutoPause', async () => { + const t = await composeServerlessDefinition(makeApp()); + const scaling = t.resources.Resources.FriggAuroraCluster.Properties.ServerlessV2ScalingConfiguration; + expect(scaling.MinCapacity).toBe(0.5); + expect(scaling.MaxCapacity).toBe(4); + expect(scaling.SecondsUntilAutoPause).toBeUndefined(); + }); + + test('Aurora ingress uses SourceSecurityGroupId (Lambda SG), not CidrIp', async () => { + const t = await composeServerlessDefinition(makeApp()); + const ingress = findResources( + t, + (r) => r.Type === 'AWS::EC2::SecurityGroupIngress' && r.Properties.FromPort === 5432 + ); + expect(ingress).toHaveLength(1); + expect(ingress[0][1].Properties.SourceSecurityGroupId).toEqual({ Ref: 'FriggLambdaSecurityGroup' }); + expect(ingress[0][1].Properties.CidrIp).toBeUndefined(); + // Logical ID unchanged for the vpc path + expect(ingress[0][0]).toBe('FriggAuroraIngressRule'); + }); + + test('Aurora instance is not publicly accessible by default', async () => { + const t = await composeServerlessDefinition(makeApp()); + expect(t.resources.Resources.FriggAuroraInstance.Properties.PubliclyAccessible).toBe(false); + }); + + test('Lambda IS attached to the VPC (provider.vpc set)', async () => { + const t = await composeServerlessDefinition(makeApp()); + expect(t.provider.vpc).toBeDefined(); + expect(t.provider.vpc.subnetIds).toBeDefined(); + expect(t.provider.vpc.securityGroupIds).toBeDefined(); + }); + + test('DATABASE_URL does not add sslmode in vpc mode', async () => { + const t = await composeServerlessDefinition(makeApp()); + const url = t.provider.environment.DATABASE_URL; + expect(url['Fn::Sub'][0]).not.toContain('sslmode=require'); + }); + }); +}); diff --git a/packages/devtools/infrastructure/domains/database/aurora-builder.js b/packages/devtools/infrastructure/domains/database/aurora-builder.js index ed6057f40..ac7c6f315 100644 --- a/packages/devtools/infrastructure/domains/database/aurora-builder.js +++ b/packages/devtools/infrastructure/domains/database/aurora-builder.js @@ -94,21 +94,123 @@ class AuroraBuilder extends InfrastructureBuilder { } // Validate capacity settings - if (dbConfig.minCapacity !== undefined && (dbConfig.minCapacity < 0.5 || dbConfig.minCapacity > 128)) { - result.addError('database.postgres.minCapacity must be between 0.5 and 128'); + // minCapacity accepts 0 (Aurora Serverless v2 scale-to-zero, GA Nov 2024) + // OR a value in [0.5, 128]. Values in (0, 0.5) are not a valid Aurora + // capacity — reject them explicitly. See ADR-033. + if ( + dbConfig.minCapacity !== undefined && + dbConfig.minCapacity !== 0 && + (dbConfig.minCapacity < 0.5 || dbConfig.minCapacity > 128) + ) { + result.addError('database.postgres.minCapacity must be 0 (scale-to-zero) or between 0.5 and 128'); } if (dbConfig.maxCapacity !== undefined && (dbConfig.maxCapacity < 0.5 || dbConfig.maxCapacity > 128)) { result.addError('database.postgres.maxCapacity must be between 0.5 and 128'); } + // Validate scale-to-zero auto-pause window (seconds). AWS-valid range is + // 300–86400 (5 minutes to 24 hours) and it must be an integer. + if (dbConfig.secondsUntilAutoPause !== undefined) { + const s = dbConfig.secondsUntilAutoPause; + if (!Number.isInteger(s) || s < 300 || s > 86400) { + result.addError('database.postgres.secondsUntilAutoPause must be an integer between 300 and 86400'); + } + } + + // Validate connectivity mode. 'vpc' (default) keeps today's behavior; + // 'public' places Aurora in public subnets and leaves the Lambda out of + // the VPC (NAT-free). See ADR-033. + if (dbConfig.connectivity !== undefined) { + const validConnectivity = ['vpc', 'public']; + if (!validConnectivity.includes(dbConfig.connectivity)) { + result.addError( + `Invalid database.postgres.connectivity: "${dbConfig.connectivity}". Must be one of: ${validConnectivity.join(', ')}` + ); + } + } + + // Validate allowedCidrs is an array of CIDR-looking strings when present. + if (dbConfig.allowedCidrs !== undefined) { + const cidrPattern = /^([0-9]{1,3}\.){3}[0-9]{1,3}\/[0-9]{1,2}$/; + if (!Array.isArray(dbConfig.allowedCidrs)) { + result.addError('database.postgres.allowedCidrs must be an array of CIDR strings'); + } else { + const bad = dbConfig.allowedCidrs.filter( + (c) => typeof c !== 'string' || !cidrPattern.test(c) + ); + if (bad.length > 0) { + result.addError( + `database.postgres.allowedCidrs contains invalid CIDR value(s): ${bad.join(', ')}` + ); + } + } + } + + // Scale-to-zero requires a supported engine version. Warn (do not + // hard-fail) if the user pins an older version with minCapacity: 0. + // Frigg's default engineVersion (15.13) qualifies. Minimum + // scale-to-zero-capable versions: Aurora PG 13.15 / 14.12 / 15.7 / 16.3. + if (dbConfig.minCapacity === 0 && dbConfig.engineVersion) { + if (!this.engineSupportsScaleToZero(dbConfig.engineVersion)) { + result.addWarning( + `database.postgres.engineVersion="${dbConfig.engineVersion}" may not support Aurora Serverless v2 scale-to-zero (minCapacity: 0). ` + + 'Scale-to-zero requires Aurora PostgreSQL 13.15+/14.12+/15.7+/16.3+.' + ); + } + } + // Warn about public accessibility in production if (dbConfig.publiclyAccessible === true) { result.addWarning('database.postgres.publiclyAccessible=true is not recommended for production'); } + // ADR-033 security posture: public connectivity exposes the DB endpoint + // to the internet. Make the trade-off loud. + if (dbConfig.connectivity === 'public') { + result.addWarning( + 'database.postgres.connectivity="public" exposes the Aurora endpoint to the internet. ' + + 'TLS is enforced (sslmode=require) and access is restricted to allowedCidrs, but prefer "vpc" for production.' + ); + const cidrs = Array.isArray(dbConfig.allowedCidrs) ? dbConfig.allowedCidrs : ['0.0.0.0/0']; + if (cidrs.includes('0.0.0.0/0')) { + result.addWarning( + 'database.postgres.allowedCidrs allows 0.0.0.0/0 (the whole internet). ' + + 'Narrow this to known egress ranges wherever the deployment can.' + ); + } + } + return result; } + /** + * Determine whether an Aurora PostgreSQL engine version supports + * Serverless v2 scale-to-zero (minCapacity: 0). Minimum capable versions: + * 13.15+, 14.12+, 15.7+, 16.3+. Unknown/unparseable versions return true + * (assume-capable) so we never hard-block on a version string we can't read; + * this only gates a warning, never a hard failure. + * @param {string} engineVersion e.g. '15.13' + * @returns {boolean} + */ + engineSupportsScaleToZero(engineVersion) { + const minByMajor = { 13: 15, 14: 12, 15: 7, 16: 3 }; + const match = /^(\d+)\.(\d+)/.exec(String(engineVersion)); + if (!match) { + return true; // can't parse — don't warn spuriously + } + const major = Number(match[1]); + const minor = Number(match[2]); + // Majors newer than the table are assumed capable. + if (major > 16) { + return true; + } + // Majors older than 13 never support scale-to-zero. + if (!(major in minByMajor)) { + return false; + } + return minor >= minByMajor[major]; + } + /** * Build Aurora infrastructure using ownership-based architecture */ @@ -327,16 +429,27 @@ class AuroraBuilder extends InfrastructureBuilder { } } - // Preserve other database config - if (appDefinition.database?.postgres?.minCapacity) { + // Preserve other database config. + // NOTE: use `!== undefined` (not truthiness) so a requested minCapacity + // of 0 (scale-to-zero) flows through instead of being dropped. + if (appDefinition.database?.postgres?.minCapacity !== undefined) { translated.database.postgres.config.minCapacity = appDefinition.database.postgres.minCapacity; } - if (appDefinition.database?.postgres?.maxCapacity) { + if (appDefinition.database?.postgres?.maxCapacity !== undefined) { translated.database.postgres.config.maxCapacity = appDefinition.database.postgres.maxCapacity; } + if (appDefinition.database?.postgres?.secondsUntilAutoPause !== undefined) { + translated.database.postgres.config.secondsUntilAutoPause = appDefinition.database.postgres.secondsUntilAutoPause; + } if (appDefinition.database?.postgres?.publiclyAccessible !== undefined) { translated.database.postgres.config.publiclyAccessible = appDefinition.database.postgres.publiclyAccessible; } + if (appDefinition.database?.postgres?.connectivity !== undefined) { + translated.database.postgres.config.connectivity = appDefinition.database.postgres.connectivity; + } + if (appDefinition.database?.postgres?.allowedCidrs !== undefined) { + translated.database.postgres.config.allowedCidrs = appDefinition.database.postgres.allowedCidrs; + } return translated; } @@ -373,7 +486,10 @@ class AuroraBuilder extends InfrastructureBuilder { console.log(' Creating new Aurora Serverless v2 cluster...'); const dbConfig = appDefinition.database.postgres; - const publiclyAccessible = dbConfig.publiclyAccessible === true; + // ADR-033: connectivity 'public' implies a publicly-accessible cluster in + // public subnets. It combines with the legacy publiclyAccessible flag. + const publicConnectivity = dbConfig.connectivity === 'public'; + const publiclyAccessible = publicConnectivity || dbConfig.publiclyAccessible === true; // Get subnet IDs for DB Subnet Group const subnetIds = publiclyAccessible @@ -452,9 +568,16 @@ class AuroraBuilder extends InfrastructureBuilder { // min when idle) and gives the DB enough headroom to // absorb bursty sync traffic. Apps can still override both // via app definition dbConfig. + // ADR-033: use nullish coalescing — `|| 0.5` silently turned a + // requested MinCapacity of 0 (scale-to-zero) back into 0.5. + // When MinCapacity is 0, emit SecondsUntilAutoPause so the cluster + // pauses to 0 ACU after the idle window (default 300s). ServerlessV2ScalingConfiguration: { - MinCapacity: dbConfig.minCapacity || 0.5, - MaxCapacity: dbConfig.maxCapacity || 4, + MinCapacity: dbConfig.minCapacity ?? 0.5, + MaxCapacity: dbConfig.maxCapacity ?? 4, + ...(dbConfig.minCapacity === 0 + ? { SecondsUntilAutoPause: dbConfig.secondsUntilAutoPause ?? 300 } + : {}), }, EnableHttpEndpoint: false, BackupRetentionPeriod: 7, @@ -482,12 +605,15 @@ class AuroraBuilder extends InfrastructureBuilder { }, }; - // Environment variables + // Environment variables. + // ADR-033: public connectivity requires TLS (sslmode=require) since the + // endpoint is reachable over the internet. result.environment.DATABASE_URL = this.buildDatabaseUrl( { 'Fn::GetAtt': ['FriggAuroraCluster', 'Endpoint.Address'] }, { 'Fn::GetAtt': ['FriggAuroraCluster', 'Endpoint.Port'] }, dbConfig.database || 'frigg', - { Ref: 'FriggDBSecret' } + { Ref: 'FriggDBSecret' }, + { requireTls: publicConnectivity } ); // IAM permissions for Secrets Manager @@ -497,19 +623,46 @@ class AuroraBuilder extends InfrastructureBuilder { Resource: { Ref: 'FriggDBSecret' }, }); - // Add self-referencing security group ingress rule to allow Lambda to connect to Aurora - // Since both Lambda and Aurora share the same security group, we need to allow the SG to accept traffic from itself - result.resources.FriggAuroraIngressRule = { - Type: 'AWS::EC2::SecurityGroupIngress', - Properties: { - GroupId: { Ref: 'FriggLambdaSecurityGroup' }, - IpProtocol: 'tcp', - FromPort: 5432, - ToPort: 5432, - SourceSecurityGroupId: { Ref: 'FriggLambdaSecurityGroup' }, - Description: 'Allow Lambda functions to connect to Aurora PostgreSQL (self-referencing rule)', - }, - }; + if (publicConnectivity) { + // ADR-033 NAT-free public connectivity: the Lambda is NOT attached to + // the VPC (see vpc-builder), so FriggLambdaSecurityGroup is not on the + // Lambda side and a SourceSecurityGroupId rule would authorize nothing. + // Instead open 5432 to the configured CIDR allowlist (default + // 0.0.0.0/0 for demos — narrow it in production). One ingress rule per + // CIDR. GroupId stays FriggLambdaSecurityGroup because that is the SG + // attached to the Aurora cluster (see VpcSecurityGroupIds above). + const allowedCidrs = + Array.isArray(dbConfig.allowedCidrs) && dbConfig.allowedCidrs.length > 0 + ? dbConfig.allowedCidrs + : ['0.0.0.0/0']; + allowedCidrs.forEach((cidr, index) => { + result.resources[`FriggAuroraIngressRule${index}`] = { + Type: 'AWS::EC2::SecurityGroupIngress', + Properties: { + GroupId: { Ref: 'FriggLambdaSecurityGroup' }, + IpProtocol: 'tcp', + FromPort: 5432, + ToPort: 5432, + CidrIp: cidr, + Description: `Allow PostgreSQL access from ${cidr} (public connectivity)`, + }, + }; + }); + } else { + // Add self-referencing security group ingress rule to allow Lambda to connect to Aurora + // Since both Lambda and Aurora share the same security group, we need to allow the SG to accept traffic from itself + result.resources.FriggAuroraIngressRule = { + Type: 'AWS::EC2::SecurityGroupIngress', + Properties: { + GroupId: { Ref: 'FriggLambdaSecurityGroup' }, + IpProtocol: 'tcp', + FromPort: 5432, + ToPort: 5432, + SourceSecurityGroupId: { Ref: 'FriggLambdaSecurityGroup' }, + Description: 'Allow Lambda functions to connect to Aurora PostgreSQL (self-referencing rule)', + }, + }; + } console.log(' ✅ Aurora Serverless v2 cluster resources created'); } @@ -554,6 +707,9 @@ class AuroraBuilder extends InfrastructureBuilder { console.log(` ✅ Using discovered Aurora cluster: ${discoveredResources.auroraClusterEndpoint}`); const dbConfig = appDefinition.database.postgres; + // ADR-033: public connectivity requires TLS on the connection string and a + // CIDR-based ingress rule (the Lambda is not VPC-attached). + const publicConnectivity = dbConfig.connectivity === 'public'; // Use discovered cluster details result.environment.DATABASE_HOST = discoveredResources.auroraClusterEndpoint; @@ -729,7 +885,8 @@ exports.handler = async (event, context) => { discoveredResources.auroraClusterEndpoint, discoveredResources.auroraPort || 5432, dbConfig.database || 'frigg', - { Ref: 'FriggDBSecret' } + { Ref: 'FriggDBSecret' }, + { requireTls: publicConnectivity } ); // Grant Lambda functions permission to read the secret @@ -747,7 +904,8 @@ exports.handler = async (event, context) => { discoveredResources.auroraClusterEndpoint, discoveredResources.auroraPort || 5432, dbConfig.database || 'frigg', - discoveredResources.databaseSecretArn + discoveredResources.databaseSecretArn, + { requireTls: publicConnectivity } ); result.iamStatements.push({ @@ -768,7 +926,10 @@ exports.handler = async (event, context) => { // Consumers that build DATABASE_URL from components at runtime MUST // append `?${DATABASE_URL_PARAMS}` to get the same hang-prevention // timeouts as the managed path. - result.environment.DATABASE_URL_PARAMS = LAMBDA_DATABASE_URL_QUERY_PARAMS; + // ADR-033: public connectivity requires TLS — include sslmode=require. + result.environment.DATABASE_URL_PARAMS = publicConnectivity + ? `${LAMBDA_DATABASE_URL_QUERY_PARAMS}&sslmode=require` + : LAMBDA_DATABASE_URL_QUERY_PARAMS; // Note: DATABASE_URL is NOT set here to avoid Serverless variable resolution errors // The application (Frigg Core) should construct it at runtime from: @@ -782,17 +943,39 @@ exports.handler = async (event, context) => { // Add security group ingress rule to allow Lambda to connect to Aurora if (discoveredResources.auroraSecurityGroupId) { - result.resources.FriggAuroraIngressRule = { - Type: 'AWS::EC2::SecurityGroupIngress', - Properties: { - GroupId: discoveredResources.auroraSecurityGroupId, - IpProtocol: 'tcp', - FromPort: discoveredResources.auroraPort || 5432, - ToPort: discoveredResources.auroraPort || 5432, - SourceSecurityGroupId: { Ref: 'FriggLambdaSecurityGroup' }, - Description: 'Allow Lambda functions to connect to Aurora PostgreSQL', - }, - }; + if (publicConnectivity) { + // ADR-033 NAT-free public connectivity: the Lambda is not in the + // VPC, so authorize the CIDR allowlist instead of the Lambda SG. + const allowedCidrs = + Array.isArray(dbConfig.allowedCidrs) && dbConfig.allowedCidrs.length > 0 + ? dbConfig.allowedCidrs + : ['0.0.0.0/0']; + allowedCidrs.forEach((cidr, index) => { + result.resources[`FriggAuroraIngressRule${index}`] = { + Type: 'AWS::EC2::SecurityGroupIngress', + Properties: { + GroupId: discoveredResources.auroraSecurityGroupId, + IpProtocol: 'tcp', + FromPort: discoveredResources.auroraPort || 5432, + ToPort: discoveredResources.auroraPort || 5432, + CidrIp: cidr, + Description: `Allow PostgreSQL access from ${cidr} (public connectivity)`, + }, + }; + }); + } else { + result.resources.FriggAuroraIngressRule = { + Type: 'AWS::EC2::SecurityGroupIngress', + Properties: { + GroupId: discoveredResources.auroraSecurityGroupId, + IpProtocol: 'tcp', + FromPort: discoveredResources.auroraPort || 5432, + ToPort: discoveredResources.auroraPort || 5432, + SourceSecurityGroupId: { Ref: 'FriggLambdaSecurityGroup' }, + Description: 'Allow Lambda functions to connect to Aurora PostgreSQL', + }, + }; + } console.log(` ✅ Added security group ingress rule for Lambda → Aurora connectivity`); } @@ -805,8 +988,11 @@ exports.handler = async (event, context) => { * @param {string|number|object} port - Database port (string/number or CloudFormation intrinsic function) * @param {string} database - Database name * @param {string|object} secretRef - Secret ARN (string) or CloudFormation Ref object + * @param {object} [options] + * @param {boolean} [options.requireTls] - Append sslmode=require (ADR-033 public connectivity) */ - buildDatabaseUrl(host, port, database, secretRef) { + buildDatabaseUrl(host, port, database, secretRef, options = {}) { + const { requireTls = false } = options; // Handle secretRef as either a string ARN or CloudFormation Ref object const resolveSecretRef = (secretRefValue) => { if (typeof secretRefValue === 'object' && secretRefValue.Ref) { @@ -838,9 +1024,16 @@ exports.handler = async (event, context) => { // Query params are defined at module scope (LAMBDA_DATABASE_URL_QUERY_PARAMS) // so runtime-URL-construction paths can emit the same timeouts as an env var. + // ADR-033: for public connectivity, TLS is mandatory — append sslmode=require + // unless it's already present in the base params. + const queryParams = + requireTls && !/(^|&)sslmode=/.test(LAMBDA_DATABASE_URL_QUERY_PARAMS) + ? `${LAMBDA_DATABASE_URL_QUERY_PARAMS}&sslmode=require` + : LAMBDA_DATABASE_URL_QUERY_PARAMS; + return { 'Fn::Sub': [ - `postgresql://\${Username}:\${Password}@\${Host}:\${Port}/\${Database}?${LAMBDA_DATABASE_URL_QUERY_PARAMS}`, + `postgresql://\${Username}:\${Password}@\${Host}:\${Port}/\${Database}?${queryParams}`, { Username: resolveSecretRef(secretRef), Password: resolveSecretPassword(secretRef), diff --git a/packages/devtools/infrastructure/domains/networking/vpc-builder.js b/packages/devtools/infrastructure/domains/networking/vpc-builder.js index 2b9d07f14..cb8ad7cdf 100644 --- a/packages/devtools/infrastructure/domains/networking/vpc-builder.js +++ b/packages/devtools/infrastructure/domains/networking/vpc-builder.js @@ -24,6 +24,31 @@ const { createEmptyDiscoveryResult } = require('../shared/types/discovery-result const { ResourceOwnership } = require('../shared/types/resource-ownership'); const { isSsmOffloadActive } = require('../parameters/offload-utils'); +/** + * ADR-033: NAT-free public database connectivity. + * + * When the app opts into `database.postgres.connectivity: 'public'`, Aurora is + * placed in public subnets with a public endpoint and the app's Lambdas are + * intentionally left OUTSIDE the VPC so they keep default internet egress and + * need no NAT Gateway. This function is the single seam that DECOUPLES the + * Lambda's VPC attachment + NAT from Aurora's networking: + * + * - The VpcBuilder still builds the VPC / public subnets / DB subnet group + * that Aurora requires (Aurora must live in a VPC — an AWS constraint). + * - But it does NOT emit a NAT Gateway, and it clears `result.vpcConfig` so the + * composer never sets `provider.vpc` (see infrastructure-composer.js:126) — + * leaving the Lambda un-attached with normal egress. + * + * @param {Object} appDefinition + * @returns {boolean} + */ +function isPublicDatabaseConnectivity(appDefinition) { + return ( + appDefinition?.database?.postgres?.enable === true && + appDefinition?.database?.postgres?.connectivity === 'public' + ); +} + class VpcBuilder extends InfrastructureBuilder { constructor() { super(); @@ -565,11 +590,35 @@ class VpcBuilder extends InfrastructureBuilder { // Build Subnets based on ownership decision this.buildSubnetsFromDecision(decisions.subnets, appDefinition, discoveredResources, result); - // Build NAT Gateway based on ownership decision - this.buildNatGatewayFromDecision(decisions.natGateway, appDefinition, discoveredResources, result); + // ADR-033: NAT-free public database connectivity. When set, Aurora sits in + // public subnets with a public endpoint and the Lambda is NOT attached to + // the VPC — so it keeps default internet egress and needs no NAT Gateway. + // We still built the VPC + public subnets above (Aurora requires them), but + // we skip the NAT Gateway here and clear vpcConfig below so the composer + // leaves provider.vpc unset. This is the seam that decouples the Lambda's + // VPC attachment + NAT from Aurora's networking. + const publicDbConnectivity = isPublicDatabaseConnectivity(appDefinition); + + if (publicDbConnectivity) { + console.log( + ' ⊝ NAT Gateway skipped (database.postgres.connectivity=public — Lambda is not VPC-attached, so no NAT is needed)' + ); + } else { + // Build NAT Gateway based on ownership decision + this.buildNatGatewayFromDecision(decisions.natGateway, appDefinition, discoveredResources, result); + } - // Build VPC Endpoints based on ownership decisions - this.buildVpcEndpointsFromDecisions(decisions.vpcEndpoints, decisions.securityGroup, appDefinition, discoveredResources, result); + // Build VPC Endpoints based on ownership decisions. + // ADR-033: in public connectivity the Lambda is not in the VPC, so VPC + // endpoints (which give in-VPC functions private AWS access) would be + // wasted spend — skip them along with the NAT Gateway. + if (publicDbConnectivity) { + console.log( + ' ⊝ VPC Endpoints skipped (database.postgres.connectivity=public — Lambda is not VPC-attached)' + ); + } else { + this.buildVpcEndpointsFromDecisions(decisions.vpcEndpoints, decisions.securityGroup, appDefinition, discoveredResources, result); + } // Set VPC_ENABLED environment variable result.environment.VPC_ENABLED = 'true'; @@ -579,6 +628,18 @@ class VpcBuilder extends InfrastructureBuilder { console.log(` - Subnets: ${result.vpcConfig.subnetIds.length}`); console.log(` - Security Groups: ${result.vpcConfig.securityGroupIds.length}`); + if (publicDbConnectivity) { + // Do NOT attach the Lambda to the VPC: leaving vpcConfig null means the + // composer never sets provider.vpc, so the function keeps normal + // internet egress. The Aurora subnets/DB subnet group built above are + // still emitted (AuroraBuilder consumes discoveredResources.publicSubnetId*), + // and the Lambda reaches Aurora over its public endpoint + TLS. + console.log( + ' ⊝ Lambda VPC attachment skipped (database.postgres.connectivity=public — provider.vpc will be unset)' + ); + result.vpcConfig = null; + } + return result; } diff --git a/packages/devtools/infrastructure/domains/shared/types/app-definition.js b/packages/devtools/infrastructure/domains/shared/types/app-definition.js index 55426bb3b..3516d4697 100644 --- a/packages/devtools/infrastructure/domains/shared/types/app-definition.js +++ b/packages/devtools/infrastructure/domains/shared/types/app-definition.js @@ -55,10 +55,26 @@ * * @property {Object} [config] - Configuration preferences * @property {'aurora-postgresql'|'aurora-mysql'} [config.engine] - Database engine - * @property {number} [config.minCapacity] - Min serverless capacity (default: 0.5) - * @property {number} [config.maxCapacity] - Max serverless capacity (default: 1) + * @property {number} [config.minCapacity] - Min serverless capacity in ACU. Accepts + * `0` for Aurora Serverless v2 scale-to-zero (cluster pauses to 0 ACU when idle; + * requires Aurora PostgreSQL 13.15+/14.12+/15.7+/16.3+, and the default 15.13 + * qualifies) OR a value in [0.5, 128]. Values in (0, 0.5) are invalid. (default: 0.5) + * @property {number} [config.maxCapacity] - Max serverless capacity in ACU, range + * [0.5, 128] (default: 4) + * @property {number} [config.secondsUntilAutoPause] - Idle window before a + * scale-to-zero cluster (minCapacity: 0) pauses to 0 ACU. Integer seconds in + * [300, 86400]. Only applied when minCapacity === 0 (default: 300) * @property {string} [config.database] - Database name (default: 'frigg') * @property {boolean} [config.publiclyAccessible] - Public access (default: false) + * @property {'vpc'|'public'} [config.connectivity] - Lambda↔Aurora connectivity + * mode. `'vpc'` (default): Aurora in private subnets, Lambda attached to the VPC, + * ingress from the Lambda security group (needs a NAT for external egress). + * `'public'` (NAT-free): Aurora in public subnets with a public endpoint, Lambda + * left OUTSIDE the VPC (no NAT Gateway, no VPC endpoints), ingress opened to + * `allowedCidrs`, TLS enforced (sslmode=require). (default: 'vpc') + * @property {string[]} [config.allowedCidrs] - CIDR blocks allowed to reach Aurora + * on 5432 when connectivity is 'public'. One ingress rule is emitted per CIDR. + * (default: ['0.0.0.0/0'] — narrow this for production) * @property {boolean} [config.autoCreateCredentials] - Auto-create credentials in Secrets Manager */ From 51406015cf670753915e87f84a29f2d6067cd5be Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 04:48:38 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(devtools):=20address=20ADR-033=20review?= =?UTF-8?q?=20=E2=80=94=20public-subnet=20routing,=20validator=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review follow-ups on the Aurora scale-to-zero / NAT-free work: - [HIGH] Public subnets are now internet-routable. Skipping NAT dropped the IGW default route + subnet associations (they were only emitted as a side effect of the NAT build), leaving the public Aurora endpoint unreachable. In public mode the VpcBuilder now calls createPublicRouting for a Frigg-created (stack) VPC — guarded by the presence of FriggInternetGateway so a discovered/existing VPC (whose subnets already route to an IGW) is left alone. - [SEC] Empty allowedCidrs with connectivity=public is now a validation ERROR instead of silently falling back to 0.0.0.0/0. - Fixed the stale aurora-builder.test.js assertion for the new minCapacity error. - Added a loud warning for connectivity=public with discover/use-existing (Frigg can't move an existing cluster to public subnets, and the Lambda is detached); useExistingAurora now applies sslmode=require in public mode. - Added minCapacity <= maxCapacity cross-check. - connectivity=public now requires vpc.enable=true (clear error, not a downstream "requires 2 public subnets" throw). - Warn when secondsUntilAutoPause is set with minCapacity !== 0 (ignored). - VPC_ENABLED is 'false' in public mode so /health doesn't misreport isInVpc. - Tightened allowedCidrs validation to real IPv4 numeric ranges (octets <=255, prefix <=32); documented IPv4-only. - Corrected the .config.* translation comment (it is a mirror, not the load-bearing 0-propagation path). Default vpc path unchanged. New + existing aurora-builder tests green; vpc-builder.test.js failures are pre-existing base-branch red (unchanged). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JDh45c1vm91ySYtvVv9Z65 --- .../aurora-scale-to-zero-connectivity.test.js | 107 +++++++++++++++- .../domains/database/aurora-builder.js | 116 ++++++++++++++++-- .../domains/database/aurora-builder.test.js | 2 +- .../domains/networking/vpc-builder.js | 34 ++++- 4 files changed, 243 insertions(+), 16 deletions(-) diff --git a/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js b/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js index 0488ac29b..04bb2fe8c 100644 --- a/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js +++ b/packages/devtools/infrastructure/__tests__/aurora-scale-to-zero-connectivity.test.js @@ -57,9 +57,15 @@ describe('ADR-033: Aurora scale-to-zero + connectivity', () => { describe('validator (AuroraBuilder.validate)', () => { const build = new AuroraBuilder(); // ValidationResult exposes hasErrors(); "valid" means no errors. - const validateResult = (postgres) => - build.validate({ database: { postgres: { enable: true, ...postgres } } }); - const isValid = (postgres) => !validateResult(postgres).hasErrors(); + // vpc.enable defaults to true so connectivity:'public' cases don't trip the + // "public requires vpc.enable" rule; pass appOverrides to change it. + const validateResult = (postgres, appOverrides = {}) => + build.validate({ + vpc: { enable: true }, + database: { postgres: { enable: true, ...postgres } }, + ...appOverrides, + }); + const isValid = (postgres, appOverrides) => !validateResult(postgres, appOverrides).hasErrors(); test('accepts minCapacity: 0 (scale-to-zero)', () => { expect(isValid({ minCapacity: 0 })).toBe(true); @@ -102,6 +108,57 @@ describe('ADR-033: Aurora scale-to-zero + connectivity', () => { expect(isValid({ allowedCidrs: ['10.0.0.0/8', '203.0.113.5/32'] })).toBe(true); }); + test('rejects out-of-range CIDR octets/prefix (numeric, not just shape)', () => { + expect(isValid({ allowedCidrs: ['999.999.999.999/99'] })).toBe(false); + expect(isValid({ allowedCidrs: ['10.0.0.0/33'] })).toBe(false); + expect(isValid({ allowedCidrs: ['256.1.1.1/24'] })).toBe(false); + expect(isValid({ allowedCidrs: ['0.0.0.0/0'] })).toBe(true); + }); + + test('rejects empty allowedCidrs in public mode (no silent full-internet fallback)', () => { + const r = validateResult({ connectivity: 'public', allowedCidrs: [] }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/allowedCidrs is empty with connectivity="public"/); + }); + + test('rejects minCapacity > maxCapacity', () => { + const r = validateResult({ minCapacity: 8, maxCapacity: 4 }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/minCapacity \(8\) must be <= maxCapacity \(4\)/); + // equal is fine + expect(isValid({ minCapacity: 4, maxCapacity: 4 })).toBe(true); + }); + + test("connectivity:'public' requires vpc.enable=true", () => { + const r = validateResult({ connectivity: 'public' }, { vpc: { enable: false } }); + expect(r.hasErrors()).toBe(true); + expect(r.errors.join(' ')).toMatch(/connectivity="public" requires vpc\.enable=true/); + // With vpc.enable true (default helper), it's valid + expect(isValid({ connectivity: 'public' })).toBe(true); + }); + + test('warns when secondsUntilAutoPause set with minCapacity !== 0', () => { + const r = validateResult({ minCapacity: 0.5, secondsUntilAutoPause: 3600 }); + expect(r.hasErrors()).toBe(false); + expect(r.warnings.join(' ')).toMatch(/secondsUntilAutoPause is ignored unless minCapacity is 0/); + // No such warning when minCapacity is 0 + const r0 = validateResult({ minCapacity: 0, secondsUntilAutoPause: 3600 }); + expect(r0.warnings.join(' ')).not.toMatch(/ignored unless minCapacity is 0/); + }); + + test("warns (not errors) for connectivity:'public' with discover/use-existing management", () => { + const rDiscover = validateResult({ connectivity: 'public', management: 'discover' }); + expect(rDiscover.hasErrors()).toBe(false); + expect(rDiscover.warnings.join(' ')).toMatch(/assumes the EXISTING Aurora cluster is already publicly accessible/); + + const rUseExisting = validateResult({ + connectivity: 'public', + management: 'use-existing', + endpoint: 'db.example.com', + }); + expect(rUseExisting.warnings.join(' ')).toMatch(/only affects TLS/); + }); + test('warns (does not fail) when minCapacity:0 with an older pinned engine version', () => { const r = validateResult({ minCapacity: 0, engineVersion: '15.4' }); expect(r.hasErrors()).toBe(false); // warning, not error @@ -188,6 +245,45 @@ describe('ADR-033: Aurora scale-to-zero + connectivity', () => { expect(nats).toHaveLength(0); }); + test('public subnets get an IGW default route + route table + both associations (create-new VPC)', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + const R = t.resources.Resources; + + // Public route table + expect(R.FriggPublicRouteTable).toBeDefined(); + expect(R.FriggPublicRouteTable.Type).toBe('AWS::EC2::RouteTable'); + + // 0.0.0.0/0 -> Internet Gateway default route + const igwRoutes = findResources( + t, + (r) => + r.Type === 'AWS::EC2::Route' && + r.Properties.DestinationCidrBlock === '0.0.0.0/0' && + r.Properties.GatewayId && + r.Properties.GatewayId.Ref === 'FriggInternetGateway' + ); + expect(igwRoutes).toHaveLength(1); + + // Both public subnet associations + expect(R.FriggPublicSubnet1RouteTableAssociation).toBeDefined(); + expect(R.FriggPublicSubnet2RouteTableAssociation).toBeDefined(); + expect(R.FriggPublicSubnet1RouteTableAssociation.Properties.RouteTableId).toEqual({ + Ref: 'FriggPublicRouteTable', + }); + + // And there must be NO NAT route (that would imply a NAT default route) + const natRoutes = findResources( + t, + (r) => r.Type === 'AWS::EC2::Route' && r.Properties.NatGatewayId + ); + expect(natRoutes).toHaveLength(0); + }); + + test('VPC_ENABLED is false in public mode (Lambda not VPC-attached)', async () => { + const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); + expect(t.provider.environment.VPC_ENABLED).toBe('false'); + }); + test('Lambda is NOT attached to the VPC (provider.vpc unset)', async () => { const t = await composeServerlessDefinition(makeApp({ connectivity: 'public' })); expect(t.provider.vpc).toBeUndefined(); @@ -251,5 +347,10 @@ describe('ADR-033: Aurora scale-to-zero + connectivity', () => { const url = t.provider.environment.DATABASE_URL; expect(url['Fn::Sub'][0]).not.toContain('sslmode=require'); }); + + test('VPC_ENABLED stays true in vpc mode', async () => { + const t = await composeServerlessDefinition(makeApp()); + expect(t.provider.environment.VPC_ENABLED).toBe('true'); + }); }); }); diff --git a/packages/devtools/infrastructure/domains/database/aurora-builder.js b/packages/devtools/infrastructure/domains/database/aurora-builder.js index ac7c6f315..64c239181 100644 --- a/packages/devtools/infrastructure/domains/database/aurora-builder.js +++ b/packages/devtools/infrastructure/domains/database/aurora-builder.js @@ -107,6 +107,17 @@ class AuroraBuilder extends InfrastructureBuilder { if (dbConfig.maxCapacity !== undefined && (dbConfig.maxCapacity < 0.5 || dbConfig.maxCapacity > 128)) { result.addError('database.postgres.maxCapacity must be between 0.5 and 128'); } + // Cross-check: minCapacity must not exceed maxCapacity, or CloudFormation + // rejects the ServerlessV2ScalingConfiguration at deploy time. + if ( + dbConfig.minCapacity !== undefined && + dbConfig.maxCapacity !== undefined && + dbConfig.minCapacity > dbConfig.maxCapacity + ) { + result.addError( + `database.postgres.minCapacity (${dbConfig.minCapacity}) must be <= maxCapacity (${dbConfig.maxCapacity})` + ); + } // Validate scale-to-zero auto-pause window (seconds). AWS-valid range is // 300–86400 (5 minutes to 24 hours) and it must be an integer. @@ -115,6 +126,12 @@ class AuroraBuilder extends InfrastructureBuilder { if (!Number.isInteger(s) || s < 300 || s > 86400) { result.addError('database.postgres.secondsUntilAutoPause must be an integer between 300 and 86400'); } + // secondsUntilAutoPause only takes effect for scale-to-zero clusters. + if (dbConfig.minCapacity !== 0) { + result.addWarning( + 'database.postgres.secondsUntilAutoPause is ignored unless minCapacity is 0 (scale-to-zero).' + ); + } } // Validate connectivity mode. 'vpc' (default) keeps today's behavior; @@ -129,20 +146,37 @@ class AuroraBuilder extends InfrastructureBuilder { } } - // Validate allowedCidrs is an array of CIDR-looking strings when present. + // Public connectivity requires an enabled VPC (Aurora must live in a VPC — + // an AWS constraint — and the builder creates the public subnets there). + // Validate it here so the failure is a clear message rather than a downstream + // "Aurora requires 2 public subnets" throw. + if (dbConfig.connectivity === 'public' && appDefinition.vpc?.enable !== true) { + result.addError( + 'database.postgres.connectivity="public" requires vpc.enable=true (Aurora needs public subnets in a VPC).' + ); + } + + // Validate allowedCidrs is an array of syntactically valid IPv4 CIDRs when + // present. NOTE: IPv4 only — IPv6 (CidrIpv6) is not supported here. if (dbConfig.allowedCidrs !== undefined) { - const cidrPattern = /^([0-9]{1,3}\.){3}[0-9]{1,3}\/[0-9]{1,2}$/; if (!Array.isArray(dbConfig.allowedCidrs)) { result.addError('database.postgres.allowedCidrs must be an array of CIDR strings'); } else { - const bad = dbConfig.allowedCidrs.filter( - (c) => typeof c !== 'string' || !cidrPattern.test(c) - ); + const bad = dbConfig.allowedCidrs.filter((c) => !this.isValidIpv4Cidr(c)); if (bad.length > 0) { result.addError( `database.postgres.allowedCidrs contains invalid CIDR value(s): ${bad.join(', ')}` ); } + // An empty allowlist in public mode is almost always a mistake that + // would silently fall back to 0.0.0.0/0 (full internet exposure) — + // reject it so "allow nothing" cannot mean "allow everything". + if (dbConfig.allowedCidrs.length === 0 && dbConfig.connectivity === 'public') { + result.addError( + 'database.postgres.allowedCidrs is empty with connectivity="public". List at least one CIDR ' + + "(use ['0.0.0.0/0'] to intentionally allow the whole internet)." + ); + } } } @@ -171,18 +205,67 @@ class AuroraBuilder extends InfrastructureBuilder { 'database.postgres.connectivity="public" exposes the Aurora endpoint to the internet. ' + 'TLS is enforced (sslmode=require) and access is restricted to allowedCidrs, but prefer "vpc" for production.' ); - const cidrs = Array.isArray(dbConfig.allowedCidrs) ? dbConfig.allowedCidrs : ['0.0.0.0/0']; + // Only the default fallback (empty array is now an error above) or an + // explicit 0.0.0.0/0 reaches here as "whole internet". + const cidrs = + Array.isArray(dbConfig.allowedCidrs) && dbConfig.allowedCidrs.length > 0 + ? dbConfig.allowedCidrs + : ['0.0.0.0/0']; if (cidrs.includes('0.0.0.0/0')) { result.addWarning( 'database.postgres.allowedCidrs allows 0.0.0.0/0 (the whole internet). ' + 'Narrow this to known egress ranges wherever the deployment can.' ); } + + // Public connectivity only makes Frigg place the cluster in public + // subnets when Frigg CREATES the cluster (management='managed'). In + // discover/use-existing mode Frigg cannot flip an existing cluster to + // public subnets / PubliclyAccessible, yet the Lambda is still detached + // from the VPC — so if the existing cluster is private it becomes + // unreachable. Warn loudly. (management defaults to 'discover'.) + const mgmt = dbConfig.management || 'discover'; + if (mgmt === 'discover' || mgmt === 'use-existing') { + result.addWarning( + `database.postgres.connectivity="public" with management="${mgmt}" assumes the EXISTING Aurora cluster ` + + 'is already publicly accessible. Frigg detaches the Lambda from the VPC in public mode, so a private ' + + 'existing cluster will be unreachable. Ensure the cluster has a public endpoint and open security group.' + ); + } + if (mgmt === 'use-existing') { + result.addWarning( + 'database.postgres.connectivity="public" with management="use-existing" only affects TLS (sslmode=require) on the ' + + 'connection string — Frigg does not manage the existing cluster\'s subnets or ingress.' + ); + } } return result; } + /** + * Validate an IPv4 CIDR string with real numeric range checks (each octet + * 0–255, prefix 0–32) — not just shape — so values like 999.999.999.999/99 + * are rejected. IPv4 only; IPv6 is out of scope for allowedCidrs. + * @param {string} cidr + * @returns {boolean} + */ + isValidIpv4Cidr(cidr) { + if (typeof cidr !== 'string') { + return false; + } + const match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})\/(\d{1,2})$/.exec(cidr); + if (!match) { + return false; + } + const octets = [match[1], match[2], match[3], match[4]].map(Number); + if (octets.some((o) => o > 255)) { + return false; + } + const prefix = Number(match[5]); + return prefix >= 0 && prefix <= 32; + } + /** * Determine whether an Aurora PostgreSQL engine version supports * Serverless v2 scale-to-zero (minCapacity: 0). Minimum capable versions: @@ -429,9 +512,14 @@ class AuroraBuilder extends InfrastructureBuilder { } } - // Preserve other database config. - // NOTE: use `!== undefined` (not truthiness) so a requested minCapacity - // of 0 (scale-to-zero) flows through instead of being dropped. + // Mirror config into translated.database.postgres.config for completeness. + // NOTE: this is NOT the load-bearing path — createNewAurora/discoverAurora + // read the TOP-LEVEL database.postgres fields (preserved by the deep clone + // above), and 0-propagation for scale-to-zero comes from that clone plus the + // `?? 0.5` in the scaling config, not from here. These `.config.*` copies are + // kept only so any future consumer that reads `config` sees the same values; + // `!== undefined` (not truthiness) is used so a minCapacity of 0 is mirrored + // rather than dropped. if (appDefinition.database?.postgres?.minCapacity !== undefined) { translated.database.postgres.config.minCapacity = appDefinition.database.postgres.minCapacity; } @@ -679,6 +767,12 @@ class AuroraBuilder extends InfrastructureBuilder { throw new Error('database.postgres.endpoint is required when management="use-existing"'); } + // ADR-033: for use-existing, `connectivity` only affects TLS on the + // connection params — Frigg does not own the cluster, so it manages neither + // its subnets nor its ingress (the validator warns about this). Public mode + // still requires TLS, so append sslmode=require. + const publicConnectivity = dbConfig.connectivity === 'public'; + // Set environment variables for existing cluster result.environment.DATABASE_HOST = dbConfig.endpoint; result.environment.DATABASE_PORT = String(dbConfig.port || 5432); @@ -687,7 +781,9 @@ class AuroraBuilder extends InfrastructureBuilder { // Consumers that build DATABASE_URL from components at runtime MUST // append `?${DATABASE_URL_PARAMS}` to get the same hang-prevention // timeouts as the managed path. - result.environment.DATABASE_URL_PARAMS = LAMBDA_DATABASE_URL_QUERY_PARAMS; + result.environment.DATABASE_URL_PARAMS = publicConnectivity + ? `${LAMBDA_DATABASE_URL_QUERY_PARAMS}&sslmode=require` + : LAMBDA_DATABASE_URL_QUERY_PARAMS; console.log(` ✅ Using existing cluster: ${dbConfig.endpoint}`); } diff --git a/packages/devtools/infrastructure/domains/database/aurora-builder.test.js b/packages/devtools/infrastructure/domains/database/aurora-builder.test.js index e07724c8e..ee29c50c4 100644 --- a/packages/devtools/infrastructure/domains/database/aurora-builder.test.js +++ b/packages/devtools/infrastructure/domains/database/aurora-builder.test.js @@ -194,7 +194,7 @@ describe('AuroraBuilder', () => { const result = auroraBuilder.validate(appDefinition); expect(result.valid).toBe(false); - expect(result.errors.some(e => e.includes('minCapacity must be between 0.5 and 128'))).toBe(true); + expect(result.errors.some(e => e.includes('minCapacity must be 0 (scale-to-zero) or between 0.5 and 128'))).toBe(true); }); it('should error when maxCapacity is out of range', () => { diff --git a/packages/devtools/infrastructure/domains/networking/vpc-builder.js b/packages/devtools/infrastructure/domains/networking/vpc-builder.js index cb8ad7cdf..a64b8f41d 100644 --- a/packages/devtools/infrastructure/domains/networking/vpc-builder.js +++ b/packages/devtools/infrastructure/domains/networking/vpc-builder.js @@ -603,6 +603,34 @@ class VpcBuilder extends InfrastructureBuilder { console.log( ' ⊝ NAT Gateway skipped (database.postgres.connectivity=public — Lambda is not VPC-attached, so no NAT is needed)' ); + + // ADR-033: the public subnets Aurora sits in still need an Internet + // Gateway default route + subnet→route-table associations to be + // internet-routable. Those are normally emitted only as a SIDE EFFECT + // of the NAT build (createPublicRouting is called from inside the NAT + // methods), so skipping NAT would otherwise leave the public subnets on + // the VPC main route table (local-only) and the public Aurora endpoint + // unreachable — a green deploy with a dead DB. Decouple the public-subnet + // routing from NAT here. + // + // GUARD: only do this for a Frigg-created (stack) VPC — signalled by the + // presence of FriggInternetGateway in the template (emitted by + // buildVpcFromDecision only for STACK ownership). createPublicRouting + // references { Ref: 'FriggInternetGateway' } and DependsOn + // 'FriggVPCGatewayAttachment', and associates the stack-created + // FriggPublicSubnet* — all of which exist only in that case. For a + // discovered/existing VPC, its public subnets already route to an IGW, so + // creating our own public route table would be redundant/conflicting. + if (result.resources.FriggInternetGateway) { + console.log( + ' → Public-subnet routing (IGW default route + associations) for stack-created VPC' + ); + this.createPublicRouting(appDefinition, discoveredResources, result); + } else { + console.log( + ' ℹ Public connectivity on a discovered/existing VPC — assuming its public subnets already route to an Internet Gateway; not creating conflicting routing' + ); + } } else { // Build NAT Gateway based on ownership decision this.buildNatGatewayFromDecision(decisions.natGateway, appDefinition, discoveredResources, result); @@ -620,8 +648,10 @@ class VpcBuilder extends InfrastructureBuilder { this.buildVpcEndpointsFromDecisions(decisions.vpcEndpoints, decisions.securityGroup, appDefinition, discoveredResources, result); } - // Set VPC_ENABLED environment variable - result.environment.VPC_ENABLED = 'true'; + // Set VPC_ENABLED environment variable. + // ADR-033: in public connectivity the Lambda is NOT attached to the VPC, so + // report VPC_ENABLED=false — otherwise /health would misreport isInVpc:true. + result.environment.VPC_ENABLED = publicDbConnectivity ? 'false' : 'true'; console.log(`\n[${this.name}] ✅ VPC infrastructure built successfully`); console.log(` - VPC ID: ${result.vpcId || 'from discovery'}`);