fix: error: No more than 1 "s3_origin_config" blocks are allowed

[Eyjafjallajokull]

what

• Fixed No more than 1 s3_origin_config blocks are allowed error when using multiple S3 origins with origin access identity enabled
• Changed for_each from iterating over var.s3_origins to using [1] to create a single s3_origin_config block

why

• AWS CloudFront only allows one s3_origin_config block per origin
• The previous implementation incorrectly created multiple blocks when multiple S3 origins were configured

references

fixes #325

to reproduce error

1. in examples/complete/main.tf#L102 replace origin_access_control with origin_access_identity
   https://github.com/Eyjafjallajokull/terraform-aws-cloudfront-s3-cdn/blob/96703043867c986ff3fc1550448118111a9f5659/examples/complete/main.tf#L102
2. terraform plan fails with the above error.

[coderabbitai]

📝 Walkthrough

Walkthrough

The s3_origin_config dynamic block's for_each expression is modified to wrap var.s3_origins in a list, changing iteration from multiple items to a single item when origin access identity is enabled. This ensures only one s3_origin_config block is generated.

Changes

Cohort / File(s)	Summary
CloudFront S3 Origin Configuration main.tf	Modified s3_origin_config dynamic block's for_each expression to wrap var.s3_origins in a one-element list ([var.s3_origins]), ensuring single-block generation instead of multi-block iteration

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The change is localized to a single dynamic block with straightforward logic. Review effort is minimal due to the targeted nature of the fix, though verification against the reported bug scenario is necessary.

Possibly related PRs

• Adding origin_access_control_id to custom_origins #326: Adds origin_access_control_id and related origin fields to S3 origin handling, working in tandem with this PR's s3_origin_config iteration fix

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: error: No more than 1 "s3_origin_config" blocks are allowed" is clear, specific, and directly summarizes the main change in the pull request. It accurately describes the bug being fixed - the Terraform error that occurs when multiple s3_origin_config blocks are created instead of a single one. The title follows conventional commit conventions and provides sufficient context for developers reviewing the history to understand the primary issue being addressed.
Linked Issues Check	✅ Passed	The pull request directly addresses the core coding requirement from linked issue #325. The issue explicitly states "The for_each shouldn't be based on a number of s3_origins, but rather a single block at most should be there," and the PR implements this by changing the for_each expression from iterating over var.s3_origins to using [1], which ensures exactly one s3_origin_config block is created regardless of the number of s3_origins configured. This aligns with AWS CloudFront's constraint of allowing only one s3_origin_config block per origin.
Out of Scope Changes Check	✅ Passed	The changes in this pull request are narrowly focused and in-scope relative to the objectives. Only the main.tf file's s3_origin_config dynamic block's for_each expression is modified, which directly addresses the requirement from issue #325. The modification is limited to fixing the specific issue of multiple s3_origin_config blocks being incorrectly created when multiple S3 origins are configured with origin access identity enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.
Description Check	✅ Passed	The pull request description is clearly related to the changeset and provides specific, meaningful information about the fix. The author explains what problem was addressed (the "No more than 1 s3_origin_config blocks are allowed" error when using multiple S3 origins with origin access identity), what change was made (modifying the for_each to use [1] instead of iterating over var.s3_origins), and why the fix was necessary (AWS CloudFront only allows one s3_origin_config block per origin). The description also includes a reference to the issue being fixed (#325) and reproduction steps, which aligns with the technical details confirmed in the raw summary showing the for_each expression was changed from var.s3_origins to a single-element list. This is not vague or generic, and the description is directly relevant to the code modifications in main.tf.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

main.tf (1)

603-613: Correct fix for s3_origin_config duplication, but consider using [1] for consistency.

Wrapping var.s3_origins in a list ensures the dynamic block creates only one s3_origin_config per origin (by iterating the for_each once), which fixes the Terraform error when multiple S3 origins are configured with origin access identity enabled. The content block correctly references the outer loop's origin.value, not the inner loop variable.

However, the loop variable from s3_origin_config.value is never used, making the wrapping of var.s3_origins unnecessary. The similar pattern at line 520 uses [1] instead, which is more idiomatic and clearer about intent.

Consider this refactor for consistency and clarity:

       dynamic "s3_origin_config" {
-        for_each = local.origin_access_identity_enabled ? [var.s3_origins] : []
+        for_each = local.origin_access_identity_enabled ? [1] : []
         content {
           # the following enables specifying the origin_access_identity used by the origin created by this module, prior to the module's creation:
           origin_access_identity = (

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b07360f and 9670304.

📒 Files selected for processing (1)

• main.tf (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-07-31T11:46:57.218Z

Learnt from: jwadolowski
PR: cloudposse/terraform-aws-cloudfront-s3-cdn#340
File: main.tf:570-578
Timestamp: 2025-07-31T11:46:57.218Z
Learning: In terraform-aws-cloudfront-s3-cdn module, custom_origin_config in the custom_origins variable was always a required parameter, not optional. CloudFront origins require either custom_origin_config or s3_origin_config but not both, so null was never a valid value for custom_origin_config in custom origins.

Applied to files:

• main.tf

🔇 Additional comments (1)

main.tf (1)

603-613: Fix is correct and successfully resolves the Terraform error.

The change from for_each = local.origin_access_identity_enabled ? var.s3_origins : [] to for_each = local.origin_access_identity_enabled ? [var.s3_origins] : [] properly eliminates the duplication issue. Wrapping the list in brackets converts multiple iterations over individual origins to a single iteration, ensuring exactly one s3_origin_config block per origin instead of N² blocks, which complies with CloudFront's constraint.

[RoseSecurity]

/terratest

[RoseSecurity]

This fix looks good to me. Thank you

[github-actions]

These changes were released in v1.1.0.