Skip to content

Commit

Permalink
Respond to reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Nuru committed Jul 7, 2022
1 parent 2c3b3c6 commit fa858fb
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ With "create before destroy" and any resources dependent on the security group a
same Terraform plan, replacement happens successfully:

1. New security group is created
2. Resource is associated with new security group and disassociated with old one
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
Expand Down Expand Up @@ -244,7 +244,7 @@ Every security group rule input to this module accepts optional identifying keys
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
`[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
Expand Down Expand Up @@ -453,7 +453,7 @@ 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 index in the input lists. If a rule is deleted and the other rules therefore move
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.
Expand Down
6 changes: 3 additions & 3 deletions README.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ usage: |-
same Terraform plan, replacement happens successfully:
1. New security group is created
2. Resource is associated with new security group and disassociated with old one
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
Expand Down Expand Up @@ -211,7 +211,7 @@ usage: |-
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
`[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
Expand Down Expand Up @@ -420,7 +420,7 @@ usage: |-
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 index in the input lists. If a rule is deleted and the other rules therefore move
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.
Expand Down
4 changes: 2 additions & 2 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ locals {
# 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)
# 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]

Expand Down

0 comments on commit fa858fb

Please sign in to comment.