From a7ff89ba6103964d830b683b3c01985c70257307 Mon Sep 17 00:00:00 2001 From: Nuru Date: Thu, 7 Jul 2022 13:35:54 -0700 Subject: [PATCH] More accurate control of create before destroy behaviors (#35) --- .github/renovate.json | 2 +- README.md | 232 +++++++++++++++++++++++++--- README.yaml | 218 ++++++++++++++++++++++++-- docs/terraform.md | 14 +- examples/complete/main.tf | 2 +- examples/complete/versions.tf | 2 +- exports/security-group-variables.tf | 89 +++++------ main.tf | 127 ++++++++++++++- test/src/examples_complete_test.go | 2 +- test/src/go.mod | 13 +- test/src/go.sum | 24 +-- variables.tf | 45 ++++-- versions.tf | 10 +- 13 files changed, 644 insertions(+), 136 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index ae4f0aa..a780298 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -4,9 +4,9 @@ ":preserveSemverRanges" ], "labels": ["auto-update"], + "dependencyDashboardAutoclose": true, "enabledManagers": ["terraform"], "terraform": { "ignorePaths": ["**/context.tf", "examples/**"] } } - diff --git a/README.md b/README.md index 1a31dea..2ca3ce9 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,185 @@ This module is primarily for setting security group rules on a security group. Y ID of an existing security group to modify, or, by default, this module will create a new security group and apply the given rules to it. +This module can be used very simply, but it is actually quite complex because it is attempting to handle +numerous interrelationships, restrictions, and a few bugs in ways that offer a choice between zero +service interruption for updates to a security group not referenced by other security groups +(by replacing the security group with a new one) versus brief service interruptions for security groups that must be preserved. + +### Avoiding Service Interruptions + +It is desirable to avoid having service interruptions when updating a security group. This is not always possible +due to a combination of the way Terraform organizes its activities and the fact that AWS will reject +an attempt to create a duplicate of an existing security group rule. There is also the issue that while many +AWS resources can be associated with and disassociated from security groups at any time, some +may not have their security group association changed, and an attempt to change the security group will +cause Terraform to delete and recreate the resource. + +#### The 2 Ways Security Group Changes Cause Service Interruptions + +Changes to a security group can cause service interruptions in 2 ways: + +1. Changing rules may be implemented as deleting existing rules and creating new ones. During the + period between deleting the old rules and creating the new rules, the security group will block + traffic intended to be allowed by the new rules. +2. Changing rules may alternately be implemented as creating a new security group with the new rules + and replacing the existing security group with the new one (then deleting the old one). + This usually works with no service interruption in the case where all resources that reference the + security group are part of the same Terraform plan. + However, if, for example, the security group ID is referenced in a security group + rule in a security group that is not part of the same Terraform plan, then AWS will not allow the + existing (referenced) security group to be deleted, and even if it did, Terraform would not know + to update the rule to reference the new security group. + +The key question you need to answer to decide which configuration to use is "will anything break +if the security group ID changes". If no, then use the defaults `create_before_destroy = true` and +`preserve_security_group_id = false` and do not worry about providing "keys" for +security group rules. This is the default because it is the easiest and safest solution when +the way the security group is being used allows it. + +If things will break when the security group ID changes, then set `preserve_security_group_id` +to `true` and read the information about keys and single source Terraform rules if you want +to mitigate against service interruptions caused by rule changes. Note that even in this case, +you probably want to keep `create_before_destroy = true` because otherwise, if some change +requires the security group to be replaced, Terraform will likely succeed in deleting all the +security group rules but fail to delete the security group itself, leaving the associated resources +completely inaccessible. At least with `create_before_destroy = true`, the new security group +will be created and will be used where Terraform is able to make the changes, even though +the old security group will still fail to be deleted. + +#### The 3 Ways to Mitigate Against Service Interruptions + +##### Security Group `create_before_destroy = true` + +The most important option is `create_before_destroy` which, when set to `true` (the default), +ensures that a new replacement security group is created before an existing one is destroyed. +This is particularly important because a security group cannot be destroyed while it is associated with +a resource (e.g. a load balancer), but "destroy before create" behavior causes Terraform +to try to destroy the security group before disassociating it from associated resources, +so plans fail to apply with the error + +``` +Error deleting security group: DependencyViolation: resource sg-XXX has a dependent object +``` + +With "create before destroy" and any resources dependent on the security group as part of the +same Terraform plan, replacement happens successfully: + +1. New security group is created +2. Resource is associated with the new security group and disassociated from the old one +3. Old security group is deleted successfully because there is no longer anything associated with it + +(If there is a resource dependent on the security group that is also outside the scope of +the Terraform plan, the old security group will fail to be deleted and you will have to +address the dependency manually.) + +Note that the module's default configuration of `create_before_destroy = true` and +`preserve_security_group_id = false` will force "create before destroy" behavior on the target security +group, even if the module did not create it and instead you provided a `target_security_group_id`. + +Unfortunately, just creating the new security group first is not enough to prevent a service interruption. Keep reading. + +##### Setting Rule Changes to Force Replacement of the Security Group + +A security group by itself is just a container for rules. It only functions as desired when all the rules are in place. +If using the Terraform default "destroy before create" behavior for rules, even when using `create_before_destroy` for the +security group itself, an outage occurs when updating the rules or security group, because the order of operations is: + +1. Delete existing security group rules (triggering a service interruption) +2. Create the new security group +3. Associate the new security group with resources and disassociate the old one (which can take a substantial + amount of time for a resource like a NAT Gateway) +4. Create the new security group rules (restoring service) +5. Delete the old security group + +To resolve this issue, the module's default configuration of `create_before_destroy = true` and +`preserve_security_group_id = false` causes any change in the security group rules +to trigger the creation of a new security group. With that, a rule change causes operations to occur in this order: + +1. Create the new security group +2. Create the new security group rules +3. Associate the new security group with resources and disassociate the old one +4. Delete the old security group rules +5. Delete the old security group + +##### Preserving the Security Group + +There can be a downside to creating a new security group with every rule change. +If you want to prevent the security group ID from changing unless absolutely necessary, perhaps because the associated +resource does not allow the security group to be changed or because the ID is referenced somewhere (like in +another security group's rules) outside of this Terraform plan, then you need to set `preserve_security_group_id` to `true`. + +The main drawback of this configuration is that there will normally be +a service outage during an update, because existing rules will be deleted before replacement +rules are created. Using keys to identify rules can help limit the impact, but even with keys, simply adding a +CIDR to the list of allowed CIDRs will cause that entire rule to be deleted and recreated, causing a temporary +access denial for all of the CIDRs in the rule. (For more on this and how to mitigate against it, see [The Importance +of Keys](#the-importance-of-keys) below.) + +Also note that setting `preserve_security_group_id` to `true` does not prevent Terraform from replacing the +security group when modifying it is not an option, such as when its name or description changes. +However, if you can control the configuration adequately, you can maintain the security group ID and eliminate +impact on other security groups by setting `preserve_security_group_id` to `true`. We still recommend +leaving `create_before_destroy` set to `true` for the times when the security group must be replaced, +to avoid the `DependencyViolation` described above. + +### Defining Security Group Rules + +We provide a number of different ways to define rules for the security group for a few reasons: +- Terraform type constraints make it difficult to create collections of objects with optional members +- Terraform resource addressing can cause resources that did not actually change to nevertheless be replaced + (deleted and recreated), which, in the case of security group rules, then causes a brief service interruption +- Terraform resource addresses must be known at `plan` time, making it challenging to create rules that + depend on resources being created during `apply` and at the same time are not replaced needlessly when something else changes +- When Terraform rules can be successfully created before being destroyed, there is no service interruption for the resources + associated with that security group (unless the security group ID is used in other security group rules outside + of the scope of the Terraform plan) + +#### The Importance of Keys + +If you are using "create before destroy" behavior for the security group and security group rules, then +you can skip this section and much of the discussion about keys in the later sections, because keys do not matter +in this configuration. However, if you are using "destroy before create" behavior, then a full understanding of keys +as applied to security group rules will help you minimize service interruptions due to changing rules. + +When creating a collection of resources, Terraform requires each resource to be identified by a key, +so that each resource has a unique "address", and changes to resources are tracked by that key. +Every security group rule input to this module accepts optional identifying keys (arbitrary strings) for each rule. +If you do not supply keys, then the rules are treated as a list, +and the index of the rule in the list will be used as its key. This has the unwelcome behavior that removing a rule +from the list will cause all the rules later in the list to be destroyed and recreated. For example, changing +`[A, B, C, D]` to `[A, C, D]` causes rules 1(`B`), 2(`C`), and 3(`D`) to be deleted and new rules 1(`C`) and +2(`D`) to be created. + +To mitigate against this problem, we allow you to specify keys (arbitrary strings) for each rule. (Exactly how you specify +the key is explained in the next sections.) Going back to our example, if the +initial set of rules were specified with keys, e.g. `[{A: A}, {B: B}, {C: C}, {D: D}]`, then removing `B` from the list +would only cause `B` to be deleted, leaving `C` and `D` intact. + +Note, however, two cautions. First, the keys must be known at `terraform plan` time and therefore cannot depend +on resources that will be created during `apply`. Second, in order to be helpful, the keys must remain consistently +attached to the same rules. For example, if you did + +```hcl +rule_map = { for i, v in rule_list : i => v } +``` + +then you will have merely recreated the initial problem with using a plain list. If you cannot attach +meaningful keys to the rules, there is no advantage to specifying keys at all. + +**But wait, there's more!*** + +A single security group rule input can actually specify multiple security group rules. For example, +`ipv6_cidr_blocks` takes a list of CIDRs. However, AWS security group rules do not allow for a list +of CIDRs, so the AWS Terraform provider converts that list of CIDRs into a list of AWS security group rules, +one for each CIDR. (This is the underlying cause of several AWS Terraform provider bugs, +such as [#25173](https://github.com/hashicorp/terraform-provider-aws/issues/25173).) +As of this writing, any change to any such element of a rule will cause +all the AWS rules specified by the Terraform rule to be deleted and recreated, causing the same kind of +service interruption we sought to avoid by providing keys for the rules. To guard against this issue, +when using "destroy before create" behavior, you should avoid the convenience of specifying multiple AWS rules +in a single Terraform rule and instead create a separate Terraform rule for each source or destination specification. + ##### `rules` and `rules_map` inputs This module provides 3 ways to set security group rules. You can use any or all of them at the same time. @@ -199,9 +378,9 @@ The `rules_map` input takes an object. which means they cannot depend on any resources created or changed by Terraform. - The values of the attributes are lists of rule objects, each object representing one Security Group Rule. As explained above in "Why the input is so complex", each object in the list must be exactly the same type. To use multiple types, - you must put them in separate lists which are values of separate attributes. + you must put them in separate lists and put the lists in a map with distinct keys. -###### Definition of a rule +###### Definition of a Rule For this module, a rule is defined as an object. - The attributes and values of the rule objects are fully compatible (have the same keys and accept the same values) as the @@ -219,7 +398,7 @@ a rule gets deleted from start of a list, causing all the other rules to shift p See ["Unexpected changes..."](#unexpected-changes-during-plan-and-apply) below for more details. -##### `rule_matrix` input +##### `rule_matrix` Input The other way to set rules is via the `rule_matrix` input. This splits the attributes of the `aws_security_group_rule` resource into two sets: one set defines the rule and description, the other set defines the subjects of the rule. Again, optional "key" values can provide stability, but cannot contain derived values. @@ -263,29 +442,33 @@ The schema for `rule_matrix` is: } ``` -##### Create before delete -This module provides a `create_before_delete` option that will, when a security group needs to be replaced, -cause Terraform to create the new one before deleting the old one. We recommend making this `true` for new security groups, -but we default it to `false` because if you import a security group with this setting `true`, that security -group will be deleted and replaced on the first `terraform apply`, which will likely cause a service outage. - ### Important Notes ##### Unexpected changes during plan and apply -The way Terraform works and the way this module is implemented causes security group rules without keys -to be dependent on their place in the input lists. If a rule is deleted and the other rules therefore move -closer to the start of the list, those rules will be deleted and recreated. This should have no significant -operational impact, but it can make a small change look like a big one when viewing the output of -Terraform plan. -You can avoid this for the most part by providing the optional keys. Rules with keys will not be +When configuring this module for "create before destroy" behavior, any change to +a security group rule will cause an entire new security group to be created with +all new rules. This can make a small change look like a big one, but is intentional +and should not cause concern. + +As explained above under [The Importance of Keys](#the-importance-of-keys), +when using "destroy before create" behavior, security group rules without keys +are identified by their indices in the input lists. If a rule is deleted and the other rules therefore move +closer to the start of the list, those rules will be deleted and recreated. This +can make a small change look like a big one when viewing the output of Terraform plan, +and will likely cause a brief (seconds) service interruption. + +You can avoid this for the most part by providing the optional keys, and limiting each rule +to a single source or destination. Rules with keys will not be 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. You cannot avoid this by sorting the -`source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error. +`source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error +because of [terraform#31035](https://github.com/hashicorp/terraform/issues/31035). ##### 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 @@ -305,7 +488,8 @@ 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 +will cause this error. (See [terraform#31035](https://github.com/hashicorp/terraform/issues/31035).) +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. @@ -468,14 +652,18 @@ Available targets: | Name | Version | |------|---------| -| [terraform](#requirement\_terraform) | >= 0.14.0 | +| [terraform](#requirement\_terraform) | >= 1.0.0 | | [aws](#requirement\_aws) | >= 3.0 | +| [null](#requirement\_null) | >= 3.0 | +| [random](#requirement\_random) | >= 3.0 | ## Providers | Name | Version | |------|---------| | [aws](#provider\_aws) | >= 3.0 | +| [null](#provider\_null) | >= 3.0 | +| [random](#provider\_random) | >= 3.0 | ## Modules @@ -489,17 +677,20 @@ Available targets: |------|------| | [aws_security_group.cbd](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource | | [aws_security_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource | +| [aws_security_group_rule.dbc](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource | | [aws_security_group_rule.keyed](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource | +| [null_resource.sync_rules_and_sg_lifecycles](https://registry.terraform.io/providers/hashicorp/null/latest/docs/resources/resource) | resource | +| [random_id.rule_change_forces_new_security_group](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/id) | resource | ## Inputs | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| | [additional\_tag\_map](#input\_additional\_tag\_map) | Additional key-value pairs to add to each map in `tags_as_list_of_maps`. Not added to `tags` or `id`.
This is for some rare cases where resources want additional configuration of tags
and therefore take a list of maps with tag key, value, and additional configuration. | `map(string)` | `{}` | no | -| [allow\_all\_egress](#input\_allow\_all\_egress) | A convenience that adds to the rules specified elsewhere a rule that allows all egress.
If this is false and no egress rules are specified via `rules` or `rule-matrix`, then no egress will be allowed. | `bool` | `false` | no | +| [allow\_all\_egress](#input\_allow\_all\_egress) | A convenience that adds to the rules specified elsewhere a rule that allows all egress.
If this is false and no egress rules are specified via `rules` or `rule-matrix`, then no egress will be allowed. | `bool` | `true` | no | | [attributes](#input\_attributes) | ID element. Additional attributes (e.g. `workers` or `cluster`) to add to `id`,
in the order they appear in the list. New attributes are appended to the
end of the list. The elements of the list are joined by the `delimiter`
and treated as a single ID element. | `list(string)` | `[]` | no | | [context](#input\_context) | Single object for setting entire context at once.
See description of individual variables for details.
Leave string and numeric variables as `null` to use default value.
Individual variable settings (non-null) override settings in context object,
except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | 
{
  "additional_tag_map": {},
  "attributes": [],
  "delimiter": null,
  "descriptor_formats": {},
  "enabled": true,
  "environment": null,
  "id_length_limit": null,
  "label_key_case": null,
  "label_order": [],
  "label_value_case": null,
  "labels_as_tags": [
    "unset"
  ],
  "name": null,
  "namespace": null,
  "regex_replace_chars": null,
  "stage": null,
  "tags": {},
  "tenant": null
}
| no | -| [create\_before\_destroy](#input\_create\_before\_destroy) | Set `true` to enable terraform `create_before_destroy` behavior on the created security group.
We recommend setting this `true` on new security groups, but default it to `false` because `true`
will cause existing security groups to be replaced.
Note that changing this value will always cause the security group to be replaced. | `bool` | `false` | no | +| [create\_before\_destroy](#input\_create\_before\_destroy) | Set `true` to enable terraform `create_before_destroy` behavior on the created security group.
We only recommend setting this `false` if you are importing an existing security group
that you do not want replaced and therefore need full control over its name.
Note that changing this value will always cause the security group to be replaced. | `bool` | `true` | no | | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | @@ -512,6 +703,7 @@ Available targets: | [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.
Default is to include all labels.
Tags with empty values will not be included in the `tags` output.
Set to `[]` to suppress all generated tags.
**Notes:**
The value of the `name` tag, if included, will be the `id`, not the `name`.
Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be
changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | 
[
  "default"
]
| no | | [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.
This is the only ID element not also included as a `tag`.
The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no | | [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no | +| [preserve\_security\_group\_id](#input\_preserve\_security\_group\_id) | When `false` and `security_group_create_before_destroy` is `true`, changes to security group rules
cause a new security group to be created with the new rules, and the existing security group is then
replaced with the new one, eliminating any service interruption.
When `true` or when changing the value (from `false` to `true` or from `true` to `false`),
existing security group rules will be deleted before new ones are created, resulting in a service interruption,
but preserving the security group itself.
**NOTE:** Setting this to `true` does not guarantee the security group will never be replaced,
it only keeps changes to the security group rules from triggering a replacement.
See the README for further discussion. | `bool` | `false` | no | | [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.
Characters matching the regex will be removed from the ID elements.
If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no | | [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
the security group itself. This is normally not needed. | `bool` | `false` | no | | [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 | diff --git a/README.yaml b/README.yaml index b86eecc..8928888 100644 --- a/README.yaml +++ b/README.yaml @@ -64,6 +64,185 @@ usage: |- ID of an existing security group to modify, or, by default, this module will create a new security group and apply the given rules to it. + This module can be used very simply, but it is actually quite complex because it is attempting to handle + numerous interrelationships, restrictions, and a few bugs in ways that offer a choice between zero + service interruption for updates to a security group not referenced by other security groups + (by replacing the security group with a new one) versus brief service interruptions for security groups that must be preserved. + + ### Avoiding Service Interruptions + + It is desirable to avoid having service interruptions when updating a security group. This is not always possible + due to a combination of the way Terraform organizes its activities and the fact that AWS will reject + an attempt to create a duplicate of an existing security group rule. There is also the issue that while many + AWS resources can be associated with and disassociated from security groups at any time, some + may not have their security group association changed, and an attempt to change the security group will + cause Terraform to delete and recreate the resource. + + #### The 2 Ways Security Group Changes Cause Service Interruptions + + Changes to a security group can cause service interruptions in 2 ways: + + 1. Changing rules may be implemented as deleting existing rules and creating new ones. During the + period between deleting the old rules and creating the new rules, the security group will block + traffic intended to be allowed by the new rules. + 2. Changing rules may alternately be implemented as creating a new security group with the new rules + and replacing the existing security group with the new one (then deleting the old one). + This usually works with no service interruption in the case where all resources that reference the + security group are part of the same Terraform plan. + However, if, for example, the security group ID is referenced in a security group + rule in a security group that is not part of the same Terraform plan, then AWS will not allow the + existing (referenced) security group to be deleted, and even if it did, Terraform would not know + to update the rule to reference the new security group. + + The key question you need to answer to decide which configuration to use is "will anything break + if the security group ID changes". If no, then use the defaults `create_before_destroy = true` and + `preserve_security_group_id = false` and do not worry about providing "keys" for + security group rules. This is the default because it is the easiest and safest solution when + the way the security group is being used allows it. + + If things will break when the security group ID changes, then set `preserve_security_group_id` + to `true` and read the information about keys and single source Terraform rules if you want + to mitigate against service interruptions caused by rule changes. Note that even in this case, + you probably want to keep `create_before_destroy = true` because otherwise, if some change + requires the security group to be replaced, Terraform will likely succeed in deleting all the + security group rules but fail to delete the security group itself, leaving the associated resources + completely inaccessible. At least with `create_before_destroy = true`, the new security group + will be created and will be used where Terraform is able to make the changes, even though + the old security group will still fail to be deleted. + + #### The 3 Ways to Mitigate Against Service Interruptions + + ##### Security Group `create_before_destroy = true` + + The most important option is `create_before_destroy` which, when set to `true` (the default), + ensures that a new replacement security group is created before an existing one is destroyed. + This is particularly important because a security group cannot be destroyed while it is associated with + a resource (e.g. a load balancer), but "destroy before create" behavior causes Terraform + to try to destroy the security group before disassociating it from associated resources, + so plans fail to apply with the error + + ``` + Error deleting security group: DependencyViolation: resource sg-XXX has a dependent object + ``` + + With "create before destroy" and any resources dependent on the security group as part of the + same Terraform plan, replacement happens successfully: + + 1. New security group is created + 2. Resource is associated with the new security group and disassociated from the old one + 3. Old security group is deleted successfully because there is no longer anything associated with it + + (If there is a resource dependent on the security group that is also outside the scope of + the Terraform plan, the old security group will fail to be deleted and you will have to + address the dependency manually.) + + Note that the module's default configuration of `create_before_destroy = true` and + `preserve_security_group_id = false` will force "create before destroy" behavior on the target security + group, even if the module did not create it and instead you provided a `target_security_group_id`. + + Unfortunately, just creating the new security group first is not enough to prevent a service interruption. Keep reading. + + ##### Setting Rule Changes to Force Replacement of the Security Group + + A security group by itself is just a container for rules. It only functions as desired when all the rules are in place. + If using the Terraform default "destroy before create" behavior for rules, even when using `create_before_destroy` for the + security group itself, an outage occurs when updating the rules or security group, because the order of operations is: + + 1. Delete existing security group rules (triggering a service interruption) + 2. Create the new security group + 3. Associate the new security group with resources and disassociate the old one (which can take a substantial + amount of time for a resource like a NAT Gateway) + 4. Create the new security group rules (restoring service) + 5. Delete the old security group + + To resolve this issue, the module's default configuration of `create_before_destroy = true` and + `preserve_security_group_id = false` causes any change in the security group rules + to trigger the creation of a new security group. With that, a rule change causes operations to occur in this order: + + 1. Create the new security group + 2. Create the new security group rules + 3. Associate the new security group with resources and disassociate the old one + 4. Delete the old security group rules + 5. Delete the old security group + + ##### Preserving the Security Group + + There can be a downside to creating a new security group with every rule change. + If you want to prevent the security group ID from changing unless absolutely necessary, perhaps because the associated + resource does not allow the security group to be changed or because the ID is referenced somewhere (like in + another security group's rules) outside of this Terraform plan, then you need to set `preserve_security_group_id` to `true`. + + The main drawback of this configuration is that there will normally be + a service outage during an update, because existing rules will be deleted before replacement + rules are created. Using keys to identify rules can help limit the impact, but even with keys, simply adding a + CIDR to the list of allowed CIDRs will cause that entire rule to be deleted and recreated, causing a temporary + access denial for all of the CIDRs in the rule. (For more on this and how to mitigate against it, see [The Importance + of Keys](#the-importance-of-keys) below.) + + Also note that setting `preserve_security_group_id` to `true` does not prevent Terraform from replacing the + security group when modifying it is not an option, such as when its name or description changes. + However, if you can control the configuration adequately, you can maintain the security group ID and eliminate + impact on other security groups by setting `preserve_security_group_id` to `true`. We still recommend + leaving `create_before_destroy` set to `true` for the times when the security group must be replaced, + to avoid the `DependencyViolation` described above. + + ### Defining Security Group Rules + + We provide a number of different ways to define rules for the security group for a few reasons: + - Terraform type constraints make it difficult to create collections of objects with optional members + - Terraform resource addressing can cause resources that did not actually change to nevertheless be replaced + (deleted and recreated), which, in the case of security group rules, then causes a brief service interruption + - Terraform resource addresses must be known at `plan` time, making it challenging to create rules that + depend on resources being created during `apply` and at the same time are not replaced needlessly when something else changes + - When Terraform rules can be successfully created before being destroyed, there is no service interruption for the resources + associated with that security group (unless the security group ID is used in other security group rules outside + of the scope of the Terraform plan) + + #### The Importance of Keys + + If you are using "create before destroy" behavior for the security group and security group rules, then + you can skip this section and much of the discussion about keys in the later sections, because keys do not matter + in this configuration. However, if you are using "destroy before create" behavior, then a full understanding of keys + as applied to security group rules will help you minimize service interruptions due to changing rules. + + When creating a collection of resources, Terraform requires each resource to be identified by a key, + so that each resource has a unique "address", and changes to resources are tracked by that key. + Every security group rule input to this module accepts optional identifying keys (arbitrary strings) for each rule. + If you do not supply keys, then the rules are treated as a list, + and the index of the rule in the list will be used as its key. This has the unwelcome behavior that removing a rule + from the list will cause all the rules later in the list to be destroyed and recreated. For example, changing + `[A, B, C, D]` to `[A, C, D]` causes rules 1(`B`), 2(`C`), and 3(`D`) to be deleted and new rules 1(`C`) and + 2(`D`) to be created. + + To mitigate against this problem, we allow you to specify keys (arbitrary strings) for each rule. (Exactly how you specify + the key is explained in the next sections.) Going back to our example, if the + initial set of rules were specified with keys, e.g. `[{A: A}, {B: B}, {C: C}, {D: D}]`, then removing `B` from the list + would only cause `B` to be deleted, leaving `C` and `D` intact. + + Note, however, two cautions. First, the keys must be known at `terraform plan` time and therefore cannot depend + on resources that will be created during `apply`. Second, in order to be helpful, the keys must remain consistently + attached to the same rules. For example, if you did + + ```hcl + rule_map = { for i, v in rule_list : i => v } + ``` + + then you will have merely recreated the initial problem with using a plain list. If you cannot attach + meaningful keys to the rules, there is no advantage to specifying keys at all. + + **But wait, there's more!*** + + A single security group rule input can actually specify multiple security group rules. For example, + `ipv6_cidr_blocks` takes a list of CIDRs. However, AWS security group rules do not allow for a list + of CIDRs, so the AWS Terraform provider converts that list of CIDRs into a list of AWS security group rules, + one for each CIDR. (This is the underlying cause of several AWS Terraform provider bugs, + such as [#25173](https://github.com/hashicorp/terraform-provider-aws/issues/25173).) + As of this writing, any change to any such element of a rule will cause + all the AWS rules specified by the Terraform rule to be deleted and recreated, causing the same kind of + service interruption we sought to avoid by providing keys for the rules. To guard against this issue, + when using "destroy before create" behavior, you should avoid the convenience of specifying multiple AWS rules + in a single Terraform rule and instead create a separate Terraform rule for each source or destination specification. + ##### `rules` and `rules_map` inputs This module provides 3 ways to set security group rules. You can use any or all of them at the same time. @@ -166,9 +345,9 @@ usage: |- which means they cannot depend on any resources created or changed by Terraform. - The values of the attributes are lists of rule objects, each object representing one Security Group Rule. As explained above in "Why the input is so complex", each object in the list must be exactly the same type. To use multiple types, - you must put them in separate lists which are values of separate attributes. + you must put them in separate lists and put the lists in a map with distinct keys. - ###### Definition of a rule + ###### Definition of a Rule For this module, a rule is defined as an object. - The attributes and values of the rule objects are fully compatible (have the same keys and accept the same values) as the @@ -186,7 +365,7 @@ usage: |- See ["Unexpected changes..."](#unexpected-changes-during-plan-and-apply) below for more details. - ##### `rule_matrix` input + ##### `rule_matrix` Input The other way to set rules is via the `rule_matrix` input. This splits the attributes of the `aws_security_group_rule` resource into two sets: one set defines the rule and description, the other set defines the subjects of the rule. Again, optional "key" values can provide stability, but cannot contain derived values. @@ -230,29 +409,33 @@ usage: |- } ``` - ##### Create before delete - This module provides a `create_before_delete` option that will, when a security group needs to be replaced, - cause Terraform to create the new one before deleting the old one. We recommend making this `true` for new security groups, - but we default it to `false` because if you import a security group with this setting `true`, that security - group will be deleted and replaced on the first `terraform apply`, which will likely cause a service outage. - ### Important Notes ##### Unexpected changes during plan and apply - The way Terraform works and the way this module is implemented causes security group rules without keys - to be dependent on their place in the input lists. If a rule is deleted and the other rules therefore move - closer to the start of the list, those rules will be deleted and recreated. This should have no significant - operational impact, but it can make a small change look like a big one when viewing the output of - Terraform plan. - You can avoid this for the most part by providing the optional keys. Rules with keys will not be + When configuring this module for "create before destroy" behavior, any change to + a security group rule will cause an entire new security group to be created with + all new rules. This can make a small change look like a big one, but is intentional + and should not cause concern. + + As explained above under [The Importance of Keys](#the-importance-of-keys), + when using "destroy before create" behavior, security group rules without keys + are identified by their indices in the input lists. If a rule is deleted and the other rules therefore move + closer to the start of the list, those rules will be deleted and recreated. This + can make a small change look like a big one when viewing the output of Terraform plan, + and will likely cause a brief (seconds) service interruption. + + You can avoid this for the most part by providing the optional keys, and limiting each rule + to a single source or destination. Rules with keys will not be 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. You cannot avoid this by sorting the - `source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error. + `source_security_group_ids`, because that leads to the "Invalid `for_each` argument" error + because of [terraform#31035](https://github.com/hashicorp/terraform/issues/31035). ##### 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 @@ -272,7 +455,8 @@ usage: |- 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 + will cause this error. (See [terraform#31035](https://github.com/hashicorp/terraform/issues/31035).) + 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. diff --git a/docs/terraform.md b/docs/terraform.md index 940d0f3..fbc32bb 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -3,14 +3,18 @@ | Name | Version | |------|---------| -| [terraform](#requirement\_terraform) | >= 0.14.0 | +| [terraform](#requirement\_terraform) | >= 1.0.0 | | [aws](#requirement\_aws) | >= 3.0 | +| [null](#requirement\_null) | >= 3.0 | +| [random](#requirement\_random) | >= 3.0 | ## Providers | Name | Version | |------|---------| | [aws](#provider\_aws) | >= 3.0 | +| [null](#provider\_null) | >= 3.0 | +| [random](#provider\_random) | >= 3.0 | ## Modules @@ -24,17 +28,20 @@ |------|------| | [aws_security_group.cbd](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource | | [aws_security_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource | +| [aws_security_group_rule.dbc](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource | | [aws_security_group_rule.keyed](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource | +| [null_resource.sync_rules_and_sg_lifecycles](https://registry.terraform.io/providers/hashicorp/null/latest/docs/resources/resource) | resource | +| [random_id.rule_change_forces_new_security_group](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/id) | resource | ## Inputs | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| | [additional\_tag\_map](#input\_additional\_tag\_map) | Additional key-value pairs to add to each map in `tags_as_list_of_maps`. Not added to `tags` or `id`.
This is for some rare cases where resources want additional configuration of tags
and therefore take a list of maps with tag key, value, and additional configuration. | `map(string)` | `{}` | no | -| [allow\_all\_egress](#input\_allow\_all\_egress) | A convenience that adds to the rules specified elsewhere a rule that allows all egress.
If this is false and no egress rules are specified via `rules` or `rule-matrix`, then no egress will be allowed. | `bool` | `false` | no | +| [allow\_all\_egress](#input\_allow\_all\_egress) | A convenience that adds to the rules specified elsewhere a rule that allows all egress.
If this is false and no egress rules are specified via `rules` or `rule-matrix`, then no egress will be allowed. | `bool` | `true` | no | | [attributes](#input\_attributes) | ID element. Additional attributes (e.g. `workers` or `cluster`) to add to `id`,
in the order they appear in the list. New attributes are appended to the
end of the list. The elements of the list are joined by the `delimiter`
and treated as a single ID element. | `list(string)` | `[]` | no | | [context](#input\_context) | Single object for setting entire context at once.
See description of individual variables for details.
Leave string and numeric variables as `null` to use default value.
Individual variable settings (non-null) override settings in context object,
except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | 
{
  "additional_tag_map": {},
  "attributes": [],
  "delimiter": null,
  "descriptor_formats": {},
  "enabled": true,
  "environment": null,
  "id_length_limit": null,
  "label_key_case": null,
  "label_order": [],
  "label_value_case": null,
  "labels_as_tags": [
    "unset"
  ],
  "name": null,
  "namespace": null,
  "regex_replace_chars": null,
  "stage": null,
  "tags": {},
  "tenant": null
}
| no | -| [create\_before\_destroy](#input\_create\_before\_destroy) | Set `true` to enable terraform `create_before_destroy` behavior on the created security group.
We recommend setting this `true` on new security groups, but default it to `false` because `true`
will cause existing security groups to be replaced.
Note that changing this value will always cause the security group to be replaced. | `bool` | `false` | no | +| [create\_before\_destroy](#input\_create\_before\_destroy) | Set `true` to enable terraform `create_before_destroy` behavior on the created security group.
We only recommend setting this `false` if you are importing an existing security group
that you do not want replaced and therefore need full control over its name.
Note that changing this value will always cause the security group to be replaced. | `bool` | `true` | no | | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | @@ -47,6 +54,7 @@ | [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.
Default is to include all labels.
Tags with empty values will not be included in the `tags` output.
Set to `[]` to suppress all generated tags.
**Notes:**
The value of the `name` tag, if included, will be the `id`, not the `name`.
Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be
changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | 
[
  "default"
]
| no | | [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.
This is the only ID element not also included as a `tag`.
The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no | | [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no | +| [preserve\_security\_group\_id](#input\_preserve\_security\_group\_id) | When `false` and `security_group_create_before_destroy` is `true`, changes to security group rules
cause a new security group to be created with the new rules, and the existing security group is then
replaced with the new one, eliminating any service interruption.
When `true` or when changing the value (from `false` to `true` or from `true` to `false`),
existing security group rules will be deleted before new ones are created, resulting in a service interruption,
but preserving the security group itself.
**NOTE:** Setting this to `true` does not guarantee the security group will never be replaced,
it only keeps changes to the security group rules from triggering a replacement.
See the README for further discussion. | `bool` | `false` | no | | [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.
Characters matching the regex will be removed from the ID elements.
If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no | | [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
the security group itself. This is normally not needed. | `bool` | `false` | no | | [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 | diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 96d2b2f..d94aa0e 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -115,7 +115,7 @@ module "new_security_group" { security_group_create_timeout = "5m" security_group_delete_timeout = "2m" - security_group_name = [format("%s-%s", module.this.id, "new")] + security_group_name = [format("%s-%s", module.this.id, "new-")] context = module.this.context } diff --git a/examples/complete/versions.tf b/examples/complete/versions.tf index 28a2720..b1598cd 100644 --- a/examples/complete/versions.tf +++ b/examples/complete/versions.tf @@ -1,5 +1,5 @@ terraform { - required_version = ">= 0.14.0" + required_version = ">= 1.0.0" required_providers { aws = { diff --git a/exports/security-group-variables.tf b/exports/security-group-variables.tf index 17d78db..b8bdedd 100644 --- a/exports/security-group-variables.tf +++ b/exports/security-group-variables.tf @@ -1,4 +1,4 @@ -# security-group-variables Version: 2 +# security-group-variables Version: 3 # # Copy this file from https://github.com/cloudposse/terraform-aws-security-group/blob/master/exports/security-group-variables.tf # and EDIT IT TO SUIT YOUR PROJECT. Update the version number above if you update this file from a later version. @@ -21,17 +21,17 @@ variable "create_security_group" { type = bool - default = true description = "Set `true` to create and configure a new security group. If false, `associated_security_group_ids` must be provided." + default = true } variable "associated_security_group_ids" { type = list(string) - default = [] description = <<-EOT A list of IDs of Security Groups to associate the created resource with, in addition to the created security group. These security groups will not be modified and, if `create_security_group` is `false`, must have rules providing the desired access. EOT + default = [] } ## @@ -47,127 +47,113 @@ variable "associated_security_group_ids" { ## - likely to invite count/for_each issues variable "allowed_security_group_ids" { type = list(string) - default = [] description = <<-EOT A list of IDs of Security Groups to allow access to the security group created by this module. The length of this list must be known at "plan" time. EOT + default = [] } variable "allowed_cidr_blocks" { type = list(string) - default = [] description = <<-EOT A list of IPv4 CIDRs to allow access to the security group created by this module. The length of this list must be known at "plan" time. EOT + default = [] } variable "allowed_ipv6_cidr_blocks" { type = list(string) - default = [] description = <<-EOT A list of IPv6 CIDRs to allow access to the security group created by this module. The length of this list must be known at "plan" time. EOT + default = [] } variable "allowed_ipv6_prefix_list_ids" { type = list(string) - default = [] description = <<-EOT A list of IPv6 Prefix Lists IDs to allow access to the security group created by this module. The length of this list must be known at "plan" time. EOT + default = [] } ## End of optional allowed_* ########### variable "security_group_name" { type = list(string) - default = [] description = <<-EOT The name to assign to the created security group. Must be unique within the VPC. If not provided, will be derived from the `null-label.context` passed in. If `create_before_destroy` is true, will be used as a name prefix. EOT + default = [] } variable "security_group_description" { type = string - default = "Managed by Terraform" description = <<-EOT The description to assign to the created Security Group. Warning: Changing the description causes the security group to be replaced. EOT + default = "Managed by Terraform" } -############################### -# -# Decide on a case-by-case basis what the default should be. -# In general, if the resource supports changing security groups without deleting -# the resource or anything it depends on, then default it to `true` and -# note in the release notes and migration documents the option to -# set it to `false` to preserve the existing security group. -# If the resource has to be deleted to change its security group, -# then set the default to `false` and highlight the option to change -# it to `true` in the release notes and migration documents. -# -################################ variable "security_group_create_before_destroy" { - type = bool - # - # Pick `true` or `false` and the associated description. - # Use `true` unless replacing the security group causes the underlying resource - # (e.g. the EC2 instance, not just the security group) to be destroyed and recreated. - # - # Replace "the resource" in the description with the name of the resource, e.g. "EC2 instance". - # - - # default = true - # description = <<-EOT - # Set `true` to enable Terraform `create_before_destroy` behavior on the created security group. - # We only recommend setting this `false` if you are upgrading this module and need to keep - # the existing security group from being replaced. - # Note that changing this value will always cause the security group to be replaced. - # EOT - - # default = false - # description = <<-EOT - # Set `true` to enable Terraform `create_before_destroy` behavior on the created security group. - # We recommend setting this `true` on new security groups, but default it to `false` because `true` - # will cause existing security groups to be replaced, possibly requiring the resource to be deleted and recreated. - # Note that changing this value will always cause the security group to be replaced. - # EOT + type = bool + description = <<-EOT + Set `true` to enable terraform `create_before_destroy` behavior on the created security group. + We only recommend setting this `false` if you are importing an existing security group + that you do not want replaced and therefore need full control over its name. + Note that changing this value will always cause the security group to be replaced. + EOT + default = true +} +variable "preserve_security_group_id" { + type = bool + description = <<-EOT + When `false` and `security_group_create_before_destroy` is `true`, changes to security group rules + cause a new security group to be created with the new rules, and the existing security group is then + replaced with the new one, eliminating any service interruption. + When `true` or when changing the value (from `false` to `true` or from `true` to `false`), + existing security group rules will be deleted before new ones are created, resulting in a service interruption, + but preserving the security group itself. + **NOTE:** Setting this to `true` does not guarantee the security group will never be replaced, + it only keeps changes to the security group rules from triggering a replacement. + See the [terraform-aws-security-group README](https://github.com/cloudposse/terraform-aws-security-group) for further discussion. + EOT + default = false } variable "security_group_create_timeout" { type = string - default = "10m" description = "How long to wait for the security group to be created." + default = "10m" } variable "security_group_delete_timeout" { type = string - default = "15m" description = <<-EOT How long to retry on `DependencyViolation` errors during security group deletion from lingering ENIs left by certain AWS services such as Elastic Load Balancing. EOT + default = "15m" } variable "allow_all_egress" { type = bool - default = true description = <<-EOT If `true`, the created security group will allow egress on all ports and protocols to all IP addresses. If this is false and no egress rules are otherwise specified, then no egress will be allowed. EOT + default = true } variable "additional_security_group_rules" { type = list(any) - default = [] description = <<-EOT A list of Security Group rule objects to add to the created security group, in addition to the ones this module normally creates. (To suppress the module's rules, set `create_security_group` to false @@ -177,6 +163,7 @@ variable "additional_security_group_rules" { For more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule and https://github.com/cloudposse/terraform-aws-security-group. EOT + default = [] } #### We do not expose an `additional_security_group_rule_matrix` input for a few reasons: @@ -214,21 +201,21 @@ variable "vpc_id" { variable "inline_rules_enabled" { type = bool - default = false description = <<-EOT NOT RECOMMENDED. Create rules "inline" instead of as separate `aws_security_group_rule` resources. See [#20046](https://github.com/hashicorp/terraform-provider-aws/issues/20046) for one of several issues with inline rules. See [this post](https://github.com/hashicorp/terraform-provider-aws/pull/9032#issuecomment-639545250) for details on the difference between inline rules and rule resources. EOT + default = false } variable "revoke_security_group_rules_on_delete" { type = bool - default = false description = <<-EOT Instruct Terraform to revoke all of the Security Group's attached ingress and egress rules before deleting the security group itself. This is normally not needed. EOT + default = false } diff --git a/main.tf b/main.tf index 495f637..1687662 100644 --- a/main.tf +++ b/main.tf @@ -6,26 +6,60 @@ locals { default_rule_description = "Managed by Terraform" - create_security_group = local.enabled && length(var.target_security_group_id) == 0 + create_security_group = local.enabled && length(var.target_security_group_id) == 0 + sg_create_before_destroy = var.create_before_destroy created_security_group = local.create_security_group ? ( - var.create_before_destroy ? aws_security_group.cbd[0] : aws_security_group.default[0] + local.sg_create_before_destroy ? aws_security_group.cbd[0] : aws_security_group.default[0] ) : null + # This clever construction makes `security_group_id` the ID of either the Target security group (SG) supplied, + # or the 1 of the 2 flavors we create: the "create before destroy (CBD)" (`create_before_destroy = true`) SG + # or the "destroy before create (DBC)" (`create_before_destroy = false`) SG. Unfortunately, the way it is constructed, + # Terraform considers `local.security_group_id` dependent on the DBC SG, which means that + # when it is referenced by the CBD security group rules, Terraform forces + # unwanted CBD behavior on the DBC SG, so we can only use it for the DBC SG rules. security_group_id = local.enabled ? ( # Use coalesce() here to hack an error message into the output local.create_security_group ? local.created_security_group.id : coalesce(var.target_security_group_id[0], - "var.target_security_group_id contains null value. Omit value if you want this module to create a security group.") + "var.target_security_group_id contains an empty value. Omit any value if you want this module to create a security group.") ) : null + + # Setting `create_before_destroy` on the security group rules forces `create_before_destroy` behavior + # on the security group, so we have to disable it on the rules if disabled on the security group. + # It also forces a new security group to be created whenever any rule changes, so we disable it + # when `var.preserve_security_group_id` is `true`. + rule_create_before_destroy = local.sg_create_before_destroy && !var.preserve_security_group_id + # We also have to make it clear to Terraform that the "create before destroy" (CBD) rules + # will never reference the "destroy before create" (DBC) security group (SG) + # by keeping any conditional reference to the DBC SG out of the expression (unlike the `security_group_id` expression above). + cbd_security_group_id = local.create_security_group ? one(aws_security_group.cbd[*].id) : var.target_security_group_id[0] + + # The only way to guarantee success when creating new rules before destroying old ones + # is to make the new rules part of a new security group. + # See https://github.com/cloudposse/terraform-aws-security-group/issues/34 + rule_change_forces_new_security_group = local.enabled && local.rule_create_before_destroy +} + +# We force a new security group by changing its name, using `random_id` to generate a part of the name prefix +resource "random_id" "rule_change_forces_new_security_group" { + count = local.rule_change_forces_new_security_group ? 1 : 0 + byte_length = 3 + keepers = { + rules = jsonencode(local.keyed_resource_rules) + } } # You cannot toggle `create_before_destroy` based on input, # you have to have a completely separate resource to change it. resource "aws_security_group" "default" { # Because we have 2 almost identical alternatives, use x == false and x == true rather than x and !x - count = local.create_security_group && var.create_before_destroy == false ? 1 : 0 + count = local.create_security_group && local.sg_create_before_destroy == false ? 1 : 0 name = concat(var.security_group_name, [module.this.id])[0] + lifecycle { + create_before_destroy = false + } ######################################################################## ## Everything from here to the end of this resource should be identical @@ -78,11 +112,20 @@ resource "aws_security_group" "default" { } +locals { + sg_name_prefix_base = concat(var.security_group_name, ["${module.this.id}${module.this.delimiter}"])[0] + # Force a new security group to be created by changing its name prefix, using `random_id` to create a short ID string + # that changes when the rules change, and adding that to the configured name prefix. + sg_name_prefix_forced = "${local.sg_name_prefix_base}${module.this.delimiter}${join("", random_id.rule_change_forces_new_security_group[*].b64_url)}${module.this.delimiter}" + sg_name_prefix = local.rule_change_forces_new_security_group ? local.sg_name_prefix_forced : local.sg_name_prefix_base +} + + resource "aws_security_group" "cbd" { # Because we have 2 almost identical alternatives, use x == false and x == true rather than x and !x - count = local.create_security_group && var.create_before_destroy == true ? 1 : 0 + count = local.create_security_group && local.sg_create_before_destroy == true ? 1 : 0 - name_prefix = concat(var.security_group_name, ["${module.this.id}${module.this.delimiter}"])[0] + name_prefix = local.sg_name_prefix lifecycle { create_before_destroy = true } @@ -138,8 +181,54 @@ resource "aws_security_group" "cbd" { } +# We would like to always have `create_before_destroy` for security group rules, +# but duplicates are not allowed so `create_before_destroy` has a high probability of failing. +# See https://github.com/hashicorp/terraform-provider-aws/issues/25173 and its References. +# You cannot toggle `create_before_destroy` based on input, +# you have to have a completely separate resource to change it. resource "aws_security_group_rule" "keyed" { - for_each = local.keyed_resource_rules + for_each = local.rule_create_before_destroy ? local.keyed_resource_rules : {} + + lifecycle { + create_before_destroy = true + } + + ######################################################################## + ## Everything from here to the end of this resource should be identical + ## (copy and paste) in aws_security_group_rule.keyed and aws_security_group.dbc + + + security_group_id = local.cbd_security_group_id + + type = each.value.type + from_port = each.value.from_port + to_port = each.value.to_port + protocol = each.value.protocol + description = each.value.description + + cidr_blocks = length(each.value.cidr_blocks) == 0 ? null : each.value.cidr_blocks + ipv6_cidr_blocks = length(each.value.ipv6_cidr_blocks) == 0 ? null : each.value.ipv6_cidr_blocks + prefix_list_ids = length(each.value.prefix_list_ids) == 0 ? [] : each.value.prefix_list_ids + self = each.value.self + source_security_group_id = each.value.source_security_group_id + + ## + ## end of duplicate block + ######################################################################## + +} + +resource "aws_security_group_rule" "dbc" { + for_each = local.rule_create_before_destroy ? {} : local.keyed_resource_rules + + lifecycle { + # This has no actual effect, it is just here for emphasis + create_before_destroy = false + } + ######################################################################## + ## Everything from here to the end of this resource should be identical + ## (copy and paste) in aws_security_group.default and aws_security_group.cbd + security_group_id = local.security_group_id @@ -155,7 +244,29 @@ resource "aws_security_group_rule" "keyed" { self = each.value.self source_security_group_id = each.value.source_security_group_id - depends_on = [aws_security_group.cbd, aws_security_group.default] + ## + ## end of duplicate block + ######################################################################## + +} + +# This null resource prevents an outage when a new Security Group needs to be provisioned +# and `local.rule_create_before_destroy` is `true`: +# 1. It prevents the deposed security group rules from being deleted until after all +# references to it have been changed to refer to the new security group. +# 2. It ensures the new security group rules are created before +# the new security group is associated with existing resources +resource "null_resource" "sync_rules_and_sg_lifecycles" { + # NOTE: This resource affects the lifecycles even when count = 0, + # see https://github.com/hashicorp/terraform/issues/31316#issuecomment-1167450615 + # Still, we can avoid creating it when we do not need it to be triggered. + count = local.rule_create_before_destroy ? 1 : 0 + # Replacement of the security group requires re-provisioning + triggers = { + sg_ids = one(aws_security_group.cbd[*].id) + } + + depends_on = [aws_security_group_rule.keyed] lifecycle { create_before_destroy = true diff --git a/test/src/examples_complete_test.go b/test/src/examples_complete_test.go index c836f66..11c4219 100644 --- a/test/src/examples_complete_test.go +++ b/test/src/examples_complete_test.go @@ -61,7 +61,7 @@ func TestExamplesComplete(t *testing.T) { assert.Contains(t, newSgID, "sg-", "SG ID should contains substring 'sg-'") assert.Contains(t, newSgARN, "arn:aws:ec2", "SG ID should contains substring 'arn:aws:ec2'") - assert.Equal(t, "eg-ue2-test-sg-"+randID+"-new", newSgName) + assert.Contains(t, newSgName, "eg-ue2-test-sg-"+randID+"-new-") // Verify that outputs are valid when an existing security group is provided targetSgID := terraform.Output(t, terraformOptions, "target_sg_id") diff --git a/test/src/go.mod b/test/src/go.mod index 62a2838..ca4e4c2 100644 --- a/test/src/go.mod +++ b/test/src/go.mod @@ -1,10 +1,11 @@ module github.com/cloudposse/terraform-aws-security-group -go 1.17 +go 1.18 require ( - github.com/gruntwork-io/terratest v0.39.0 - github.com/stretchr/testify v1.7.0 + // Known security flaws in terratest dependencies prior to v0.40.15 + github.com/gruntwork-io/terratest v0.40.17 + github.com/stretchr/testify v1.8.0 k8s.io/apimachinery v0.20.6 ) @@ -33,7 +34,7 @@ require ( github.com/gruntwork-io/go-commons v0.8.0 // indirect github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect - github.com/hashicorp/go-getter v1.5.11 // indirect + github.com/hashicorp/go-getter v1.6.1 // indirect github.com/hashicorp/go-multierror v1.1.0 // indirect github.com/hashicorp/go-safetemp v1.0.0 // indirect github.com/hashicorp/go-version v1.3.0 // indirect @@ -65,7 +66,7 @@ require ( golang.org/x/mod v0.4.2 // indirect golang.org/x/net v0.0.0-20210614182718-04defd469f4e // indirect golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c // indirect - golang.org/x/sys v0.0.0-20210603125802-9665404d3644 // indirect + golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e // indirect golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect golang.org/x/text v0.3.6 // indirect golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect @@ -78,7 +79,7 @@ require ( google.golang.org/protobuf v1.26.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.20.6 // indirect k8s.io/client-go v0.20.6 // indirect k8s.io/klog/v2 v2.4.0 // indirect diff --git a/test/src/go.sum b/test/src/go.sum index 417452b..f32db96 100644 --- a/test/src/go.sum +++ b/test/src/go.sum @@ -177,8 +177,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -213,14 +213,14 @@ github.com/googleapis/gnostic v0.4.1/go.mod h1:LRhVm6pbyptWbWbuZ38d1eyptfvIytN3i github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= github.com/gruntwork-io/go-commons v0.8.0 h1:k/yypwrPqSeYHevLlEDmvmgQzcyTwrlZGRaxEM6G0ro= github.com/gruntwork-io/go-commons v0.8.0/go.mod h1:gtp0yTtIBExIZp7vyIV9I0XQkVwiQZze678hvDXof78= -github.com/gruntwork-io/terratest v0.39.0 h1:Lq7aNCoFxhhmdQIyuBFBf8N87aCnypmNBFYgvsdIfCQ= -github.com/gruntwork-io/terratest v0.39.0/go.mod h1:CjHsEgP1Pe987X5N8K5qEqCuLtu1bqERGIAF8bTj1s0= +github.com/gruntwork-io/terratest v0.40.17 h1:veSr7MUtk5GRDg5/pZ9qLUvrylr7yuRw0fY9UaSHbuI= +github.com/gruntwork-io/terratest v0.40.17/go.mod h1:enUpxNMsQfiJTTJMGqiznxohqJ4cYPU+nzI3bQNw1WM= github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= -github.com/hashicorp/go-getter v1.5.11 h1:wioTuNmaBU3IE9vdFtFMcmZWj0QzLc6DYaP6sNe5onY= -github.com/hashicorp/go-getter v1.5.11/go.mod h1:9i48BP6wpWweI/0/+FBjqLrp9S8XtwUGjiu0QkWHEaY= +github.com/hashicorp/go-getter v1.6.1 h1:NASsgP4q6tL94WH6nJxKWj8As2H/2kop/bB1d8JMyRY= +github.com/hashicorp/go-getter v1.6.1/go.mod h1:IZCrswsZPeWv9IkVnLElzRU/gz/QPi6pZHn4tv6vbwA= github.com/hashicorp/go-multierror v1.1.0 h1:B9UzwGQJehnUY1yNrnwREHc3fGbC2xefo8g4TbElacI= github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA= github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo= @@ -330,13 +330,15 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/tmccombs/hcl2json v0.3.3 h1:+DLNYqpWE0CsOQiEZu+OZm5ZBImake3wtITYxQ8uLFQ= github.com/tmccombs/hcl2json v0.3.3/go.mod h1:Y2chtz2x9bAeRTvSibVRVgbLJhLJXKlUeIvjeVdnm4w= github.com/ulikunitz/xz v0.5.8 h1:ERv8V6GKqVi23rgu5cj9pVfVzJbOqAY2Ntl88O6c2nQ= @@ -524,8 +526,8 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210603125802-9665404d3644 h1:CA1DEQ4NdKphKeL70tvsWNdT5oFh1lOjihRcEDROi0I= -golang.org/x/sys v0.0.0-20210603125802-9665404d3644/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e h1:w36l2Uw3dRan1K3TyXriXvY+6T56GNmlKGcqiQUJDfM= +golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -725,8 +727,8 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/variables.tf b/variables.tf index 9ebd409..28ef2e3 100644 --- a/variables.tf +++ b/variables.tf @@ -1,12 +1,12 @@ variable "target_security_group_id" { type = list(string) - default = [] description = <<-EOT The ID of an existing Security Group to which Security Group rules will be assigned. The Security Group's description will not be changed. Not compatible with `inline_rules_enabled` or `revoke_rules_on_delete`. Required if `create_security_group` is `false`, ignored otherwise. EOT + default = [] validation { condition = length(var.target_security_group_id) < 2 error_message = "Only 1 security group can be targeted." @@ -15,12 +15,12 @@ variable "target_security_group_id" { variable "security_group_name" { type = list(string) - default = [] description = <<-EOT The name to assign to the security group. Must be unique within the VPC. If not provided, will be derived from the `null-label.context` passed in. If `create_before_destroy` is true, will be used as a name prefix. EOT + default = [] validation { condition = length(var.security_group_name) < 2 error_message = "Only 1 security group name can be provided." @@ -30,36 +30,51 @@ variable "security_group_name" { variable "security_group_description" { type = string - default = "Managed by Terraform" description = <<-EOT The description to assign to the created Security Group. Warning: Changing the description causes the security group to be replaced. EOT + default = "Managed by Terraform" } variable "create_before_destroy" { type = bool - default = false description = <<-EOT Set `true` to enable terraform `create_before_destroy` behavior on the created security group. - We recommend setting this `true` on new security groups, but default it to `false` because `true` - will cause existing security groups to be replaced. + We only recommend setting this `false` if you are importing an existing security group + that you do not want replaced and therefore need full control over its name. Note that changing this value will always cause the security group to be replaced. EOT + default = true } -variable "allow_all_egress" { +variable "preserve_security_group_id" { type = bool + description = <<-EOT + When `false` and `security_group_create_before_destroy` is `true`, changes to security group rules + cause a new security group to be created with the new rules, and the existing security group is then + replaced with the new one, eliminating any service interruption. + When `true` or when changing the value (from `false` to `true` or from `true` to `false`), + existing security group rules will be deleted before new ones are created, resulting in a service interruption, + but preserving the security group itself. + **NOTE:** Setting this to `true` does not guarantee the security group will never be replaced, + it only keeps changes to the security group rules from triggering a replacement. + See the README for further discussion. + EOT default = false +} + +variable "allow_all_egress" { + type = bool description = <<-EOT A convenience that adds to the rules specified elsewhere a rule that allows all egress. If this is false and no egress rules are specified via `rules` or `rule-matrix`, then no egress will be allowed. EOT + default = true } variable "rules" { type = list(any) - default = [] description = <<-EOT A list of Security Group rule objects. All elements of a list must be exactly the same type; use `rules_map` if you want to supply multiple lists of different types. @@ -70,11 +85,11 @@ variable "rules" { ___Note:___ The length of the list must be known at plan time. This means you cannot use functions like `compact` or `sort` when computing the list. EOT + default = [] } variable "rules_map" { type = any - default = {} description = <<-EOT A map-like object of lists of Security Group rule objects. All elements of a list must be exactly the same type, so this input accepts an object with keys (attributes) whose values are lists so you can separate different @@ -84,6 +99,7 @@ variable "rules_map" { and known at "plan" time. To get more info see the `security_group_rule` [documentation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule). EOT + default = {} } variable "rule_matrix" { @@ -112,34 +128,34 @@ variable "rule_matrix" { # }] type = any - default = [] description = <<-EOT A convenient way to apply the same set of rules to a set of subjects. See README for details. EOT + default = [] } variable "security_group_create_timeout" { type = string - default = "10m" description = "How long to wait for the security group to be created." + default = "10m" } variable "security_group_delete_timeout" { type = string - default = "15m" description = <<-EOT How long to retry on `DependencyViolation` errors during security group deletion from lingering ENIs left by certain AWS services such as Elastic Load Balancing. EOT + default = "15m" } variable "revoke_rules_on_delete" { type = bool - default = false description = <<-EOT Instruct Terraform to revoke all of the Security Group's attached ingress and egress rules before deleting the security group itself. This is normally not needed. EOT + default = false } variable "vpc_id" { @@ -149,11 +165,10 @@ variable "vpc_id" { variable "inline_rules_enabled" { type = bool - default = false description = <<-EOT NOT RECOMMENDED. Create rules "inline" instead of as separate `aws_security_group_rule` resources. See [#20046](https://github.com/hashicorp/terraform-provider-aws/issues/20046) for one of several issues with inline rules. See [this post](https://github.com/hashicorp/terraform-provider-aws/pull/9032#issuecomment-639545250) for details on the difference between inline rules and rule resources. EOT + default = false } - diff --git a/versions.tf b/versions.tf index fc6bdc5..94c2e78 100644 --- a/versions.tf +++ b/versions.tf @@ -1,10 +1,18 @@ terraform { - required_version = ">= 0.14.0" + required_version = ">= 1.0.0" required_providers { aws = { source = "hashicorp/aws" version = ">= 3.0" } + null = { + source = "hashicorp/null" + version = ">= 3.0" + } + random = { + source = "hashicorp/random" + version = ">= 3.0" + } } }