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

Handle self = false, add warning about compact and sort #33

Merged
merged 1 commit into from
May 11, 2022
Merged
Show file tree
Hide file tree
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
55 changes: 49 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,15 @@ object do not all have to be the same type.
The "type" of an object is itself an object: the keys are the same, and the values are the types of the values in the object.

So although `{ foo = "bar", baz = {} }` and `{ foo = "bar", baz = [] }` are both objects,
they are not of the same type. This means you cannot put them both in the same list or the same map,
they are not of the same type, and you can get error messages like

```
Error: Inconsistent conditional result types
The true and false result expressions must have consistent types. The given
expressions are object and object, respectively.
```

This means you cannot put them both in the same list or the same map,
even though you can put them in a single tuple or object.
Similarly, and closer to the problem at hand,

Expand All @@ -137,23 +145,29 @@ cidr_rule = {
}
```
is not the same type as

```hcl
self_rule = {
type = "ingress"
self = true
}
```

This means you cannot put both of those in the same list.

```hcl
rules = tolist([local.cidr_rule, local.self_rule])
```

Generates the error

```text
Invalid value for "v" parameter: cannot convert tuple to list of any single type.
```

You could make them the same type and put them in a list,
like this:

```hcl
rules = tolist([{
type = "ingress"
Expand All @@ -166,9 +180,11 @@ rules = tolist([{
self = true
}])
```

That remains an option for you when generating the rules, and is probably better when you have full control over all the rules.
However, what if some of the rules are coming from a source outside of your control? You cannot simply add those rules
to your list. So, what to do? Create an object whose attributes' values can be of different types.

```hcl
{ mine = local.my_rules, theirs = var.their_rules }
```
Expand Down Expand Up @@ -266,7 +282,32 @@ You can avoid this for the most part by providing the optional keys. Rules with
changed if their keys do not change and the rules themselves do not change, except in the case of
`rule_matrix`, where the rules are still dependent on the order of the security groups in
`source_security_group_ids`. You can avoid this by using `rules` instead of `rule_matrix` when you have
more than one security group in the list.
more than one security group in the list. You cannot avoid this by sorting the
`source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error.

##### Invalid for_each argument
You can supply a number of rules as inputs to this module, and they (usually) get transformed into
`aws_security_group_rule` resources. However, Terraform works in 2 steps: a `plan` step where it
calculates the changes to be made, and an `apply` step where it makes the changes. This is so you
can review and approve the plan before changing anything. One big limitation of this approach is
that it requires that Terraform be able to count the number of resources to create without the
benefit of any data generated during the `apply` phase. So if you try to generate a rule based
on something you are creating at the same time, you can get an error like

```
Error: Invalid for_each argument
The "for_each" value depends on resource attributes that cannot be determined until apply,
so Terraform cannot predict how many instances will be created.
```

This module uses lists to minimize the chance of that happening, as all it needs to know
is the length of the list, not the values in it, but this error still can
happen for subtle reasons. Most commonly, using a function like `compact` on a list
will cause the length to become unknown (since the values have to be checked and `null`s removed).
In the case of `source_security_group_ids`, just sorting the list using `sort`
will cause this error. If you run into this error, check for functions like `compact` somewhere
in the chain that produces the list and remove them if you find them.


##### WARNINGS and Caveats

Expand Down Expand Up @@ -474,8 +515,8 @@ Available targets:
| <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br>Characters matching the regex will be removed from the ID elements.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
| <a name="input_revoke_rules_on_delete"></a> [revoke\_rules\_on\_delete](#input\_revoke\_rules\_on\_delete) | Instruct Terraform to revoke all of the Security Group's attached ingress and egress rules before deleting<br>the security group itself. This is normally not needed. | `bool` | `false` | no |
| <a name="input_rule_matrix"></a> [rule\_matrix](#input\_rule\_matrix) | A convenient way to apply the same set of rules to a set of subjects. See README for details. | `any` | `[]` | no |
| <a name="input_rules"></a> [rules](#input\_rules) | A list of Security Group rule objects. All elements of a list must be exactly the same type;<br>use `rules_map` if you want to supply multiple lists of different types.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `list(any)` | `[]` | no |
| <a name="input_rules_map"></a> [rules\_map](#input\_rules\_map) | A map-like object of lists of Security Group rule objects. All elements of a list must be exactly the same type,<br>so this input accepts an object with keys (attributes) whose values are lists so you can separate different<br>types into different lists and still pass them into one input. Keys must be known at "plan" time.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `any` | `{}` | no |
| <a name="input_rules"></a> [rules](#input\_rules) | A list of Security Group rule objects. All elements of a list must be exactly the same type;<br>use `rules_map` if you want to supply multiple lists of different types.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see the `security_group_rule` [documentation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule).<br>\_\_\_Note:\_\_\_ The length of the list must be known at plan time.<br>This means you cannot use functions like `compact` or `sort` when computing the list. | `list(any)` | `[]` | no |
| <a name="input_rules_map"></a> [rules\_map](#input\_rules\_map) | A map-like object of lists of Security Group rule objects. All elements of a list must be exactly the same type,<br>so this input accepts an object with keys (attributes) whose values are lists so you can separate different<br>types into different lists and still pass them into one input. Keys must be known at "plan" time.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see the `security_group_rule` [documentation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule). | `any` | `{}` | no |
| <a name="input_security_group_create_timeout"></a> [security\_group\_create\_timeout](#input\_security\_group\_create\_timeout) | How long to wait for the security group to be created. | `string` | `"10m"` | no |
| <a name="input_security_group_delete_timeout"></a> [security\_group\_delete\_timeout](#input\_security\_group\_delete\_timeout) | How long to retry on `DependencyViolation` errors during security group deletion from<br>lingering ENIs left by certain AWS services such as Elastic Load Balancing. | `string` | `"15m"` | no |
| <a name="input_security_group_description"></a> [security\_group\_description](#input\_security\_group\_description) | The description to assign to the created Security Group.<br>Warning: Changing the description causes the security group to be replaced. | `string` | `"Managed by Terraform"` | no |
Expand Down Expand Up @@ -652,12 +693,14 @@ Check out [our other projects][github], [follow us on twitter][twitter], [apply
### Contributors

<!-- markdownlint-disable -->
| [![Erik Osterman][osterman_avatar]][osterman_homepage]<br/>[Erik Osterman][osterman_homepage] | [![Vladimir][SweetOps_avatar]][SweetOps_homepage]<br/>[Vladimir][SweetOps_homepage] | [![RB][nitrocode_avatar]][nitrocode_homepage]<br/>[RB][nitrocode_homepage] | [![Andriy Knysh][aknysh_avatar]][aknysh_homepage]<br/>[Andriy Knysh][aknysh_homepage] |
|---|---|---|---|
| [![Erik Osterman][osterman_avatar]][osterman_homepage]<br/>[Erik Osterman][osterman_homepage] | [![Nuru][Nuru_avatar]][Nuru_homepage]<br/>[Nuru][Nuru_homepage] | [![Vladimir][SweetOps_avatar]][SweetOps_homepage]<br/>[Vladimir][SweetOps_homepage] | [![RB][nitrocode_avatar]][nitrocode_homepage]<br/>[RB][nitrocode_homepage] | [![Andriy Knysh][aknysh_avatar]][aknysh_homepage]<br/>[Andriy Knysh][aknysh_homepage] |
|---|---|---|---|---|
<!-- markdownlint-restore -->

[osterman_homepage]: https://github.com/osterman
[osterman_avatar]: https://img.cloudposse.com/150x150/https://github.com/osterman.png
[Nuru_homepage]: https://github.com/Nuru
[Nuru_avatar]: https://img.cloudposse.com/150x150/https://github.com/Nuru.png
[SweetOps_homepage]: https://github.com/SweetOps
[SweetOps_avatar]: https://img.cloudposse.com/150x150/https://github.com/SweetOps.png
[nitrocode_homepage]: https://github.com/nitrocode
Expand Down
47 changes: 45 additions & 2 deletions README.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,15 @@ usage: |-
The "type" of an object is itself an object: the keys are the same, and the values are the types of the values in the object.

So although `{ foo = "bar", baz = {} }` and `{ foo = "bar", baz = [] }` are both objects,
they are not of the same type. This means you cannot put them both in the same list or the same map,
they are not of the same type, and you can get error messages like

```
Error: Inconsistent conditional result types
The true and false result expressions must have consistent types. The given
expressions are object and object, respectively.
```

This means you cannot put them both in the same list or the same map,
even though you can put them in a single tuple or object.
Similarly, and closer to the problem at hand,

Expand All @@ -104,23 +112,29 @@ usage: |-
}
```
is not the same type as

```hcl
self_rule = {
type = "ingress"
self = true
}
```

This means you cannot put both of those in the same list.

```hcl
rules = tolist([local.cidr_rule, local.self_rule])
```

Generates the error

```text
Invalid value for "v" parameter: cannot convert tuple to list of any single type.
```

You could make them the same type and put them in a list,
like this:

```hcl
rules = tolist([{
type = "ingress"
Expand All @@ -133,9 +147,11 @@ usage: |-
self = true
}])
```

That remains an option for you when generating the rules, and is probably better when you have full control over all the rules.
However, what if some of the rules are coming from a source outside of your control? You cannot simply add those rules
to your list. So, what to do? Create an object whose attributes' values can be of different types.

```hcl
{ mine = local.my_rules, theirs = var.their_rules }
```
Expand Down Expand Up @@ -233,7 +249,32 @@ usage: |-
changed if their keys do not change and the rules themselves do not change, except in the case of
`rule_matrix`, where the rules are still dependent on the order of the security groups in
`source_security_group_ids`. You can avoid this by using `rules` instead of `rule_matrix` when you have
more than one security group in the list.
more than one security group in the list. You cannot avoid this by sorting the
`source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error.

##### Invalid for_each argument
You can supply a number of rules as inputs to this module, and they (usually) get transformed into
`aws_security_group_rule` resources. However, Terraform works in 2 steps: a `plan` step where it
calculates the changes to be made, and an `apply` step where it makes the changes. This is so you
can review and approve the plan before changing anything. One big limitation of this approach is
that it requires that Terraform be able to count the number of resources to create without the
benefit of any data generated during the `apply` phase. So if you try to generate a rule based
on something you are creating at the same time, you can get an error like

```
Error: Invalid for_each argument
The "for_each" value depends on resource attributes that cannot be determined until apply,
so Terraform cannot predict how many instances will be created.
```

This module uses lists to minimize the chance of that happening, as all it needs to know
is the length of the list, not the values in it, but this error still can
happen for subtle reasons. Most commonly, using a function like `compact` on a list
will cause the length to become unknown (since the values have to be checked and `null`s removed).
In the case of `source_security_group_ids`, just sorting the list using `sort`
will cause this error. If you run into this error, check for functions like `compact` somewhere
in the chain that produces the list and remove them if you find them.


##### WARNINGS and Caveats

Expand Down Expand Up @@ -387,6 +428,8 @@ include:
contributors:
- name: "Erik Osterman"
github: "osterman"
- name: "Nuru"
github: "Nuru"
- name: "Vladimir"
github: "SweetOps"
- name: "RB"
Expand Down
4 changes: 2 additions & 2 deletions docs/terraform.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
| <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br>Characters matching the regex will be removed from the ID elements.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
| <a name="input_revoke_rules_on_delete"></a> [revoke\_rules\_on\_delete](#input\_revoke\_rules\_on\_delete) | Instruct Terraform to revoke all of the Security Group's attached ingress and egress rules before deleting<br>the security group itself. This is normally not needed. | `bool` | `false` | no |
| <a name="input_rule_matrix"></a> [rule\_matrix](#input\_rule\_matrix) | A convenient way to apply the same set of rules to a set of subjects. See README for details. | `any` | `[]` | no |
| <a name="input_rules"></a> [rules](#input\_rules) | A list of Security Group rule objects. All elements of a list must be exactly the same type;<br>use `rules_map` if you want to supply multiple lists of different types.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `list(any)` | `[]` | no |
| <a name="input_rules_map"></a> [rules\_map](#input\_rules\_map) | A map-like object of lists of Security Group rule objects. All elements of a list must be exactly the same type,<br>so this input accepts an object with keys (attributes) whose values are lists so you can separate different<br>types into different lists and still pass them into one input. Keys must be known at "plan" time.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `any` | `{}` | no |
| <a name="input_rules"></a> [rules](#input\_rules) | A list of Security Group rule objects. All elements of a list must be exactly the same type;<br>use `rules_map` if you want to supply multiple lists of different types.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see the `security_group_rule` [documentation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule).<br>\_\_\_Note:\_\_\_ The length of the list must be known at plan time.<br>This means you cannot use functions like `compact` or `sort` when computing the list. | `list(any)` | `[]` | no |
| <a name="input_rules_map"></a> [rules\_map](#input\_rules\_map) | A map-like object of lists of Security Group rule objects. All elements of a list must be exactly the same type,<br>so this input accepts an object with keys (attributes) whose values are lists so you can separate different<br>types into different lists and still pass them into one input. Keys must be known at "plan" time.<br>The keys and values of the Security Group rule objects are fully compatible with the `aws_security_group_rule` resource,<br>except for `security_group_id` which will be ignored, and the optional "key" which, if provided, must be unique<br>and known at "plan" time.<br>To get more info see the `security_group_rule` [documentation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule). | `any` | `{}` | no |
| <a name="input_security_group_create_timeout"></a> [security\_group\_create\_timeout](#input\_security\_group\_create\_timeout) | How long to wait for the security group to be created. | `string` | `"10m"` | no |
| <a name="input_security_group_delete_timeout"></a> [security\_group\_delete\_timeout](#input\_security\_group\_delete\_timeout) | How long to retry on `DependencyViolation` errors during security group deletion from<br>lingering ENIs left by certain AWS services such as Elastic Load Balancing. | `string` | `"15m"` | no |
| <a name="input_security_group_description"></a> [security\_group\_description](#input\_security\_group\_description) | The description to assign to the created Security Group.<br>Warning: Changing the description causes the security group to be replaced. | `string` | `"Managed by Terraform"` | no |
Expand Down
Loading