Skip to content

Conversation

milldr
Copy link
Contributor

@milldr milldr commented Aug 14, 2025

what

This pull request introduces support for using S3-based state locking as an alternative to DynamoDB for Terraform state management. It adds a new variable to control this behavior and ensures that DynamoDB resources are not created when S3 state locking is enabled. The changes also make sure that documentation and outputs reflect the new logic.

Support for S3 State Locking:

  • Added a new variable s3_state_lock_enabled to allow switching between DynamoDB and S3 for state locking, with documentation updates in src/README.md and variable definition in src/variables.tf. [1] [2]
  • Updated local logic in src/main.tf to disable DynamoDB automatically when s3_state_lock_enabled is true, and passed this value to the tfstate_backend module.

Conditional Resource and Output Handling:

  • Modified the IAM policy document in src/iam.tf to use the new local variable for enabling DynamoDB, ensuring the correct resource is referenced.
  • Updated outputs in src/outputs.tf to use the local DynamoDB enabled flag, so DynamoDB outputs are empty when S3 state locking is used.

why

  • Fix issues using S3 statefile locking rather than DynamoDB

references

Summary by CodeRabbit

  • New Features
    • Added option to enable S3-based state locking via a new input; when enabled, DynamoDB state-lock resources are not created.
    • Module now honors the new setting across configuration, ensuring consistent behavior.
  • Documentation
    • Updated README to document the new input and its behavior.
  • Refactor
    • Updated internal logic so DynamoDB-related outputs and permissions are conditionally exposed based on the new setting.

Copy link

coderabbitai bot commented Aug 14, 2025

Walkthrough

Introduces s3_state_lock_enabled to enable S3-based state locking and conditionally disable DynamoDB resources. Refactors logic to compute local.dynamodb_enabled and updates references across IAM, outputs, and module invocation. Updates README to document the new input. No exported/public APIs changed beyond variable addition.

Changes

Cohort / File(s) Summary
Inputs & Docs
src/variables.tf, src/README.md
Added boolean variable s3_state_lock_enabled (default false) and documented it; clarifies that enabling S3 locking prevents DynamoDB table creation.
Control Logic & Module Wiring
src/main.tf
Added local.dynamodb_enabled = var.s3_state_lock_enabled ? false : var.dynamodb_enabled; passed enabled = local.enabled, dynamodb_enabled = local.dynamodb_enabled, and s3_state_lock_enabled to tfstate_backend.
IAM Policy Condition
src/iam.tf
Switched dynamic policy for_each condition from var.dynamodb_enabled to local.dynamodb_enabled.
Outputs Gating
src/outputs.tf
Gated DynamoDB-related outputs on local.dynamodb_enabled instead of var.dynamodb_enabled.

Sequence Diagram(s)

sequenceDiagram
  participant User as Caller
  participant Module as tfstate_backend (root)
  participant IAM as IAM Policies
  participant DDB as DynamoDB Table
  participant S3 as S3 Locking

  User->>Module: set s3_state_lock_enabled, dynamodb_enabled
  Module->>Module: compute local.dynamodb_enabled = s3_lock ? false : dynamodb_enabled
  Module-->>IAM: apply policies if local.dynamodb_enabled
  alt S3 lock enabled
    Module--X DDB: skip creation
    Module->>S3: use S3 for state locking
  else DynamoDB enabled
    Module->>DDB: create table for locking
  end
  Module-->>User: outputs gated by local.dynamodb_enabled
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Assessment against linked issues

Objective Addressed Explanation
Provide variable to control DynamoDB creation (#33) var.dynamodb_enabled is referenced but not added in this PR; addition status unclear.
Optionally disable the DynamoDB table (#33)
Enable S3-based state locking without DynamoDB (#33)
Documentation reflects new/updated locking options (#33)

Possibly related PRs

Suggested labels

minor, needs-test

Poem

In burrows of code where states reside,
I toggled the lock with S3 as guide.
Dynamo snoozes when clouds take lead,
A carrot of config for every need.
Flip the bit—hip hop hooray!
My terraform state is safe today. 🥕🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/statefile-locking

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@milldr
Copy link
Contributor Author

milldr commented Aug 14, 2025

/terratest

@mergify mergify bot requested review from a team August 14, 2025 21:12
Copy link

mergify bot commented Aug 14, 2025

Important

Do not edit the README.md directly. It's auto-generated from the README.yaml

Please update the README.yaml file instead.

Could you fix it @milldr? 🙏

@mergify mergify bot added the triage Needs triage label Aug 14, 2025
Copy link

@coderabbitai coderabbitai bot left a 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 (2)
src/README.md (1)

193-193: Document OpenTofu compatibility and behavior changes when S3 locking is enabled

The input is clear. Please add a short “Note” in the README (outside the terraform-docs table) stating:

  • S3 state locking requires OpenTofu >= 1.10.1 (HashiCorp Terraform CLI does not support S3 lockfiles).
  • When s3_state_lock_enabled = true, no DynamoDB resources or IAM permissions for DynamoDB are created, and DynamoDB-related outputs will be empty.

This will reduce confusion for users toggling between Terraform and OpenTofu and clarify the IAM/output implications.

I can propose a small note snippet and where to place it if helpful.

src/variables.tf (1)

54-58: Input addition looks good; consider adding a validation to avoid confusing configurations

The variable is well-described. Since var.s3_state_lock_enabled = true overrides var.dynamodb_enabled, consider surfacing that to users with a validation to prevent inadvertently setting both to true.

Follow-up (applies outside the changed lines): add a validation block to variable "dynamodb_enabled":

variable "dynamodb_enabled" {
  type        = bool
  default     = true
  description = "Whether to create the DynamoDB table."

  validation {
    condition     = !(var.dynamodb_enabled && var.s3_state_lock_enabled)
    error_message = "dynamodb_enabled cannot be true when s3_state_lock_enabled is true. Either disable DynamoDB or disable S3 state locking."
  }
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 286799e and f991e68.

📒 Files selected for processing (5)
  • src/README.md (1 hunks)
  • src/iam.tf (1 hunks)
  • src/main.tf (1 hunks)
  • src/outputs.tf (1 hunks)
  • src/variables.tf (1 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 (4)
src/iam.tf (1)

72-72: Correct gating for DynamoDB IAM permissions

Switching the guard to local.dynamodb_enabled aligns IAM policy with the new logic that disables DDB when S3 locking is enabled. The dynamic block prevents referencing the DDB ARN when disabled. LGTM.

src/main.tf (2)

4-6: Good: single source of truth for DynamoDB enablement

The local expression cleanly enforces “S3 lockfile overrides DDB.” This centralizes the branching and avoids diverging checks across files.


12-20: No action required — module v1.6.0 supports s3_state_lock_enabled

Verified: cloudposse/terraform-aws-tfstate-backend added the s3_state_lock_enabled input in v1.6.0 (PR #192), default = false — so passing that argument is supported.

  • Files to note: src/main.tf (lines 12–20) — current use of s3_state_lock_enabled = var.s3_state_lock_enabled is valid if the module is pinned to v1.6.0 or later.
  • Action only if necessary: if the module is pinned to an older version, bump the module version to >= v1.6.0.
src/outputs.tf (1)

18-18: Outputs correctly gated on the computed local

Switching the DynamoDB-related outputs to depend on local.dynamodb_enabled keeps outputs consistent with the IAM logic and the module inputs. This prevents attempts to read DDB attributes when S3 locking is enabled.

Also applies to: 23-23, 28-28

@milldr milldr enabled auto-merge August 14, 2025 21:40
@milldr milldr added this pull request to the merge queue Aug 15, 2025
@mergify mergify bot removed the triage Needs triage label Aug 15, 2025
@mergify mergify bot requested a review from a team August 15, 2025 01:50
Merged via the queue into main with commit acd3ae9 Aug 15, 2025
19 checks passed
@milldr milldr deleted the fix/statefile-locking branch August 15, 2025 01:57
Copy link

These changes were released in v1.537.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants