Add Flexion Forms sandbox deployment

[danielnaab]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 92e6807

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

This PR includes no changesets

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

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

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

[claude]

Pull Request Review: Add Flexion Forms Sandbox Deployment

Summary

This PR introduces AWS App Runner infrastructure for deploying the Forms sandbox environment, supporting both main and demo branches with automated Docker image builds and deployments.

✅ Strengths

1. Well-documented deployment process: The updated README provides clear, step-by-step instructions for infrastructure deployment and the CI/CD workflow.

2. Good infrastructure design:

   • VPC with proper subnet configuration across multiple AZs
   • Security groups correctly isolate RDS access to App Runner only
   • Uses Secrets Manager for credential management

3. Automated deployments: GitHub Actions workflow properly builds and pushes to both GHCR and ECR, with App Runner auto-deployment enabled.

🔒 Security Concerns

1. SecretsManagerReadWrite is overly permissive (infra/cdktf/src/lib/aws/sandbox-stack.ts:251)

   • The instance role has SecretsManagerReadWrite policy attached
   • Should use a more restrictive inline policy that grants only GetSecretValue for the specific secret ARN
   • Recommendation:

   new IamRolePolicy(this, `${id}-apprunner-secrets-policy`, {
     name: `${id}-secrets-read`,
     role: appRunnerInstanceRole.name,
     policy: Fn.jsonencode({
       Version: '2012-10-17',
       Statement: [{
         Effect: 'Allow',
         Action: ['secretsmanager:GetSecretValue'],
         Resource: dbSecret.arn
       }]
     })
   })

2. Bedrock policy allows all resources (infra/cdktf/src/lib/aws/sandbox-stack.ts:268)

   • Resource: '*' is too broad
   • Should specify specific model ARNs if known, or at least document why wildcard is necessary

3. Database password generation (infra/cdktf/src/lib/aws/sandbox-stack.ts:156-158)

   • Using Fn.uuid() with base64encode for password generation is creative but may not provide sufficient entropy
   • Consider using AWS Secrets Manager's generate_random_password feature or a proper random password resource

4. skipFinalSnapshot: true (infra/cdktf/src/lib/aws/sandbox-stack.ts:201)

   • While acceptable for sandbox/demo, ensure this is intentional
   • Consider making this configurable per environment

🐛 Potential Issues

1. Database credentials passed differently than expected (apps/sandbox/src/index.ts:20-39)

   • Environment variable is DB_SECRET_ARN but documentation and code suggest different approaches
   • The secret only contains username/password, but host/port/db are separate env vars - this is correct but should be clearly documented

2. RDS uses public subnets (infra/cdktf/src/lib/aws/sandbox-stack.ts:180)

   • Database subnet group uses public subnets, though publiclyAccessible: false
   • Best practice is to use private subnets for databases, even with public access disabled
   • Recommendation: Create separate private subnets for RDS

3. No NAT Gateway for private subnets

   • If moving RDS to private subnets, you'll need NAT Gateway for RDS to reach AWS services (if needed)
   • Current design works but isn't ideal for production

4. Error handling in sandbox app (apps/sandbox/src/index.ts:43-46)

   • process.exit(1) on secrets failure is appropriate, but error message could be more specific about which secrets are missing

📝 Code Quality

1. Large fixture file committed (apps/cli/fixtures/ai-cache/8f/8fd9b9039c736540e0f3becade55e2d36a3d5afe3f448c8efdb86b10525cdfd4.json)

   • 2109 lines of JSON fixture data added
   • Consider if this is intentionally part of this PR or if it's unrelated
   • If needed for testing, ensure it's documented

2. GitHub Actions workflow comments are helpful

   • Good inline documentation in deploy.yml explaining prerequisites and flow

3. Type safety in sandbox app

   • Good use of TypeScript, but could add type definitions for secret structure:

   interface DbSecret {
     username: string;
     password: string;
   }
   const dbSecret: DbSecret = JSON.parse(dbSecretString);

⚡ Performance Considerations

1. RDS instance sizing

   • db.t3.micro with 20GB storage is reasonable for sandbox
   • maxAllocatedStorage: 100 provides good headroom

2. App Runner instance sizing

   • 1 vCPU / 2GB RAM seems appropriate for sandbox
   • Monitor actual usage and adjust if needed

3. Health check configuration (infra/cdktf/src/lib/aws/sandbox-stack.ts:353-360)

   • 10-second interval with 5 unhealthy threshold = 50 seconds to mark unhealthy
   • This is reasonable; consider tuning based on app startup time

🧪 Testing & CI/CD

1. Missing infrastructure tests

   • No tests for the new SandboxStack
   • Consider adding CDKTF snapshot tests to catch infrastructure drift

2. Deployment order documented well

   • README clearly explains that infrastructure must be deployed before GitHub Actions can push images

3. No rollback strategy documented

   • Consider documenting how to rollback a failed deployment

📋 Minor Issues

1. Environment naming inconsistency

   • Stack uses demo-aws and main-aws (infra/cdktf/src/spaces/aws/demo.ts:21)
   • But ECR repo names use flexion-forms-sandbox-demo and flexion-forms-sandbox-main
   • Ensure naming is intentional and consistent

2. Region changed from us-east-2 to us-east-1

   • Workflow changed AWS_REGION from us-east-2 to us-east-1
   • Ensure all resources are in the same region (confirmed: they are)

🎯 Recommendations Summary

High Priority (Security):

• Replace SecretsManagerReadWrite with least-privilege inline policy
• Restrict Bedrock policy resource scope or document wildcard necessity
• Use proper password generation for RDS

Medium Priority (Best Practices):

• Move RDS to private subnets
• Add infrastructure tests
• Document rollback procedures
• Review and document the large fixture file inclusion

Low Priority (Nice to Have):

• Add TypeScript interfaces for secret structures
• Make skipFinalSnapshot configurable per environment
• Consider adding monitoring/alerting configuration

✨ Overall Assessment

This is a solid infrastructure implementation that follows many AWS best practices. The main concerns are around IAM permissions being too broad (security) and database subnet placement (architecture). The documentation is excellent, and the deployment workflow is well thought out. With the security improvements, this would be production-ready for a sandbox environment.

Recommendation: Request changes for the IAM security concerns, then approve after those are addressed.