Skip to content
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

fix: Do not create cors_configuration when variable not provided #114

Merged
merged 3 commits into from
Aug 2, 2024
Merged
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
rev: v1.92.0
rev: v1.92.1
hooks:
- id: terraform_fmt
- id: terraform_wrapper_module_for_each
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -204,7 +204,7 @@ module "api_gateway" {
| <a name="input_api_version"></a> [api\_version](#input\_api\_version) | A version identifier for the API. Must be between 1 and 64 characters in length | `string` | `null` | no |
| <a name="input_authorizers"></a> [authorizers](#input\_authorizers) | Map of API gateway authorizers to create | <pre>map(object({<br> authorizer_credentials_arn = optional(string)<br> authorizer_payload_format_version = optional(string)<br> authorizer_result_ttl_in_seconds = optional(number)<br> authorizer_type = optional(string, "REQUEST")<br> authorizer_uri = optional(string)<br> enable_simple_responses = optional(bool)<br> identity_sources = optional(list(string))<br> jwt_configuration = optional(object({<br> audience = optional(list(string))<br> issuer = optional(string)<br> }))<br> name = optional(string)<br> }))</pre> | `{}` | no |
| <a name="input_body"></a> [body](#input\_body) | An OpenAPI specification that defines the set of routes and integrations to create as part of the HTTP APIs. Supported only for HTTP APIs | `string` | `null` | no |
| <a name="input_cors_configuration"></a> [cors\_configuration](#input\_cors\_configuration) | The cross-origin resource sharing (CORS) configuration. Applicable for HTTP APIs | <pre>object({<br> allow_credentials = optional(bool)<br> allow_headers = optional(list(string))<br> allow_methods = optional(list(string))<br> allow_origins = optional(list(string))<br> expose_headers = optional(list(string), [])<br> max_age = optional(number)<br> })</pre> | `{}` | no |
| <a name="input_cors_configuration"></a> [cors\_configuration](#input\_cors\_configuration) | The cross-origin resource sharing (CORS) configuration. Applicable for HTTP APIs | <pre>object({<br> allow_credentials = optional(bool)<br> allow_headers = optional(list(string))<br> allow_methods = optional(list(string))<br> allow_origins = optional(list(string))<br> expose_headers = optional(list(string), [])<br> max_age = optional(number)<br> })</pre> | `null` | no |
| <a name="input_create"></a> [create](#input\_create) | Controls if resources should be created | `bool` | `true` | no |
| <a name="input_create_certificate"></a> [create\_certificate](#input\_create\_certificate) | Whether to create a certificate for the domain | `bool` | `true` | no |
| <a name="input_create_domain_name"></a> [create\_domain\_name](#input\_create\_domain\_name) | Whether to create API domain name resource | `bool` | `true` | no |
2 changes: 1 addition & 1 deletion main.tf
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ resource "aws_apigatewayv2_api" "this" {
body = local.is_http ? var.body : null

dynamic "cors_configuration" {
for_each = local.is_http && length(var.cors_configuration) > 0 ? [var.cors_configuration] : []
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a valid change

for_each = local.is_http && var.cors_configuration != null ? [var.cors_configuration] : []

content {
allow_credentials = cors_configuration.value.allow_credentials
2 changes: 1 addition & 1 deletion variables.tf
Original file line number Diff line number Diff line change
@@ -30,7 +30,7 @@ variable "cors_configuration" {
expose_headers = optional(list(string), [])
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be a valid change - will test it out

Copy link
Member

Choose a reason for hiding this comment

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

not valid - updated in b7ec703

max_age = optional(number)
})
default = {}
default = null
}

variable "credentials_arn" {
2 changes: 1 addition & 1 deletion wrappers/main.tf
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ module "wrapper" {
api_version = try(each.value.api_version, var.defaults.api_version, null)
authorizers = try(each.value.authorizers, var.defaults.authorizers, {})
body = try(each.value.body, var.defaults.body, null)
cors_configuration = try(each.value.cors_configuration, var.defaults.cors_configuration, {})
cors_configuration = try(each.value.cors_configuration, var.defaults.cors_configuration, null)
create = try(each.value.create, var.defaults.create, true)
create_certificate = try(each.value.create_certificate, var.defaults.create_certificate, true)
create_domain_name = try(each.value.create_domain_name, var.defaults.create_domain_name, true)