r/security_group: improve PR #1587 to fix issue #2069

[svanharmelen]

The logic in PR #1587 has some flaws with regards to how the security group API works in relation with TF.

The problem will not show itself when you manage everything properly from TF, but when someone makes a manual change directly in AWS you could be hit by the problem described in issue #2069.

This PR aims to fix that problem. As for the code itself, it does look like it’s not really DRY, but if you look closely the code is slightly different every time. So making an abstraction seems questionable in this case…

FYI: I've updated one unit test to represent and test the updated logic and run all related acceptance tests to make sure the PR works as expected.

[bflad]

Can you please fix the make test failure?

--- FAIL: TestRulesMixedMatching (0.00s)
panic: interface conversion: interface {} is nil, not string [recovered]
	panic: interface conversion: interface {} is nil, not string
goroutine 1824 [running]:
testing.tRunner.func1(0xc420203b30)
	/home/travis/.gimme/versions/go1.9.1.linux.amd64/src/testing/testing.go:711 +0x2d2
panic(0x2a62b60, 0xc420175b80)
	/home/travis/.gimme/versions/go1.9.1.linux.amd64/src/runtime/panic.go:491 +0x283
github.com/terraform-providers/terraform-provider-aws/aws.matchRules(0x2f59141, 0x7, 0xc420519970, 0x1, 0x1, 0xc42000f1f0, 0x1, 0x1, 0x0, 0x0, ...)
	/home/travis/gopath/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_security_group.go:866 +0x3701

[svanharmelen]

Whoops 😳Yes, of course! I amended the changes to matchRules just after I was finished fixing the initial issue. Before amending them I did re-run the acceptance tests, but somehow managed to forget running make test again. Sorry about that...

[ewbankkit]

Thanks for fixing this.

[bflad]

Hey @svanharmelen I just realized I completely forgot about this PR and went ahead and merged a similar fix in #4416. 😓 I am really sorry about that. The maintainers here want to ensure everyone gets fair credit and this was my fault. The worst part is that your fix here was likely on the right track, if not completely correct. I'll give you a shout out for your efforts for all the tickets that are hopefully getting closed.

If you have anything specific you would like to see done, please let me know and I'll personally make sure it gets some top priority attention. Thank you again for submitting this and sorry it did not get a proper review beforehand.

[svanharmelen]

Stuff happens, no worries! 👍

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!