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

provider/aws: Fix security group rule import #13630

Merged
merged 1 commit into from
Apr 21, 2017
Merged

Conversation

AMeng
Copy link
Contributor

@AMeng AMeng commented Apr 13, 2017

Fixes #8166 and #8827

The issue was being caused when the same rules were applied to both IP ranges and security groups. The previous implementation would combine them into a single rule, which according to the docs is not allowed. This fix creates a separate rule for each case.

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSecurityGroup_import'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/13 10:33:21 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurityGroup_import -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (19.55s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (19.48s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (23.05s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (22.80s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (24.14s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (22.49s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	131.525s

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! I was able to see the behavior described by using TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules and playing with removing bits from state and trying to import them, and then verify the behavior after this patch, so big thanks for including that 😄

@catsby
Copy link
Contributor

catsby commented Apr 21, 2017

Verified all the tests pass too, for posterity:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurity -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (22.60s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (25.22s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (28.55s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (27.67s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (32.52s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (25.58s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (14.32s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (22.04s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (21.34s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (12.50s)
=== RUN   TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (15.11s)
=== RUN   TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (13.95s)
=== RUN   TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (22.33s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (0.01s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (0.01s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (24.85s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (24.12s)
=== RUN   TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (11.01s)
=== RUN   TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (206.86s)
=== RUN   TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (22.88s)
=== RUN   TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (24.40s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (20.54s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (21.73s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (13.45s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (10.84s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (20.31s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (21.01s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (21.12s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (20.31s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (25.07s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (34.60s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (21.10s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (20.56s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (9.96s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (15.58s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (24.65s)
=== RUN   TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (0.02s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (33.22s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (24.74s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (23.65s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (13.84s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (25.63s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (24.96s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    1044.776s

@ghost
Copy link

ghost commented Apr 13, 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 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not create a resource to match import from AWS Security Group with cidr/sourceSg
3 participants