Skip to content

feat(terraform): allow overriding R2 media bucket name#591

Open
donnfelker wants to merge 2 commits intoColeMurray:mainfrom
donnfelker:feat/r2-media-bucket-name-override
Open

feat(terraform): allow overriding R2 media bucket name#591
donnfelker wants to merge 2 commits intoColeMurray:mainfrom
donnfelker:feat/r2-media-bucket-name-override

Conversation

@donnfelker
Copy link
Copy Markdown
Contributor

@donnfelker donnfelker commented May 4, 2026

Add an optional r2_media_bucket_name variable so the bucket can be pre-created out-of-band and the deployment can point at it instead of having Terraform create one. This unblocks environments where the Cloudflare API token used by Terraform/CI does not have R2 bucket- creation rights and an admin must provision the bucket manually.

When the override is empty, behavior is unchanged: the bucket is created as open-inspect-media-<deployment_name>. When set, that exact name is used and operators are expected to terraform import the existing bucket so applies do not try to recreate it.

The example tfvars documents manual-setup requirements (same account, private, no CORS/lifecycle, matching location) and clarifies that the Terraform token only needs Workers R2 Storage Read plus Workers Scripts Edit once the bucket exists — object-level R2 permissions are never required because runtime access flows through the in-account MEDIA_BUCKET Worker binding.

Summary by CodeRabbit

  • New Features

    • Support for specifying a custom R2 media bucket name in deployment configuration while preserving the automatic default when left unset.
  • Documentation

    • Added guidance on how to override the default bucket name and on the required manual pre-creation and import workflow when automated bucket creation isn’t possible.

Add an optional r2_media_bucket_name variable so the bucket can be
pre-created out-of-band and the deployment can point at it instead of
having Terraform create one. This unblocks environments where the
Cloudflare API token used by Terraform/CI does not have R2 bucket-
creation rights and an admin must provision the bucket manually.

When the override is empty, behavior is unchanged: the bucket is
created as open-inspect-media-<deployment_name>. When set, that exact
name is used and operators are expected to terraform import the
existing bucket so applies do not try to recreate it.

The example tfvars documents manual-setup requirements (same account,
private, no CORS/lifecycle, matching location) and clarifies that the
Terraform token only needs Workers R2 Storage Read plus Workers
Scripts Edit once the bucket exists — object-level R2 permissions are
never required because runtime access flows through the in-account
MEDIA_BUCKET Worker binding.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

Introduces an optional Terraform variable r2_media_bucket_name and updates the cloudflare_r2_bucket.media.name to use that variable when non-empty, falling back to the prior open-inspect-media-${local.name_suffix} pattern. Example tfvars and usage/import documentation were added.

Changes

R2 Bucket Naming Flexibility

Layer / File(s) Summary
Data Shape / Inputs
terraform/environments/production/variables.tf
Adds variable "r2_media_bucket_name" (string, default "") to permit an explicit R2 media bucket name.
Core Resource
terraform/environments/production/r2.tf
cloudflare_r2_bucket.media.name now uses var.r2_media_bucket_name != "" ? var.r2_media_bucket_name : "open-inspect-media-${local.name_suffix}".
Examples / Docs
terraform/environments/production/terraform.tfvars.example
Adds r2_media_bucket_name = "" example and detailed instructions for pre-creating/importing a bucket when Terraform cannot create it.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • open-inspect

"I hopped through variables, light and quick,
A name to bend at your choice—how slick.
Default or custom, both paths fine,
R2 storage now fits your line. 🐇✨"

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding the ability to override the R2 media bucket name through a Terraform variable, which is the core purpose of this pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@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: 1

🧹 Nitpick comments (1)
terraform/environments/production/terraform.tfvars.example (1)

177-205: ⚡ Quick win

Add explicit data-loss warnings for the two dangerous rename/mismatch scenarios

The current docs say "Location should match r2_media_location" (line 188) but omit the consequence of a mismatch: a location drift forces bucket replacement, destroying all stored objects. Similarly, there's no warning covering the scenario where an operator sets this variable on an existing deployment (where Terraform already owns a default-named bucket) without first running terraform import — that would also produce a destroy-then-create plan.

📝 Suggested doc additions
 # Manual setup requirements (when an admin pre-creates the bucket):
 #   - Must live in the same Cloudflare account as the Workers (cloudflare_account_id).
 #   - Private bucket — do NOT enable public access. The control-plane Worker
 #     reads/writes objects through its binding; clients never hit R2 directly.
 #   - No CORS rules or lifecycle policies are required.
-#   - Location should match r2_media_location (default ENAM).
+#   - Location should match r2_media_location (default ENAM). A mismatch causes
+#     Terraform to force-replace the bucket (destroy + recreate), wiping all objects.
+#     Either match the location, or override r2_media_location to match the bucket.
+#
+# WARNING — renaming an existing Terraform-managed bucket:
+#   If you previously deployed without this variable (Terraform owns the default-named
+#   bucket), setting r2_media_bucket_name to a different name WITHOUT running
+#   `terraform import` first will cause Terraform to destroy the existing bucket and
+#   create a new one, permanently deleting all stored objects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@terraform/environments/production/terraform.tfvars.example` around lines 177
- 205, Add explicit data-loss warnings near the existing R2 guidance: explain
that changing r2_media_location (or creating a bucket in a different location)
will cause Terraform to replace the R2 bucket and permanently delete objects,
and that setting r2_media_bucket_name on an existing deployment without first
running terraform import (terraform import cloudflare_r2_bucket.media
<account_id>/<bucket-name>) will produce a destroy-then-create plan; instruct
operators to verify matching r2_media_location, run terraform import before
assigning r2_media_bucket_name for any pre-created or existing bucket, and to
back up/transfer objects prior to making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@terraform/environments/production/r2.tf`:
- Around line 5-9: Update the Cloudflare provider to v5.16.0+ to pick up the fix
that ignores location changes for imported R2 buckets; if you cannot upgrade
immediately, add a lifecycle block to the cloudflare_r2_bucket resource "media"
to ignore_changes = ["location"] to prevent forced replacement until the
provider is updated. Ensure you modify the resource named
cloudflare_r2_bucket.media (and any other R2 bucket resources) and verify the
provider version constraint in your Terraform configuration and lockfile.

---

Nitpick comments:
In `@terraform/environments/production/terraform.tfvars.example`:
- Around line 177-205: Add explicit data-loss warnings near the existing R2
guidance: explain that changing r2_media_location (or creating a bucket in a
different location) will cause Terraform to replace the R2 bucket and
permanently delete objects, and that setting r2_media_bucket_name on an existing
deployment without first running terraform import (terraform import
cloudflare_r2_bucket.media <account_id>/<bucket-name>) will produce a
destroy-then-create plan; instruct operators to verify matching
r2_media_location, run terraform import before assigning r2_media_bucket_name
for any pre-created or existing bucket, and to back up/transfer objects prior to
making these changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62cdb632-e569-4ebc-817a-c846a4ea7cb1

📥 Commits

Reviewing files that changed from the base of the PR and between cd68977 and daa896f.

📒 Files selected for processing (3)
  • terraform/environments/production/r2.tf
  • terraform/environments/production/terraform.tfvars.example
  • terraform/environments/production/variables.tf

Comment on lines 5 to 9
resource "cloudflare_r2_bucket" "media" {
account_id = var.cloudflare_account_id
name = "open-inspect-media-${local.name_suffix}"
name = var.r2_media_bucket_name != "" ? var.r2_media_bucket_name : "open-inspect-media-${local.name_suffix}"
location = var.r2_media_location
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

cloudflare_r2_bucket terraform location ForceNew forces replacement provider issue

💡 Result:

The cloudflare_r2_bucket resource in the Cloudflare Terraform provider has issues where changes or mismatches in the location attribute can force resource replacement (marked with # forces replacement in terraform plan). This is because location is a "location hint" that is only honored on initial bucket creation and cannot be updated afterward—Cloudflare ignores attempts to change it post-creation, but older provider versions detect a diff between config and state, triggering unnecessary replacement plans. Key causes and evidence: - Mismatch between configured location and actual bucket location (e.g., config "ENAM" vs. state "WNAM") repeatedly forces replacement, even though Cloudflare enforces the original location. Fixed in provider v5.16.0 by ignoring location changes after creation. - Case sensitivity: Using "weur" vs. "WEUR" triggers diff/replacement due to strict string matching. Validation added in v4.37.0; use lowercase as per docs ("apac", "eeur", etc.). - Provider bugs in v5.x (e.g., v5.2.0-v5.3.0): Perpetual diffs on location, jurisdiction, storage_class after creation, leading to failed updates. Fixed in v5.5.0+ via improved param handling. - Import issues: Imported buckets show diffs on location etc., requiring manual state edits (set location, jurisdiction="default", storage_class="Standard"). Use import format <account_id>/<bucket_name>/ with "default" for non-jurisdictional buckets. Official docs confirm: location is optional, best-effort, creation-only. No ForceNew: true explicitly documented, but behavior acts as if it is due to immutability. No evidence of provider-specific "location ForceNew forces replacement provider issue" beyond these; symptoms match known GitHub issues in cloudflare/terraform-provider-cloudflare repo. Update to latest provider (v5.16.0+ as of 2026) resolves most cases. For imports, manually align state or omit location in config after import. Workarounds: - Omit location after initial creation/import to avoid diffs. - Ensure consistent casing (lowercase). - For perpetual diffs, edit state JSON or downgrade temporarily.

Citations:


Update to Cloudflare provider v5.16.0+ to resolve location mismatch on imported R2 buckets

A known provider bug caused location mismatches to trigger forced bucket replacement. This issue has been fixed in provider v5.16.0+ by having the provider ignore location changes after initial creation.

If upgrading is not immediately possible, lifecycle { ignore_changes = [location] } prevents the replacement, though this is a workaround pending the provider update.

Recommended action: Update to Cloudflare provider v5.16.0 or later. If an immediate update is not feasible, apply the ignore_changes workaround below:

Workaround for older provider versions
 resource "cloudflare_r2_bucket" "media" {
   account_id = var.cloudflare_account_id
   name       = var.r2_media_bucket_name != "" ? var.r2_media_bucket_name : "open-inspect-media-${local.name_suffix}"
   location   = var.r2_media_location
+
+  lifecycle {
+    # Workaround: location is immutable post-creation. Older provider versions detect
+    # a diff between config and actual bucket location, forcing replacement.
+    # This fix is unnecessary in provider v5.16.0+.
+    ignore_changes = [location]
+  }
 }
📝 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.

Suggested change
resource "cloudflare_r2_bucket" "media" {
account_id = var.cloudflare_account_id
name = "open-inspect-media-${local.name_suffix}"
name = var.r2_media_bucket_name != "" ? var.r2_media_bucket_name : "open-inspect-media-${local.name_suffix}"
location = var.r2_media_location
}
resource "cloudflare_r2_bucket" "media" {
account_id = var.cloudflare_account_id
name = var.r2_media_bucket_name != "" ? var.r2_media_bucket_name : "open-inspect-media-${local.name_suffix}"
location = var.r2_media_location
lifecycle {
# Workaround: location is immutable post-creation. Older provider versions detect
# a diff between config and actual bucket location, forcing replacement.
# This fix is unnecessary in provider v5.16.0+.
ignore_changes = [location]
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@terraform/environments/production/r2.tf` around lines 5 - 9, Update the
Cloudflare provider to v5.16.0+ to pick up the fix that ignores location changes
for imported R2 buckets; if you cannot upgrade immediately, add a lifecycle
block to the cloudflare_r2_bucket resource "media" to ignore_changes =
["location"] to prevent forced replacement until the provider is updated. Ensure
you modify the resource named cloudflare_r2_bucket.media (and any other R2
bucket resources) and verify the provider version constraint in your Terraform
configuration and lockfile.

Copy link
Copy Markdown

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@terraform/environments/production/terraform.tfvars.example`:
- Line 222: Update the Cloudflare R2 import example for resource
cloudflare_r2_bucket.media so its import ID includes the jurisdiction suffix;
change the example from "<cloudflare_account_id>/<bucket-name>" to
"<cloudflare_account_id>/<bucket-name>/default" so Terraform sees the
non-jurisdictional bucket as having jurisdiction "default" and stops trying to
add it on each plan/apply.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f9fa8aaa-d29c-481b-b027-fc42e667939c

📥 Commits

Reviewing files that changed from the base of the PR and between 4535dd9 and aa744b6.

📒 Files selected for processing (3)
  • terraform/environments/production/r2.tf
  • terraform/environments/production/terraform.tfvars.example
  • terraform/environments/production/variables.tf

#
# After manual creation, import the bucket so Terraform manages the binding without
# trying to recreate it:
# terraform import cloudflare_r2_bucket.media <cloudflare_account_id>/<bucket-name>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

cloudflare_r2_bucket terraform import ID format jurisdiction default non-jurisdictional bucket

💡 Result:

For terraform import of Cloudflare R2 buckets (resource cloudflare_r2_bucket), the import ID format is: <account_id>/<bucket_name>/ And the provider’s jurisdiction field accepts values: "default", "eu", "fedramp". [1] Example: terraform import cloudflare_r2_bucket.my_backups "6be2041a37d48a9799c6aaaaaaaaaaaa/aaa-aaaaaa/eu" [2][1] Default (non-jurisdictional) buckets: - Use jurisdiction "default" in the import ID, i.e. <account_id>/<bucket_name>/default. [3] Important note about importing jurisdictional buckets: - There are provider/API quirks that require using the correct jurisdiction when importing buckets that have jurisdictional restrictions (separate from the location hint). [2] - In at least one issue report, the docs were effectively “wrong” and working imports required using /default rather than the real jurisdiction string at the end; switching to the correct behavior is tracked/fixed in later provider versions. [3]

Citations:


Add the /default jurisdiction suffix to the Cloudflare R2 bucket import example.

The Cloudflare Terraform provider expects the import ID as <account_id>/<bucket_name>/<jurisdiction>. For non-jurisdictional buckets, the jurisdiction value is "default". Omitting it can cause Terraform to perpetually try to add jurisdiction = "default" on every subsequent plan/apply.

📝 Proposed fix
-#   terraform import cloudflare_r2_bucket.media <cloudflare_account_id>/<bucket-name>
+#   terraform import cloudflare_r2_bucket.media <cloudflare_account_id>/<bucket-name>/default
📝 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.

Suggested change
# terraform import cloudflare_r2_bucket.media <cloudflare_account_id>/<bucket-name>
# terraform import cloudflare_r2_bucket.media <cloudflare_account_id>/<bucket-name>/default
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/environments/production/terraform.tfvars.example` at line 222,
Update the Cloudflare R2 import example for resource cloudflare_r2_bucket.media
so its import ID includes the jurisdiction suffix; change the example from
"<cloudflare_account_id>/<bucket-name>" to
"<cloudflare_account_id>/<bucket-name>/default" so Terraform sees the
non-jurisdictional bucket as having jurisdiction "default" and stops trying to
add it on each plan/apply.

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