-
Notifications
You must be signed in to change notification settings - Fork 11
feat(pulumi): add AWS infrastructure code and deployment guide #1052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(pulumi): add AWS infrastructure code and deployment guide #1052
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.
|
📝 WalkthroughWalkthroughAdds a full Pulumi-based AWS infrastructure suite for Antenna: networking, RDS, ElastiCache, ECR, image build/push, Elastic Beanstalk multi-container deployment, CloudFront + S3 frontend hosting, IAM and Secrets Manager integration, and S3 test/demo helpers. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, v10 is just an example. I’ll add a note explaining how contributors should choose version numbers so it's clear and consistent.
|
|
||
| - `/api/v2/` returns `200` | ||
| - Django container remains healthy | ||
| - Celery worker connects to Redis successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a section that explains exactly how each validation step was performed (API 200 check, EB health check, Worker/Beat logs, and Flower UI access). This clarifies how each item was confirmed during deployment.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add example versions of these files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
👷 Deploy request for antenna-ssec pending review.Visit the deploys page to approve it
|
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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..." )
| pulumi.log.info("Building UI inside Pulumi: cd ui && nvm use && yarn install && yarn build") | ||
| subprocess.run(["bash", "-lc", cmd], check=True) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use an absolute bash path for the UI build subprocess.
Avoid PATH hijacking by calling /bin/bash explicitly.
🔧 Safer executable path
- subprocess.run(["bash", "-lc", cmd], check=True)
+ subprocess.run(["/bin/bash", "-lc", cmd], check=True)🧰 Tools
🪛 Ruff (0.14.14)
[error] 133-133: subprocess call: check for execution of untrusted input
(S603)
[error] 133-133: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
In `@aws-infra/cloudfront.py` around lines 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.
| "name": "awscli", | ||
| "image": awscli_image, | ||
| "essential": False, | ||
| "memory": 256, | ||
| "command": ["sleep", "9999999"], | ||
| "environment": [{"name": "AWS_REGION", "value": "us-west-2"}], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and view the specific lines
if [ -f "aws-infra/eb.py" ]; then
echo "=== File exists, showing lines 160-180 for context ==="
sed -n '160,180p' "aws-infra/eb.py"
else
echo "=== Searching for eb.py files ==="
find . -name "eb.py" -type f 2>/dev/null | head -20
fiRepository: RolnickLab/antenna
Length of output: 937
🌐 Web query:
Pulumi aws.config.region Python API documentation
💡 Result:
aws.config.region is the Node.js/TypeScript convenience accessor. In Python, the equivalent is to read the AWS package config key aws:region via pulumi.Config("aws"), or to set region= on an explicit aws.Provider.
Read aws:region from config (Python)
import pulumi
aws_config = pulumi.Config("aws")
region = aws_config.require("region") # reads `aws:region`Pulumi config keys are namespaced like aws:region, and the docs show this exact Python pattern. [1]
Set it with:
pulumi config set aws:region us-west-2[1]
Set region on a provider (Python)
import pulumi_aws as aws
provider = aws.Provider("aws", region="us-west-2")The AWS provider’s region setting is documented (and can also come from AWS_REGION / AWS_DEFAULT_REGION). [2]
Sources: Pulumi configuration docs (including aws:region and Python example) [1]; Pulumi AWS Provider docs (region option and env vars) [2].
Avoid hardcoding AWS_REGION in the awscli container; use the configured region instead.
Hardcoded regions reduce stack portability and can target the wrong region in multi-region deployments. Use Pulumi's AWS configuration to dynamically read the region.
Use the configured region
+import pulumi
# Inside container definition:
- "environment": [{"name": "AWS_REGION", "value": "us-west-2"}],
+ "environment": [{"name": "AWS_REGION", "value": pulumi.Config("aws").get("region") or "us-west-2"}],Alternatively, set the region via pulumi config set aws:region <region> or configure it on an explicit aws.Provider.
📝 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.
| "name": "awscli", | |
| "image": awscli_image, | |
| "essential": False, | |
| "memory": 256, | |
| "command": ["sleep", "9999999"], | |
| "environment": [{"name": "AWS_REGION", "value": "us-west-2"}], | |
| }, | |
| "name": "awscli", | |
| "image": awscli_image, | |
| "essential": False, | |
| "memory": 256, | |
| "command": ["sleep", "9999999"], | |
| "environment": [{"name": "AWS_REGION", "value": pulumi.Config("aws").get("region") or "us-west-2"}], | |
| }, |
🤖 Prompt for AI Agents
In `@aws-infra/eb.py` around lines 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).
| "environment": backend_environment + [ | ||
| {"name": "CELERY_FLOWER_USER", "value": "admin"}, | ||
| {"name": "CELERY_FLOWER_PASSWORD", "value": "password"}, | ||
| {"name": "DEFAULT_PROCESSING_SERVICE_ENDPOINT", "value": "http://ml-backend-minimal:2000"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t hardcode Flower credentials.
admin/password in Dockerrun is a security footgun. Use Secrets Manager (or stack config) instead.
🔐 Move Flower creds to secrets
# Other secrets
backend_secrets += [
{"name": "SENDGRID_API_KEY", "valueFrom": _require_secret_arn(secret_arns, "SENDGRID_API_KEY_SECRET_ARN")},
{"name": "SENTRY_DSN", "valueFrom": _require_secret_arn(secret_arns, "SENTRY_DSN_SECRET_ARN")},
{"name": "REDIS_URL", "valueFrom": _require_secret_arn(secret_arns, "REDIS_URL_SECRET_ARN")},
{"name": "CELERY_BROKER_URL", "valueFrom": _require_secret_arn(secret_arns, "CELERY_BROKER_URL_SECRET_ARN")},
+ {"name": "CELERY_FLOWER_USER", "valueFrom": _require_secret_arn(secret_arns, "CELERY_FLOWER_USER_SECRET_ARN")},
+ {"name": "CELERY_FLOWER_PASSWORD", "valueFrom": _require_secret_arn(secret_arns, "CELERY_FLOWER_PASSWORD_SECRET_ARN")},
]
...
- "environment": backend_environment + [
- {"name": "CELERY_FLOWER_USER", "value": "admin"},
- {"name": "CELERY_FLOWER_PASSWORD", "value": "password"},
- {"name": "DEFAULT_PROCESSING_SERVICE_ENDPOINT", "value": "http://ml-backend-minimal:2000"},
- ],
+ "environment": backend_environment + [
+ {"name": "DEFAULT_PROCESSING_SERVICE_ENDPOINT", "value": "http://ml-backend-minimal:2000"},
+ ],🧰 Tools
🪛 Ruff (0.14.14)
[warning] 230-234: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
🤖 Prompt for AI Agents
In `@aws-infra/eb.py` around lines 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.
| # Scan on push = ON | ||
| image_scanning_configuration=aws.ecr.RepositoryImageScanningConfigurationArgs( | ||
| scan_on_push=False | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment contradicts code: scanning is disabled, not enabled.
The comment states "Scan on push = ON" but scan_on_push=False. Fix the comment to match the actual configuration.
📝 Proposed fix
- # Scan on push = ON
+ # Scan on push = OFF (enable for production)
image_scanning_configuration=aws.ecr.RepositoryImageScanningConfigurationArgs(
scan_on_push=False
),📝 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.
| # Scan on push = ON | |
| image_scanning_configuration=aws.ecr.RepositoryImageScanningConfigurationArgs( | |
| scan_on_push=False | |
| ), | |
| # Scan on push = OFF (enable for production) | |
| image_scanning_configuration=aws.ecr.RepositoryImageScanningConfigurationArgs( | |
| scan_on_push=False | |
| ), |
🤖 Prompt for AI Agents
In `@aws-infra/ecr.py` around lines 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".
| aws.iam.RolePolicy( | ||
| "ecs-execution-secrets-readonly", | ||
| role=ecs_execution_role.name, | ||
| policy=json.dumps( | ||
| { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| { | ||
| "Sid": "AllowSecretsReadForEcsTasks", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "secretsmanager:GetSecretValue", | ||
| "secretsmanager:DescribeSecret", | ||
| ], | ||
| "Resource": "arn:aws:secretsmanager:*:*:secret:*", | ||
| } | ||
| ], | ||
| } | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "iam_roles.py" -type fRepository: RolnickLab/antenna
Length of output: 86
🏁 Script executed:
wc -l ./aws-infra/iam_roles.pyRepository: RolnickLab/antenna
Length of output: 90
🏁 Script executed:
cat -n ./aws-infra/iam_roles.pyRepository: RolnickLab/antenna
Length of output: 9351
🏁 Script executed:
# Search for references to Secrets Manager ARNs or secret names in the codebase
rg -i "secrets" --max-count=20Repository: RolnickLab/antenna
Length of output: 2893
🏁 Script executed:
cat -n ./aws-infra/secrets_manager.pyRepository: RolnickLab/antenna
Length of output: 8120
🏁 Script executed:
# Check if there's a Pulumi config file to understand the configuration structure
find . -name "Pulumi*.yaml" -o -name "Pulumi*.yml" | head -5Repository: RolnickLab/antenna
Length of output: 44
🏁 Script executed:
# Check how the secrets are referenced in the ECS/EB configuration
grep -n "SECRET_ARN\|valueFrom" ./aws-infra/eb.py | head -30Repository: RolnickLab/antenna
Length of output: 1701
🏁 Script executed:
grep -n "pulumi.export.*SECRET" ./aws-infra/secrets_manager.pyRepository: RolnickLab/antenna
Length of output: 115
🏁 Script executed:
# Check if there's a way to reference or collect secret ARNs in iam_roles.py
head -20 ./aws-infra/iam_roles.pyRepository: RolnickLab/antenna
Length of output: 653
🏁 Script executed:
# Check if secrets_manager exports are accessible and how to collect them
grep -A5 "pulumi.export" ./aws-infra/secrets_manager.py | tail -20Repository: RolnickLab/antenna
Length of output: 269
🏁 Script executed:
# Let's check the project and stack naming patterns in Pulumi config or main
grep -n "get_project\|get_stack" ./aws-infra/*.pyRepository: RolnickLab/antenna
Length of output: 269
🏁 Script executed:
# Check if there's any mechanism to get all secret ARNs back from secrets_manager
grep -n "^[A-Z_]*secret" ./aws-infra/secrets_manager.py | grep -v "def\|#"Repository: RolnickLab/antenna
Length of output: 466
🏁 Script executed:
# Check if iam_roles.py could import secrets from secrets_manager.py
grep -n "^from\|^import" ./aws-infra/iam_roles.pyRepository: RolnickLab/antenna
Length of output: 194
🏁 Script executed:
# Check if EB_ENV could be imported and used
grep -n "from.*secrets_manager\|import.*secrets_manager" ./aws-infra/*.pyRepository: RolnickLab/antenna
Length of output: 171
Secrets Manager access is overly broad.
arn:aws:secretsmanager:*:*:secret:* grants access to all secrets. Since secrets follow the naming pattern antenna-{STACK}-*, scope to that pattern instead. This is achievable by replacing the wildcard with arn:aws:secretsmanager:*:*:secret:antenna-${STACK}-* or by importing the actual secret objects from secrets_manager.py and building an ARN list.
🔐 Example: scope to naming pattern
+STACK = pulumi.get_stack()
...
- "Resource": "arn:aws:secretsmanager:*:*:secret:*",
+ "Resource": f"arn:aws:secretsmanager:*:*:secret:antenna-{STACK}-*",Apply the same scope to the EC2 role policy (lines 109-125).
Also applies to: 109-125
🤖 Prompt for AI Agents
In `@aws-infra/iam_roles.py` around lines 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.
| from networking.subnets import redis_default_subnets | ||
| from networking.security_group import redis_sg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for security_group files in networking directory
fd -t f 'security_group' aws-infra/networking/
# Also explore the networking directory structure
echo "=== Networking directory structure ==="
ls -la aws-infra/networking/ 2>/dev/null || echo "Directory not found"
# Find all Python files in networking
echo "=== Python files in networking ==="
fd -t f '\.py$' aws-infra/networking/Repository: RolnickLab/antenna
Length of output: 661
🏁 Script executed:
#!/bin/bash
# Check if redis_sg is defined in security_groups.py
rg 'redis_sg' aws-infra/networking/security_groups.pyRepository: RolnickLab/antenna
Length of output: 137
Correct the import path from security_group to security_groups.
The import at line 5 references networking.security_group, but the actual file is aws-infra/networking/security_groups.py (plural). Change the import to from networking.security_groups import redis_sg.
🤖 Prompt for AI Agents
In `@aws-infra/redis.py` around lines 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.
| pulumi>=3.0.0,<4.0.0 | ||
| pulumi-aws>=7.0.0,<8.0.0 | ||
| pulumi-command>=0.9.0,<1.0.0 | ||
| boto3>=1.26,<2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
GHSA-2xpw-w6gg-jr37 urllib3 vulnerability
💡 Result:
GHSA-2xpw-w6gg-jr37 (CVE-2025-66471) — urllib3 streaming decompression DoS
- What it is: urllib3’s streaming response API can fully decompress a small, highly-compressed response in a single operation, causing excessive CPU use and very large memory allocation on the client (resource exhaustion / DoS). This is triggered when streaming responses with
Content-Encodingsuch asgzip,deflate,br, orzstd. [1][3][4] - Affected versions:
urllib3 >= 1.0, < 2.6.0(often described as “2.5.0 and earlier” for the v2 line). [1][3] - Fixed version: 2.6.0. [1]
- Commonly affected calls:
HTTPResponse.stream(),read(),read1(),read_chunked(),readinto()when decoding/decompression is enabled. [1] - Remediation:
- Upgrade to
urllib3 >= 2.6.0. [1] - If you use Brotli (
urllib3[brotli]/Content-Encoding: br), also ensure Brotli/brotlicffi is updated as required by the advisory (and some distros note an extra brotli package update may be needed). [1][2]
- Upgrade to
Sources: GitHub Advisory Database and OSV entries for GHSA-2xpw-w6gg-jr37; third-party summaries referencing the same CVE/advisory. [1][3][4]
References
- GitHub Advisory: GHSA-2xpw-w6gg-jr37 (urllib3) — patched in 2.6.0. [1]
- Rapid7 (Ubuntu advisories context; brotli note). [2]
- OSV entry for GHSA-2xpw-w6gg-jr37. [3]
- Tenable CVE page for CVE-2025-66471. [4]
🏁 Script executed:
# Check what versions of urllib3 and protobuf are required by pulumi 3.x
# This requires looking at pulumi's dependencies
python3 << 'EOF'
import json
import urllib.request
# Fetch pulumi 3.x from PyPI to see its dependencies
try:
url = "https://pypi.org/pypi/pulumi/3.0.0/json"
with urllib.request.urlopen(url) as response:
data = json.loads(response.read())
print("=== Pulumi 3.0.0 dependencies ===")
if 'requires_dist' in data.get('info', {}):
for req in data['info']['requires_dist']:
if req and ('urllib3' in req or 'protobuf' in req):
print(req)
except Exception as e:
print(f"Error fetching: {e}")
# Try latest 3.x version
try:
url = "https://pypi.org/pypi/pulumi/3.120.0/json"
with urllib.request.urlopen(url) as response:
data = json.loads(response.read())
print("\n=== Pulumi 3.120.0 dependencies ===")
if 'requires_dist' in data.get('info', {}):
for req in data['info']['requires_dist']:
if req and ('urllib3' in req or 'protobuf' in req):
print(req)
except Exception as e:
print(f"Error: {e}")
EOFRepository: RolnickLab/antenna
Length of output: 340
🏁 Script executed:
# Try to find pulumi's setup.py or pyproject.toml to see its dependencies
python3 << 'EOF'
import subprocess
import sys
# Use pip-compile or pip to show dependencies without installing
# First, let's check if we can query pip's cache or use pip index
try:
result = subprocess.run(
["pip", "index", "versions", "pulumi"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
print("=== Pulumi versions available ===")
print(result.stdout[:500])
except Exception as e:
print(f"pip index error: {e}")
# Alternative: check if we can use pipdeptree-like approach
# Let's search for pulumi on GitHub to find setup.py
print("\nSearching for pulumi dependencies...")
EOF
# Try to search for pulumi's setup.py or pyproject.toml in a GitHub context
# or use alternative method to get dependency info
rg "urllib3|protobuf" --type py -i 2>/dev/null | head -20 || echo "No urllib3/protobuf mentions in repo"Repository: RolnickLab/antenna
Length of output: 821
🌐 Web query:
pulumi 3.x transitive dependencies urllib3 version requirement
💡 Result:
Pulumi’s Python SDK package (pulumi on PyPI) in the 3.x line does not depend on urllib3 (directly or transitively). Its required dependencies are things like grpcio, protobuf, pyyaml, semver, etc., but not requests/urllib3. [1][2]
So if you’re seeing an urllib3 constraint in a Pulumi 3.x Python environment, it’s coming from other packages you installed alongside Pulumi—most commonly requests, whose dependency constraint is:
urllib3 >= 1.21.1, < 3[3][4]
Bottom line: Pulumi 3.x itself doesn’t impose an urllib3 version requirement; requests (or another dependency like botocore) does. [1][3]
Sources: [1] https://pypistats.org/packages/pulumi, [2] pulumi/pulumi#19398, [3] https://urllib3.readthedocs.io/en/latest/v2-migration-guide.html, [4] psf/requests#6734
🌐 Web query:
pulumi 3.0 setup.py requires urllib3
💡 Result:
pulumi (the Pulumi Python SDK) does not directly depend on urllib3. Its published dependency set (e.g., grpcio, protobuf, pyyaml, semver, etc.) does not include urllib3.[2] Also, Pulumi’s PyPI releases are distributed as wheels (so a normal pip install pulumi shouldn’t be running setup.py for Pulumi itself).[1]
If you’re seeing “setup.py requires urllib3” during a Pulumi-related install, it’s almost always because another package in your environment is being built/installed from source and that package pulls in HTTP tooling—commonly requests, which in turn depends on urllib3 (and has historically had strict version constraints).[4]
What to do:
-
Install using pip in a venv (not
setup.py install)
Pulumi’s own docs recommend usingpip/poetry/uvand virtual environments.[3] -
Upgrade your build tooling first (avoids many “setup.py”/build isolation oddities):
python -m pip install -U pip setuptools wheel- If the failing build mentions
requestsorurllib3, install/upgradeurllib3explicitly:
python -m pip install -U urllib3(urllib3 installation is standard via pip.)[5]
If you paste the exact error log (the package name whose setup.py is running, and the traceback), I can pinpoint which dependency is actually demanding urllib3 and what version constraint to use.
🏁 Script executed:
# Check boto3 and botocore dependency chains
python3 << 'EOF'
import subprocess
# Check boto3 dependencies - it likely depends on botocore -> requests -> urllib3
try:
result = subprocess.run(
["pip", "show", "boto3"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
print("=== Boto3 Info ===")
print(result.stdout)
except Exception as e:
print(f"Error: {e}")
# Let's also check what requests version constraint is
try:
result = subprocess.run(
["pip", "show", "requests"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
print("\n=== Requests Info ===")
print(result.stdout)
except Exception as e:
print(f"Error: {e}")
EOF
# Check the actual requirements.txt to see if requests is listed
if [ -f aws-infra/requirements.txt ]; then
echo "=== Current aws-infra/requirements.txt ==="
cat aws-infra/requirements.txt
fiRepository: RolnickLab/antenna
Length of output: 567
🌐 Web query:
boto3 1.26 dependencies botocore requests urllib3
💡 Result:
For the boto3 1.26.x line, the dependency chain looks like this:
- boto3 1.26.x → botocore:
botocore>=1.29.x,<1.30.0(exact patch depends on the boto3 patch) [1] - boto3 1.26.x → s3transfer:
s3transfer>=0.6.0,<0.7.0[1] - boto3 1.26.x → jmespath:
jmespath>=0.7.1,<2.0.0[1]
Then, for the transitive HTTP layer:
- botocore 1.29.x → urllib3:
- commonly:
urllib3>=1.25.4,<1.27(this is what you’ll hit on Python < 3.10, and in many 1.29.x environments) [1][2] - on Python 3.10+, botocore later introduced a relaxed cap allowing urllib3 2.0.x but not 2.1.x (i.e.
<2.1)—this is discussed against botocore 1.29.1 [3]
- commonly:
And regarding requests:
- botocore does not depend on
requests(botocore’s required deps arejmespath,python-dateutil,urllib3) [2] - If you separately install requests, it typically allows urllib3 2.x (e.g.
urllib3>=1.21.1,<3) [4]—so conflicts usually come from botocore’s tighter urllib3 cap, not from requests.
Sources: [1] [2] [3] [4]
🌐 Web query:
protobuf transitive dependencies pulumi 3.x HIGH severity vulnerability
💡 Result:
Pulumi 3.x projects often pull Protocol Buffers libraries transitively (most commonly via google.golang.org/grpc → google.golang.org/protobuf in Go, or via protobufjs in some JS/SDK tooling). The “HIGH severity protobuf transitive dependency” findings usually map to one of these:
1) Go: google.golang.org/protobuf (HIGH)
CVE-2024-24786 (HIGH, DoS)
protojson.Unmarshal can get stuck in an infinite loop on certain invalid JSON inputs; fixed in google.golang.org/protobuf v1.33.0. [2][3]
What to do (Go / Pulumi SDK v3 projects):
- Ensure your build resolves protobuf to >= v1.33.0 (even if it’s only transitive), e.g.:
go get google.golang.org/protobuf@v1.33.0go mod tidy
- If you maintain a Pulumi provider/plugin (Go binary), rebuild with protobuf >= v1.33.0. [2][3]
CVE-2023-24535 (HIGH, DoS)
Parsing certain invalid text-format protobuf inputs can panic; fixed in v1.29.1. [1]
2) JS (and some Pulumi tooling/SDKs): protobufjs (HIGH)
CVE-2022-25878 (HIGH)
This shows up as a transitive dependency issue in Pulumi’s Policy Python SDK history; pulumi-policy < 1.6.0 was affected and 1.6.0 bumped protobufjs to a fixed version. [4]
What to do:
- If your scan is specifically against
pulumi-policy, upgrade topulumi-policy >= 1.6.0. [4]
Quick way to confirm what you’re actually hitting
- Go:
go mod graph | grep protobuf(and check the resolvedgoogle.golang.org/protobufversion) - Node:
npm ls protobufjs/yarn why protobufjs
If you paste the exact vulnerability ID your scanner reports (e.g., CVE-2024-24786 vs CVE-2022-25878) and which Pulumi package/component it flags (CLI? sdk/v3? provider plugin? pulumi-policy?), I can tell you the smallest safe upgrade/override to apply.
Sources: [1] [2] [3] [4]
🌐 Web query:
CVE protobuf python HIGH severity pulumi 3.x transitive dependencies
💡 Result:
The HIGH severity protobuf Python CVE you’re likely seeing in dependency scanners for Pulumi Python projects is CVE-2025-4565 (GitHub advisory GHSA-8qvm-5x2c-j2w7) affecting the protobuf PyPI package. It’s a Denial-of-Service (DoS) issue in the pure-Python protobuf backend (can crash with RecursionError) when parsing maliciously crafted, deeply-recursive protobuf payloads. [1][2]
Affected / fixed protobuf versions (Python)
Affected versions (per advisory/NVD) include:
< 4.25.8>= 5.26.0rc1, < 5.29.5>= 6.30.0rc1, < 6.31.1[1][2]
Fixed versions:
- 4.25.8
- 5.29.5
- 6.31.1 [1][2]
What to do for Pulumi 3.x (Python) when it’s transitive
- Identify which protobuf you actually have installed (this is what matters for scanners):
python -c "import google.protobuf; print(google.protobuf.__version__)"- or
pip show protobuf
- Ensure your environment resolves protobuf to a fixed version, e.g. one of:
protobuf>=4.25.8,<5protobuf>=5.29.5,<6protobuf>=6.31.1,<7(pick the major line compatible with your stack) [1][2]
- If Pulumi (or another dependency) allows a vulnerable protobuf range, you can usually remediate by pinning protobuf in your project (requirements/lockfile) to a patched version, then re-lock/rebuild.
If you paste your pipdeptree output (or poetry.lock / uv.lock) showing where protobuf is coming from, I can point to the exact transitive edge and the safest pin for your specific Pulumi 3.x version.
Sources: [1] GHSA-8qvm-5x2c-j2w7 [2] https://nvd.nist.gov/vuln/detail/CVE-2025-4565
🏁 Script executed:
# Check what protobuf version pulumi 3.x would require
python3 << 'EOF'
import subprocess
try:
result = subprocess.run(
["pip", "show", "protobuf"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
print("=== Protobuf Info ===")
print(result.stdout)
except Exception as e:
print(f"Error: {e}")
# Check pip cache for pulumi's declared dependencies
try:
result = subprocess.run(
["pip", "index", "versions", "pulumi-aws"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
print("\n=== Pulumi-AWS versions ===")
print(result.stdout[:300])
except Exception as e:
print(f"Error: {e}")
EOFRepository: RolnickLab/antenna
Length of output: 738
Correct the source and version constraints for known transitive dependency vulnerabilities.
The requirements pull in HIGH severity vulnerabilities through transitive dependencies:
-
urllib3 (GHSA-2xpw-w6gg-jr37): Not from
pulumidirectly, but fromboto3 → botocore → urllib3. Botocore 1.29.x (required by boto3 1.26.x) constrains urllib3 to< 1.27or< 2.1, pulling in vulnerable versions. Fixed in urllib3 >= 2.6.0 (not just 2.0). Mitigation: explicitly pinurllib3>=2.6.0if your environment allows it, or monitor for botocore updates that relax this constraint. -
protobuf (CVE-2025-4565): Pulumi transitively depends on protobuf. Fixed versions are 4.25.8, 5.29.5, or 6.31.1+. If your resolved version falls below these, consider pinning protobuf to a patched release.
Recommendation: Run pip list | grep -E "urllib3|protobuf" to confirm your actual resolved versions, then use pip-audit or similar tooling to track when upstream dependencies (botocore, pulumi) release compatible fixes.
🧰 Tools
🪛 OSV Scanner (2.3.2)
[HIGH] 1-1: protobuf 5.29.5: protobuf affected by a JSON recursion depth bypass
[HIGH] 1-1: urllib3 1.26.20: urllib3 streaming API improperly handles highly compressed data
[HIGH] 1-1: urllib3 1.26.20: Decompression-bomb safeguards bypassed when following HTTP redirects (streaming API)
[HIGH] 1-1: urllib3 1.26.20: urllib3 allows an unbounded number of links in the decompression chain
[HIGH] 1-1: urllib3 1.26.20: urllib3 redirects are not disabled when retries are disabled on PoolManager instantiation
🤖 Prompt for AI Agents
In `@aws-infra/requirements.txt` around lines 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.
| config = pulumi.Config("aws_infra") | ||
|
|
||
| AWS_ACCOUNT_ID = config.require("aws_account_id") | ||
| CLOUDFRONT_DISTRIBUTION_ID = config.get("CLOUDFRONT_DISTRIBUTION_ID") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CloudFront distribution ID may be None, causing invalid policy ARN.
CLOUDFRONT_DISTRIBUTION_ID uses config.get() which returns None if unset. If None, the CloudFront policy statement at lines 72-75 will contain an invalid ARN (arn:aws:cloudfront::<account>:distribution/None), potentially causing deployment failures or an ineffective policy condition.
🛡️ Proposed fix: validate or skip CloudFront statement
config = pulumi.Config("aws_infra")
AWS_ACCOUNT_ID = config.require("aws_account_id")
-CLOUDFRONT_DISTRIBUTION_ID = config.get("CLOUDFRONT_DISTRIBUTION_ID")
+CLOUDFRONT_DISTRIBUTION_ID = config.get("CLOUDFRONT_DISTRIBUTION_ID") # Optional: omit CF statement if not setThen conditionally include the CloudFront statement:
statements = []
# Only add CloudFront statement if distribution ID is configured
if CLOUDFRONT_DISTRIBUTION_ID:
statements.append({
"Sid": "AllowCloudFrontServicePrincipalOnly",
# ... rest of statement
})
# Always add EB and public statements
statements.extend([
# EB bucket access
{ ... },
# EB object access
{ ... },
# Public read
{ ... },
])Also applies to: 70-78
🤖 Prompt for AI Agents
In `@aws-infra/storage/s3_policies.py` at 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".
| # --------------------------------------------------------- | ||
| # Object Ownership (DISABLE ACLs properly) | ||
| # --------------------------------------------------------- | ||
| ownership = aws.s3.BucketOwnershipControls( | ||
| f"{bucket_name}-ownership", | ||
| bucket=assets_bucket.id, | ||
| rule=aws.s3.BucketOwnershipControlsRuleArgs( | ||
| object_ownership="ObjectWriter" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
AWS S3 BucketOwnerEnforced vs ObjectWriter best practice ACL 2024 2025
💡 Result:
Best practice (2024–2025): use BucketOwnerEnforced (ACLs disabled) for almost everything
- BucketOwnerEnforced is the default for new S3 buckets and disables ACLs. The bucket owner automatically owns and has full control of every object, and access is managed only with policies (IAM, bucket/access point policies, SCPs, etc.). AWS recommends keeping ACLs disabled except for unusual cases. [1][2][3]
- Operationally: with BucketOwnerEnforced, ACLs no longer grant permissions; attempts to set/update ACLs fail with
AccessControlListNotSupported(reads are still supported). [4]
When to use ObjectWriter (ACLs enabled): rare / legacy only
Use ObjectWriter only if you must keep the legacy S3 model where:
- The uploader account owns the object, and
- You rely on object ACLs to grant access (per-object ACL management). [1][2]
This is generally not recommended for modern architectures because it reintroduces ACL complexity and cross-account ownership surprises (policy-only access patterns break down). AWS’s guidance is to disable ACLs for most use cases. [2][3]
The usual alternative if you can’t fully disable ACLs: BucketOwnerPreferred
If you need cross-account uploads but still can’t turn ACLs off (compatibility reasons), AWS positions BucketOwnerPreferred as the “keep ACLs enabled, but make bucket owner the owner” approach when uploads include the bucket-owner-full-control canned ACL—and AWS strongly recommends enforcing that in bucket policy. [1][3]
Practical decision rule
- Choose BucketOwnerEnforced (recommended default): you want policy-only access control and consistent bucket-owner ownership. [1][2][3]
- Choose BucketOwnerPreferred: you need ACLs enabled for compatibility, but you still want the bucket owner to own objects (and you can enforce
bucket-owner-full-control). [1][3] - Choose ObjectWriter: only when you must rely on object ACLs and require uploader-owned objects (legacy/edge cases). [1][2]
Sources: AWS S3 User Guide pages on Object Ownership, ACL guidance, and enforcement recommendations. [1][2][3][4]
Fix the contradiction between code comment and ownership model.
The code comment says "DISABLE ACLs properly," but ObjectWriter actually enables ACLs (legacy model where the uploader owns objects). AWS best practice for new buckets is BucketOwnerEnforced, which disables ACLs entirely and manages access through policies only.
Change to:
object_ownership="BucketOwnerEnforced" # Disables ACLs; recommended default for new bucketsUse ObjectWriter only if cross-account object uploads with per-object ACL control are required (rare/legacy compatibility cases). BucketOwnerPreferred is the alternative if ACLs must remain enabled but the bucket owner should own all objects.
🤖 Prompt for AI Agents
In `@aws-infra/storage/s3.py` around lines 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.
| # --------------------------------------------------------- | ||
| # Block Public Access (ON) - safest defaults | ||
| # --------------------------------------------------------- | ||
| public_access = aws.s3.BucketPublicAccessBlock( | ||
| f"{bucket_name}-public-access", | ||
| bucket=assets_bucket.id, | ||
| block_public_acls=False, | ||
| ignore_public_acls=False, | ||
| block_public_policy=True, | ||
| restrict_public_buckets=True, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public access block settings contradict the "safest defaults" comment.
The comment states "safest defaults" but block_public_acls=False and ignore_public_acls=False allow public ACLs on objects. Combined with object_ownership="ObjectWriter" (which enables ACLs), this could permit public object access.
If public access is intentional (e.g., for static assets), update the comment to reflect that. If not, consider:
Proposed fix for truly blocking public access
public_access = aws.s3.BucketPublicAccessBlock(
f"{bucket_name}-public-access",
bucket=assets_bucket.id,
- block_public_acls=False,
- ignore_public_acls=False,
+ block_public_acls=True,
+ ignore_public_acls=True,
block_public_policy=True,
restrict_public_buckets=True,
)Alternatively, if public read is needed for assets served via CloudFront or direct S3 URLs, clarify the intent in the comment.
🤖 Prompt for AI Agents
In `@aws-infra/storage/s3.py` around lines 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| eb_bundle_bucket = aws.s3.Bucket( | ||
| "antenna-eb-bundles-pulumi", | ||
| force_destroy=True, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
force_destroy=True may cause accidental data loss.
Setting force_destroy=True allows Pulumi to delete the S3 bucket even when it contains objects. While convenient for development, this is risky for production environments where deployment bundles might need to be retained for rollback or audit purposes.
💡 Suggested approach
Consider making this configurable based on environment:
+import pulumi
+
+config = pulumi.Config()
+is_production = config.get_bool("is_production") or False
+
eb_bundle_bucket = aws.s3.Bucket(
"antenna-eb-bundles-pulumi",
- force_destroy=True,
+ force_destroy=not is_production,
)🤖 Prompt for AI Agents
In `@aws-infra/eb.py` around lines 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.
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
✏️ Tip: You can customize this high-level summary in your review settings.