Feature - flexible s3 lifecycle rules #54
Conversation
- Add expiration_days variable for current object expiration - Add noncurrent_version_expiration_days variable for old version expiration - Update lifecycle rules to use variables instead of hardcoded values - Expiration automatically enables when days > 0 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add variables for all S3 lifecycle rule fields - Include abort_incomplete_multipart_upload_days - Add deep archive transition variables - Add standard IA transition variables - Add separate cloudtrail-specific glacier transition variables - All lifecycle behaviors now fully configurable via variables 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Rename lifecycle variables to have cloudtrail_ prefix for clarity - Add complete set of cloudtrail-specific lifecycle variables - Separate archive and cloudtrail bucket lifecycle configurations - All cloudtrail lifecycle behaviors now independently configurable 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Replace individual lifecycle variables with archive_lifecycle_config and cloudtrail_lifecycle_config maps - Use Terraform optional() function with sensible defaults for all fields - Clean up variables.tf by removing 20+ individual lifecycle variables - Maintain backward compatibility through default values - Improve configuration organization and readability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughLifecycle configuration for S3 buckets in the Terraform codebase has been refactored. The previous use of individual variables for lifecycle settings is replaced with structured object variables, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Terraform
participant ArchiveBucketModule
participant CloudTrailBucketModule
User->>Terraform: Apply configuration
Terraform->>ArchiveBucketModule: Pass archive_lifecycle_config object
Terraform->>CloudTrailBucketModule: Pass cloudtrail_lifecycle_config object
ArchiveBucketModule->>ArchiveBucketModule: Dynamically assign lifecycle rules from config
CloudTrailBucketModule->>CloudTrailBucketModule: Dynamically assign lifecycle rules from config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/variables.tf (2)
24-40: Re-evaluate default Glacier transition for archives (enabled=true by default)The PR goal is “flexible lifecycle rules” and avoiding automatic Glacier moves when undesired. Defaulting enable_glacier_transition = true (365 days) keeps Glacier enabled by default. If you intend an opt-in posture, set default to false; otherwise, please confirm this preserves prior behavior for existing users.
24-40: Breaking change note: variable interface changed to objects — plan migration pathSwitching from discrete variables to object configs breaks existing tfvars. Either:
- Provide a deprecation bridge (keep old variables and merge into new object via locals), or
- Document a migration guide and bump component version accordingly.
I can draft a migration snippet (locals + merge) if you prefer a non-breaking path.
Also applies to: 42-58
src/main.tf (1)
176-189: Optional: guard against mis-ordered transitions at the module edgeIf you don’t add variable validations, you can enforce ordering here to reduce plan-time failures:
+ # Optional guards; remove if validations are added at variables layer + standard_transition_days = var.archive_lifecycle_config.standard_transition_days > 0 + && var.archive_lifecycle_config.standard_transition_days < var.archive_lifecycle_config.glacier_transition_days ? var.archive_lifecycle_config.standard_transition_days : var.archive_lifecycle_config.standard_transition_days + glacier_transition_days = var.archive_lifecycle_config.glacier_transition_days > 0 + && var.archive_lifecycle_config.glacier_transition_days < var.archive_lifecycle_config.deeparchive_transition_days ? var.archive_lifecycle_config.glacier_transition_days : var.archive_lifecycle_config.glacier_transition_daysPrefer validations in variables.tf for clearer UX; this is just a fallback idea.
Also applies to: 240-252
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main.tf(2 hunks)src/variables.tf(2 hunks)
⏰ 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). (1)
- GitHub Check: Summary
🔇 Additional comments (2)
src/variables.tf (1)
44-45: No action needed: downstream module supports null for abort_incomplete_multipart_upload_daysIn cloudposse/s3-bucket v4.10.0, omitting or setting
abort_incomplete_multipart_upload_daystonulldisables the abort-incomplete-multipart-upload feature. Your default ofnullinsrc/variables.tfis therefore safe and requires no changes.[Citation: cloudposse/s3-bucket docs (v4.10.0)]
src/main.tf (1)
185-188: Nice: conditional enablement of expirationsTying enable_current_object_expiration and enable_noncurrent_version_expiration to days > 0 is clean and avoids “enabled with zero days” traps.
Also applies to: 249-252
|
/terratest |
oycyc
added
enhancement
|
There are no real tests for this component. So we set terratest statuses to successful execution without running any tests |
|
Thanks @brucex for creating this pull request! A maintainer will review your changes shortly. Please don't be discouraged if it takes a while. While you wait, make sure to review our contributor guidelines. Tip Need help or want to ask for a PR review to be expedited?Join us on Slack in the |
40bce16
|
These changes were released in v1.536.0. |
2 participants
what
why
Summary by CodeRabbit