-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
✨ feature: Adds distribution custom domain support #7
Conversation
By default a custom domain name is assigned using my terraform-null-label logic. Also adds support for custom or vanity domain names. Ensures that a proper TLS certificate is issued covering the domain names. Finally, I typically try to keep the number of Route53 domains to a minimum since they cost $0.50 for just existing, so this module allows specifying a dedicated AWS profile for those changes.
Here's the code health analysis summary for commits Analysis Summary
|
WalkthroughThe changes involve updates to several Terraform configuration files and a GitHub Actions workflow. Key modifications include the addition of a Go analyzer in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant AWS
participant Terraform
User->>GitHub Actions: Trigger workflow
GitHub Actions->>AWS: Assume role using environment variables
AWS->>GitHub Actions: Provide temporary credentials
GitHub Actions->>Terraform: Execute Terraform scripts
Terraform->>AWS: Create/update resources (ACM, CloudFront, Route 53)
AWS-->>Terraform: Resources created/updated
Terraform-->>GitHub Actions: Completion status
GitHub Actions-->>User: Notify completion
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (16)
- .deepsource.toml (1 hunks)
- .github/workflows/test.yml (1 hunks)
- aws-acm.tf (1 hunks)
- aws-cloudfront.tf (2 hunks)
- aws-route53.tf (1 hunks)
- examples/simple/ctx.tf (1 hunks)
- examples/simple/infracost-usage.yml (4 hunks)
- examples/simple/main.tf (1 hunks)
- examples/simple/outputs.tf (1 hunks)
- examples/simple/variables.tf (1 hunks)
- main.tf (1 hunks)
- outputs.tf (1 hunks)
- test/.golangci.yml (2 hunks)
- test/examples_simple_test.go (1 hunks)
- variables.tf (2 hunks)
- versions.tf (1 hunks)
Additional comments not posted (32)
versions.tf (1)
7-7
: LGTM! Addition ofconfiguration_aliases
enhances flexibility.The inclusion of
configuration_aliases = [aws.route53]
allows for more flexible provider configurations, which is beneficial for managing multiple AWS service configurations.examples/simple/ctx.tf (1)
3-8
: LGTM! Verify the impact of the newlong_dns
parameter.The update to version
0.5.0
and the addition oflong_dns = true
suggest enhancements in DNS management. Ensure that the new parameter is correctly implemented and aligns with the project's requirements.Consider verifying the impact of the
long_dns
parameter in the module's behavior and documentation..deepsource.toml (1)
9-13
: LGTM! Verify the import root path for the Go analyzer.The addition of the Go analyzer enhances the static analysis capabilities for Go code. Ensure that the import root
github.com/bendoerr-terraform-modules/terraform-aws-cloudfront-and-s3-origin/test
is correct and aligns with the intended analysis scope.Consider verifying the correctness of the import root path to ensure it targets the appropriate codebase for analysis.
main.tf (2)
5-5
: Ensurevar.name
is defined and used correctly.The
name
attribute in thelabel_site
module is now parameterized withvar.name
. Verify thatvar.name
is defined in your variables and is being passed correctly when invoking the module.
8-10
: Review the logic for local variablesdefault_alias
andextra_aliases
.The local variables are well-constructed for generating domain aliases. Ensure that
var.domain_zone_name
andvar.extra_domain_prefixes
are correctly defined and used within your Terraform configuration.aws-route53.tf (1)
1-14
: Verify the configuration of AWS Route53 records and dependencies.The
aws_route53_record
resource is configured to handle multiple aliases. Ensure thatvar.extra_domain_prefixes
,var.domain_zone_id
, and theaws_cloudfront_distribution
resource are correctly defined and configured in your Terraform setup.examples/simple/variables.tf (3)
6-10
: Ensureroute53_profile
is documented and used correctly.The
route53_profile
variable is non-nullable and must be provided. Verify its documentation and usage in your Terraform configuration to ensure it aligns with your AWS profile management.
12-16
: Ensureroute53_zone_id
is documented and used correctly.The
route53_zone_id
variable is non-nullable and must be provided. Verify its documentation and usage in your Terraform configuration to ensure it aligns with your Route53 zone management.
18-22
: Ensureroute53_zone_name
is documented and used correctly.The
route53_zone_name
variable is non-nullable and must be provided. Verify its documentation and usage in your Terraform configuration to ensure it aligns with your Route53 zone management.examples/simple/main.tf (2)
28-35
: Verify the existence of required variables.Ensure that the variables
var.route53_zone_name
,var.route53_zone_id
, andvar.namespace
are defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definitions of these variables:
Verification successful
Variables are correctly defined.
The variables
var.route53_zone_name
,var.route53_zone_id
, andvar.namespace
are defined in theexamples/simple/variables.tf
file, ensuring that the Terraform configuration is complete regarding these variables.
- Found in
examples/simple/variables.tf
:
variable "namespace"
variable "route53_zone_id"
variable "route53_zone_name"
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of required variables. # Test: Search for the variable definitions. Expect: At least one occurrence for each variable. rg --type terraform 'variable "(route53_zone_name|route53_zone_id|namespace)"'Length of output: 114
Script:
#!/bin/bash # Description: Verify the existence of required variables in .tf files. # Test: Search for the variable definitions in all .tf files. rg 'variable "(route53_zone_name|route53_zone_id|namespace)"' --glob '*.tf'Length of output: 247
18-22
: Verify the existence ofvar.route53_profile
.Ensure that the variable
var.route53_profile
is defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definition of
var.route53_profile
:Verification successful
Variable
var.route53_profile
is correctly defined.The variable
route53_profile
is defined inexamples/simple/variables.tf
, ensuring that the AWS provider configuration will work as intended. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `var.route53_profile`. # Test: Search for the variable definition. Expect: At least one occurrence. rg --type terraform 'variable "route53_profile"'Length of output: 84
Script:
#!/bin/bash # Description: Verify the existence of `var.route53_profile`. # Test: Search for the variable definition. Expect: At least one occurrence. rg 'variable "route53_profile"'Length of output: 91
outputs.tf (2)
26-29
: Verify the existence oflocal.default_alias
.Ensure that
local.default_alias
is defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definition of
local.default_alias
:Verification successful
local.default_alias
is correctly defined.The
local.default_alias
variable is defined inmain.tf
and appears to be correctly structured for generating a custom domain name. No issues were found with its definition.
- File:
main.tf
- Definition:
default_alias = format("%s.%s", module.label_site.dns_name, var.domain_zone_name)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `local.default_alias`. # Test: Search for the local definition. Expect: At least one occurrence. rg --type terraform 'locals\s*{[^}]*default_alias'Length of output: 173
Script:
#!/bin/bash # Description: Verify the existence of `local.default_alias`. # Test: Search for the local definition. Expect: At least one occurrence. rg --type terraform 'locals\s*{[^}]*default_alias\s*='Length of output: 181
Script:
#!/bin/bash # Description: Verify the existence of `local.default_alias`. # Test: Search for the local definition. Expect: At least one occurrence. rg --type terraform 'locals\s*\{[^}]*default_alias\s*=' --multilineLength of output: 103
Script:
#!/bin/bash # Description: Verify the existence of `local.default_alias`. # Test: Search for the local definition. Expect: At least one occurrence. rg 'locals\s*\{[^}]*default_alias\s*=' --multilineLength of output: 161
31-34
: Verify the existence oflocal.extra_aliases
.Ensure that
local.extra_aliases
is defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definition of
local.extra_aliases
:aws-acm.tf (3)
31-34
: Configuration approved foraws_acm_certificate_validation
.The configuration for validating the ACM certificate using Route53 records is correct.
12-29
: Verify the existence ofvar.domain_zone_id
.Ensure that
var.domain_zone_id
is defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definition of
var.domain_zone_id
:Verification successful
Let's re-run the search for the
var.domain_zone_id
definition without specifying the file type.
Variable
domain_zone_id
is defined.The variable
domain_zone_id
is correctly defined in your Terraform configuration, ensuring that theaws_route53_record
resource can be deployed without issues related to this variable.
- Location:
variables.tf
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `var.domain_zone_id`. # Test: Search for the variable definition. Expect: At least one occurrence. rg --type terraform 'variable "domain_zone_id"'Length of output: 83
Script:
#!/bin/bash # Description: Verify the existence of `var.domain_zone_id`. # Test: Search for the variable definition. Expect: At least one occurrence. rg 'variable "domain_zone_id"'Length of output: 73
1-5
: Verify the existence oflocal.default_alias
andlocal.extra_aliases
.Ensure that
local.default_alias
andlocal.extra_aliases
are defined in your Terraform configuration to avoid errors during deployment.Run the following script to check for the definitions of these locals:
examples/simple/outputs.tf (1)
26-29
: Ensure Consistency in Output Descriptions.The description for
cloudfront_distribution_alias_domain_name
is clear and provides context about its source. Ensure that this aligns with the actual implementation and usage in the Terraform module.variables.tf (1)
40-44
: Ensure Domain Zone Variables are Properly Documented.The descriptions for
domain_zone_name
anddomain_zone_id
are identical. Ensure they accurately reflect the purpose of each variable.aws-cloudfront.tf (2)
9-9
: Review the Use offlatten
for Aliases.The use of
flatten
to combine aliases is a flexible approach. Ensure thatlocal.default_alias
andlocal.extra_aliases
are correctly defined and populated.
38-39
: Validate ACM Certificate Configuration.The addition of
acm_certificate_arn
andssl_support_method
enhances security. Verify that the ACM certificate is correctly provisioned and associated with the intended domain names..github/workflows/test.yml (4)
35-38
: AWS CLI profile setup looks good.The AWS CLI is configured correctly with access keys and session tokens for the
core-profile
.
40-42
: Second AWS credentials configuration is secure.The use of environment variables for
role-to-assume
is consistent and secure.
47-50
: Terraform environment variables are correctly set.The environment variables for Terraform are set correctly to manage Route 53 resources.
32-34
: LGTM! Verify environment variable usage.The use of environment variables for
role-to-assume
enhances security by avoiding hardcoded credentials.Ensure that the environment variables
${{ vars.CORE_APPLY_ROLE }}
and${{ env.AWS_ACCESS_KEY_ID }}
are correctly set in the repository settings.Run the following script to verify the environment variable usage:
Verification successful
Environment Variables Correctly Used in Workflow
The environment variables
CORE_APPLY_ROLE
andAWS_ACCESS_KEY_ID
are correctly referenced in the.github/workflows/test.yml
file, ensuring secure and flexible configuration.
CORE_APPLY_ROLE
is used forrole-to-assume
.AWS_ACCESS_KEY_ID
is used in AWS CLI configuration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of environment variables in the workflow. # Test: Search for environment variable usage. Expect: Correct usage of CORE_APPLY_ROLE and AWS_ACCESS_KEY_ID. rg --type yaml $'\\$\\{\\{ vars\\.CORE_APPLY_ROLE \\}\\}' rg --type yaml $'\\$\\{\\{ env\\.AWS_ACCESS_KEY_ID \\}\\}'Length of output: 106
Script:
#!/bin/bash # Description: Verify the usage of environment variables in the specific workflow file. # Test: Search for environment variable usage in the .github/workflows/test.yml file. rg '\$\{\{ vars\.CORE_APPLY_ROLE \}\}' .github/workflows/test.yml rg '\$\{\{ env\.AWS_ACCESS_KEY_ID \}\}' .github/workflows/test.ymlLength of output: 288
test/examples_simple_test.go (2)
148-150
: Domain names setup is correct.The domain names are correctly retrieved from Terraform outputs and stored in an array for testing.
152-185
: Iteration over domain names is well-implemented.The iteration logic correctly covers HTTP GET requests for both domain names and validates the responses.
examples/simple/infracost-usage.yml (4)
11-18
: Route 53 record usage estimates are well-defined.The new parameters for tracking query costs are correctly defined for Route 53 records.
21-80
: S3 bucket usage estimates are comprehensive.The detailed configurations for various S3 storage classes are comprehensive and align with cost estimations.
Line range hint
83-110
: CloudFront distribution usage estimates are realistic.The updated values for data transfer and HTTPS requests are realistic and align with cost estimations.
131-134
: Route 53 certificate usage estimates are well-defined.The new parameters for tracking query costs are correctly defined for Route 53 certificates.
test/.golangci.yml (2)
75-75
: Verify the rationale for increasingmin-complexity
.The
min-complexity
forgocognit
has been increased from 20 to 30. Ensure that this change aligns with the team's standards for code complexity and maintainability.
317-318
: LGTM! Shadowing declaration detection added.The addition of shadowing declaration detection for
err
andctx
usinggovet
is a positive enhancement for code quality.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ben Doerr <[email protected]>
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/simple/outputs.tf (1 hunks)
- variables.tf (2 hunks)
Additional comments not posted (6)
examples/simple/outputs.tf (2)
26-29
: LGTM! The output is well-defined.The
cloudfront_distribution_alias_domain_name
output is correctly defined and the description is clear.
31-34
: LGTM! The output is well-defined.The
cloudfront_distribution_extra_domain_names
output is correctly defined and the description provides context for multi-domain setups.variables.tf (4)
19-24
: LGTM! The variable is well-defined.The
name
variable is correctly defined with a clear description.
40-44
: LGTM! The variable is well-defined.The
domain_zone_name
variable is correctly defined with a clear description.
46-50
: LGTM! The variable is well-defined.The
domain_zone_id
variable is correctly defined with a clear description.
52-56
: LGTM! The variable is well-defined.The
extra_domain_prefixes
variable is correctly defined with a clear description.
By default a custom domain name is assigned using my terraform-null-label logic. Also adds support for custom or vanity domain names. Ensures that a proper TLS certificate is issued covering the domain names. Finally, I typically try to keep the number of Route53 domains to a minimum since they cost $0.50 for just existing, so this module allows specifying a dedicated AWS profile for those changes.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation