diff --git a/README.md b/README.md index ac69c89..36b365d 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,11 @@ Engineering conventions and best practices for Sea Haven Industries. - [Commit Messages](commit-messages.md) -- imperative mood, 50/72 rule, explain "why" - [Pull Requests](pull-requests.md) -- scope, title, description format, merge strategy - [Code Review](code-review.md) -- what to look for, giving feedback, turnaround expectations +- [Code Review Rubric](code-review-rubric.md) -- BLOCK/FIX/NIT/QUESTION finding categories and output format - [GitHub Standards](github-standards.md) -- branch defaults, repo hygiene, Dependabot - [AWS Infrastructure](aws-infrastructure.md) -- SAM vs CDK, Lambda defaults, CloudFormation - [SAM Project Layout](sam-project-layout.md) -- standard directory structure for serverless projects +- [CDK Project Layout](cdk-project-layout.md) -- standard directory structure for CDK projects - [Lambda Starter Template](lambda-template.md) -- minimal SAM scaffold for a new Python Lambda - [Secrets and Configuration](secrets-and-config.md) -- Secrets Manager vs SSM Parameter Store - [CI/CD Pipelines](cicd.md) -- every deployable repo gets a pipeline, no manual deploys diff --git a/cdk-project-layout.md b/cdk-project-layout.md new file mode 100644 index 0000000..4a70ba4 --- /dev/null +++ b/cdk-project-layout.md @@ -0,0 +1,219 @@ +# CDK Project Layout + +## When to Use CDK + +SAM is the default for new serverless stacks (Lambda + API Gateway + DynamoDB). Use CDK when the infrastructure goes beyond what SAM handles cleanly: + +- ECS Fargate tasks +- VPCs, subnets, security groups +- Multi-service compositions (e.g., SES + Lambda + DynamoDB + Bedrock Agent) +- L2/L3 constructs not available in SAM (Bedrock agents, Knowledge Bases) +- Docker-based Lambda or container workloads + +If the project is a handful of Lambdas behind API Gateway, use SAM. See [sam-project-layout.md](sam-project-layout.md). + +## Standard Directory Structure + +``` +project-name/ +├── bin/ +│ └── app.ts # CDK app entry point +├── lib/ +│ └── project-name-stack.ts # Stack definition +├── lambdas/ +│ ├── function-name/ +│ │ ├── index.ts # Handler (Node) or app.py (Python) +│ │ └── package.json # Per-function deps (Node) or requirements.txt (Python) +│ └── shared/ # Shared utilities across functions (if needed) +├── cdk.json +├── package.json +├── package-lock.json +├── tsconfig.json +├── .gitignore +└── .github/ + └── workflows/ + ├── ci.yaml + └── deploy.yaml +``` + +## File Purposes + +### `bin/app.ts` + +The CDK app entry point. Instantiates the stack with an explicit `stackName` to enforce kebab-case (see Naming below). + +```typescript +import * as cdk from 'aws-cdk-lib'; +import { ProjectNameStack } from '../lib/project-name-stack'; + +const app = new cdk.App(); +new ProjectNameStack(app, 'ProjectNameStack', { + stackName: 'project-name', + env: { account: '328440206208', region: 'us-east-1' }, +}); +``` + +### `lib/project-name-stack.ts` + +All resource definitions. For larger projects, split into multiple constructs under `lib/` and compose them in the stack file. Keep the stack class thin -- it wires constructs together, not defines low-level resources. + +### `lambdas/` + +Lambda handler source code. One directory per function, same convention as SAM's `src/` directory. Each function has its own dependency file. CDK references these via `Code.fromAsset('lambdas/function-name')` or `NodejsFunction`'s `entry` property. + +### `cdk.json` + +CDK context and feature flags. Committed to the repo. Generated by `cdk init` -- keep the default `app` command pointing at `bin/app.ts`. + +## Naming + +CDK generates PascalCase stack names by default. Always set an explicit `stackName` to enforce kebab-case: + +```typescript +new MyStack(app, 'MyStack', { + stackName: 'my-stack', +}); +``` + +All resource names within the stack follow the same kebab-case convention: `my-stack-process-orders` for Lambdas, `my-stack-orders` for tables, `my-stack/api-key` for secrets. See [naming-conventions.md](naming-conventions.md). + +### Legacy stacks + +A handful of stacks predate this convention and remain PascalCase (e.g., `SeaHavenDoorUnlockStack`). Do not rename these -- changing the stack name requires replacement of all resources. New stacks must use kebab-case from day one. + +## Version Pinning + +Pin `aws-cdk-lib` to the blessed version: **2.253.1**. + +`aws-cdk-lib` bundles transitive dependencies (`inBundle: true`). Certain versions break `npm ci` with phantom missing-package errors that cannot be fixed via npm `overrides`. Always test before bumping: + +1. Update `package.json` to the new version +2. Run `rm -rf node_modules package-lock.json && npm install` +3. Run `npm ci` -- if it fails, the version is not safe +4. Run `npx cdk synth` -- if it fails, the version is not safe + +Dependabot will flag vulnerabilities in bundled transitive deps. Dismiss these alerts with **"waiting for upstream fix"** -- there is no action available until `aws-cdk-lib` publishes a patched release. + +See [aws-infrastructure.md](aws-infrastructure.md#cdk-version-policy) for more detail. + +## overrideLogicalId + +Never remove `overrideLogicalId` calls from existing resources. + +CDK auto-generates CloudFormation logical IDs with hash suffixes (e.g., `OrdersTable4A3B2C1D`). When you override the logical ID to something stable (e.g., `OrdersTable`), CloudFormation tracks the resource under that name. Removing the override changes the logical ID back to the hashed version, which CloudFormation interprets as: + +1. **Delete** the resource with the old logical ID +2. **Create** a new resource with the new logical ID + +For named resources (Secrets Manager secrets, S3 buckets, IAM roles), the create fails because the physical name already exists. For unnamed resources (DynamoDB tables without explicit names), the delete succeeds -- and takes your data with it. + +If you need to refactor construct tree paths, add new `overrideLogicalId` calls to preserve the existing logical IDs. Never remove existing ones without understanding the CloudFormation diff (`cdk diff`). + +## ARM64 Docker Builds + +When using `ContainerImage.fromAsset()` for ARM64 Fargate tasks (or any ARM64 container), two things are required: + +### 1. Platform flag in CDK + +```typescript +import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets'; + +ContainerImage.fromAsset('path/to/docker', { + platform: ecr_assets.Platform.LINUX_ARM64, +}); +``` + +Without the `platform` flag, CDK builds an x86 image regardless of what the Dockerfile or CI runner does. The container will crash on an ARM64 Fargate task with `exec format error`. + +### 2. QEMU in GitHub Actions + +The CI runner is x86. To build ARM64 images, the workflow needs QEMU: + +```yaml +- uses: docker/setup-qemu-action@v3 +``` + +Add this step before `cdk deploy` or `cdk synth` in the deploy workflow. Both the platform flag and QEMU are required -- one without the other produces a broken image. + +## Lambda Defaults + +Same defaults as SAM projects. Verify these on every Lambda in every CDK stack: + +| Setting | Value | +|---|---| +| Runtime | Python 3.12 or Node 24.x | +| Architecture | arm64 | +| Log retention | 60 days (explicit `RetentionInDays`) | +| Naming | kebab-case, prefixed with stack name | + +CDK's `NodejsFunction` and `PythonFunction` constructs auto-create log groups, but the default retention is **never expire**. Always set it explicitly: + +```typescript +new logs.LogGroup(this, 'HandlerLogs', { + logGroupName: `/aws/lambda/${fn.functionName}`, + retention: logs.RetentionDays.TWO_MONTHS, +}); +``` + +See [aws-infrastructure.md](aws-infrastructure.md#lambda-defaults). + +## CI/CD + +Same OIDC deploy role pattern as SAM. Each repo gets its own `githubdeploy-` IAM role -- never share deploy roles across repos. + +CDK projects use the TypeScript CDK reusable workflows: + +```yaml +# .github/workflows/ci.yaml +name: CI +on: + pull_request: + branches: [main] +jobs: + ci: + uses: Sea-Haven-Industries/.github/.github/workflows/ci-typescript-cdk.yaml@main + with: + node-version: "24" + +# .github/workflows/deploy.yaml +name: Deploy +on: + push: + branches: [main] +jobs: + deploy: + uses: Sea-Haven-Industries/.github/.github/workflows/cd-cdk.yaml@main + with: + node-version: "24" + secrets: + deploy-role-arn: ${{ secrets.AWS_DEPLOY_ROLE_ARN }} +``` + +Always pass `node-version: "24"` explicitly. See [cicd.md](cicd.md) for the full pipeline convention. + +## Bedrock Agents + +CDK is the standard IaC for Bedrock Agents and Knowledge Bases (SAM lacks L2 constructs for these). Key gotchas: + +- Use cross-region inference profile IDs for `CfnAgent.foundationModel`, not direct model IDs +- Bump the alias `description` to force version rotation after model or instruction changes +- `VectorKnowledgeBase` requires Docker Desktop on the build machine + +See [bedrock.md](bedrock.md) for inference profile formats, IAM permissions, alias version pinning, and action group conventions. + +## Standard .gitignore + +``` +cdk.out/ +node_modules/ +*.js +*.d.ts +*.js.map +.env +``` + +The `*.js` / `*.d.ts` / `*.js.map` entries assume TypeScript source with compiled output excluded from version control. If you have JavaScript Lambda handlers under `lambdas/`, add a negation: + +``` +!lambdas/**/*.js +``` diff --git a/code-review-rubric.md b/code-review-rubric.md new file mode 100644 index 0000000..c724cd0 --- /dev/null +++ b/code-review-rubric.md @@ -0,0 +1,36 @@ +# Code Review Rubric + +## Finding Categories + +| Category | Meaning | Action Required | +|---|---|---| +| BLOCK | Must fix before merge | Yes -- PR cannot merge | +| FIX | Should fix, strong recommendation | Yes -- unless explicitly deferred with issue | +| NIT | Optional improvement, style preference | No -- author's discretion | +| QUESTION | Needs clarification before reviewer can assess | Yes -- answer required | + +## Review Checklist + +- Correctness: does the code do what the PR says? +- Security: OWASP top 10, secrets handling, input validation at boundaries +- Sea Haven conventions: naming, secrets placement, Lambda defaults, IaC patterns +- Operational readiness: logging, error handling, monitoring, CI/CD +- Tests: appropriate coverage for the change + +## Output Format + +``` +Verdict: APPROVE | REQUEST CHANGES | NEEDS DISCUSSION + +### BLOCK +- **file.py:42** -- [problem]. Why it matters: [impact]. Fix: [suggestion]. + +### FIX +- ... + +### NIT +- ... + +### QUESTION +- ... +```