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.

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

dynamically create website inputs with the new provider and its colli… #142

Closed
wants to merge 6 commits into from
Closed

dynamically create website inputs with the new provider and its colli… #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
61bdc50
dynamically create website inputs with the new provider and its colli…
jurgenweber Mar 14, 2022
8584b65
Auto Format
cloudpossebot Mar 14, 2022
b801051
Merge branch 'master' into master
Nuru Apr 21, 2022
3e17e9c
Merge branch 'master' into master
Nuru May 6, 2022
5af915d
Merge branch 'master' into master
Nuru May 16, 2022
1831939
Merge branch 'master' into master
jurgen-weber-deltatre Jun 24, 2022
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 README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ Available targets:
| <a name="input_transfer_acceleration_enabled"></a> [transfer\_acceleration\_enabled](#input\_transfer\_acceleration\_enabled) | Set this to true to enable S3 Transfer Acceleration for the bucket. | `bool` | `false` | no |
| <a name="input_user_enabled"></a> [user\_enabled](#input\_user\_enabled) | Set to `true` to create an IAM user with permission to access the bucket | `bool` | `false` | no |
| <a name="input_versioning_enabled"></a> [versioning\_enabled](#input\_versioning\_enabled) | A state of versioning. Versioning is a means of keeping multiple variants of an object in the same bucket | `bool` | `true` | no |
| <a name="input_website_inputs"></a> [website\_inputs](#input\_website\_inputs) | Specifies the static website hosting configuration object. | <pre>list(object({<br> index_document = string<br> error_document = string<br> redirect_all_requests_to = string<br> routing_rules = string<br> }))</pre> | `null` | no |
| <a name="input_website_inputs"></a> [website\_inputs](#input\_website\_inputs) | Specifies the static website hosting configuration object. | `map(any)` | `null` | no |

## Outputs

Expand Down
2 changes: 1 addition & 1 deletion docs/terraform.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
| <a name="input_transfer_acceleration_enabled"></a> [transfer\_acceleration\_enabled](#input\_transfer\_acceleration\_enabled) | Set this to true to enable S3 Transfer Acceleration for the bucket. | `bool` | `false` | no |
| <a name="input_user_enabled"></a> [user\_enabled](#input\_user\_enabled) | Set to `true` to create an IAM user with permission to access the bucket | `bool` | `false` | no |
| <a name="input_versioning_enabled"></a> [versioning\_enabled](#input\_versioning\_enabled) | A state of versioning. Versioning is a means of keeping multiple variants of an object in the same bucket | `bool` | `true` | no |
| <a name="input_website_inputs"></a> [website\_inputs](#input\_website\_inputs) | Specifies the static website hosting configuration object. | <pre>list(object({<br> index_document = string<br> error_document = string<br> redirect_all_requests_to = string<br> routing_rules = string<br> }))</pre> | `null` | no |
| <a name="input_website_inputs"></a> [website\_inputs](#input\_website\_inputs) | Specifies the static website hosting configuration object. | `map(any)` | `null` | no |

## Outputs

Expand Down
30 changes: 21 additions & 9 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,24 +85,36 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "default" {
}

resource "aws_s3_bucket_website_configuration" "default" {
for_each = local.enabled && var.website_inputs != null ? toset(var.website_inputs) : toset([])
for_each = local.enabled && var.website_inputs != null ? var.website_inputs : {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot and should not be for_each, because you can only have one configuration per bucket

bucket = join("", aws_s3_bucket.default.*.id)

index_document {
suffix = each.value.index_document
dynamic "index_document" {
for_each = each.value.index_document == null ? [] : [1]

content {
suffix = each.value.index_document
}
}

error_document {
key = each.value.error_document
dynamic "error_document" {
for_each = each.value.error_document == null ? [] : [1]

content {
key = each.value.error_document
}
}

redirect_all_requests_to {
host_name = each.value.redirect_all_requests_to
protocol = each.value.protocol
dynamic "redirect_all_requests_to" {
for_each = length(each.value.redirect_all_requests_to) > 0 ? each.value.redirect_all_requests_to : []

content {
host_name = redirect_all_requests_to.value.hostname
protocol = try(redirect_all_requests_to.value.protocol, "https")
}
}

dynamic "routing_rule" {
for_each = length(jsondecode(each.value.routing_rules)) > 0 ? jsondecode(each.value.routing_rules) : []
for_each = length(each.value.routing_rules) > 0 ? each.value.routing_rules : []
content {
dynamic "condition" {
for_each = routing_rule.value["Condition"]
Expand Down
7 changes: 1 addition & 6 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,7 @@ variable "object_lock_configuration" {

variable "website_inputs" {

type = list(object({
index_document = string
error_document = string
redirect_all_requests_to = string
routing_rules = string
}))
type = map(any)
default = null
Copy link
Member

Choose a reason for hiding this comment

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

could we keep the variable type the same for backwards compatibility?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, but as linked 'redirect_all_requests_to' is mutually exclusive with the other 3 options.. So, you are either setting 'redirect_all_requests_to' or 'index_document', 'error_document' and 'routing_rules'. Not sure how to handle that? I saw there is an experimental 'optional' feature but that is subject to change.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, I would really appreciate some feedback to this, not sure how to handle it.

Copy link
Member

Choose a reason for hiding this comment

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

the workaround has been to set unused options to null but maybe your way is the correct way forward, provided backwards compatibility is maintained

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this never worked in v2.0, I'm OK with having a breaking change here. Unfortunately, the "optional" feature is not usable (changes to it have been announced).

Let's make this 2 inputs.

  1. website_redirect_all_requests_to which is a list(object) with host_name and protocol and a validation of length <= 2 and default []
  2. website_configuration which is list(object) with the remaining configuration, including a list of routing_rule objects, and a validation of length <= 2 and default []

What do you think @jurgen-weber-deltatre and @nitrocode ?


description = "Specifies the static website hosting configuration object."
Expand Down