Fix a nil access error when security group can't be found.

[thegedge]

Not sure of the specifics on how I hit this codepath, but this fixed a panic I was seeing where sg was nil.

[catsby]

Hey @thegedge thanks for submitting this – I'm curious, do you have a configuration that reproduces this, so I can verify? A crash log would be helpful too, if you have it. The code change looks fine, I'm just trying to understand what happened.

Thanks!

[catsby]

It seems like something that needs to be fixed in this method https://github.com/hashicorp/terraform/blob/b-aws-route53record-errors/builtin/providers/aws/resource_aws_security_group_rule.go#L252

[thegedge]

Unfortunately I lost the crash.log file, and can't remember exactly what led to this error. I feel like it was a rare case where we may have had a security group that existed in our state file but didn't exist in AWS, or the other way around.

You're right that this may need to be fixed in findResourceSecurityGroup because I can spot a couple of other places where a similar panic could occur.

I'll try to put together a scenario (state + config + world) that triggers this issue.

[catsby]

Thanks @thegedge !

[thegedge]

Okay, really easy to trigger. Whip up a simple config:

resource "aws_security_group" "test" {
  name = "test"
  description = "Test for hashicorp/terraform#2797"
  vpc_id = "vpc-1234abcd"   // change to an actual VPC id
}

resource "aws_security_group_rule" "test" {
  type = "ingress"
  from_port = 0
  to_port = 0
  protocol = "tcp"
  security_group_id = "${aws_security_group.test.id}"
  self = true
}

Then follow these steps:

1. terraform apply
2. Go to AWS console and delete the security group
3. terraform plan
4. Panic!!!

[EDIT]
I should note that this may not be the exact circumstances under which I originally saw this error.

[thegedge]

@catsby let me know if you'd like an alternative fix, or if you're happy with this approach. If the latter, I can update create/delete so that they don't panic for similar reasons.

Another thought I had was that, in my example, should aws_security_group_rule.test wait for the read operation to finish on aws_security_group.test? If it did, we would notice that aws_security_group.test no longer exists and has to be created, and could somehow propagate that information along. Maybe automatically tainting the non-existent resource? Some food for thought :)

[catsby]

Hey @thegedge thanks for the contribution here. I'm going to close this in favor of #2897 , which fixes the problem up the code path a bit and returns a nil where we originally just emptied the ID and kept going...

Thanks for digging in!

[ghost]

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.