Skip to content

feat(origins): added optional origin_shield section for additional origins #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
atimofeev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(origins): added optional origin_shield section for additional origins #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,14 @@ resource "aws_cloudfront_distribution" "default" {
origin_keepalive_timeout = lookup(origin.value.custom_origin_config, "origin_keepalive_timeout", 60)
origin_read_timeout = lookup(origin.value.custom_origin_config, "origin_read_timeout", 60)
}

dynamic "origin_shield" {
for_each = origin.value.origin_shield != null ? [origin.value.origin_shield] : []
content {
enabled = origin_shield.value.enabled
origin_shield_region = origin_shield.value.region
}
}
Comment on lines +574 to +580
Copy link

@coderabbitai coderabbitai bot Jun 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Apply region fallback and filter disabled shields for custom origins.
Only include the block when enabled is true and coalesce a null region to local.origin_shield_region:

-for_each = origin.value.origin_shield != null ? [origin.value.origin_shield] : []
+for_each = origin.value.origin_shield.enabled ? [origin.value.origin_shield] : []

 content {
   enabled = origin_shield.value.enabled
-  origin_shield_region = origin_shield.value.region
+  origin_shield_region = coalesce(origin_shield.value.region, local.origin_shield_region)
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In main.tf around lines 574 to 580, the dynamic block for origin_shield should
only be included if the shield is enabled. Update the for_each expression to
filter out any origin_shield where enabled is false, and use a coalesce function
to set origin_shield_region to local.origin_shield_region if the region is null.
This ensures the block is conditionally included and applies the correct region
fallback.

}
}

Expand All @@ -589,6 +597,14 @@ resource "aws_cloudfront_distribution" "default" {
origin_access_identity = local.origin_access_identity_enabled && try(length(origin.value.s3_origin_config.origin_access_identity), 0) > 0 ? origin.value.s3_origin_config.origin_access_identity : local.origin_access_identity_enabled ? local.cf_access.path : ""
}
}

dynamic "origin_shield" {
for_each = origin.value.origin_shield != null ? [origin.value.origin_shield] : []
content {
enabled = origin_shield.value.enabled
origin_shield_region = origin_shield.value.region
}
}
Comment on lines +601 to +607
Copy link

@coderabbitai coderabbitai bot Jun 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Apply region fallback and filter disabled shields for S3 origins.
Mirror the custom‐origin logic: only render when enabled is true and fallback null region:

-for_each = origin.value.origin_shield != null ? [origin.value.origin_shield] : []
+for_each = origin.value.origin_shield.enabled ? [origin.value.origin_shield] : []

 content {
   enabled = origin_shield.value.enabled
-  origin_shield_region = origin_shield.value.region
+  origin_shield_region = coalesce(origin_shield.value.region, local.origin_shield_region)
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In main.tf around lines 601 to 607, the origin_shield block should only be
rendered if origin_shield is enabled and the region should fallback to null if
not set. Update the for_each condition to filter out disabled origin_shield
entries and modify the origin_shield_region assignment to use a fallback to null
when the region is not specified, matching the custom-origin logic.

}
}

Expand Down
8 changes: 8 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ variable "custom_origins" {
origin_keepalive_timeout = number
origin_read_timeout = number
})
origin_shield = optional(object({
enabled = optional(bool, false)
region = optional(string, null)
}), null)
}))
default = []
description = <<-EOT
Expand All @@ -482,6 +486,10 @@ variable "s3_origins" {
s3_origin_config = object({
origin_access_identity = string
})
origin_shield = optional(object({
enabled = optional(bool, false)
region = optional(string, null)
}), null)
}))
Comment on lines +489 to 493
Copy link

@coderabbitai coderabbitai bot Jun 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add fallback for unset origin_shield.region.
If origin_shield.enabled = true but region remains null, downstream will configure origin_shield_region = null, which is invalid. Consider defaulting region to the computed local.origin_shield_region when it’s not provided.

🤖 Prompt for AI Agents 
In variables.tf around lines 489 to 493, the origin_shield.region can be null
even when origin_shield.enabled is true, causing invalid downstream
configuration. Modify the code to provide a fallback so that if
origin_shield.region is null and origin_shield.enabled is true,
origin_shield.region defaults to local.origin_shield_region. This ensures a
valid region value is always set when origin_shield is enabled.

default = []
description = <<-EOT
Expand Down