Adding origin_access_control_id to custom_origins #326
Conversation
fix specification fix specification 2
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request primarily enhance the configuration options for custom origins in Terraform, particularly for AWS CloudFront distributions. A new field, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 2
🧹 Outside diff range comments (1)
examples/complete/custom-origins.tf (1)
Line range hint
20-34: Consider demonstrating Origin Access Control usage in the example.While the configuration structure supports Origin Access Control, this example could better demonstrate its practical application with Lambda@Edge. Consider adding a complete example showing:
- Creation of Origin Access Control resource
- Assignment to custom origins
- Required IAM permissions
Here's a suggested addition to demonstrate the setup:
resource "aws_cloudfront_origin_access_control" "example" { name = "example" description = "Example Origin Access Control for Lambda@Edge" origin_access_control_config { name = "example" origin_access_control_origin_type = "custom" signing_behavior = "always" signing_protocol = "sigv4" } } # Then in your origin configuration: local.additional_custom_origin_primary = local.additional_custom_origins_enabled ? merge( local.default_custom_origin_configuration, { domain_name = module.additional_custom_origin.bucket_website_endpoint origin_id = module.additional_custom_origin.bucket_id origin_access_control_id = aws_cloudfront_origin_access_control.example.id } ) : null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
docs/terraform.md(1 hunks)examples/complete/custom-origins.tf(1 hunks)main.tf(1 hunks)variables.tf(1 hunks)
🔇 Additional comments (4)
examples/complete/custom-origins.tf (1)
Line range hint 89-102: Verify security configuration with Origin Access Control.
The current configuration disables all public access blocks on the S3 buckets. With Origin Access Control in place, you might want to:
- Review if public access is still needed
- Update bucket policies to work with Origin Access Control
- Consider enabling public access blocks if direct bucket access isn't required
Let's verify the current security settings in the module:
Also applies to: 126-139
✅ Verification successful
Let me gather more information about the Origin Access Control configuration and related security settings.
Based on the gathered information, I can now provide the final response:
Public access blocks should remain disabled for this custom origin bucket
The current configuration with disabled public access blocks is correct because:
- This is a custom origin bucket configured as a website endpoint (as indicated by the comments in the code)
- The bucket has proper security controls in place:
- BucketOwnerEnforced ownership control is set
- The module supports Origin Access Control (OAC) as evidenced by
origin_access_type = "origin_access_control"in the configuration - Appropriate IAM policies are implemented through
aws_iam_policy_document.s3_origin_access_control
The security configuration aligns with AWS best practices for S3 buckets used as website endpoints with CloudFront distributions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any existing bucket policies or security configurations
rg -A 5 "aws_s3_bucket_policy|block_public_acls.*true|restrict_public_buckets.*true"
# Check for any existing Origin Access Control configurations
ast-grep --pattern 'resource "aws_cloudfront_origin_access_control"'
Length of output: 3065
Script:
#!/bin/bash
# Check for Origin Access Control configurations and usage
rg -A 10 "origin_access_control|aws_cloudfront_origin_access_control"
# Check for S3 bucket configurations in the custom origins file
rg -A 10 "aws_s3_bucket" examples/complete/custom-origins.tf
# Check for any IAM policies that might be related
rg -A 5 "aws_iam_policy_document"
Length of output: 57905
docs/terraform.md (2)
Line range hint 1-999: Documentation is complete and well-structured.
The Terraform module documentation:
- Properly documents all variables, outputs, and dependencies
- Follows standard Terraform documentation practices
- Clearly describes the new Origin Access Control feature
91-91: LGTM: The custom_origins variable documentation is correctly updated.
The addition of origin_access_control_id to the custom_origins variable type definition aligns with AWS CloudFront requirements and the PR objectives.
Let's verify the implementation matches the documentation:
✅ Verification successful
The variable definition in the implementation matches the documentation perfectly
The verification confirms that the custom_origins variable definition in variables.tf exactly matches the documentation in docs/terraform.md, including:
- All fields and their types are identical
- The
origin_access_control_idfield is present as a string type - The default value is set to
[] - The nested object structures for
custom_headersandcustom_origin_configmatch
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the variable definition in the Terraform code matches the documentation
# Test: Search for the custom_origins variable definition
rg -A 20 'variable "custom_origins"'
Length of output: 936
main.tf (1)
554-556: Proper inclusion of origin_access_control_id in custom origins
The addition of origin_access_control_id to the origin block allows for specifying access control settings for individual custom origins. This enhances the flexibility of the module by enabling fine-grained access control configurations for each custom origin in the CloudFront distribution.
coderabbitai suggestions
|
/terratest |
|
These changes were released in v0.96.0. |
Adding Origin Access Control Id to Custom Origins
what
Custom Origins didn't have
Origin Access ControlImplements this infrastructure
https://aws.amazon.com/blogs/networking-and-content-delivery/image-optimization-using-amazon-cloudfront-and-aws-lambda/
why
Custom Origins didn't have
Origin Access Controlif we wanted to invoke alambdawe were not able to do itreferences
Summary by CodeRabbit
New Features
origin_access_control_id.custom_originsands3_originsto include access control ID.Bug Fixes
Documentation