Add S3/ECR "down" command resource handling#772
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a secret-backed random suffix to Pulumi production config and wires it into infrastructure code to produce unique S3 bucket names; updates import scripts and resource options (retain_on_delete / force_delete) for S3 and ECR resources. Changes
Sequence Diagram(s)sequenceDiagram
participant DevEnv as Dev/Script
participant Pulumi as Pulumi Program
participant Config as Pulumi Stack Config (secret)
participant AWS as AWS Resources (S3, ECR)
DevEnv->>Config: ensure `fund:randomSuffix` exists (generate if missing)
Pulumi->>Config: read `randomSuffix` (secret)
Pulumi->>Pulumi: concat(prefix, randomSuffix) -> bucket names
Pulumi->>AWS: create S3 buckets, encryption, public access, versioning (retain_on_delete)
Pulumi->>AWS: create ECR repositories (force_delete=True)
AWS-->>Pulumi: resource ARNs/names
Pulumi->>DevEnv: export bucket names (unsecret)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ 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 |
Contributor
Greptile SummaryThis PR improves infrastructure resource management by enabling proper teardown of S3 buckets and ECR repositories. The changes implement deterministic S3 bucket naming using a random suffix stored in Pulumi secrets, add Key improvements:
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| infrastructure/Pulumi.production.yaml | Added randomSuffix configuration for unique S3 bucket naming across deployments |
| infrastructure/main.py | Replaced bucket prefixes with deterministic names using suffix, added retain_on_delete to S3 resources, added force_delete to ECR repositories, and wrapped bucket exports in unsecret() |
| maskfile.md | Added random suffix generation/retrieval, added S3 resource imports with suffix interpolation, added --generate-code=false flag to imports, and excluded .scratchpad from markdown linting |
Last reviewed commit: 5cbb2f7
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances infrastructure lifecycle management by enabling proper handling of S3 buckets and ECR repositories during stack teardown and recreation. The changes introduce a deterministic naming scheme for S3 buckets using a randomly generated suffix stored in Pulumi secrets, add resource import statements for S3 resources, configure ECR repositories for automatic deletion on teardown, and suppress code generation during imports.
Changes:
- Added random suffix generation and storage for deterministic S3 bucket naming across infrastructure up/down cycles
- Added S3 resource import statements (bucket, encryption, versioning, public access blocks) to maskfile
- Added
force_delete=Trueto all ECR repositories andretain_on_delete=Trueto S3 bucket sub-resources - Added
--generate-code=falseflag to all pulumi import commands to suppress code generation recommendations - Added
.scratchpadto markdown linter ignore list
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| maskfile.md | Adds random suffix generation, S3 resource imports with --generate-code=false flag, and .scratchpad to markdown lint ignore list |
| infrastructure/main.py | Changes S3 buckets from bucket_prefix to explicit bucket names with random suffix, adds retain_on_delete to S3 sub-resources, adds force_delete to ECR repositories with documentation comment |
| infrastructure/Pulumi.production.yaml | Adds randomSuffix as encrypted Pulumi secret, removes YAML document separator |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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`:
- Line 50: The exported S3 bucket Outputs (data_bucket.bucket and
model_artifacts_bucket.bucket) are tainted secret because random_suffix is
created with stack_config.require_secret("randomSuffix"); fix this by calling
.unsecret() on the bucket name Outputs before exporting them (i.e., wrap the
Outputs you pass to pulumi.export or any final export variable with
.unsecret()), ensuring the exports return the plain bucket names rather than
“[secret]”. Locate uses of data_bucket.bucket and model_artifacts_bucket.bucket
in the export/exporting block (exports around where aws_s3_data_bucket_name and
aws_s3_model_artifacts_bucket_name are set) and apply Output.unsecret() to those
outputs.
In `@maskfile.md`:
- Around line 196-200: The export of S3 bucket names is currently secret-tainted
because random_suffix is loaded with require_secret (random_suffix) and
concatenated into data_bucket.bucket and artifacts_bucket.bucket, causing
pulumi.stack output to return [secret]; fix this in __main__.py by unsecreting
the Outputs before exporting—wrap the bucket name Outputs with
pulumi.Output.unsecret() (e.g., pulumi.Output.unsecret(data_bucket.bucket) and
pulumi.Output.unsecret(artifacts_bucket.bucket)) so aws_s3_data_bucket_name and
aws_s3_model_artifacts_bucket_name exports are not secret-tainted.
forstmeier
commented
Feb 25, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
infrastructure/__main__.py (2)
1799-1803: LGTM —pulumi.Output.unsecret()correctly strips the secret taint before export.This addresses the previously flagged issue: stack outputs
aws_s3_data_bucket_nameandaws_s3_model_artifacts_bucket_namewill now return the plain bucket name instead of[secret], unblocking the dependentmaskfile.mdcommands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/__main__.py` around lines 1799 - 1803, Exports currently wrap data_bucket.bucket and model_artifacts_bucket.bucket with pulumi.Output.unsecret to remove the secret taint before exporting; ensure the pulumi.export calls use pulumi.Output.unsecret for both exports (the pulumi.export lines and the pulumi.Output.unsecret(...) calls referencing data_bucket and model_artifacts_bucket) so stack outputs return plain bucket names instead of "[secret]".
1799-1803: LGTM —pulumi.Output.unsecret()correctly strips the secret taint before export.This addresses the previously flagged issue: stack outputs
aws_s3_data_bucket_nameandaws_s3_model_artifacts_bucket_namewill now return the plain bucket name instead of[secret].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/__main__.py` around lines 1799 - 1803, Wrap the S3 bucket output values with pulumi.Output.unsecret so exported stack outputs return plain bucket names; specifically ensure aws_s3_data_bucket_name and aws_s3_model_artifacts_bucket_name use pulumi.Output.unsecret(data_bucket.bucket) and pulumi.Output.unsecret(model_artifacts_bucket.bucket) respectively (using the pulumi.Output.unsecret API) and apply the same pattern to any other exported secret-tainted outputs you find.
🤖 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`:
- Line 50: After calling stack_config.require_secret("randomSuffix") and
assigning to random_suffix, validate that random_suffix is not an empty string
and fail fast if it is; add a guard immediately after the assignment (checking
if random_suffix == "" or if not random_suffix.strip()) and raise a clear
exception (or exit with an error) that mentions the missing/empty randomSuffix
so downstream bucket-name construction (e.g., the fund-data- and
fund-model-artifacts- suffix usage) cannot produce unintended empty suffixes.
- Line 50: Validate that the secret returned by
stack_config.require_secret("randomSuffix") is non-empty by applying a guard
function: create a small validator (e.g., _assert_non_empty_suffix) that raises
a ValueError if the suffix is an empty string and use .apply on the Pulumi
secret Output (random_suffix =
stack_config.require_secret("randomSuffix").apply(_assert_non_empty_suffix)) so
the empty value is rejected at runtime and downstream uses of random_suffix
(bucket names, IAM ARNs, exported outputs) cannot silently degrade.
---
Duplicate comments:
In `@infrastructure/__main__.py`:
- Around line 1799-1803: Exports currently wrap data_bucket.bucket and
model_artifacts_bucket.bucket with pulumi.Output.unsecret to remove the secret
taint before exporting; ensure the pulumi.export calls use
pulumi.Output.unsecret for both exports (the pulumi.export lines and the
pulumi.Output.unsecret(...) calls referencing data_bucket and
model_artifacts_bucket) so stack outputs return plain bucket names instead of
"[secret]".
- Around line 1799-1803: Wrap the S3 bucket output values with
pulumi.Output.unsecret so exported stack outputs return plain bucket names;
specifically ensure aws_s3_data_bucket_name and
aws_s3_model_artifacts_bucket_name use
pulumi.Output.unsecret(data_bucket.bucket) and
pulumi.Output.unsecret(model_artifacts_bucket.bucket) respectively (using the
pulumi.Output.unsecret API) and apply the same pattern to any other exported
secret-tainted outputs you find.
Collaborator
Author
|
Coveralls failed because the service is down externally. |
chrisaddy
approved these changes
Feb 25, 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
Another bit of cleanup for stuff I encountered running the infrastructure.
Summary by CodeRabbit