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

providers/aws: aws_security_group v0.6.10 regression #4872

Closed
pmoust opened this issue Jan 28, 2016 · 27 comments
Closed

providers/aws: aws_security_group v0.6.10 regression #4872

pmoust opened this issue Jan 28, 2016 · 27 comments
Labels
bug provider/aws waiting-response An issue/pull request is waiting for a response from the community

Comments

@pmoust
Copy link
Contributor

pmoust commented Jan 28, 2016

Running terraform plan -out /tmp/plan -target=aws_security_group.pph_webserver with terraform v0.6.10

No change in the terraform resource. This happens to other SGs as well (not all of them though), added one resource to refrain from TL;DR.

Expected result: no change plan
Actual results pasted below

In 0.6.9 it's ok.

resource "aws_security_group" "pph_webserver" {
     name        = "pph_webserver"
     vpc_id      = "${aws_vpc.pph.id}"
     ingress {
           from_port   = 80
           to_port     = 80
           protocol    = "tcp"
           self        = true
      }
 }
~/pph/pph-iac/terraform/peopleperhour   master ●  terraform plan -out /tmp/plan -target=aws_security_group.pph_webserver
Refreshing Terraform state prior to plan...

aws_vpc.pph: Refreshing state... (ID: vpc-871689e2)
aws_security_group.pph_webserver: Refreshing state... (ID: sg-d0b443b4)

The Terraform execution plan has been generated and is shown below.
Resources are shown in alphabetical order for quick scanning. Green resources
will be created (or destroyed and then created if an existing resource
exists), yellow resources are being changed in-place, and red resources
will be destroyed.

Your plan was also saved to the path below. Call the "apply" subcommand
with this plan file and Terraform will exactly execute this execution
plan.

Path: /tmp/plan

~ aws_security_group.pph_webserver
    ingress.1844393598.cidr_blocks.#:              "0" => "0"
    ingress.1844393598.from_port:                  "" => "80"
    ingress.1844393598.protocol:                   "" => "tcp"
    ingress.1844393598.security_groups.#:          "0" => "0"
    ingress.1844393598.self:                       "" => "1"
    ingress.1844393598.to_port:                    "" => "80"
    ingress.2938818379.cidr_blocks.#:              "0" => "0"
    ingress.2938818379.from_port:                  "80" => "0"
    ingress.2938818379.protocol:                   "tcp" => ""
    ingress.2938818379.security_groups.#:          "1" => "0"
    ingress.2938818379.security_groups.1214646136: "sg-d8b443bc" => ""
    ingress.2938818379.self:                       "1" => "0"
    ingress.2938818379.to_port:                    "80" => "0"


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

State diff

diff --git a/terraform/peopleperhour/terraform.tfstate b/terraform/peopleperhour/terraform.tfstate
index c00ea98..c86f417 100644
--- a/terraform/peopleperhour/terraform.tfstate
+++ b/terraform/peopleperhour/terraform.tfstate
@@ -1,6 +1,6 @@
 {
     "version": 1,
-    "serial": 1111,
+    "serial": 1112,
     "modules": [
         {
             "path": [
@@ -7435,12 +7435,13 @@
                             "egress.482069346.to_port": "0",
                             "id": "sg-d0b443b4",
                             "ingress.#": "1",
-                            "ingress.1844393598.cidr_blocks.#": "0",
-                            "ingress.1844393598.from_port": "80",
-                            "ingress.1844393598.protocol": "tcp",
-                            "ingress.1844393598.security_groups.#": "0",
-                            "ingress.1844393598.self": "true",
-                            "ingress.1844393598.to_port": "80",
+                            "ingress.2938818379.cidr_blocks.#": "0",
+                            "ingress.2938818379.from_port": "80",
+                            "ingress.2938818379.protocol": "tcp",
+                            "ingress.2938818379.security_groups.#": "1",
+                            "ingress.2938818379.security_groups.1214646136": "sg-d8b443bc",
+                            "ingress.2938818379.self": "true",
+                            "ingress.2938818379.to_port": "80",
                             "name": "pph_webserver",
@phinze
Copy link
Contributor

phinze commented Jan 28, 2016

Hi @pmoust, thanks for the report.

This looks like it might be the SG rule drift detection fix from #4779 in action. We discovered a long-standing bug in nested security group rules that prevented rule drift from being written back to the state during an aws_security_group Refresh operation.

I'll add to the release notes to call out this possibility for other users.

If you compare the diffs you're seeing to the state of the rules on the SGs is AWS, does this seem like the proper explanation?

@pmoust
Copy link
Contributor Author

pmoust commented Jan 29, 2016

Will get back to you on this @phinze , that must be it.

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Jan 29, 2016
@akramhussein
Copy link
Contributor

I'm also experiencing this issue. Is there any info I can provide to help?

@danielnbarbosa
Copy link

I'm also seeing this problem on a handful of security groups. FWIW it only happens with security groups that have more then one ingress rule. With the exception of one security group that has two ingress rules where they are both CIDRs.

Here's an example:

resource "aws_security_group" "elasticache" {
  name        = "${var.environment_name}_elasticache"
  description = "Allow traffic to elasticache redis database"
  vpc_id      = "${aws_vpc.mod.id}"
  ingress {
    from_port       = "6379"
    to_port         = "6379"
    protocol        = "tcp"
    security_groups = ["${aws_security_group.papi.id}"]
  }
  ingress {
    from_port   = "6379"
    to_port     = "6379"
    protocol    = "tcp"
    cidr_blocks = ["${var.default_vpc_cidr}"]
  }
  egress {
    from_port   = 0
    to_port     = 0
    protocol    = "-1"
    cidr_blocks = ["0.0.0.0/0"]
  }
  tags {
    Name = "${var.environment_name}_elasticache"
  }
}

And the output of the plan:

~ module.demo.aws_security_group.elasticache
    ingress.#:                                    "1" => "2"
    ingress.445243727.cidr_blocks.#:              "0" => "1"
    ingress.445243727.cidr_blocks.0:              "" => "172.31.0.0/16"
    ingress.445243727.from_port:                  "" => "6379"
    ingress.445243727.protocol:                   "" => "tcp"
    ingress.445243727.security_groups.#:          "0" => "0"
    ingress.445243727.self:                       "" => "0"
    ingress.445243727.to_port:                    "" => "6379"
    ingress.842332913.cidr_blocks.#:              "1" => "0"
    ingress.842332913.cidr_blocks.0:              "172.31.0.0/16" => ""
    ingress.842332913.from_port:                  "6379" => "0"
    ingress.842332913.protocol:                   "tcp" => ""
    ingress.842332913.security_groups.#:          "1" => "0"
    ingress.842332913.security_groups.1507947190: "sg-ee8af98a" => ""
    ingress.842332913.self:                       "0" => "0"
    ingress.842332913.to_port:                    "6379" => "0"
    ingress.996992325.cidr_blocks.#:              "0" => "0"
    ingress.996992325.from_port:                  "" => "6379"
    ingress.996992325.protocol:                   "" => "tcp"
    ingress.996992325.security_groups.#:          "0" => "1"
    ingress.996992325.security_groups.1507947190: "" => "sg-ee8af98a"
    ingress.996992325.self:                       "" => "0"
    ingress.996992325.to_port:                    "" => "6379"

Doing a terraform apply doesn't help it converge. It comes back again on the next terraform plan.

@iserko
Copy link

iserko commented Feb 3, 2016

Having the same problem here.
Happens on security groups with multiple ingress/egress rules that specify the same port/protocol.

Example terraform definition: https://gist.github.com/iserko/d69fd1a9a8092825d5bc
terraform.tfstate changes https://gist.github.com/iserko/76581086e29f2810b704/revisions

As soon as I merge security_groups AND cidr_blocks into the same ingress block it starts working fine again. However this goes against the documentation https://www.terraform.io/docs/providers/aws/r/security_group.html ... should the documentation be changed now or will this be fixed?

@catsby
Copy link
Contributor

catsby commented Feb 3, 2016

Hello – I just opened #4983 for consideration. I think the documentation is out of date and is remedied there, but also it should fix a bug with EC2 Classic environments

@danielnbarbosa
Copy link

i can confirm that as iserko said this problem happens when you have multiple ingress/egress blocks that specify the same port/protocol. i modified the port range to: 6379-6380 on one of the blocks and it worked. not ideal but better then before.

@r0ps3c
Copy link

r0ps3c commented Feb 4, 2016

I've also just hit this problem, in my case the security groups' rules differed in that one used a cidr block and the other used "self=true" (i.e. same protocol/ports otherwise). Using both self=true and a cidr block constraint within the same "ingress" or "egress" block, which did not previously work, now also does, as a workaround/fix.

@catsby
Copy link
Contributor

catsby commented Feb 4, 2016

Hey @r0p0s3c –

which did not previously work

did this not work in v0.6.9? Or farther back from then?
In #4983 I remove the docs that say you cannot use cidr_blocks and security_groups in the same rule, I haven't been able to reproduce anything that prohibits us from doing so.

If anyone else is still having issue that are not resolved by #4983 , or are not sure how to verify that, please let me know!

@r0ps3c
Copy link

r0ps3c commented Feb 4, 2016

Hi @catsby,

correct, this didn't work with the last release I used, which was 0.6.9. I don't believe that self=true was documented as being incompatible with security_groups, unlike cidr_block, but that's why I found.

@catsby
Copy link
Contributor

catsby commented Feb 4, 2016

the security_group_rule resources does have a conflict with self and cidr_blocks, but I think we're talking bout security_group with in-line rules. Am I confused? If you have a config that demonstrates anything that may help 😄

@r0ps3c
Copy link

r0ps3c commented Feb 4, 2016

Correct, the issue I was having was with aws_security_group, so inline rules. Ingress/egress rules such as this:

resource "aws_security_group" "g1" {
    ingress {
        from_port = 22
        to_port = 22
        protocol = "tcp"
        self = true
        security_groups = ["${aws_security_group.g2.id}"]
    }
}

previously did not work for me.

@catsby
Copy link
Contributor

catsby commented Feb 8, 2016

Hello friends – 

We've identified more information regarding the regression revealed/caused by #4779 and I'm currently working on a fix. Unfortunately that fix involves replacing much of how this resource handles it's in-line rules, so it's taking a bit of time. I do anticipate having a patch to address it early this week though.

In the meantime, the work-around is to converge your in-line rules by to_port, from_port and type, so all rules (including cidr blocks and security groups) fit under those blocks. This is counter to the documentation, however the patch I'm working on should again enforce that. I sincerely apologize for the run-around here.

Let me know if you have other questions, otherwise I'll try to keep this issue updated

Also related:

@vancluever
Copy link
Contributor

Guys,

Another big thing that I have had in particular problems with that causes drift is using a unaligned CIDR.

I don't have output, but it should be easy to reproduce. Some behaviour I saw from it:

  • Constant update of network ACLs as it tried to correct the properly aligned CIDR to the improperly aligned one.
  • For security group rules - zero state in one of my modules (probably more classically exhibitive of the drift issue in general).

Some CIDR checks are probably in order to correct some of this at least.

@pmoust
Copy link
Contributor Author

pmoust commented Feb 14, 2016

Thanks for the heads up @catsby

@mechastorm
Copy link
Contributor

This seemed to have been fixed in 0.6.12 for me ... can anyone else confirm?

@stack72
Copy link
Contributor

stack72 commented Feb 28, 2016

@pmoust can you verify if terraform 0.6.12 fixes your issue?

@ephemeralsnow
Copy link
Contributor

It does not solve in 0.6.12.

Currently, security group ID of self is always written to terraform.tfstate.

"ingress.3889940180.security_groups.#": "1",
"ingress.3889940180.security_groups.190436319": "sg-ea9752ea",   # security group ID of self
"ingress.3889940180.self": "true",

This seems to cause a change occurs.

If self = true, this problem is solved by so that it does not attempt to write a security group ID of self.

"ingress.3889940180.self": "true",

I issued a pull request #5184, including the resolution of this problem.

This is the location of the problem.
https://github.com/hashicorp/terraform/pull/5184/files#diff-98ff2e41a9f03e30bb15680732bf8a24R459

@pmoust
Copy link
Contributor Author

pmoust commented Mar 3, 2016

@stack72 it doesn't
@ephemeralsnow will give it a go as soon as I hit the office, cheers

@ephemeralsnow
Copy link
Contributor

I'm sorry, in 0.6.12 this problem seemed to have already been fixed.
This fixes were not able to catch up.
280054a

My pull request that I put out is also likely to be room for reconsideration.

@danielnbarbosa
Copy link

FWIW 0.6.12 fixed my issue

@pmoust
Copy link
Contributor Author

pmoust commented Mar 4, 2016

My original repro still stands

@pmoust
Copy link
Contributor Author

pmoust commented Mar 4, 2016

My bad, it is indeed fixed!

@pmoust pmoust closed this as completed Mar 4, 2016
@stack72
Copy link
Contributor

stack72 commented Mar 5, 2016

Nice one @pmoust

@catsby
Copy link
Contributor

catsby commented Mar 7, 2016

Thank you all for confirming a fix here. If you're still having trouble, please open a new issue for me to track separately .

Thanks

@joelgriffiths
Copy link

joelgriffiths commented Jun 23, 2016

I'm seeing this with 0.6.16.

`resource "aws_security_group" "redacted" {
name = "RedactedSG"
description = "Ingess for Redacted servers"

ingress {
    from_port = 0
    to_port = 65535
    protocol = "tcp"
    cidr_blocks = ["${split(",", var.web_access_cidr_list)}"]
}
egress {
    from_port = -1
    to_port = -1
    protocol = "icmp"
    cidr_blocks = ["0.0.0.0/0"]
}
egress {
    from_port = 3306
    to_port = 3306
    protocol = "tcp"
    cidr_blocks = ["${var.rds-subnet-az1}", "${var.rds-subnet-az2}"]
}
egress {
    from_port = 0
    to_port = 65535
    protocol = "tcp"
    cidr_blocks = ["0.0.0.0/0"]
}
egress {
    from_port = 0
    to_port = 65535
    protocol = "udp"
    cidr_blocks = ["0.0.0.0/0"]
}

vpc_id = "${aws_vpc.main.id}"

tags {
    Name = "RedactedSG"
}

}
`

~ aws_security_group.redacted
ingress.2277961572.cidr_blocks.#: "0" => "10"
ingress.2277961572.cidr_blocks.0: "" => "4.18.12.0/24"
ingress.2277961572.cidr_blocks.1: "" => "7.9.8.5/32"
ingress.2277961572.cidr_blocks.2: "" => "20.18.4.17/32"
ingress.2277961572.cidr_blocks.3: "" => "2.7.23.4/32"
ingress.2277961572.cidr_blocks.4: "" => "9.14.13.15/32"
ingress.2277961572.cidr_blocks.5: "" => "21.8.16.19/32"
ingress.2277961572.cidr_blocks.6: "" => "2.17.13.11/32"
ingress.2277961572.cidr_blocks.7: "" => "7.9.7.23/32"
ingress.2277961572.cidr_blocks.8: "" => "2.20.5.13/32"
ingress.2277961572.cidr_blocks.9: "" => "10.17.2.3/32"
ingress.2277961572.from_port: "" => "0"
ingress.2277961572.protocol: "" => "tcp"
ingress.2277961572.security_groups.#: "0" => "0"
ingress.2277961572.self: "" => "false"
ingress.2277961572.to_port: "" => "65535"
ingress.3806979990.cidr_blocks.#: "9" => "0"
ingress.3806979990.cidr_blocks.0: "4.18.12.0/24" => ""
ingress.3806979990.cidr_blocks.1: "7.9.8.5/32" => ""
ingress.3806979990.cidr_blocks.2: "20.18.4.17/32" => ""
ingress.3806979990.cidr_blocks.3: "2.7.23.4/32" => ""
ingress.3806979990.cidr_blocks.4: "9.14.13.15/32" => ""
ingress.3806979990.cidr_blocks.5: "21.8.16.19/32" => ""
ingress.3806979990.cidr_blocks.6: "2.17.13.11/32" => ""
ingress.3806979990.cidr_blocks.7: "7.9.7.23/32" => ""
ingress.3806979990.cidr_blocks.8: "2.20.5.13/32" => ""
ingress.3806979990.from_port: "0" => "0"
ingress.3806979990.protocol: "tcp" => ""
ingress.3806979990.security_groups.#: "0" => "0"
ingress.3806979990.self: "false" => "false"
ingress.3806979990.to_port: "65535" => "0"

I changed the IP's to protect the innocent. If the IPs don't match, it's a typo that occurred when I manually obfuscated the IPs.

@ghost
Copy link

ghost commented Apr 24, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

No branches or pull requests