Merged
Conversation
forstmeier
added
python
Contributor
📝 WalkthroughWalkthroughThis PR refactors infrastructure code across AWS IAM, Pulumi configuration, and GitHub Actions workflows. Changes include renaming workflow jobs, rotating secrets, refactoring IAM roles and policies (particularly GitHub OIDC integration and SageMaker execution), updating S3 encryption configuration, introducing bootstrap logic, and aligning container definitions and test assertions with new resource structures. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
chrisaddy
approved these changes
Feb 18, 2026
Contributor
Greptile SummaryThis PR consolidates infrastructure fixes from manual local testing, focusing on resource ordering, IAM policy refactoring, and bootstrap tooling. Key Changes
Issues Found
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| infrastructure/Pulumi.production.yaml | Removed document separator and rotated encrypted secret values |
| infrastructure/main.py | Reordered resources, consolidated IAM policies, fixed array indexing bugs, changed encryption API, added recovery_window_in_days, and added protect flags |
| maskfile.md | Added bootstrap flag with resource import and GitHub environment secret configuration |
| tools/download_model_artifacts.py | Renamed environment variable from AWS_S3_ARTIFACTS_BUCKET_NAME to AWS_S3_MODEL_ARTIFACTS_BUCKET_NAME |
Last reviewed commit: 473c8f3
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR contains infrastructure fixes discovered during manual local testing, focusing on correcting resource references, reordering resource creation, adding bootstrap logic, and updating secret configurations.
Changes:
- Fixed environment variable naming from
AWS_S3_ARTIFACTS_BUCKET_NAMEtoAWS_S3_MODEL_ARTIFACTS_BUCKET_NAMEfor consistency - Refactored IAM role/policy structure to use
managed_policy_arnsinstead of separateRolePolicyAttachmentresources - Added bootstrap functionality for initial local setup, including resource import and GitHub Actions secret configuration
- Fixed bugs in ECS task definitions and IAM policies where
account_idwas incorrectly used instead of proper resource ARNs - Added resource protection flags to critical IAM resources
- Updated secret recovery windows and reverted S3 encryption API to non-V2 variants
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/download_model_artifacts.py | Renamed environment variable to AWS_S3_MODEL_ARTIFACTS_BUCKET_NAME for consistency |
| maskfile.md | Added bootstrap flag and logic for importing existing resources and configuring GitHub secrets |
| libraries/python/tests/test_infrastructure_configuration.py | Updated tests to match new managed_policy_arns pattern and args indexing |
| infrastructure/main.py | Major refactoring: reordered role/policy creation, fixed ARN bugs, added protect flags, changed to managed_policy_arns pattern, set recovery windows, reverted to non-V2 S3 encryption API |
| infrastructure/Pulumi.production.yaml | Updated encrypted secret values and removed invalid YAML separator |
| .github/workflows/teardown_infrastructure.yaml | Added explanatory comments about cron schedule timing |
| .github/workflows/launch_infrastructure.yaml | Renamed jobs for clarity and added explanatory comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/__main__.py`:
- Around line 135-154: The three Secret resources datamanager_secret,
portfoliomanager_secret, and shared_secret are created with
recovery_window_in_days=0 which permanently disables recovery; change these to a
non-zero recovery window (e.g., recovery_window_in_days=7) or add resource
protection by passing opts=pulumi.ResourceOptions(protect=True) to each
aws.secretsmanager.Secret (or do both) so production secrets are not
irreversibly deleted accidentally.
In `@maskfile.md`:
- Around line 198-220: The pulumi import commands (e.g., the
github_actions_infrastructure_role, github_actions_infrastructure_policy via
GITHUB_POLICY_ARN, sagemaker_execution_role, and sagemaker_execution_policy via
SAGEMAKER_POLICY_ARN) are currently silencing all errors with `2>/dev/null ||
true`; change these to detect and log failures instead: run each `pulumi import`
and capture its exit status and stderr, print a clear message including the
resource identifier and the captured error when non-zero, and only suppress the
specific "resource does not exist" case if you detect that exact condition;
apply this pattern for all imports inside the BOOTSTRAP block so import outcomes
are visible for debugging.
This was referenced Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Changes
Context
I'll reactivate the GitHub infrastructure workflows and test those as well.
Summary by CodeRabbit
Infrastructure & Configuration
Chores