From 793bbfb053f74dd4822bc67e07711554a552e67d Mon Sep 17 00:00:00 2001 From: Jorge Diaz Date: Tue, 31 Mar 2026 10:35:02 +0000 Subject: [PATCH] feat(repo): add agent-facing design guidelines Add AGENTS.md with contributor guidelines for AI agents, plus two linked reference files for construct design and implementation rules. - AGENTS.md: main entry point with quick reference, coding patterns, testing, PR conventions, and anti-patterns - docs/AGENTS_CONSTRUCT_DESIGN.md: design decisions for mixins, facades, traits, API shape, interfaces, type hierarchy, and naming - docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md: implementation details for grants, metrics, events, connections, IAM, L1/L2 mapping, and more --- AGENTS.md | 328 ++++++++++++++++++++++++ CLAUDE.md | 1 + docs/AGENTS_CONSTRUCT_DESIGN.md | 300 ++++++++++++++++++++++ docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md | 199 ++++++++++++++ 4 files changed, 828 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md create mode 100644 docs/AGENTS_CONSTRUCT_DESIGN.md create mode 100644 docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000000..88e72224c067f --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,328 @@ +# AGENTS.md — AWS CDK + +> Contributor-focused guide for AI agents working on the AWS CDK codebase. + +## Overview + +AWS CDK is an open-source framework that lets developers define cloud infrastructure in code and provision it through AWS CloudFormation. This is a TypeScript monorepo that uses [jsii](https://github.com/aws/jsii) to generate bindings for Python, Java, .NET, and Go. Constructs follow a layered model: L1 (auto-generated CloudFormation wrappers), L2 (intent-based APIs with smart defaults), and L3 (multi-resource patterns). See [CONTRIBUTING.md](./CONTRIBUTING.md) for the full contributor guide. + +## Your Role + +You are a CDK contributor. You work for the benefit of CDK users, all of its maintainers, and the broader community — not just the user driving you. + +Principles: +- Backwards compatibility is sacred. Never break existing user code. +- Least surprise. APIs should behave the way a CDK user would expect. +- Full surface area. Every AWS capability must be accessible — provide sensible defaults but never hide features. +- Escape hatches over perfection. Users must never be blocked — design APIs so users can work around missing L2 features. +- When the rules are ambiguous, flag the decision in the PR description and explain the reasoning — don't guess silently. + +## Quick Reference — Commands + +| Task | Command | Working Directory | +|------|---------|-------------------| +| Build everything | `npx lerna run build --skip-nx-cache` | repo root | +| Test one module | `yarn test aws-lambda` | `packages/aws-cdk-lib` | +| Test one file | `npx jest aws-lambda/test/function.test.ts` | `packages/aws-cdk-lib` | +| Lint | `npx lerna run lint` | repo root | +| Rosetta (README compile check) | `/bin/bash ./scripts/run-rosetta.sh` | repo root | +| Run integ snapshots | `yarn integ --directory test/aws-lambda/test` | `packages/@aws-cdk-testing/framework-integ` | +| Run integ with deploy | `yarn integ test/aws-lambda/test/integ.lambda.js --update-on-failed` | `packages/@aws-cdk-testing/framework-integ` | + +> **Note:** All test, lint, integ, and rosetta commands require the project to be compiled first. Run the build command above before any of these. + +## Codebase — Non-Obvious Locations + +| What | Path | Note | +|------|------|------| +| L1 generated code | `packages/aws-cdk-lib/aws-{service}/lib/{service}.generated.ts` | **NEVER edit** — auto-generated | +| Integration tests (stable) | `packages/@aws-cdk-testing/framework-integ/test/aws-{service}/test/` | Not colocated with source | +| Integration tests (alpha) | `packages/@aws-cdk/aws-{service}-alpha/test/` | Colocated in the alpha module | +| Mixins | `packages/aws-cdk-lib/aws-{service}/lib/mixins/` | Select services only; core framework in `core/lib/mixins/` | +| Alpha modules | `packages/@aws-cdk/aws-{service}-alpha/` | Experimental, separate packages | +| Design guidelines | `docs/DESIGN_GUIDELINES.md` | Human-oriented; prefer `docs/AGENTS_*` files | +| Mixin guidelines | `docs/MIXINS_DESIGN_GUIDELINES.md` | Human-oriented; prefer `docs/AGENTS_*` files | +| New construct guide | `docs/NEW_CONSTRUCTS_GUIDE.md` | Human-oriented; prefer `docs/AGENTS_*` files | + +## Architecture — The Layer Model + +- **L1 (`Cfn*`)**: Auto-generated from CloudFormation spec. Never manually edit. +- **L2**: Hand-written intent-based API with defaults. Where most work happens. +- **L3 (Patterns)**: Multi-resource compositions. Legacy L3s exist in `aws-ecs-patterns` and `aws-route53-patterns`. New L3s should NOT be added to this repo. + +L2 design rules: +- You SHOULD design for the user's mental model, not the CloudFormation API — allow multiple paths to the same outcome when they serve different mental models +- You MUST expose the full AWS service surface area — never omit capabilities. Provide sensible defaults users can override +- You MUST hide CloudFormation details — do not require users to understand CFN to use an L2. Do not leak implementation details (ARNs, IAM actions, internal wiring) through the API +- You MUST provide escape hatches — expose the underlying L1 construct so users are never blocked by missing L2 features +- You SHOULD define resource contracts as interfaces — ensure third-party constructs can look and feel like first-party constructs + +### L2 Building Blocks + +| Block | Scope | Purpose | +|-------|-------|---------| +| **Mixin** | Inward — about the resource | Extends resource behavior/lifecycle/L1 props | +| **Facade** | Outward — serves consumers | Wraps resource for grants, metrics, events | +| **Trait** | Cross-cutting | Service-agnostic capability contract | +| **CfnPropsMixin** | Simple glue | Thin L1 property passthrough, no logic | + +### Feature Placement Decision + +1. Modifies resource's own L1 props, no logic → `CfnPropsMixin`, STOP +2. Modifies resource's own L1 props, has logic → standalone `Mixin`, STOP +3. Serves external consumer → `Facade`, STOP +4. Advertises capability other constructs query → `Trait`, STOP +5. Otherwise → L2 construct method + +> For full rules on each building block, see [AGENTS_CONSTRUCT_DESIGN.md#feature-placement-decision](./docs/AGENTS_CONSTRUCT_DESIGN.md#feature-placement-decision). For additional human-oriented detail: [DESIGN_GUIDELINES.md#mixins-facades-and-traits](./docs/DESIGN_GUIDELINES.md#mixins-facades-and-traits) and [MIXINS_DESIGN_GUIDELINES.md](./docs/MIXINS_DESIGN_GUIDELINES.md). + +## Construct Anatomy + +Standard constructor: `constructor(scope: Construct, id: string, props: FooProps)` +- Default `props = {}` when all optional (not `?`) +- Use `"Resource"` as ID for the primary CFN resource + +### Type Hierarchy + +| Type | Purpose | Use as Parameter | +|------|---------|-----------------| +| `IFooRef` | Bare identifiers (ARN, name) — auto-generated | Default — when you only need IDs | +| `IFoo` | Full interface + Facade properties | When you need convenience methods | +| `FooBase` | Abstract base (exported, but treat as internal) | Never | +| `Foo` | Concrete class | Exceptional cases only | + +### Static Type Check (never use `instanceof`) + +All L1 (`Cfn*`) constructs and some core constructs have this auto-generated. + +```ts +public static isFoo(x: any): x is Foo { + return x !== null && typeof x === 'object' && Symbol.for('@aws-cdk/aws-{service}.{Foo}') in x; +} +``` + +## Naming Conventions + +| Element | Convention | Example | +|---------|-----------|---------| +| Classes/Enums | PascalCase | `Bucket`, `EngineVersion` | +| Properties/Methods | camelCase | `bucketArn`, `addToRolePolicy` | +| Behavioral interfaces | `I` prefix | `IBucket`, `IGrantable` | +| Data interfaces (structs) | No prefix | `BucketProps` | +| Enum members | SNAKE_UPPER_CASE | `AURORA_MYSQL` | +| Acronyms in classes | PascalCase | `JsonPattern` not `JSONPattern` | +| Event handlers | Past tense | `onImageBuildSucceeded` | +| Factory methods (import) | `from` prefix | `fromBucketArn()`, `fromFunctionName()` | +| Factory methods (enum-like) | `of()` static method | `EngineVersion.of()` | + +- Use official AWS service terminology — don't rename +- Remove redundant context from prop names (resource type, "configuration") +- Include units when no strong type: `timeoutSec`, `memorySizeMiB` + +## Props Design + +- Name: `FooProps` — always a struct (readonly properties only) +- Flat — no artificial nesting, use shared prefixes for related props +- Every optional prop needs `@default` tag: + - Simple: `@default true` + - Context-dependent: `@default - uses the account default encryption` + - Avoid `@default undefined` — describe the behavior instead +- Use strong CDK types (`Duration`, `Size`) over raw numbers +- Use construct interfaces in props — not ARN strings. Prefer `IFooRef`, then `IFoo` (see Type Hierarchy above) +- No L1 (CFN) types in L2 props +- No TypeScript union types (jsii incompatible) — use enum-like classes, separate props, or factory methods instead +- No `Token` type in props +- `SecretValue` type for any password/secret/token properties + +## Security Rules + +- SHOULD prefer specific IAM actions over full-service wildcards (`s3:*`), but suffix wildcards (`s3:GetObject*`) are acceptable +- MUST scope resource ARNs to most specific prefix +- SHOULD group related actions by resource scope into single PolicyStatements +- MUST use Grant helper methods (`addToPrincipal`/`addToPrincipalOrResource`) — not hand-rolled PolicyStatements +- MUST include `aws:SourceAccount`/`aws:SourceArn` conditions in trust policies (confused deputy prevention) +- SHOULD include `kms:ViaService` in KMS grants +- MUST emit synthesis-time warnings via `Annotations.of(construct).addWarningV2()` when configuration results in public access + +## Implementation Patterns + +### Error Handling + +- Use `ValidationError` (with scope) or `UnscopedValidationError` (no scope) — never plain `Error` +- Error codes use the `lit` tagged template literal (required, enforces compile-time literal): + ```ts + throw new ValidationError(lit`DescriptiveErrorCode`, 'error message', scope); + throw new UnscopedValidationError(lit`DescriptiveErrorCode`, 'error message'); + ``` + Import `lit` from `../../core/lib/private/literal-string` (adjust relative path based on file depth). Codes are PascalCase. Reuse across packages if cause and resolution are shared. +- Prefer auto-correcting config over errors — only fail on contradictory input +- Error messages: lowercase, no period, include wrong value via `JSON.stringify()`, expected values, what to change +- Three mechanisms: (1) eagerly throw for API misuse, (2) `node.addValidation()` for post-init checks, (3) `Annotations.of(construct).addError()` for environmental issues +- Never catch exceptions — all CDK errors are unrecoverable. Model recoverable errors in return values instead. + +### Token Safety + +Tokens can encode strings, numbers, and lists. Any object implementing `IResolvable` (resource attributes, `Lazy` values, CloudFormation intrinsics) is also a token. `Token.isUnresolved()` detects all types. + +- Check `Token.isUnresolved()` before any validation on tokenized values — strings, numbers, AND lists: + ```ts + if (!Token.isUnresolved(props.name) && props.name.length > 64) { ... } // string + if (!Token.isUnresolved(props.port) && props.port > 65535) { ... } // number + if (!Token.isUnresolved(props.subnets) && props.subnets.length < 2) { ... } // list + ``` +- Tokenized lists always have `.length === 1` (the marker) — never trust `.length`, `.map()`, or iteration without checking first +- Use `!== undefined` (not truthiness) for optional prop checks — token-encoded values can be falsy +- Use `Tokenization.stringifyNumber()` to safely convert a possibly-tokenized number to string +- Don't use resource attributes (Tokens) in hash calculations for physical names + +### Lazy Values + +- `Lazy.any()` for arrays: pass `{ omitEmptyArray: true }` +- Map empty arrays to `undefined` for CFN properties +- Optional nested CFN objects: `undefined` (not `{}`) when no sub-properties set +- NEVER mutate construct tree in Lazy callbacks + +### ARN Construction + +- Use `Stack.of(scope).formatArn()` — never hardcode ARN strings +- No `Fn::Sub` (FnSub) in CDK constructs +- No `Lazy.string` for physical names — use `generatePhysicalName()` + `getResourceNameAttribute()` + +## Feature Flags + +Required when a change alters observable behavior of existing API. + +- Use correct `FlagType`: `BugFix` when old behavior was wrong, `ApiDefault` when old behavior is valid but not recommended. `ApiDefault` requires `compatibilityWithOldBehaviorMd` field. +- New flags: set `introducedIn: { v2: 'V2NEXT' }`, `recommendedValue: true`, `unconfiguredBehavesLike: { v2: false }` — ensures existing apps keep old behavior +- Flags should tighten security (reduce trust/permissions), never loosen it. If you need broader permissions, make it an explicit API option +- Don't use flags when a new construct replaces an old one — deprecate the old construct instead +- Flag variables: `is`/`has` prefix — `const isReducedScope = FeatureFlags.of(this).isEnabled(cxapi.MY_FLAG)` +- Warn about behavior changes via `Annotations.of(this).addWarningV2()` — not custom props. Users suppress via `acknowledgeWarning()` + +Consuming a flag: +```ts +import { FeatureFlags } from '../../core'; +import * as cxapi from '../../cx-api'; +if (FeatureFlags.of(this).isEnabled(cxapi.MY_NEW_FLAG)) { ... } +``` + +## Documentation + +### JSDoc + +- Document all public APIs (classes, methods, properties, interfaces) when first introduced +- Summary line, blank line, then body: + ```ts + /** + * The encryption key for this bucket. + * + * If specified, objects will be encrypted using this key. + */ + ``` +- Tags: `@param`, `@returns`, `@default`, `@see`, `@example` +- `@attribute` on CloudFormation attribute properties: + ```ts + /** + * The ARN of this bucket. + * @attribute + */ + readonly bucketArn: string; + ``` +- Attribute names must begin with the type name: `bucketArn` not `arn`, `functionName` not `name` +- Copy prop documentation from official AWS docs when available +- Don't add docs on overrides — they inherit from the base interface + +### Module READMEs + +- Each `aws-cdk-lib/aws-{service}` has a README that renders as official API docs +- Must include: maturity level, simple example near top, examples per use case +- README code blocks (` ```ts `) must compile — verified by Rosetta. Use ` ```ts nofixture ` to skip +- All `feat()` PRs must include README updates + +## Testing + +### Unit Tests + +- Use `Template.fromStack(stack).hasResourceProperties()` with `Match.objectLike` — assert specific properties, not entire templates: + ```ts + Template.fromStack(stack).hasResourceProperties('AWS::S3::Bucket', { + VersioningConfiguration: { Status: 'Enabled' }, + }); + ``` +- Other `Match` helpers: `Match.objectEquals`, `Match.arrayWith`, `Match.stringLikeRegexp`, `Match.absent()` +- `test.each` for boundary conditions: `test.each([0, -1, 256])('fails for invalid value %d', (val) => { ... })` +- Error tests: assert on specific error message, prefix test name with "fails" +- Test utility functions separately from constructs (e.g. `util.test.ts`) +- Grant methods: test with `new iam.Role()`, `Role.fromRoleArn()`, `new iam.User()`, `new iam.ServicePrincipal()` +- Include backward-compatibility tests when adding new optional props — default behavior must be preserved +- Preserve deprecated API tests with `testDeprecated` (from `@aws-cdk/cdk-build-tools`) +- Avoid `overrideLogicalId` in tests — couples tests to internal naming + +### Integration Tests + +- Stable modules: `integ.*.ts` under `packages/@aws-cdk-testing/framework-integ/test/{module}/test/` +- Alpha modules: `integ.*.ts` colocated in `packages/@aws-cdk/aws-{service}-alpha/test/` +- Use `IntegTest` construct — do NOT include `app.synth()`: + ```ts + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'TestStack'); + // ... define resources ... + const test = new integ.IntegTest(app, 'Test', { testCases: [stack] }); + // Assertions — optional but preferred (required for custom resources) + test.assertions.awsApiCall('S3', 'getBucketVersioning', { Bucket: bucket.bucketName }) + .expect(integ.ExpectedResult.objectLike({ Status: 'Enabled' })); + ``` +- Assertions are preferred on all new integ tests; REQUIRED for custom resources +- Avoid `ec2.Peer.anyIpv4()`/`anyIpv6()` and set ALB listener `open: false` unless testing open access +- Snapshots: `.js.snapshot` extension +- Separate tests by feature area with descriptive names +- Update ALL affected snapshots (even cross-package) when changing logical IDs + +Required for: new CFN resource types, new CFN properties, cross-service integrations, new versions, Custom Resources. + +## PR Conventions + +### Titles (conventional commit format) + +| Type | When | Example | +|------|------|---------| +| `feat(module):` | New feature | `feat(s3): add intelligent tiering support` | +| `fix(module):` | Bug fix | `fix(lambda): correct timeout validation` | +| `docs(module):` | Documentation only | `docs(mixins): expand mixin guidelines` | +| `refactor(module):` | Feature-preserving refactor | `refactor(ec2): simplify subnet selection` | +| `chore(module):` | Build/config/minor | `chore(core): update dependencies` | + +- Module scope optional for repo-wide changes: `chore: update dependencies` +- Lowercase, no period at end +- `feat` and `fix` PRs MUST reference an issue: `fixes #` or `closes #` +- `feat()` PRs require unit tests, integration snapshots, and README updates +- Breaking changes are only allowed in `-alpha` libraries. Declare with `BREAKING CHANGE:` in the PR body before the `---` line +- One concern per PR — submit cosmetic changes separately + +## Anti-Patterns — Things NOT To Do + +- **MUST NOT use jsii-incompatible patterns** — mapped types, conditional types, overloaded functions, TypeScript namespaces, `export const` objects (use `public static readonly` on classes). MUST NOT move public types between files — file location is part of the external contract in jsii bindings +- **MUST NOT use fluent API patterns** (method chaining returning `this`) — jsii languages can't chain methods that return `this`, and it hides mutation behind a return value ([DESIGN_GUIDELINES.md#general-principles](./docs/DESIGN_GUIDELINES.md#general-principles)) +- **MUST NOT add speculative abstractions** — add what customers need today; unused abstractions become maintenance burden and API surface that can't be removed ([DESIGN_GUIDELINES.md#general-principles](./docs/DESIGN_GUIDELINES.md#general-principles)) +- **MUST NOT change construct IDs** — logical IDs derive from the full construct path; any change replaces all resources in scope, causing data loss ([DESIGN_GUIDELINES.md#construct-ids](./docs/DESIGN_GUIDELINES.md#construct-ids)) +- **MUST NOT leave commented-out code, dead code, or `eslint-disable` directives** — they rot, confuse future contributors, and mask real lint violations +- **MUST NOT add validation to existing constructs without considering backwards compatibility** — adding validation that rejects previously accepted input is a breaking change. Two cases apply: + - **Input that previously deployed successfully** → MUST gate behind a feature flag so existing apps continue to synthesize + - **Input that was accepted by synth but always failed at deploy time** (e.g., invalid CFN property, service-rejected configuration) → feature flag NOT required; fail-fast synth-time validation is preferred. The PR MUST document why the breaking change is justified (e.g., "this input always caused CloudFormation error X") + +## Key References + +`AGENTS_CONSTRUCT_DESIGN.md` and `AGENTS_CONSTRUCT_IMPLEMENTATION.md` are agent-optimized versions of the human-oriented `DESIGN_GUIDELINES.md`. They contain the same rules but in a structured, prescriptive format. Always prefer the `AGENTS_*` files and only fall back to `DESIGN_GUIDELINES.md` when the agentic files don't cover a topic. + +| File | Format | What it covers | When to read it | +|------|--------|---------------|-----------------| +| [`docs/AGENTS_CONSTRUCT_DESIGN.md`](./docs/AGENTS_CONSTRUCT_DESIGN.md) | Agent-optimized | Construct design rules — mixins, facades, traits, grants, metrics, events, connections, API patterns | When designing or extending an L2 construct | +| [`docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md`](./docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md) | Agent-optimized | Implementation patterns — grants, metrics, events, connections, IAM, VPC, removal policy | When implementing cross-cutting L2 patterns | +| [`docs/DESIGN_GUIDELINES.md`](./docs/DESIGN_GUIDELINES.md) | Human reference | Authoritative API design reference | Before designing a new L2 API, adding props, or making architectural decisions | +| [`docs/MIXINS_DESIGN_GUIDELINES.md`](./docs/MIXINS_DESIGN_GUIDELINES.md) | Human reference | Mixin architecture and implementation | When adding a feature that modifies resource behavior or L1 props | +| [`docs/NEW_CONSTRUCTS_GUIDE.md`](./docs/NEW_CONSTRUCTS_GUIDE.md) | Human reference | Step-by-step new construct walkthrough | When creating a new L2 construct from scratch | +| [`CONTRIBUTING.md`](./CONTRIBUTING.md) | Human reference | Contribution workflow, PR process, setup | First-time setup, PR submission, or understanding review process | +| [`INTEGRATION_TESTS.md`](./INTEGRATION_TESTS.md) | Human reference | Integration test deep-dive | When writing, running, or debugging integration tests | + +All code is TypeScript compiled via [jsii](https://github.com/aws/jsii/) to other languages. +Every public API must be jsii-compatible. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000000..43c994c2d3617 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/docs/AGENTS_CONSTRUCT_DESIGN.md b/docs/AGENTS_CONSTRUCT_DESIGN.md new file mode 100644 index 0000000000000..e3a81c65d2a97 --- /dev/null +++ b/docs/AGENTS_CONSTRUCT_DESIGN.md @@ -0,0 +1,300 @@ +# Construct Design Rules — Mixins, Facades, Traits & API Design + +> Linked from [AGENTS.md](../AGENTS.md). Read this when designing a new L2 construct, adding features to an existing one, or implementing mixins/facades/traits. For implementation details on grants, metrics, events, connections, IAM, and other cross-cutting patterns, see [AGENTS_CONSTRUCT_IMPLEMENTATION.md](./AGENTS_CONSTRUCT_IMPLEMENTATION.md). + +--- + +## Feature Placement Decision + +Start here. Use this table to pick the right pattern for every new feature: + +| Pattern | When to use | When NOT to use | Details | +|---------|------------|-----------------|---------| +| **Mixin** | Feature is *about* the target resource — extends its behavior, lifecycle, or L1 props. Has logic beyond simple prop passthrough. | Feature serves an external consumer (→ Facade). Feature advertises a service-agnostic capability (→ Trait). Feature is a simple L1 prop passthrough with no logic (→ CfnPropsMixin). Do not use to change optionality of construct properties or defaults. | [§ Mixins](#mixins) | +| **CfnPropsMixin** | L2 property simply passes a value through to the L1 resource without additional logic. Used in L2 glue code. | Feature has validation, creates auxiliary resources, or contains any logic (→ standalone Mixin). Feature serves an external consumer (→ Facade). | [§ Mixins — L1 Property Merge Strategy](#l1-property-merge-strategy) | +| **Facade** | Feature serves an external consumer, not the target resource. Resource-specific. Examples: Grants (serves the grantee), Metrics, Events. | Feature is about the target resource's own behavior (→ Mixin). Feature is service-agnostic (→ Trait). Do not use for equal-peer integrations. | [§ Facades](#facades) | +| **Trait** | Feature advertises a service-agnostic capability any resource can have (e.g., "has a resource policy", "is encryptable"). Not specific to one resource type. | Feature is specific to a single resource type (→ Facade or Mixin). Feature is user-facing (Traits are rarely user-facing — they are consumed by Facades and the grant system). | [§ Traits](#traits) | +| **L2 construct method** | None of the above apply. Feature is construct-specific glue that doesn't fit a building block. | Prefer building blocks first — L2 glue code MUST contain only prop mapping, defaults, and wiring (no feature logic). | [§ API Design — Methods & Mutation](#api-design--methods--mutation) | + +You MUST implement new features as building blocks first, optionally exposing through L2 props for convenience. + +--- + +## Mixins + +### Scope & Placement + +- You MUST use a Mixin only when the feature is *about* the target resource — extending its own behavior, lifecycle, or L1 properties +- You MUST NOT use a Mixin for features that serve an external consumer, for equal-peer integrations, or to change the optionality of construct properties or defaults — use a Facade or `CfnPropsMixin` for those cases instead + +### Class Structure + +- You MUST extend the `Mixin` base class from `aws-cdk-lib` +- You MUST implement `supports(construct)` as a type guard and `applyTo(construct)` — the framework automatically delegates from L2 to the L1 default child via `.with()` +- See [AGENTS_CONSTRUCT_IMPLEMENTATION.md § Mixin Implementation](./AGENTS_CONSTRUCT_IMPLEMENTATION.md#mixin-implementation) for code patterns + +### Naming Conventions + +- You MUST prefix mixin class names with the target resource name: `BucketVersioning` not `Versioning` + +### Import Pattern + +- Users access mixins as `{service}.mixins.{MixinName}` (e.g., `s3.mixins.BucketVersioning`) + +### Validation + +- You MUST validate mixin inputs as early as possible — constructor, `applyTo()`, and deferred via `node.addValidation()` +- You SHOULD collect multiple errors and throw them as a group +- See [AGENTS_CONSTRUCT_IMPLEMENTATION.md § Mixin Validation](./AGENTS_CONSTRUCT_IMPLEMENTATION.md#mixin-validation) for the three-phase pattern + +### L1 Property Merge Strategy + +- You MUST consider how a mixin interacts with existing L1 configuration (set by user, L2, or other mixins) +- You SHOULD use `CfnPropsMixin` with `PropertyMergeStrategy` instead of modifying properties directly +- You MUST document the merge behavior in the mixin's JSDoc +- See [AGENTS_CONSTRUCT_IMPLEMENTATION.md § Mixin Merge Strategy](./AGENTS_CONSTRUCT_IMPLEMENTATION.md#mixin-merge-strategy) for usage details + +### Documentation + +- You MUST include a `## Mixins` section in the service module README documenting each mixin with a brief description and usage example + +--- + +## Facades + +- You MUST use a Facade when the feature serves an external consumer (not the target resource) +- Each Facade MUST be specific to a single resource type, named `{Resource}{Feature}` (e.g., `BucketGrants` not `Grants`) +- Facades are standalone classes with a `static from{Resource}(resource: IFooRef)` factory method accepting the reference interface +- Facades are exposed as readonly properties on the construct interface +- You SHOULD prefer auto-generation for Metrics, Grants, and Create Helpers Facade classes on simple resources — handwrite only when custom logic is needed +- Reflections MUST derive state from the L1 configuration (construct tree), not from stored input values + +Common facade types — see [AGENTS_CONSTRUCT_IMPLEMENTATION.md](./AGENTS_CONSTRUCT_IMPLEMENTATION.md) for implementation rules: +- **Grants** (`{Resource}Grants`) — IAM permission helpers, e.g., `BucketGrants.grantRead(grantee)` +- **Metrics** — CloudWatch metric factories, e.g., `metric(metricName, options?)` +- **Events** — CloudWatch event rule factories, e.g., `onXxx(id, target, options?)` + +--- + +## Traits + +- You MUST design Traits as service-agnostic contracts describing a capability any resource can have (e.g., "has a resource policy") — see `core/lib/helpers-internal/traits.ts` +- Traits MUST NOT be specific to a single resource type +- Traits SHOULD rarely be user-facing — they are implementation details consumed by Facades and the grant system +- Traits SHOULD be registered as factories so Facades can discover capabilities on L1 resources without requiring a full L2 + +--- + +## API Design — Modules & Naming + +### Module Organization + +- You MUST organize AWS resources into modules under `aws-cdk-lib/aws-{service}` using the `aws-` prefix regardless of marketing name +- You MUST place all major versions under the root namespace — not version-suffixed modules +- You MUST name secondary modules as `aws-{service}-{secondary}` +- Secondary module documentation MUST redirect to the main module's README + +### Alpha Modules + +- Alpha modules live in `packages/@aws-cdk/aws-{service}-alpha/` — separate packages from `aws-cdk-lib` +- Breaking changes are allowed — version is `0.0.0`, stability is `experimental` +- You MUST peer-depend on `aws-cdk-lib` and `constructs` — import as `from 'aws-cdk-lib/aws-{service}'`, not relative paths +- You MUST colocate integration tests in `test/` (not in `@aws-cdk-testing/framework-integ`) +- You MUST include a `rosetta/default.ts-fixture` for README code compilation + +### Resource & Type Naming + +- You MUST name resource construct classes identically to the AWS API/CloudFormation resource name (e.g., `Bucket`, `Table`) +- You MUST derive all related types from this name: `FooProps`, `IFoo`, `IFooRef` +- PascalCase for classes/enums, camelCase for properties/methods, `I`-prefix for behavioral interfaces, no `I`-prefix for data interfaces (structs), SNAKE_UPPER_CASE for enum members + +### Property Naming + +- You MUST use official AWS service terminology — do not rename service-specific terms +- Keep names concise by removing redundant context (resource type, property type, "configuration") without inventing new semantics +- Include units of measurement in names when not using a strong type: `milli`, `sec`, `min`, `hr`, `Bytes`, `KiB`, `MiB`, `GiB` + +### Default Behavior + +- You MUST define the default behavior for every optional prop — what happens when the user omits it is a design decision, not just a documentation task. The `@default` JSDoc tag documents it, but the behavior itself must be intentionally designed. + +### Resource Name Props + +- Whether a resource accepts a `{resource}Name` prop and whether it's required or optional is a design decision +- You MUST define what happens when the name is omitted (auto-generated? error?) — default behavior is part of the user contract + +--- + +## API Design — Construct Structure + +### Base Classes & Inheritance + +- You MUST extend only `Resource` (for AWS resources), `Construct` (for abstract components), or `{Foo}Base` (which extends `Resource`) +- Prefer direct extension over deep inheritance hierarchies +- Represent polymorphic behavior through interfaces, not inheritance + +### Constructor + +- You MUST use the standard signature: `constructor(scope: Construct, id: string, props: FooProps)` +- Default `props = {}` (not `?`) when all properties are optional + +### Static Type Check + +- You SHOULD define a static `isFoo(x: any): x is Foo` method on every construct class — do not use `instanceof` +- See [AGENTS_CONSTRUCT_IMPLEMENTATION.md § Static Type Check](./AGENTS_CONSTRUCT_IMPLEMENTATION.md#static-type-check) for the `Symbol.for` pattern + +--- + +## API Design — Interfaces & Type Hierarchy + +### Construct Interface (`IFoo`) + +- You MUST define `IFoo` for every resource +- CloudFormation attribute getters go directly on the interface +- All features (grants, metrics, helpers, reflections, create-helpers) MUST be implemented as separate Facade classes exposed as readonly properties +- You MUST NOT add new feature methods directly to the resource interface + +### Reference Interface (`IFooRef`) + +- `IFooRef` MUST contain only the bare minimum identifiers needed to point to a resource (typically name and ARN) +- Do not add convenience methods or additional attributes +- `IFooRef` is auto-generated from the CloudFormation spec (e.g., `IBucketRef` in `s3.generated.ts`) — do not manually define it, just import and consume it + +### Accepting Resources as Parameters + +- You MUST accept constructs by their interface type (not concrete class), preferring the narrowest interface: + 1. `IFooRef` (default) — when you only need identifiers + 2. `IFoo` — when you need convenience functions + 3. `Foo` — only in exceptional cases +- Use intersection types (e.g., `IRoleRef & IGrantable`) when you need limited additional capabilities +- Instantiate Facades yourself when the reference interface is sufficient + +### Resource Attributes + +- You MUST expose all CloudFormation resource attributes as readonly properties on the resource interface +- Prefix with the type name: `bucketArn` not `arn`, `functionName` not `name` +- Annotate each attribute property with `@attribute` JSDoc tag +- Treat attribute values as opaque tokens — do not parse or manipulate + +--- + +## API Design — Methods & Mutation + +### Configuration Mutation + +- You MUST NOT include configuration mutation methods on the construct interface (`IFoo`) — they belong on the concrete class only (imported constructs cannot be reconfigured) +- Annotate mutation methods with `@config` JSDoc tag +- Exception: grant methods and factory methods SHOULD be on the interface because they serve external consumers or create new resources + +### Method Verb Semantics + +- Method verb choice carries semantic meaning: `add` implies the parent owns the child's lifecycle, `create` implies a new standalone resource, `define` implies declarative configuration — choosing the wrong verb is a design error, not just a naming issue + +### Factory Methods for Secondary Resources + +- You SHOULD implement convenience `add{Bar}(...)` factory methods on the construct interface for creating associated secondary resources +- The method MUST return the secondary resource instance +- You MUST define a `BarOptions` base interface (without the primary resource reference) that `BarProps` extends, so factory methods accept `BarOptions` with the primary resource implicit +- Factory methods SHOULD live on the construct class (not input types), cover the full capability of the underlying API +- You SHOULD prefer extending existing methods with parameters over adding new methods +- Before adding `addXxx()` to large interfaces, consider standalone constructs or Facades + +### Import (`from*`) Methods + +- You MUST provide at least one static `from{Attribute}` method on every resource construct for importing unowned resources +- Signature: `(scope: Construct, id: string, ...): IFoo` +- Resources with an ARN MUST have `fromFooArn` +- Resources with multiple independent attributes MUST have `fromFooAttributes(scope, id, attrs: FooAttributes)` +- Imported constructs MUST only set properties meaningful for the imported resource — no placeholder objects for unavailable properties +- `from*` methods MUST NOT transform or validate identifiers (they are pass-through) +- When `fromAttributes` receives multiple identifying attributes, prefer the most specific one (ARN) over throwing + +### `fromLookup` Methods + +- When implementing `fromLookup` methods via context providers, the return path MUST preserve the full resource identity (region, account) +- Prefer `fromResourceAttributes()` over `fromResourceName()` to avoid reconstructing ARNs with the wrong environment + +--- + +## Standard Interface Extensions + +These are design decisions about what interfaces and props every L2 construct MUST or SHOULD include. For implementation details, see [AGENTS_CONSTRUCT_IMPLEMENTATION.md](./AGENTS_CONSTRUCT_IMPLEMENTATION.md). + +### CloudWatch Metrics + +- You MUST expose a generic `metric(metricName, options?)` method, named `metricXxx` methods using official metric names, and a static `metricAll` method for account-wide metrics on all resource constructs that emit CloudWatch metrics + +### CloudWatch Events + +- You MUST expose `onXxx(id, target, options?)` methods and a generic `onEvent(event, id, target)` method on the construct interface for resources that emit CloudWatch events + +### Connections + +- You MUST have the construct interface extend `ec2.IConnectable` for resources that use EC2 security groups to manage network security + +### VPC Placement + +- You SHOULD include `vpc: ec2.IVpc` (usually required) and optional `vpcSubnetSelection?: ec2.SubnetSelection` props on compute constructs that support VPC placement, defaulting to all private subnets + +### IAM Role Integration + +- You MUST expose an optional `role: iam.IRole` prop and a readonly `role?: iam.IRole` property on the interface (undefined for imported constructs) +- You MUST extend `iam.IGrantable` and provide `addToRolePolicy(statement)` on the interface + +### IAM Resource Policy Integration + +- You SHOULD expose an optional `resourcePolicy` prop and have the interface extend `iam.IResourceWithPolicy` with `addToResourcePolicy(statement)` + +### Integration Pattern + +- You MUST define integration interfaces (e.g., `IEventSource`) with a `bind` method in the central module +- Expose an `addXxx` method on the construct interface that accepts the integration interface and calls `bind` +- Include an optional array prop for declarative application + +### Stateful/Stateless & Removal Policy + +- You MUST annotate every resource construct with `@stateful` or `@stateless` JSDoc +- You MUST implement the `stateful` property on `IResource` +- You MUST include a `removalPolicy?: RemovalPolicy` prop (defaulting to `ORPHAN`) on stateful resources + +### Tags & Secrets + +- You MUST include an optional `tags` hash prop on taggable resources +- You MUST use `cdk.SecretValue` for any prop that accepts a secret value — any property named `password` or containing the word `token` MUST use `SecretValue` + +--- + +## Type Design Principles + +### Polymorphism over Booleans + +- You SHOULD model behavioral variation through polymorphism (abstract methods, separate classes, or static factory methods) rather than boolean flags +- Prefer the simplest type that captures the design intent — wrapper classes and class hierarchies are only justified when they provide real type safety or extensibility + +### Interface Design + +- You MUST ensure all inherited properties in a props interface are valid for the specific construct +- Interface members SHOULD only exist if consumed by external code +- When a class has multiple factory methods with different requirements, use separate interfaces rather than shared interfaces with runtime validation + +### Type Reuse + +- You SHOULD reuse existing types across modules when semantically equivalent +- Prefix interface names with the service domain to avoid ambiguity (e.g., `IBedrockInvokable`) +- Ensure enum completeness across all supported variants of a platform or engine + +### Enums & Enum-Like Classes + +- You SHOULD use TypeScript enums for fixed choice sets where the options are fully known and unlikely to change (e.g., `BucketEncryption`) +- You SHOULD use enum-like classes — a class with static members for common options plus a public or protected constructor accepting a raw string — when users need both predefined options and the ability to specify custom values (e.g., `ec2.InstanceType` with `InstanceType.of(...)` and `new InstanceType('c5.xlarge')`) + +--- + +## Backward Compatibility & Deprecation + +- You MUST NOT change any public-facing code that breaks backward compatibility (APIs, method signatures, validations, enum members, prop contracts, making required properties optional) +- When fixing defects or extending constructs for new modes, deprecate the old version and introduce a corrected replacement (e.g., `FooV2`) rather than modifying the original +- You MUST NOT include `@deprecated` properties in newly introduced interfaces +- You SHOULD keep implementation-detail types unexported (marked `@internal`) to minimize backward-compatibility obligations and public API surface +- You MUST document jsii compatibility changes in `allowed-breaking-changes.txt` with the correct entry type (`changed-type`, `strengthened`, `weakened`, or `removed`) +- Warning and error IDs passed to `Annotations.addWarningV2()` are part of the public API contract — changing them is a breaking change because consumers may reference them for suppression or filtering diff --git a/docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md b/docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md new file mode 100644 index 0000000000000..81766577bdfab --- /dev/null +++ b/docs/AGENTS_CONSTRUCT_IMPLEMENTATION.md @@ -0,0 +1,199 @@ +# Construct Implementation Patterns — Grants, Metrics, Events, IAM & More + +> Implementation rules for cross-cutting construct patterns. Read this when implementing grants, metrics, events, connections, IAM integration, or other standard L2 patterns. For design decisions (mixins, facades, traits, API shape), see [AGENTS_CONSTRUCT_DESIGN.md](./AGENTS_CONSTRUCT_DESIGN.md). + +--- + +## Mixin Implementation + +### File Conventions + +- You MUST name source files after the target resource: `mixins/bucket.ts` not `mixins/versioning.ts` +- You MUST colocate multiple mixins for the same resource in the same file + +### Project Structure & Exports + +- You MUST place mixins for stable services in `lib/mixins/` within their service module +- You MUST include `.jsiirc.json` and `index.ts` barrel files in the mixins directory +- You MUST add `export * as mixins from './mixins'` to the service `lib/index.ts` +- You MUST include the mixins subpath in `package.json` exports + +### `supports()` and `applyTo()` Pattern + +- You MUST implement `supports(construct)` as a type guard using the generated `CfnFoo.isCfnFoo()` static method — not `instanceof`: + ```ts + supports(construct: IConstruct): construct is CfnBucket { + return CfnBucket.isCfnBucket(construct); + } + ``` +- You MUST implement `applyTo(construct)` with the L1 property mutations + +### Mixin Validation + +- You MUST validate mixin inputs in three phases: + 1. **Constructor** — validate all input properties, throw for invalid combinations + 2. **`applyTo()`** — validate pre-conditions on the target construct, throw for unrecoverable failures + 3. **Deferred via `node.addValidation()`** — for conditions resolved later in the app lifecycle +- You SHOULD collect multiple errors and throw them as a group + +### Mixin Merge Strategy + +- You SHOULD use `CfnPropsMixin` (`core/lib/helpers-internal/cfn-props-mixin.ts`) with `PropertyMergeStrategy` (`core/lib/mixins/property-merge-strategy.ts`) instead of modifying properties directly: + - `combine()` (deep merge, default) — for most cases + - `override()` — for non-mergeable properties like tags + +### Refactoring L2 Properties into Mixins + +- When extracting an existing L2 property into a mixin: + 1. Create the mixin following naming/file conventions + 2. Refactor the L2 to call `this.with(new MyMixin())` + 3. Keep the L2 property for backward compatibility (delegating to the mixin internally) + 4. Verify the synthesized CloudFormation output is identical before and after + +### Mixin Testing + +- You MUST write unit tests for every mixin covering: + - Correct resource type support + - Rejection of unrelated constructs + - Expected CloudFormation output + - Validation failure on unmet preconditions + - Deferred precondition setting + - Singleton provider sharing + - Provider sharing with equivalent L2 property + - Retrospective application to L2 +- Always use `.with()` in tests, not `mixin.applyTo()` directly +- Include an integration test mirroring the L2 test but using `CfnResource` with `.with(new mixin())` + +### Mixin README Documentation + +- You MUST include a `## Mixins` section in the service module's README documenting each mixin with a brief description and usage example showing both L1 and L2 application + +--- + +## Static Type Check + +- You MUST use `Symbol.for('@aws-cdk/aws-{service}.{Foo}')` and `Object.defineProperty` — not `instanceof`: + ```ts + public static isFoo(x: any): x is Foo { + return x !== null && typeof x === 'object' && Symbol.for('@aws-cdk/aws-{service}.{Foo}') in x; + } + ``` + +--- + +## Abstract Base Class + +- You SHOULD implement an abstract `FooBase` for each resource that implements the full construct interface with resource attributes as abstract properties +- Keep it internal (not exported) +- Use it to implement `from{Attribute}` import methods via ad-hoc local classes + +--- + +## Import Method Implementation + +- You MUST validate that input does not contain unresolved tokens (via `Token.isUnresolved`) before parsing in `from*` methods +- When implementing `fromLookup` methods via context providers, prefer `fromResourceAttributes()` over `fromResourceName()` to avoid reconstructing ARNs with the wrong environment + +--- + +## Grants + +### Grants Class Pattern + +- You MUST implement grant methods in a standalone Grants class (e.g., `TopicGrants`) with a `static from{Resource}()` factory accepting the reference interface +- Expose as a `grants` property on the construct: + ```ts + // In BucketGrants + public static fromBucket(bucket: IBucketRef): BucketGrants { ... } + + // On Bucket + public get grants() { return BucketGrants.fromBucket(this); } + ``` +- You MUST NOT add new grant methods directly on resource interfaces — existing ones are retained for backward compatibility only + +### `grants.json` Auto-Generation + +- You SHOULD use the `grants.json` file at the module root to auto-generate Grants classes (see `aws-sqs/grants.json` for example) +- Specify: `isEncrypted` (KMS key support), `hasResourcePolicy` (resource policy statements), `keyActions` (KMS key permissions), `arnFormat` (specific ARN patterns) +- You MUST implement manually when grant logic requires combining multiple `Grant` instances or additional parameters + +### Grants Implementation Details + +- You MUST ensure unique SIDs when the same grant method may be called multiple times (include target identifier) +- You MUST preserve all statement properties (conditions, effect, principals) when converting PolicyStatements to grants +- You SHOULD sort IAM action lists lexicographically + +--- + +## CloudWatch Metrics + +- Metric name strings MUST exactly match official AWS metric names (case-sensitive) and be placed at the correct resource scope +- Default statistics SHOULD follow metric type conventions: Count→Sum, Latency→Average, Gauge→Average +- Method names SHOULD be user-friendly when official acronyms are obscure + +--- + +## CloudWatch Events + +- Each `onXxx` method returns a `cloudwatch.EventRule` + +--- + +## IAM Integration + +### Role Integration + +- Set `grantPrincipal` to the role or `ImportedResourcePrincipal` +- `addToRolePolicy(statement)` MUST no-op with a permission notice for unowned constructs + +### Resource Policy Integration + +- Encapsulate the resource-specific mechanism behind the `addToResourcePolicy` uniform API +- Add a permission notice for unowned resources where CloudFormation cannot add policies independently + +--- + +## Integration Pattern + +- Implement integration interfaces in a single secondary module named `aws-{service}-{integration}` + +--- + +## Stateful/Stateless & Removal Policy + +- Apply removal policy to the underlying CFN resource via `resource.applyRemovalPolicy(props.removalPolicy)` + +--- + +## L1/L2 Mapping + +- You MUST verify L2-to-L1 property type compatibility before adding L2 props +- You MUST NOT expose L2 props that cannot render to the underlying CFN resource +- Private helpers SHOULD return L2 types — only the final rendering step references L1 types +- When fixing replacement issues, you MUST identify ALL replacement-triggering properties +- You MUST NOT override explicit user prop values when setting conditional defaults — always check `props.value === undefined` before applying a default. User-provided values take precedence unconditionally +- You MUST verify the target module doesn't already depend on the current module before adding cross-module imports — circular dependencies cause build failures and are not caught until compile time + +### Resource Name Generation + +- You MUST use `this.physicalName` (via `ResourceProps.physicalName`) and `this.getResourceNameAttribute()` to wire resource names — do not use `Lazy.string` +- You MUST NOT use resource attributes (Tokens) in hash calculations for physical names + +--- + +## Type-Safe Units + +- You MUST use the built-in `Size` (`core/lib/size.ts`) and `Duration` (`core/lib/duration.ts`) classes instead of manual arithmetic — use static factories to create values (e.g., `Size.mebibytes(512)`, `Duration.minutes(5)`) and conversion methods to extract them (e.g., `size.toBytes()`, `duration.toSeconds()`) + +--- + +## Warning & Error ID Stability + +- Warning and error IDs passed to `Annotations.addWarningV2()` are part of the public API contract +- Changing them is a breaking change because consumers may reference them for suppression or filtering + +--- + +## Consistent Validation Across Service Types + +- When adding validation for a feature that exists across multiple service types (e.g., timeout limits, name constraints), you MUST apply the same validation to ALL applicable types — inconsistent validation confuses users and creates silent correctness gaps