Giter VIP home page Giter VIP logo

Comments (15)

antonbabenko avatar antonbabenko commented on July 30, 2024 1

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.

from terraform-aws-security-group.

antonbabenko avatar antonbabenko commented on July 30, 2024

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.

from terraform-aws-security-group.

aolley avatar aolley commented on July 30, 2024

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.

from terraform-aws-security-group.

nagas avatar nagas commented on July 30, 2024

does setting proper lifecycle solve the problem?

lifecycle {
    create_before_destroy = true
}

from terraform-aws-security-group.

mikealbert avatar mikealbert commented on July 30, 2024

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.

from terraform-aws-security-group.

antonbabenko avatar antonbabenko commented on July 30, 2024

@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.

from terraform-aws-security-group.

mikealbert avatar mikealbert commented on July 30, 2024

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.

from terraform-aws-security-group.

antonbabenko avatar antonbabenko commented on July 30, 2024

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.

from terraform-aws-security-group.

mikealbert avatar mikealbert commented on July 30, 2024

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.

from terraform-aws-security-group.

antonbabenko avatar antonbabenko commented on July 30, 2024

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.

from terraform-aws-security-group.

aolley avatar aolley commented on July 30, 2024

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

from terraform-aws-security-group.

antonbabenko avatar antonbabenko commented on July 30, 2024

@aolley #125 (comment)

from terraform-aws-security-group.

aolley avatar aolley commented on July 30, 2024

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).

from terraform-aws-security-group.

aolley avatar aolley commented on July 30, 2024

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

from terraform-aws-security-group.

github-actions avatar github-actions commented on July 30, 2024

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.

from terraform-aws-security-group.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.