feat: configurable removalPolicy [INFRA-80168]

CLAUDE.md

@@ -6,15 +6,25 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 `@time-loop/cdk-aurora` is an opinionated AWS CDK construct library for deploying PostgreSQL Aurora clusters with best practices baked in. It provides a high-level abstraction that includes user management, secrets rotation, RDS Proxy, and database provisioning through custom CloudFormation resources.
 
+## Current Work: Configurable Removal Policy (optional-retain branch)
+
+The `optional-retain` branch contains work to implement the requirements in `AURORA-REMOVAL-POLICY-PRD.md`:
+- Making the `removalPolicy` prop functional again (currently deprecated and ignored)
+- Changing default from automatic RETAIN to DESTROY
+- Allowing users to explicitly configure RETAIN for production databases
+- This is a **breaking change** from the current hardcoded RETAIN behavior
+- Integration with downstream `@time-loop/cdk-library` which has already implemented support for this
+
 ## Key Architecture Components
 
 ### Main Construct (`src/aurora.ts`)
 The `Aurora` class is the primary construct that orchestrates:
-- **Aurora PostgreSQL Cluster**: Configured with encryption, CloudWatch logs, and performance insights
+- **Aurora PostgreSQL Cluster**: Configured with encryption, CloudWatch logs, performance insights, and extensive default parameters optimized for production use
 - **Three User Types**: Manager (admin), Writer (DML), and Reader (read-only) with separate SecretsManager secrets
 - **RDS Proxy**: Optional proxy for connection pooling (default enabled, incompatible with multi-user rotation)
 - **Custom Provisioners**: Lambda-backed CloudFormation custom resources for database and user provisioning
 - **Activity Stream**: Optional database activity monitoring
+- **Automatic Removal Policies**: Infrastructure resources are retained, ephemeral resources are destroyed (via `ApplyRemovalPolicyAspect` at line 46-86)
 
 ### Provisioning System
 Custom CloudFormation resources handle runtime database configuration:
@@ -98,10 +108,35 @@ The construct supports two mutually exclusive approaches for cluster configurati
 **Default parameters** (used when neither is specified):
 ```typescript
 {
+  // Logical replication
   'rds.logical_replication': '1',
-  max_replication_slots: '10',
-  max_wal_senders: '10',
-  wal_sender_timeout: '0'
+
+  // Replication configuration
+  max_replication_slots: '20',
+  max_wal_senders: '20',
+  max_logical_replication_workers: '8',
+
+  // Security
+  'rds.force_ssl': '1',
+
+  // Performance monitoring and logging
+  log_min_duration_statement: '200',
+  log_lock_waits: '1',
+  log_statement: 'ddl',
+
+  // Query timeout protection
+  statement_timeout: '30000',
+  idle_in_transaction_session_timeout: '5000',
+
+  // Performance tuning
+  random_page_cost: '1.1',
+  work_mem: '4096',
+  max_parallel_workers_per_gather: '4',
+
+  // Autovacuum tuning
+  autovacuum_analyze_scale_factor: '0.01',
+
+  // ... and more (see src/aurora.ts:385-427 for complete list)
 }
 ```
 
@@ -118,7 +153,16 @@ The construct supports two mutually exclusive approaches for cluster configurati
 
 4. **Secret Naming**: Uses `multi-convention-namer` for consistent naming. Can prefix with `secretPrefix` for multiple clusters in same account.
 
-5. **Removal Policy**: The construct accepts `removalPolicy` prop but applies it differently across resources. Consider implementing the aspect pattern from `AURORA_DELETION_POLICY_IMPLEMENTATION.md` for comprehensive control.
+5. **Removal Policy** (⚠️ In Transition - see AURORA-REMOVAL-POLICY-PRD.md):
+   - **Current State**: The `removalPolicy` prop is deprecated and ignored. The `ApplyRemovalPolicyAspect` (src/aurora.ts:46-86) automatically applies:
+     - **RETAINED** (for safety): KMS keys, RDS clusters/instances, parameter groups, subnet groups, security groups (including ingress/egress rules), Secrets Manager secrets, and secret target attachments
+     - **DESTROYED** (ephemeral): Lambda functions, IAM roles/policies, CloudFormation nested stacks, resource policies, rotation schedules, SSM parameters, and CloudWatch log groups
+     - This was designed to enable safe CDK→Terragrunt migrations
+   - **Planned Change** (per PRD): Make `removalPolicy` configurable again:
+     - **Default**: `RemovalPolicy.DESTROY` for all infrastructure resources (breaking change from current RETAIN behavior)
+     - **Configurable**: Users can explicitly set `RETAIN` for production or `DESTROY` for dev/QA environments
+     - Remove deprecation warning
+     - Maintain automatic DESTROY for ephemeral resources (lambdas, IAM, etc.)
 
 ## Troubleshooting Database Issues
 

src/aurora.ts

@@ -37,21 +37,20 @@ declare global {
 /**
  * Aspect that applies appropriate RemovalPolicies based on resource type.
  *
- * Infrastructure resources (KMS, RDS, Security Groups, Secrets) are RETAINED for safety.
- * Ephemeral resources (Lambdas, IAM, nested stacks, secret attachments) are DESTROYED.
- *
- * This enables safe CDK->Terragrunt migrations where infrastructure is imported
- * but temporary CDK resources are cleaned up.
+ * Infrastructure resources (KMS, RDS, Security Groups, Secrets) use the configurable removal policy.
+ * Ephemeral resources (Lambdas, IAM, nested stacks) are always DESTROYED.
  */
 class ApplyRemovalPolicyAspect implements IAspect {
+  constructor(private readonly removalPolicy: RemovalPolicy) {}
+
   visit(node: IConstruct): void {
     if (!(node instanceof CfnResource)) {
       return;
     }
 
     const resourceType = node.cfnResourceType;
 
-    const retainedTypes = [
+    const infrastructureTypes = [
       'AWS::KMS::Key',
       'AWS::RDS::DBCluster',
       'AWS::RDS::DBInstance',
@@ -64,7 +63,7 @@ class ApplyRemovalPolicyAspect implements IAspect {
       'AWS::SecretsManager::SecretTargetAttachment',
     ];
 
-    const destroyedTypes = [
+    const ephemeralTypes = [
       'AWS::CDK::Metadata',
       'AWS::CloudFormation::Stack',
       'AWS::Lambda::Function',
@@ -76,9 +75,9 @@ class ApplyRemovalPolicyAspect implements IAspect {
       'AWS::Logs::LogGroup',
     ];
 
-    if (retainedTypes.includes(resourceType)) {
-      node.applyRemovalPolicy(RemovalPolicy.RETAIN);
-    } else if (destroyedTypes.includes(resourceType)) {
+    if (infrastructureTypes.includes(resourceType)) {
+      node.applyRemovalPolicy(this.removalPolicy);
+    } else if (ephemeralTypes.includes(resourceType)) {
       node.applyRemovalPolicy(RemovalPolicy.DESTROY);
     }
     // For Custom:: resources and anything else, let CDK defaults apply
@@ -150,12 +149,14 @@ export interface AuroraProps {
    */
   readonly proxySecurityGroups?: aws_ec2.ISecurityGroup[];
   /**
-   * @deprecated This property is no longer used. Removal policies are now automatically
-   * set based on resource type: infrastructure resources (KMS, RDS, Security Groups, Secrets)
-   * are RETAINED for safety, while ephemeral resources (Lambdas, IAM, nested stacks) are DESTROYED.
-   * This enables safe CDK->Terragrunt migrations.
+   * Removal policy for infrastructure resources (RDS cluster, instances, secrets, security groups).
+   *
+   * Use `RemovalPolicy.RETAIN` for production databases to prevent accidental deletion.
+   * Use `RemovalPolicy.DESTROY` for development/QA environments to enable automatic cleanup.
+   *
+   * Ephemeral resources (lambdas, IAM roles, log groups) are always DESTROYED regardless of this setting.
    *
-   * @default - Automatic based on resource type
+   * @default RemovalPolicy.DESTROY
    */
   readonly removalPolicy?: RemovalPolicy;
   /**
@@ -706,16 +707,9 @@ export class Aurora extends Construct {
     }
 
     // Apply removal policies based on resource type
-    // Infrastructure resources (KMS, RDS, Security Groups, Secrets) are RETAINED
-    // Ephemeral resources (Lambdas, IAM, nested stacks) are DESTROYED
-    Aspects.of(this).add(new ApplyRemovalPolicyAspect());
-
-    // Warn if deprecated removalPolicy prop is used
-    if (props.removalPolicy !== undefined) {
-      Annotations.of(this).addWarning(
-        'The removalPolicy property is deprecated and no longer has any effect. ' +
-          'Removal policies are now automatically set based on resource type.',
-      );
-    }
+    // Infrastructure resources use the configurable removal policy (default: DESTROY)
+    // Ephemeral resources (Lambdas, IAM, nested stacks) are always DESTROYED
+    const removalPolicy = props.removalPolicy ?? RemovalPolicy.DESTROY;
+    Aspects.of(this).add(new ApplyRemovalPolicyAspect(removalPolicy));
   }
 }

test/aurora.test.ts

@@ -147,7 +147,7 @@ describe('Aurora', () => {
     it('proxyName', () => {
       template.hasResourceProperties('AWS::RDS::DBProxy', { DBProxyName: 'Test' });
     });
-    it('removalPolicy defaults to RETAIN for infrastructure resources', () => {
+    it('removalPolicy defaults to DESTROY for infrastructure resources', () => {
       const templateJson = template.toJSON();
       const resources = templateJson.Resources || {};
 
@@ -168,7 +168,7 @@ describe('Aurora', () => {
       expect(infrastructureResources.length).toBeGreaterThan(0);
 
       infrastructureResources.forEach(([, resource]: [string, any]) => {
-        expect(resource.DeletionPolicy).toBe('Retain');
+        expect(['Delete', undefined]).toContain(resource.DeletionPolicy);
       });
     });
     it('retention', () => {
@@ -349,13 +349,30 @@ describe('Aurora', () => {
       ]);
       expect(aurora.proxySecurityGroups).toStrictEqual([sg]);
     });
-    it('removalPolicy is deprecated and warns when used', () => {
-      createAurora({ ...defaultAuroraProps, removalPolicy: RemovalPolicy.DESTROY });
-      // Should warn that the property is deprecated
-      annotations.hasWarning('*', Match.stringLikeRegexp('removalPolicy property is deprecated'));
-      // Should still apply automatic policies (infrastructure retained, ephemeral destroyed)
-      template.hasResource('AWS::RDS::DBCluster', {
-        DeletionPolicy: 'Retain',
+    it('removalPolicy RETAIN applies to infrastructure resources', () => {
+      createAurora({ ...defaultAuroraProps, removalPolicy: RemovalPolicy.RETAIN });
+
+      const templateJson = template.toJSON();
+      const resources = templateJson.Resources || {};
+
+      const infrastructureTypes = [
+        'AWS::RDS::DBCluster',
+        'AWS::RDS::DBInstance',
+        'AWS::SecretsManager::Secret',
+        'AWS::RDS::DBClusterParameterGroup',
+        'AWS::EC2::SecurityGroup',
+        'AWS::EC2::SecurityGroupEgress',
+        'AWS::EC2::SecurityGroupIngress',
+      ];
+
+      const infrastructureResources = Object.entries(resources).filter(([_, resource]: [string, any]) =>
+        infrastructureTypes.includes(resource.Type),
+      );
+
+      expect(infrastructureResources.length).toBeGreaterThan(0);
+
+      infrastructureResources.forEach(([, resource]: [string, any]) => {
+        expect(resource.DeletionPolicy).toBe('Retain');
       });
     });
     it('retention', () => {
@@ -457,8 +474,8 @@ describe('Aurora', () => {
       });
     });
 
-    describe('automatic removal policy aspect', () => {
-      it('applies RETAIN to infrastructure resources', () => {
+    describe('configurable removal policy', () => {
+      it('defaults to DESTROY for infrastructure resources', () => {
         createAurora({ ...defaultAuroraProps });
 
         const templateJson = template.toJSON();
@@ -482,9 +499,9 @@ describe('Aurora', () => {
         // Verify we found infrastructure resources
         expect(infrastructureResources.length).toBeGreaterThan(0);
 
-        // Check that these resources have DeletionPolicy: Retain
+        // Check that these resources have DeletionPolicy: Delete or undefined (which defaults to Delete)
         infrastructureResources.forEach(([, resource]: [string, any]) => {
-          expect(resource.DeletionPolicy).toBe('Retain');
+          expect(['Delete', undefined]).toContain(resource.DeletionPolicy);
         });
       });