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

Update of cidr_blocks in ingress_with_cidr_blocks forces new resource #67

Closed
amontalban opened this issue Jul 4, 2018 · 15 comments
Closed

Comments

@amontalban
Copy link

Hi,

I have the following state (https://gist.github.com/amontalban/0af3064d958f2d1e648848f342b34b2f) and whenever I add/remove an IP to sysadmins_networks variable it produces:

-/+ module.common_sysadmins_sg.aws_security_group_rule.ingress_with_cidr_blocks[0] (new resource required)
      id:                       "sgrule-2061399776" => <computed> (forces new resource)
      cidr_blocks.#:            "4" => "5" (forces new resource)
      cidr_blocks.0:            "5.6.7.8/32" => "5.6.7.8/32"
      cidr_blocks.1:            "9.9.9.9/32" => "9.9.9.9/32"
      cidr_blocks.2:            "1.1.1.1/32" => "1.1.1.1/32"
      cidr_blocks.3:            "2.2.2.2/32" => "2.2.2.2/32"
      cidr_blocks.4:            "" => "1.2.3.4/32" (forces new resource)
      description:              "SSH Access" => "SSH Access"
      from_port:                "22" => "22"
      protocol:                 "tcp" => "tcp"
      security_group_id:        "sg-63991f1d" => "sg-63991f1d"
      self:                     "false" => "false"
      source_security_group_id: "" => <computed>
      to_port:                  "22" => "22"
      type:                     "ingress" => "ingress"

Thing is that I have to frequently add/remove IPs from security groups, I have been doing this by using plain Terraform security groups just fine but can't find the way to do it with this module.

Any input is highly appreciated!

Thanks!

@antonbabenko
Copy link
Member

Hi Andres!

The code you have is good and every time you change IPs it just recreates new aws_security_group_rule and not aws_security_group, so security group ID is the same.

Please tell me if I am missing the point of what you are trying to do.

@aolley
Copy link

aolley commented Apr 9, 2019

I suspect the issue, which I have as well, is not about it regenerating a security group. The issue is that in order to alter the cidr list, this actually removes the entire existing rule set, and then creates a new one.

So in practice, this means you have a brief moment where your security group no longer has any of the rules from the list.

So say we have cidr_blocks a, b and c.
We want to add block d.
It does this by removing all the rules - so a, b and c are gone. It then posts a new set of a, b, c and d.

@nagas
Copy link

nagas commented May 29, 2019

does setting proper lifecycle solve the problem?

lifecycle {
    create_before_destroy = true
}

@mikealbert
Copy link

I'm seeing this same issue as well. I tried applying the lifecycle, but that didn't work. I get the error "lifecycle" is not a valid argument.

Like @aolley mentioned, I think the problem is the rules get dropped before the new ones are added. Here's an example where I switched the ingress cidr block from 0.0.0.0/0 to 10.16.0.0/16.

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

-/+ module.foo_sg.aws_security_group_rule.ingress_rules[0] (new resource required)
      id:                       "sgrule-59628504" => <computed> (forces new resource)
      cidr_blocks.#:            "1" => "1"
      cidr_blocks.0:            "0.0.0.0/0" => "10.16.0.0/16" (forces new resource)
      description:              "SSH" => "SSH"
      from_port:                "22" => "22"
      protocol:                 "tcp" => "tcp"
      security_group_id:        "sg-023303e02b74449ae" => "sg-023303e02b74449ae"
      self:                     "false" => "false"
      source_security_group_id: "" => <computed>
      to_port:                  "22" => "22"
      type:                     "ingress" => "ingress"

-/+ module.foo_sg.aws_security_group_rule.ingress_rules[1] (new resource required)
      id:                       "sgrule-1430954925" => <computed> (forces new resource)
      cidr_blocks.#:            "1" => "1"
      cidr_blocks.0:            "0.0.0.0/0" => "10.16.0.0/16" (forces new resource)
      description:              "Oracle" => "Oracle"
      from_port:                "1521" => "1521"
      protocol:                 "tcp" => "tcp"
      security_group_id:        "sg-023303e02b74449ae" => "sg-023303e02b74449ae"
      self:                     "false" => "false"
      source_security_group_id: "" => <computed>
      to_port:                  "1521" => "1521"
      type:                     "ingress" => "ingress"

-/+ module.foo_sg.aws_security_group_rule.ingress_rules[2] (new resource required)
      id:                       "sgrule-1060948688" => <computed> (forces new resource)
      cidr_blocks.#:            "1" => "1"
      cidr_blocks.0:            "0.0.0.0/0" => "10.16.0.0/16" (forces new resource)
      description:              "All IPV4 ICMP" => "All IPV4 ICMP"
      from_port:                "-1" => "-1"
      protocol:                 "icmp" => "icmp"
      security_group_id:        "sg-023303e02b74449ae" => "sg-023303e02b74449ae"
      self:                     "false" => "false"
      source_security_group_id: "" => <computed>
      to_port:                  "-1" => "-1"
      type:                     "ingress" => "ingress"

-/+ module.foo_sg.aws_security_group_rule.ingress_rules[3] (new resource required)
      id:                       "sgrule-3160914071" => <computed> (forces new resource)
      cidr_blocks.#:            "1" => "1"
      cidr_blocks.0:            "0.0.0.0/0" => "10.16.0.0/16" (forces new resource)
      description:              "All TCP ports" => "All TCP ports"
      from_port:                "0" => "0"
      protocol:                 "tcp" => "tcp"
      security_group_id:        "sg-023303e02b74449ae" => "sg-023303e02b74449ae"
      self:                     "false" => "false"
      source_security_group_id: "" => <computed>
      to_port:                  "65535" => "65535"
      type:                     "ingress" => "ingress"


Plan: 4 to add, 0 to change, 4 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

module.foo_sg.aws_security_group_rule.ingress_rules[0]: Destroying... (ID: sgrule-59628504)
module.foo_sg.aws_security_group_rule.ingress_rules[1]: Destroying... (ID: sgrule-1430954925)
module.foo_sg.aws_security_group_rule.ingress_rules[2]: Destroying... (ID: sgrule-1060948688)
module.foo_sg.aws_security_group_rule.ingress_rules[3]: Destroying... (ID: sgrule-3160914071)
module.foo_sg.aws_security_group_rule.ingress_rules[3]: Destruction complete after 1s
module.foo_sg.aws_security_group_rule.ingress_rules[3]: Creating...
  cidr_blocks.#:            "" => "1"
  cidr_blocks.0:            "" => "10.16.0.0/16"
  description:              "" => "All TCP ports"
  from_port:                "" => "0"
  protocol:                 "" => "tcp"
  security_group_id:        "" => "sg-023303e02b74449ae"
  self:                     "" => "false"
  source_security_group_id: "" => "<computed>"
  to_port:                  "" => "65535"
  type:                     "" => "ingress"
module.foo_sg.aws_security_group_rule.ingress_rules[1]: Destruction complete after 1s
module.foo_sg.aws_security_group_rule.ingress_rules[1]: Creating...
  cidr_blocks.#:            "" => "1"
  cidr_blocks.0:            "" => "10.16.0.0/16"
  description:              "" => "Oracle"
  from_port:                "" => "1521"
  protocol:                 "" => "tcp"
  security_group_id:        "" => "sg-023303e02b74449ae"
  self:                     "" => "false"
  source_security_group_id: "" => "<computed>"
  to_port:                  "" => "1521"
  type:                     "" => "ingress"
module.foo_sg.aws_security_group_rule.ingress_rules[2]: Destruction complete after 2s
module.foo_sg.aws_security_group_rule.ingress_rules[2]: Creating...
  cidr_blocks.#:            "" => "1"
  cidr_blocks.0:            "" => "10.16.0.0/16"
  description:              "" => "All IPV4 ICMP"
  from_port:                "" => "-1"
  protocol:                 "" => "icmp"
  security_group_id:        "" => "sg-023303e02b74449ae"
  self:                     "" => "false"
  source_security_group_id: "" => "<computed>"
  to_port:                  "" => "-1"
  type:                     "" => "ingress"
module.foo_sg.aws_security_group_rule.ingress_rules[0]: Destruction complete after 2s
module.foo_sg.aws_security_group_rule.ingress_rules[0]: Creating...
  cidr_blocks.#:            "" => "1"
  cidr_blocks.0:            "" => "10.16.0.0/16"
  description:              "" => "SSH"
  from_port:                "" => "22"
  protocol:                 "" => "tcp"
  security_group_id:        "" => "sg-023303e02b74449ae"
  self:                     "" => "false"
  source_security_group_id: "" => "<computed>"
  to_port:                  "" => "22"
  type:                     "" => "ingress"
module.foo_sg.aws_security_group_rule.ingress_rules[3]: Creation complete after 2s (ID: sgrule-292050268)
module.foo_sg.aws_security_group_rule.ingress_rules[1]: Creation complete after 2s (ID: sgrule-3565003292)
module.foo_sg.aws_security_group_rule.ingress_rules[2]: Creation complete after 2s (ID: sgrule-2232245650)
module.foo_sg.aws_security_group_rule.ingress_rules[0]: Creation complete after 2s (ID: sgrule-1688316956)

Apply complete! Resources: 4 added, 0 changed, 4 destroyed.

@antonbabenko
Copy link
Member

@mikealbert Could you paste the complete code which fails and tell which argument you are changing. I will try to run it myself and see if there is anything we can do to improve this module.

@mikealbert
Copy link

Sure. Here's what I've been testing with.

module "foo_sg" {
  source  = "terraform-aws-modules/security-group/aws"
  version = "2.16.0"

  name        = "foo-lab"
  description = "Security group for foo instance"
  vpc_id      = "${data.terraform_remote_state.vpc.vpc_id}"

  ingress_cidr_blocks = ["0.0.0.0/0"]
  ingress_rules       = ["ssh-tcp", "oracle-db-tcp", "all-icmp", "all-tcp"]
  egress_rules        = ["all-all"]
}

Updating ingress_cidr_blocks triggers a destruction of the existing rules before the new ones are created. I submitted a pr that I think should fix the issue.

@antonbabenko
Copy link
Member

I tested it as well for Terraform 0.11 and 0.12 (using latest release), and it works as expected. It deletes rules one by one and recreates them. I also tried to change the number of blocks in ingress_cidr_blocks.

@mikealbert
Copy link

Thanks for testing.

I'm new to terraform, so I might be misinterpreting the output but I think the question is can the new rules be created before the old ones are dropped. So if a single ingress_cidr_block entry is modified, can the new rules be added first.

Perhaps it'd be better to just add the new cidr block to the list, apply, remove the old cidr block and then apply.

Thanks again for the help.

@antonbabenko
Copy link
Member

You are welcome!

Terraform should do this for you natively, so you don't have to apply this in multiple steps.

In the output individual rules are being created after deletion which is the right way.

Closing this issue.

@aolley
Copy link

aolley commented Jun 7, 2019

We want the new rules created before the old ones are deleted in order to avoid a brief outage.

@antonbabenko
Copy link
Member

@aolley #125 (comment)

@aolley
Copy link

aolley commented Jun 7, 2019

Right, I read that, the solution proposed won't work sure. But that doesn't mean the problem doesn't exist anymore.

If you're suggesting there's no way to address the issue within this module due to whatever limitations imposed on it from how terraform does things - then that's fine.

But it's definitely possible to edit an existing security group in-place via the AWS console without having to delete all the rules in it and re-add them (last I checked at least).

@antonbabenko
Copy link
Member

There is a limitation in Terraform and, as a result, this module identifies individual rules by index. For example, when the new rule is added at the beginning then all rules after that are recreated because their index has increased.

I don't think we can do anything with it without increasing complexity very much.

@aolley
Copy link

aolley commented Jun 10, 2019

Fair enough, I was afraid that was the case. At least there's a workaround for when this does come up :)

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants