feat(pulumi): add AWS infrastructure code and deployment guide#1052
feat(pulumi): add AWS infrastructure code and deployment guide#1052bhagyashreewagh wants to merge 66 commits intoRolnickLab:mainfrom
Conversation
…ation This document provides a comprehensive guide on deploying the Antenna backend to AWS, detailing the architecture, environment variables, and deployment workflow.
Updated the README to enhance clarity and detail about the Antenna backend deployment process, including infrastructure components and future improvements.
✅ Deploy Preview for antenna-preview canceled.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds complete AWS infrastructure-as-code deployment framework using Pulumi for the Antenna platform, including provisioning of VPC, networking, security groups, RDS PostgreSQL, ElastiCache Redis, ECR repositories, Elastic Beanstalk ECS environment, CloudFront CDN, S3 storage, IAM roles/policies, Secrets Manager, and Docker image building and pushing to ECR. Changes
Sequence Diagram(s)sequenceDiagram
actor Operator
participant Pulumi as Pulumi CLI
participant AWS as AWS APIs
participant ECR as ECR Registry
participant EB as Elastic Beanstalk
participant RDS as RDS Instance
participant Redis as ElastiCache
participant S3 as S3 Bucket
participant CF as CloudFront
Operator->>Pulumi: pulumi up (deploy)
Pulumi->>AWS: Create VPC & Networking
AWS-->>Pulumi: VPC, Subnets, SGs created
Pulumi->>AWS: Create RDS PostgreSQL
AWS-->>Pulumi: RDS endpoint ready
Pulumi->>AWS: Create ElastiCache Redis
AWS-->>Pulumi: Redis endpoint ready
Pulumi->>AWS: Create ECR Repositories
AWS-->>Pulumi: ECR repos created
Pulumi->>ECR: Build & push Docker images
ECR-->>Pulumi: Images uploaded
Pulumi->>AWS: Create IAM roles & policies
AWS-->>Pulumi: Roles configured
Pulumi->>AWS: Create Secrets Manager entries
AWS-->>Pulumi: Secrets stored
Pulumi->>AWS: Create EB Application & Environment
AWS->>ECR: Pull images
ECR-->>AWS: Images retrieved
AWS->>RDS: Connect for migrations
RDS-->>AWS: Migrations run
AWS->>Redis: Connect & configure
Redis-->>AWS: Connected
AWS-->>Pulumi: EB Environment running
Pulumi->>S3: Create assets bucket
S3-->>Pulumi: Bucket created
Pulumi->>S3: Upload UI assets
S3-->>Pulumi: Assets stored
Pulumi->>AWS: Create CloudFront distribution
AWS->>S3: Configure origin access
S3-->>AWS: OAC configured
AWS-->>Pulumi: CloudFront ready
Pulumi->>AWS: Invalidate CloudFront cache
AWS-->>Pulumi: Invalidation submitted
Pulumi-->>Operator: Deployment complete (outputs exported)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
aws-infra/README.md
Outdated
| ### Step 1 — Build and push image to ECR | ||
|
|
||
| docker build -t antenna-backend . | ||
| docker tag antenna-backend:latest <ECR_URI>:v10 |
There was a problem hiding this comment.
is v10 an example version number? If so maybe add a small note about how to think about and use container versions
There was a problem hiding this comment.
Yes, v10 is just an example. I’ll add a note explaining how contributors should choose version numbers so it's clear and consistent.
aws-infra/README.md
Outdated
|
|
||
| - /.ebextensions/00_setup.config : EB environment variables and settings | ||
| - /.ebignore : Exclusion list for EB deployment bundle | ||
| - /Dockerrun.aws.json : Multi-container EB deployment config |
There was a problem hiding this comment.
can we add example versions of these files?
There was a problem hiding this comment.
Absolutely! Some of these files may contain secrets, so I’ll sanitize the sensitive parts and upload minimal example versions.
…guration notes Added guidelines for Docker image versioning and deployment validation steps.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
aws-infra/README.md (5)
41-43: Minor: Improve sentence structure for formal documentation tone.The text uses an implied subject in the second sentence. Consider making the subject explicit:
- Environment type: - - Single-instance environment (used for development/testing to reduce cost). - - Can be upgraded later to a load-balanced environment for production. + Environment type: + - Single-instance environment (used for development/testing to reduce cost). + - This can be upgraded later to a load-balanced environment for production.
55-58: Optional: Tighten the explanatory note for conciseness.The phrase "originally created for" is slightly verbose. Consider simplifying:
- - Additional outbound egress security group - *(originally created for App Runner, now reused for EB networking)* + - Additional outbound egress security group + *(created for App Runner, reused for EB networking)*
334-341: Fix: Add missing language specifiers to code blocks and tighten adverb usage.Several code blocks are missing language identifiers, and one sentence has redundant adverb usage:
- Line 339: Remove repeated adverb for conciseness:
- Celery worker connects to Redis successfully - Celery Beat schedules run successfully + Celery worker connects to Redis + Celery Beat schedules run successfully
- Lines 351, 362, 396, 411, 425: Add language specifiers to fenced code blocks. For example:
At line 351, change:
- ``` + ```bashApply the same fix to lines 362 (
bash), 396 (bash), 411 (bash), and 425 (bashorhtml).
- Line 343: Remove blank line inside blockquote before "How to validate..." explanation for proper markdown formatting.
248-260: Fix: Reduce repetitive adjective usage in least-privilege recommendations.Lines 254-255 use the word "only" twice in consecutive bullet points. Vary the language:
- Restricting S3 access to only specific buckets - Restricting ECR access to only required repositories - Minimizing CloudWatch permissions + Restricting S3 access to specific buckets + Restricting ECR access to required repositories + Minimizing CloudWatch permissions
522-524: Fix: Use asterisks for emphasis instead of underscores (markdown convention).Line 524 uses underscores for italics, which is inconsistent with markdown best practices. Change:
-_End of documentation._ +*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
aws-infra/README.md(1 hunks)aws-infra/policies/iam_elastic_beanstalk_policy.json(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
aws-infra/policies/iam_elastic_beanstalk_policy.json
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
🪛 LanguageTool
aws-infra/README.md
[style] ~43-~43: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~58-~58: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~255-~255: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~339-~339: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
343-343: Blank line inside blockquote
(MD028, no-blanks-blockquote)
351-351: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
362-362: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
396-396: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
411-411: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
425-425: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
524-524: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
524-524: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (3)
aws-infra/README.md (3)
307-321: Approve: Comprehensive responses to past review feedback.The developer has thoroughly addressed all prior review comments:
- ✓ Added version strategy guidance (lines 307–321) in response to version numbering questions
- ✓ Included detailed validation methodology (lines 344–435) explaining how each deployment step is confirmed
- ✓ Clarified Redis TLS fix location (lines 450–471) explaining the role of
.ebextensions/00_setup.configThese additions significantly improve clarity for maintainers and contributors.
Also applies to: 344-435, 450-471
1-524: Overall Assessment: Comprehensive documentation with strong team feedback integration.This README provides excellent coverage of the AWS Elastic Beanstalk deployment architecture, infrastructure components, deployment workflow, and troubleshooting guidance. The structure progressively builds from high-level overview → detailed configuration → practical deployment steps → validation methodology → common issues.
Strengths:
- Clear, logical section flow suitable for onboarding maintainers and contributors
- Detailed validation section with concrete examples (
curl,eb logs) helps teams confirm successful deployments- IAM and networking sections properly document current state and acknowledge least-privilege future improvements
- All four prior review comments from the team have been thoughtfully incorporated
Resolution needed before merge:
- Resolve the port inconsistency identified in the earlier comment (5000 vs 8000).
Optional polish items:
- Apply the markdown and style fixes noted in the separate comments above for professional consistency.
87-96: Verify port inconsistency claim: Django container port mapping (lines 91 vs. 376).The review identifies a discrepancy stating line 91 mentions Django listening on port 5000 (exposed as 80), while line 376 states port 8000. Unable to access the repository to independently verify these specific line references and claims. Please confirm:
- Actual port number configured in the Django container
- Whether both references in the document match the actual configuration
- Update the documentation to reflect the correct port mapping if inconsistency exists
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
File is empty and contains no valid JSON policy content.
The IAM policy file is currently empty and fails JSON validation. A valid Elastic Beanstalk IAM policy should follow the structure with "Version" and "Statement" keys, but this file contains none.
Since the PR adds comprehensive AWS infrastructure documentation, this policy file is critical infrastructure-as-code. An empty file cannot be used for actual deployment and contradicts the PR's intent.
Do you want me to generate a starter IAM policy tailored to the Antenna Elastic Beanstalk deployment? Based on the PR objectives mentioning an ECS-based multi-container setup with RDS and ElastiCache, I can provide a policy that aligns with those resources. Please clarify:
- Intended scope: Is this policy for the EC2 instance role (what EB instances assume) or for deployment/CI-CD users?
- Least-privilege requirement: Should the policy follow least-privilege with scoped resources, or use broader wildcards for initial setup?
- Additional integrations: Does it need permissions for ECR, RDS, ElastiCache, CloudWatch, or other services mentioned in the infrastructure guide?
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
🤖 Prompt for AI Agents
In aws-infra/policies/iam_elastic_beanstalk_policy.json around line 1, the file
is empty and must be replaced with a valid IAM policy JSON (including "Version"
and "Statement"). Fix by creating a proper JSON policy for the intended role:
decide whether this is an EC2/Elastic Beanstalk instance profile or a
CI/CD/deployer role, pick least-privilege vs wildcard scope, and either (A)
reference AWS-managed Elastic Beanstalk policies (e.g.,
AWSElasticBeanstalkWebTier/WorkerTier plus any needed service policies like
AmazonEC2ContainerRegistryReadOnly, CloudWatchLogsFullAccess, or scoped
RDS/ElastiCache permissions) or (B) author a custom policy JSON with a Version
and Statements granting only the required actions (S3 get/put for deployments,
ecs/ecr describe/pull if ECS tasks used, logs:CreateLogStream/PutLogEvents,
ec2/autoscaling describe and tagging, and scoped RDS/ElastiCache access if
needed). Ensure the final file is valid JSON and add comments in PR describing
scope and resource ARNs used for least-privilege.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
aws-infra/README.md (4)
43-44: Fix incomplete sentence; add subject to line 44.Line 44 is missing a subject: "Can be upgraded later..." → who/what can be upgraded? Likely should be "It can be upgraded later..." or "The environment can be upgraded later..."
- Deployment bundle includes: - - `Dockerrun.aws.json` - - `.ebextensions/00_setup.config` - Environment type: - - Single-instance environment (used for development/testing to reduce cost). - - Can be upgraded later to a load-balanced environment for production. + Deployment bundle includes: + - `Dockerrun.aws.json` + - `.ebextensions/00_setup.config` + Environment type: + - Single-instance environment (used for development/testing to reduce cost). + - It can be upgraded later to a load-balanced environment for production.
56-60: Remove redundant phrase on line 59; simplify "originally created" to "created".The phrase "originally created" is redundant in context; "created" alone is sufficient.
- - EB-managed instance security group (default inbound + outbound rules) - - Additional outbound egress security group - *(originally created for App Runner, now reused for EB networking)* + - EB-managed instance security group (default inbound + outbound rules) + - Additional outbound egress security group + *(created for App Runner, now reused for EB networking)*
357-451: Fix markdown issues in deployment workflow section: add language specs to code blocks and remove blank line in blockquote.The extensive blockquote section containing validation instructions has several markdown linting violations:
- Line 358: Blank line inside blockquote (should have no blank lines within blockquote)
- Lines 366, 377, 411, 426, 440: Fenced code blocks missing language specifications (should be
```bash,```json, etc.)Additionally, line 354 uses the adverb "successfully" twice in consecutive bullet points ("connects to Redis successfully" and "schedules run successfully"), which is redundant. Consider replacing one with a synonym or restructuring.
Apply these fixes:
### Step 5 — Validate Deployment - `/api/v2/` returns `200` - Django container remains healthy - - Celery worker connects to Redis successfully - - Celery Beat schedules run successfully + - Celery worker connects to Redis without errors + - Celery Beat schedules tasks as expected - Flower UI loads on port 5555 (if security groups permit) -> > ### How to Validate this deployment? > The points listed above describe the expected state of a successful AWS deployment. > > --- > > ### 1. Confirming that `/api/v2/` returns a successful response > When you open: - > ``` + > ```bash > https://<EB-URL>/api/v2/ > ``` > the browser shows the JSON content returned by Django. > This means that simply opening the URL visually confirms "the API is working," but not the status code. > > To check the actual HTTP status code, use: - > ```bash + > ```bash > curl -I https://<EB-URL>/api/v2/ > ``` > This command returns the HTTP headers. A successful response looks like: - > ``` + > ``` > HTTP/1.1 200 OK > ```For all missing language specs, add the appropriate language identifier after the opening backticks:
```bash,```json, etc.
539-539: Fix markdown emphasis style on final line.Line 539 uses underscore emphasis (
_End of documentation._) instead of the consistent asterisk style used throughout the document. Change to*End of documentation.*for markdown consistency.-_End of documentation._ +*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~44-~44: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~59-~59: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~270-~270: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~354-~354: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
358-358: Blank line inside blockquote
(MD028, no-blanks-blockquote)
366-366: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
377-377: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
411-411: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
426-426: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
539-539: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
539-539: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (3)
aws-infra/README.md (3)
505-535: Comprehensive future improvements roadmap is well-prioritized.The list of recommended enhancements covers critical areas (Secrets Manager, RDS/Redis hardening, IAM least-privilege, CI/CD) and scalability improvements. This aligns well with the earlier acknowledgment in section 5.4 about using broad EB-managed policies as a temporary baseline. Consider adding a priority or phasing note (e.g., "Phase 1: Critical for production" vs. "Phase 2: Scalability enhancements") to help maintainers prioritize implementation order.
229-275: Verify thataws-infra/policies/iam_elastic_beanstalk_policy.jsoncontains an example or template.Section 5.4 documents IAM roles comprehensively and notes "least privilege" recommendations for future hardening. The policy file should include either a concrete, sanitized example of the current EB-managed policy configuration, or a template showing the recommended least-privilege hardened version. If currently empty, maintainers lack a concrete reference artifact to implement or review.
26-31: Include sanitized example versions of the three deployment configuration files referenced in this section.Past review comments requested adding example versions of
.ebextensions/00_setup.config,Dockerrun.aws.json, and.ebignorewith sensitive parts sanitized. Maintainers need concrete, runnable templates for these critical deployment files to understand the expected configuration structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
aws-infra/README.md (9)
44-44: Clarify incomplete sentence about load-balanced environment upgrade.Line 44 lacks a subject: "Can be upgraded later to a load-balanced environment for production" reads incomplete. Suggest revising to: "The environment can be upgraded later to a load-balanced configuration for production."
59-59: Simplify redundant phrase in security group description.The phrase "originally created for App Runner, now reused" is redundant. Consider simplifying to: "An outbound-only egress security group (reused from prior App Runner setup)."
188-188: Fix emphasis marker formatting on line 188.Markdown linter flags spaces inside emphasis markers. Line 188 reads
** Inbound:**with an extra space after the opening**. Remove the space to comply with markdown formatting.
276-276: Remove redundant adverb in IAM least-privilege section.The adverb "only" is used twice in this sentence: "Restricting S3 access to only specific buckets…Restricting ECR access to only required repositories." Consider removing the second instance or replacing with a synonym like "necessary" for clarity.
365-365: Strengthen the adjective describing CloudFront logging enhancement.The phrase "if deeper monitoring is needed" uses a weak comparative adjective. Consider replacing with: "if comprehensive monitoring is needed" or "if detailed access logs are required."
429-429: Remove redundant adverb in validation checklist.The adverb "successfully" appears twice in quick succession: "Celery worker connects to Redis successfully…Celery Beat schedules run successfully." Consider rewording the second instance to: "Celery Beat schedules are dispatched without errors."
433-433: Remove blank line inside blockquote on line 433.Markdown linter flags a blank line inside the blockquote block starting at line 433. Remove the blank line between the
>markers to maintain proper blockquote formatting.
441-451: Add language specifiers to all fenced code blocks.Several fenced code blocks lack language identifiers, which markdown linters expect:
- Line 441:
```→```bash- Line 452:
```→```bash- Line 486:
```→```text(or omit if showing plaintext output)- Line 501:
```→```text- Line 515:
```httpor```text(HTTP request)Adding language tags improves syntax highlighting and follows markdown best practices.
Also applies to: 452-462, 486-495, 501-510, 515-523
614-614: Update emphasis style to match markdown linter expectations.Markdown linter flags underscore emphasis (
_End of documentation._) on line 614. Replace with asterisks to comply with style guide:*End of documentation.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~44-~44: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~59-~59: This phrase is redundant. Consider writing “created”.
Context: ... outbound egress security group *(originally created for App Runner, now reused for EB netwo...
(ORIGINALLY_CREATED)
[style] ~276-~276: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~365-~365: Consider a different adjective to strengthen your wording.
Context: ...Disabled** (Can be enabled later if deeper monitoring is needed) --- ## 6. .ebe...
(DEEP_PROFOUND)
[style] ~429-~429: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
188-188: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
433-433: Blank line inside blockquote
(MD028, no-blanks-blockquote)
441-441: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
452-452: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
486-486: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
501-501: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
515-515: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
614-614: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
614-614: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (5)
aws-infra/README.md (5)
185-185: RDS public accessibility security issue resolved.Good news: RDS public accessibility is now disabled, which addresses the prior critical security gap flagged in previous reviews. The private subnet configuration and restricted security group (inbound only from EB SG) are properly in place.
398-411: Version numbering guidance clearly documented.The explanation of container versioning (lines 398–411) directly addresses prior feedback requesting clarity on how to choose version tags. This addition is helpful and well-integrated into the deployment workflow.
434-526: Detailed validation section comprehensively addresses prior feedback.The "How to Validate this deployment?" section (lines 434–526) provides clear step-by-step validation instructions with concrete examples (curl commands, log snippets, UI access). This directly addresses the prior reviewer question "how?" and significantly improves usability of the guide.
540-561: Redis TLS configuration location clearly explained.The dedicated subsection explaining where the Redis TLS fix is applied (lines 540–561) directly addresses prior feedback asking "where would I fix this?" The explanation of
.ebextensions/00_setup.configas the source and how EB forwards values to containers is clear and actionable.
26-31: Verify that example configuration files are present or planned.A prior reviewer requested "example versions of these files" (section 2, configuration files) with the response that "sanitized minimal example versions" would be uploaded. The current documentation references these files (
.ebextensions/00_setup.config,Dockerrun.aws.json,.ebignore) but no example snippets or separate example directory is visible in this PR.Please confirm:
- Are sanitized example versions of these configuration files included elsewhere in the PR (e.g., under
aws-infra/examples/)?- If not yet added, should this be tracked as a follow-up task, or should examples be added before merging this PR?
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
aws-infra/images/aws-architecture.pngis excluded by!**/*.png
📒 Files selected for processing (1)
aws-infra/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
aws-infra/README.md
[style] ~49-~49: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
[style] ~280-~280: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...c buckets - Restricting ECR access to only required repositories - Minimizing Cl...
(ADVERB_REPETITION_PREMIUM)
[style] ~369-~369: Consider a different adjective to strengthen your wording.
Context: ...Disabled** (Can be enabled later if deeper monitoring is needed) --- ## 6. .ebe...
(DEEP_PROFOUND)
[style] ~433-~433: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...uccessfully - Celery Beat schedules run successfully - Flower UI loads on port 5555 (if secu...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
aws-infra/README.md
192-192: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
437-437: Blank line inside blockquote
(MD028, no-blanks-blockquote)
445-445: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
456-456: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
490-490: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
505-505: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
519-519: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🔇 Additional comments (5)
aws-infra/README.md (5)
92-176: Well-organized container and environment variable documentation.The Docker container descriptions and environment variable taxonomy are clear, well-categorized, and easy to navigate. This will help maintainers understand the deployment model and required configuration.
178-302: RDS security concern is properly addressed.The RDS configuration correctly specifies public accessibility as Disabled (line 189) with restrictive inbound rules from only the EB security group. This resolves the prior concern. The overall infrastructure documentation is comprehensive, with RDS and Redis properly isolated in private subnets and secured via security group rules. The IAM section appropriately documents the use of broad managed policies for initial deployment while recommending least-privilege refinement as a future hardening step.
305-371: CloudFront architecture is well-documented and secure.The distribution strategy (S3 for static files, EB for /api/* routes), origin access control, and security posture are clearly explained. The HTTPS enforcement and S3 bucket protection via OAC demonstrate good security practices.
387-427: Deployment workflow is clear and actionable.The five-step deployment process with Docker image versioning guidance is well-structured and includes helpful notes about semantic versioning choices. This makes it easy for contributors to follow a consistent versioning scheme.
532-582: Troubleshooting section thoroughly addresses common deployment issues.The Redis TLS configuration and EB health check sections provide clear fixes with explanations of how Elastic Beanstalk injects environment variables into containers. This directly addresses prior questions about where to apply fixes and demonstrates good understanding of the deployment mechanics.
| - `.ebextensions/00_setup.config` | ||
| - Environment type: | ||
| - Single-instance environment (used for development/testing to reduce cost). | ||
| - Can be upgraded later to a load-balanced environment for production. |
There was a problem hiding this comment.
Fix fragment sentence.
This line lacks a subject. Restructure to: "This environment can be upgraded later to a load-balanced environment for production."
🧰 Tools
🪛 LanguageTool
[style] ~49-~49: To form a complete sentence, be sure to include a subject.
Context: ...velopment/testing to reduce cost). - Can be upgraded later to a load-balanced en...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
In aws-infra/README.md around line 49, the sentence "Can be upgraded later to a
load-balanced environment for production." is a fragment without a subject;
replace it with a full sentence such as "This environment can be upgraded later
to a load-balanced environment for production." to fix grammar and clarity.
aws-infra/README.md
Outdated
|
|
||
| ## 9. Future Improvements | ||
|
|
||
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | ||
|
|
||
| - **Move secrets to AWS Secrets Manager** | ||
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | ||
|
|
||
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | ||
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | ||
|
|
||
| - **IAM hardening and least-privilege review** | ||
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | ||
|
|
||
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | ||
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | ||
|
|
||
| - **Add staging environment** | ||
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | ||
|
|
||
| - **Migrate to load-balanced EB environment (optional)** | ||
| Enables rolling deployments, zero-downtime updates, and better scalability. | ||
|
|
||
| - **Enable RDS Multi-AZ + automated backups** | ||
| Ensures database failover and improves disaster recovery readiness. | ||
|
|
||
| - **Add health checks for Celery worker & beat** | ||
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | ||
|
|
||
|
|
||
| --- | ||
|
|
||
| _End of documentation._ |
There was a problem hiding this comment.
Future improvements are well-prioritized and realistic.
The roadmap appropriately covers security hardening (Secrets Manager, IAM least-privilege), reliability enhancements (Multi-AZ, health monitoring), and operational maturity (CI/CD, staging). However, fix the emphasis style on line 615: change _End of documentation._ to *End of documentation.* to match the file's asterisk emphasis convention (MD049).
-_End of documentation._
+*End of documentation.*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 9. Future Improvements | |
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | |
| - **Move secrets to AWS Secrets Manager** | |
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | |
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | |
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | |
| - **IAM hardening and least-privilege review** | |
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | |
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | |
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | |
| - **Add staging environment** | |
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | |
| - **Migrate to load-balanced EB environment (optional)** | |
| Enables rolling deployments, zero-downtime updates, and better scalability. | |
| - **Enable RDS Multi-AZ + automated backups** | |
| Ensures database failover and improves disaster recovery readiness. | |
| - **Add health checks for Celery worker & beat** | |
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | |
| --- | |
| _End of documentation._ | |
| ## 9. Future Improvements | |
| To harden the deployment and move toward a production-grade architecture, the following enhancements are recommended: | |
| - **Move secrets to AWS Secrets Manager** | |
| Centralize all sensitive variables (DB password, Redis URL, Django secret key, Sentry key, SendGrid, etc.) and replace `.ebextensions` injection with runtime retrieval. | |
| - **Enable ElastiCache Multi-AZ + Auto-Failover** | |
| Improves high availability for Celery and Django caching; eliminates single-node Redis failure risks. | |
| - **IAM hardening and least-privilege review** | |
| Replace broad EB-managed policies with reduced IAM policies scoped only to required S3, ECR, CloudWatch, and ECS resources. | |
| - **Add CI/CD pipeline (GitHub Actions -> ECR -> EB)** | |
| Automate build, tag, push of images and deployments to Elastic Beanstalk for consistent, reproducible releases. | |
| - **Add staging environment** | |
| Separate EB environment (staging) for testing migrations, image builds, and infrastructure changes before production. | |
| - **Migrate to load-balanced EB environment (optional)** | |
| Enables rolling deployments, zero-downtime updates, and better scalability. | |
| - **Enable RDS Multi-AZ + automated backups** | |
| Ensures database failover and improves disaster recovery readiness. | |
| - **Add health checks for Celery worker & beat** | |
| Custom EB or CloudWatch alarms to alert on worker failures, broker connectivity issues, or long task queues. | |
| --- | |
| *End of documentation.* |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
615-615: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
🤖 Prompt for AI Agents
In aws-infra/README.md around lines 583 to 615, the closing emphasis uses
underscores (`_End of documentation._`) which violates the file's asterisk
emphasis convention (MD049); change that line to use asterisks (`*End of
documentation.*`) to match the rest of the document's emphasis style. Ensure
only the delimiter characters are changed and no trailing spaces or other
formatting are altered.
This file contains the implementation for managing secrets in AWS Secrets Manager using Pulumi. It includes functions to create secrets and export necessary configurations for a Django application.
Implement Redis ElastiCache setup using Pulumi with subnet and security group configuration.
This script creates multiple AWS ECR repositories with specific configurations, including encryption and tagging.
Implement S3 bucket policies and public access block for assets bucket.
Added detailed installation instructions for deploying Antenna infrastructure using Pulumi on AWS, including prerequisites, installation steps for AWS CLI and Pulumi, configuration settings, and deployment instructions.
This script sets up AWS subnets for Redis and RDS, using default subnets for Redis and creating new private subnets for RDS to ensure security and isolation.
This file defines security groups for Elastic Beanstalk, RDS, and Redis using Pulumi. It includes ingress and egress rules to control traffic for each service.
Updated security group rules to remove specific IP addresses.
This script creates a private route table for RDS subnets, ensuring database security by restricting access to the VPC only.
bhagyashreewagh
changed the title
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Fix all issues with AI agents
In `@aws-infra/cloudfront.py`:
- Around line 132-134: The subprocess call currently uses "bash" which can be
hijacked via PATH; update the subprocess.run invocation that executes cmd (see
subprocess.run([...], check=True) and the cmd variable used alongside
pulumi.log.info) to call the absolute shell path "/bin/bash" (i.e., replace
"bash" with "/bin/bash") so the UI build runs with a fixed, safe executable
path.
In `@aws-infra/eb.py`:
- Around line 230-233: The Dockerrun/env block currently hardcodes Flower
credentials via backend_environment including entries CELERY_FLOWER_USER and
CELERY_FLOWER_PASSWORD; remove those hardcoded values and instead retrieve the
credentials from Secrets Manager (or stack config) at deployment/runtime and
inject them into the environment list. Concretely: update the code that builds
backend_environment (the list where CELERY_FLOWER_USER and
CELERY_FLOWER_PASSWORD are added) so it fetches the secret (using
boto3.client('secretsmanager').get_secret_value or your existing secret helper)
and appends {"name":"CELERY_FLOWER_USER","value": <secret.user>} and
{"name":"CELERY_FLOWER_PASSWORD","value": <secret.password>} (or reference the
stack parameter) rather than the literal "admin"/"password"; keep
DEFAULT_PROCESSING_SERVICE_ENDPOINT unchanged. Ensure errors fetching secrets
fail fast with clear logs.
- Around line 167-173: The awscli container currently hardcodes "us-west-2" for
the AWS_REGION environment variable; replace that literal with the configured
Pulumi AWS region (e.g., use aws.config.region or aws.get_region().name) in the
container spec so the "environment" entry for name "AWS_REGION" uses the
stack/provider-configured region instead of "us-west-2" (locate the awscli
container dict and update its "environment" value accordingly, keeping
awscli_image and other fields unchanged).
In `@aws-infra/ecr.py`:
- Around line 21-24: Update the misleading inline comment to match the actual
configuration: change the comment near the image_scanning_configuration argument
where scan_on_push is set to False
(aws.ecr.RepositoryImageScanningConfigurationArgs with scan_on_push=False) so it
indicates "Scan on push = OFF" (or similar) instead of "Scan on push = ON".
In `@aws-infra/iam_roles.py`:
- Around line 40-58: The RolePolicy currently grants SecretsManager access to
all secrets via the Resource "arn:aws:secretsmanager:*:*:secret:*"; restrict
this to only antenna secrets by replacing the wildcard ARN with the stack-scoped
pattern (e.g., "arn:aws:secretsmanager:*:*:secret:antenna-${STACK}-*") or,
alternatively, import the secret objects from secrets_manager.py and build a
list of exact ARNs, then use that list as the policy Resource; apply the same
change to the EC2 role policy (the other aws.iam.RolePolicy for the EC2 role) so
both ecs_execution_role and the EC2 role only have access to antenna-{STACK}-*
secrets.
In `@aws-infra/images.py`:
- Around line 75-88: The DOCKER_SETUP_BLOCK currently creates the Docker config
in DOCKER_CONFIG_DIR with default perms; update the shell snippet to restrict
permissions by setting a safe umask (e.g., umask 077) before creating files or
explicitly chmod the directory and file: ensure the created DOCKER_CONFIG
directory (DOCKER_CONFIG_DIR) is mode 0700 and the created DOCKER_CONFIG_JSON
file is mode 0600 after the cat heredoc completes; reference DOCKER_SETUP_BLOCK,
DOCKER_CONFIG_DIR and DOCKER_CONFIG_JSON when locating and updating the snippet.
- Around line 58-62: The _hash_build_inputs function currently hashes only the
path strings; change it to hash the actual contents by reading the Dockerfile at
dockerfile_path and by walking the context_path directory recursively: collect
all file paths under context_path, sort them deterministically, and for each
file update the sha256 with the relative file path and file bytes so changes to
any file alter the hash; also handle missing/unreadable files gracefully (e.g.,
skip or raise a clear error) and include any relevant metadata (like file mode
or modification time) only if needed for your build semantics.
In `@aws-infra/modifications/storage.py`:
- Around line 88-96: placeholder_key is being built with prefix but
s3.write_file already calls key_with_prefix, causing a double-prefix; change to
pass the raw key (e.g., ".placeholder" or a variable like placeholder_name) to
s3.write_file instead of placeholder_key and let s3.write_file/key_with_prefix
add the prefix. Also replace logger.error(...) in the except block with
logger.exception(...) so the traceback is captured; keep the same descriptive
message that includes prefix and any context (mention S3_TEST_CONFIG and prefix
variables to locate the call).
In `@aws-infra/networking/security_groups.py`:
- Around line 58-65: The SecurityGroup ingress rule using
aws.ec2.SecurityGroupIngressArgs with cidr_blocks=[""] is invalid; change the
admin UI (Flower) ingress to only be created when a configured CIDR value is
present (e.g. check your ADMIN_CIDR/ENV var) or remove the rule entirely, and
ensure any created rule supplies a valid source field (cidr_blocks,
ipv6_cidr_blocks, prefix_list_ids or security_groups) rather than an empty
string; apply the same conditional pattern/fix to the RDS and Redis admin
ingress rules so none are emitted with empty cidr_blocks.
In `@aws-infra/networking/subnets.py`:
- Around line 53-59: The code assumes cidr_blocks has at least 9 /20 subnets and
will IndexError when it doesn't; validate the available CIDR count before
indexing cidr_blocks and handle the shortfall (either raise a clear ValueError
with context or choose a safe fallback selection). Specifically, after computing
vpc_cidr and cidr_blocks, check len(cidr_blocks) >= 9 (or >= max index+1) and if
not, raise an informative exception mentioning vpc_cidr.cidr_block and
len(cidr_blocks) or compute/select alternate indices; update the db_cidrs
assignment (the symbol db_cidrs and variables vpc_cidr, cidr_blocks) to only use
validated indices so you never access out-of-range elements.
- Around line 63-75: The code assumes three availability zones by slicing azs =
aws.get_availability_zones(...).names[:3] and then indexing db_cidrs[i], which
can IndexError in regions with fewer AZs; before creating subnets
(private_db_subnets loop) validate the AZ count: get azs via
aws.get_availability_zones, then if len(azs) < len(db_cidrs) either raise a
clear exception (e.g., ValueError) or truncate/align db_cidrs to len(azs) and
proceed, and ensure the loop uses the validated azs and matching db_cidrs
indices so subnet creation (aws.ec2.Subnet, variables azs, db_cidrs,
private_db_subnets) always has matching lengths.
In `@aws-infra/pulumi_docs/installation.md`:
- Line 187: Update the Pulumi config key to include the project namespace
prefix: replace the bare config key "deployFrontend" with the namespaced key
"aws_infra:deployFrontend" wherever the command is used (the pulumi config set
invocation that currently references deployFrontend) so Pulumi reads the setting
from the same aws_infra namespace as the other keys.
In `@aws-infra/rds.py`:
- Around line 121-123: The RDS resource currently sets deletion_protection=False
and skip_final_snapshot=True which is unsafe for production; change the defaults
to deletion_protection=True and skip_final_snapshot=False on the RDS
creation/config (refer to the deletion_protection and skip_final_snapshot
parameters) and expose these as configurable flags/env-vars so non-prod can
override them (e.g., if env != "prod" or via a config object) while keeping the
safer defaults for production.
In `@aws-infra/README.md`:
- Line 341: The README's validation section contains several fenced code blocks
missing language identifiers; update each triple-backtick fence to include the
appropriate language spec (e.g., ```bash for shell/URL examples, ```text or
```http for raw HTTP response headers and log output) so syntax highlighting and
linters work properly—search the validation section for the unannotated ```
fences and add `bash`, `text`, or `http` respectively to the five occurrences
flagged (the URL examples -> bash/text, HTTP headers -> http/text, log outputs
-> text).
- Line 443: Fix the trailing space inside the bold emphasis for the Security
Group label: locate the bolded string containing "antenna-rds-sg" (the fragment
currently written as **antenna-rds-sg`) **) and remove the extra space before
the closing bold marker so it reads **antenna-rds-sg`)**; update the Markdown
accordingly to eliminate the stray space inside the emphasis markers.
In `@aws-infra/redis.py`:
- Around line 4-5: Update the import for the Redis security group: change the
module import from networking.security_group to networking.security_groups so
the symbol redis_sg is imported from the correct module; adjust the import
statement that currently references networking.security_group to read
networking.security_groups to match the actual file and ensure redis_sg is
resolved.
In `@aws-infra/requirements.txt`:
- Around line 1-4: The requirements file allows transitive vulnerabilities via
urllib3 and protobuf; update the dependency constraints in aws-infra's
requirements.txt by explicitly pinning secure versions: add/modify entries to
require urllib3>=2.6.0 and protobuf>=4.25.8 (or protobuf>=5.29.5 / >=6.31.1+)
alongside the existing pulumi, pulumi-aws, pulumi-command, and boto3 constraints
so the resolver selects patched releases, then re-run dependency resolution (pip
install / pip-compile) and verify with pip list and pip-audit; ensure
boto3/botocore compatibility when choosing the protobuf/urllib3 pins.
In `@aws-infra/storage/s3_policies.py`:
- Line 10: CLOUDFRONT_DISTRIBUTION_ID can be None (since it uses config.get), so
update the policy construction in s3_policies.py to validate this value and only
add the CloudFront policy statement when CLOUDFRONT_DISTRIBUTION_ID is truthy;
specifically, build a statements list, append the CloudFront statement (the
block that references
arn:aws:cloudfront::<account>:distribution/{CLOUDFRONT_DISTRIBUTION_ID}) only if
CLOUDFRONT_DISTRIBUTION_ID is set, then extend the list with the existing EB and
public statements so you never emit an ARN with "distribution/None".
In `@aws-infra/storage/s3.py`:
- Around line 31-41: The public access block configuration in
aws.s3.BucketPublicAccessBlock (public_access) contradicts the "safest defaults"
comment because block_public_acls=False and ignore_public_acls=False permit
ACL-based public access—either make the code match the comment by setting
block_public_acls=True and ignore_public_acls=True and, if applicable, change
object_ownership to "BucketOwnerEnforced" to fully disable ACLs, or if public
read is intentional (e.g., for static assets), update the comment to state that
public ACLs are allowed and document why; locate and update the
aws.s3.BucketPublicAccessBlock declaration (public_access) and any
object_ownership setting to apply the chosen fix.
- Around line 20-29: The comment says to disable ACLs but the
BucketOwnershipControls rule sets object_ownership="ObjectWriter" which enables
legacy ACLs; update the aws.s3.BucketOwnershipControls (the ownership variable)
to use object_ownership="BucketOwnerEnforced" so ACLs are disabled and ownership
is enforced by the bucket owner (use ObjectWriter only for legacy cross-account
upload cases or BucketOwnerPreferred if you must keep ACLs). Ensure the change
is applied where ownership is constructed for the assets_bucket.id and keep the
descriptive comment aligned with the new setting.
🧹 Nitpick comments (7)
aws-infra/pulumi_docs/installation.md (1)
381-393: Convert bare URLs to proper markdown links.Per markdownlint, bare URLs should use markdown link syntax for better accessibility and rendering consistency.
Proposed fix
-http://localhost:5555 +<http://localhost:5555>-- Pulumi Docs: https://www.pulumi.com/docs/ -- AWS Systems Manager: https://docs.aws.amazon.com/systems-manager/ -- Elastic Beanstalk: https://docs.aws.amazon.com/elasticbeanstalk/ +- Pulumi Docs: <https://www.pulumi.com/docs/> +- AWS Systems Manager: <https://docs.aws.amazon.com/systems-manager/> +- Elastic Beanstalk: <https://docs.aws.amazon.com/elasticbeanstalk/>aws-infra/modifications/storage.py (1)
54-58: Consider usinggetattrwith a default for safer attribute access.While
hasattr()works,getattr(settings, "DJANGO_AWS_ACCESS_KEY_ID", None)would be more explicit and allow checking for empty/falsy values in one step.USE_AWS = all([ getattr(settings, "DJANGO_AWS_ACCESS_KEY_ID", None), getattr(settings, "DJANGO_AWS_SECRET_ACCESS_KEY", None), getattr(settings, "DJANGO_AWS_STORAGE_BUCKET_NAME", None), ])This also handles cases where the setting exists but is empty.
aws-infra/storage/__init__.py (1)
1-2: Consider defining__all__in submodules for explicit exports.Wildcard imports work but can make the public API unclear. Adding
__all__tos3.pyands3_policies.pywould document the intended exports and satisfy linters.This is a minor suggestion—the current approach is acceptable for Pulumi infrastructure modules.
aws-infra/networking/routes.py (1)
26-31: Hardcoded[:2]slice limits route table associations.This associates only the first 2 subnets regardless of how many
private_db_subnetsexist. If additional subnets are added for multi-AZ redundancy, they won't receive this private route table and may have unintended routing.If the limit is intentional (e.g., RDS subnet group minimum), consider documenting why or using a constant:
RDS_REQUIRED_AZ_COUNT = 2 # RDS requires at least 2 AZs for i, subnet in enumerate(private_db_subnets[:RDS_REQUIRED_AZ_COUNT]):If all private DB subnets should be private, remove the slice:
for i, subnet in enumerate(private_db_subnets):aws-infra/__main__.py (1)
21-24: Consider explicit imports for better maintainability.Wildcard imports (
from ... import *) can make it difficult to trace where specific resources are defined and may cause namespace conflicts. For infrastructure-as-code modules, explicit imports improve clarity when debugging deployment issues.♻️ Suggested improvement
-from networking.security_group import * +from networking.security_group import eb_sg, rds_sg, redis_sg # --- Storage (S3 + policies) --- -from storage import * +from storage.s3 import assets_bucket +from storage.s3_policies import bucket_policy, public_access_blockaws-infra/secrets_manager.py (1)
18-45: Consider returning the secret version for complete resource tracking.The
create_secretfunction creates aSecretVersionbut doesn't return it. While the secret ARN is exported, tracking the version resource could be useful for debugging or dependencies.♻️ Optional improvement
def create_secret( key: str, value: pulumi.Input[str], description: str = "", -) -> aws.secretsmanager.Secret: +) -> tuple[aws.secretsmanager.Secret, aws.secretsmanager.SecretVersion]: secret_name = f"{PROJECT}-{STACK}-{key}" secret = aws.secretsmanager.Secret( # ... ) - aws.secretsmanager.SecretVersion( + version = aws.secretsmanager.SecretVersion( f"{secret_name}-version", secret_id=secret.id, secret_string=value, ) pulumi.export(f"{key}_SECRET_ARN", secret.arn) - return secret + return secret, versionaws-infra/redis.py (1)
21-44: Improve exception handling specificity.Catching bare
Exception(line 28) can hide unexpected errors like network issues or permission problems. Consider catching a more specific exception or at least logging the actual error for debugging.♻️ Proposed improvement
def get_or_create_subnet_group(): try: existing = aws.elasticache.get_subnet_group(name=REDIS_SUBNET_GROUP_NAME) + pulumi.log.info( + f"[Redis] Using existing ElastiCache subnet group: {existing.name}" + ) + return existing.name + except aws.elasticache.GetSubnetGroupError: + # Subnet group doesn't exist, create it pulumi.log.info( - f"[Redis] Using existing ElastiCache subnet group: {existing.name}" + f"[Redis] Subnet group '{REDIS_SUBNET_GROUP_NAME}' not found. Creating new one..." ) - return existing.name - except Exception: - pulumi.log.warn( - f"[Redis] Subnet group '{REDIS_SUBNET_GROUP_NAME}' not found. Creating new one..." - ) - sg = aws.elasticache.SubnetGroup( # ... ) return sg.nameIf the specific exception type isn't available, at minimum log the actual exception:
- except Exception: + except Exception as e: pulumi.log.warn( - f"[Redis] Subnet group '{REDIS_SUBNET_GROUP_NAME}' not found. Creating new one..." + f"[Redis] Subnet group lookup failed ({e}). Creating new one..." )
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@aws-infra/eb.py`:
- Around line 310-313: The S3 bucket resource eb_bundle_bucket is created with
force_destroy=True which risks accidental data loss; update the aws.s3.Bucket
creation to make force_destroy configurable (e.g., derive a boolean from
environment/config such as a Pulumi config key or an is_production flag) and
default it to False for production, while allowing True for disposable dev/test
stacks; change the force_destroy argument on eb_bundle_bucket to use that
config-derived variable so production retains objects unless explicitly
overridden.
🧹 Nitpick comments (2)
aws-infra/eb.py (2)
50-53: Consider pinning image tags instead of usinglatest.Using
latestfor all image tags can lead to non-reproducible deployments and unexpected behavior when images are updated. For production environments, consider using specific version tags or commit SHAs.
294-297: Consider being more specific with the solution stack regex.The regex
"64bit Amazon Linux 2.*running ECS"withmost_recent=Truewill always pick the latest matching stack. While generally safe, AWS occasionally deprecates or changes solution stacks. Consider logging or outputting the selected stack name for debugging.
carlosgjs
left a comment
carlosgjs
left a comment
There was a problem hiding this comment.
Added a few comments. There are also some good catches from coderabbitai.
| "{context_abs}" \ | ||
| --push | ||
|
|
||
| echo "Pushed -> $REPO_URL:{tag}" |
There was a problem hiding this comment.
Could we use this instead of shelling out to docker?
https://www.pulumi.com/registry/packages/docker/api-docs/image/
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@aws-infra/networking/security_group.py`:
- Around line 125-133: The egress rule using aws.ec2.SecurityGroupEgressArgs
with protocol "-1" and cidr_blocks ["0.0.0.0/0"] unnecessarily allows the
DB/Redis to initiate outbound connections (the comment claiming return traffic
needs it is misleading because security groups are stateful); remove the wide
open egress or replace it with a minimal, explicit egress policy that only
allows required destinations/ports (or omit egress so default stateful return
traffic still works) for both the DB security group block containing the egress
array and the Redis security group (the block around the SecurityGroupEgressArgs
at lines ~178–184); ensure any retained egress is justified and scoped to
specific CIDR or security group IDs rather than 0.0.0.0/0.
- Around line 58-65: The SecurityGroupIngressArgs entry opening port 5555 for
Flower is a security risk; remove or disable this ingress rule (the
aws.ec2.SecurityGroupIngressArgs with from_port/to_port 5555 and
cidr_blocks=[""]) and instead either (A) do not create a public ingress for port
5555 at all, or (B) restrict access to an internal bastion/VPN by replacing
cidr_blocks with a reference to an internal security group (e.g.
security_groups=[bastion_sg.id]) so only the bastion can reach port 5555, or (C)
keep no rule and require SSH tunneling/VPN to access Flower. Ensure the change
is applied to the block that constructs the admin UI ingress for port 5555.
🧹 Nitpick comments (2)
aws-infra/networking/security_group.py (2)
8-8: Consider using a dedicated VPC instead of the default VPC.The default VPC uses public subnets, which means RDS and ElastiCache instances could be publicly routable depending on subnet group configuration. For production workloads, a dedicated VPC with private subnets for databases and caches is strongly recommended.
23-24: Hardcoded resource names prevent multi-environment deployments.Names like
"antenna-eb-sg-pulumi"are static. If you need staging + production stacks, they'll collide. Usepulumi.get_stack()or a config value to namespace resources.Suggested approach
+stack = pulumi.get_stack() # e.g. "dev", "staging", "prod" + eb_sg = aws.ec2.SecurityGroup( - "antenna-eb-sg-pulumi", + f"antenna-eb-sg-{stack}", ... tags={ - "Name": "antenna-eb-sg-pulumi" + "Name": f"antenna-eb-sg-{stack}" } )Apply the same pattern to
rds_sgandredis_sg.
| # Admin UI (e.g. Flower) - restricted to your IP | ||
| aws.ec2.SecurityGroupIngressArgs( | ||
| protocol="tcp", | ||
| from_port=5555, # Port used for admin UI (e.g. Flower) | ||
| to_port=5555, | ||
| cidr_blocks=[""], # Only allow your personal IP | ||
| description="Admin access (Flower)", | ||
| ), |
There was a problem hiding this comment.
Exposing Flower (port 5555) directly on the EB security group is a security risk.
Even with a restricted admin CIDR, Flower's built-in authentication is minimal. Consider accessing Flower through an SSH tunnel or a VPN rather than opening a dedicated ingress rule on the public-facing SG.
🤖 Prompt for AI Agents
In `@aws-infra/networking/security_group.py` around lines 58 - 65, The
SecurityGroupIngressArgs entry opening port 5555 for Flower is a security risk;
remove or disable this ingress rule (the aws.ec2.SecurityGroupIngressArgs with
from_port/to_port 5555 and cidr_blocks=[""]) and instead either (A) do not
create a public ingress for port 5555 at all, or (B) restrict access to an
internal bastion/VPN by replacing cidr_blocks with a reference to an internal
security group (e.g. security_groups=[bastion_sg.id]) so only the bastion can
reach port 5555, or (C) keep no rule and require SSH tunneling/VPN to access
Flower. Ensure the change is applied to the block that constructs the admin UI
ingress for port 5555.
| # Databases need open egress so they can respond to clients. | ||
| # Security groups are stateful, so this does NOT expose the DB publicly. | ||
| egress=[ | ||
| aws.ec2.SecurityGroupEgressArgs( | ||
| protocol="-1", | ||
| from_port=0, | ||
| to_port=0, | ||
| cidr_blocks=["0.0.0.0/0"], | ||
| ) |
There was a problem hiding this comment.
Misleading comment: SGs are stateful, so response traffic is already allowed without explicit egress rules.
The comment says "Databases need open egress so they can respond to clients," but since AWS security groups are stateful, return traffic for established connections is automatically permitted. This egress rule allows the DB to initiate new outbound connections (e.g., to the internet), which is unnecessary for RDS and widens the blast radius if the instance is compromised. Consider restricting egress to only what's needed, or removing it entirely.
The same applies to the Redis SG egress on lines 178–184.
🤖 Prompt for AI Agents
In `@aws-infra/networking/security_group.py` around lines 125 - 133, The egress
rule using aws.ec2.SecurityGroupEgressArgs with protocol "-1" and cidr_blocks
["0.0.0.0/0"] unnecessarily allows the DB/Redis to initiate outbound connections
(the comment claiming return traffic needs it is misleading because security
groups are stateful); remove the wide open egress or replace it with a minimal,
explicit egress policy that only allows required destinations/ports (or omit
egress so default stateful return traffic still works) for both the DB security
group block containing the egress array and the Redis security group (the block
around the SecurityGroupEgressArgs at lines ~178–184); ensure any retained
egress is justified and scoped to specific CIDR or security group IDs rather
than 0.0.0.0/0.
2 participants
This PR adds end-to-end AWS deployment and infrastructure documentation for the Antenna platform, along with safe templates for deployment configuration and a small test fixture update to support S3 vs MinIO.
Closes: #967
What’s Included
Documentation
New
aws-infra/README.mdcovering the full Antenna platform deployment:/api/*proxying to EBDeployment templates (no secrets)
.ebextensions/00_setup.configtemplate with placeholders (no credentials committed)Dockerrun.aws.jsontemplate with placeholders for account/region/image tags and runtime configCode change
Updated
ami/tests/fixtures/storage.pyto support:DJANGO_AWS_*settings are present (Elastic Beanstalk/prod-like env)Why This Matters
The project previously lacked cohesive, end-to-end infrastructure documentation. These additions make it easier for maintainers and contributors to reproduce the deployment, understand AWS resource relationships, and safely operate/update the platform.
Notes
Summary by CodeRabbit
New Features
Documentation