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.

Sign up for GitHub

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

Jump to bottom

provider/aws: Add support for IPv6 to aws_security_group #12655

Merged
merged 1 commit into from
Mar 14, 2017
Merged

provider/aws: Add support for IPv6 to aws_security_group #12655

merged 1 commit into from
Mar 14, 2017

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Mar 13, 2017 •
edited
Loading 

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSecurityGroup_'                                                                     ✹ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/13 19:07:33 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (48.67s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (51.71s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (58.46s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (65.99s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (62.18s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (47.38s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (30.48s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (16.90s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (45.36s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (45.76s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (44.50s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (45.01s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (50.67s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (76.34s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (46.29s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (48.10s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (16.71s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (22.73s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (54.14s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (74.12s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (55.83s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (54.40s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (26.93s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (61.39s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (55.93s)
PASS
ok	github.com/hashicorp/terraform/builtin/providers/aws	1136.500s

@stack72 stack72 force-pushed the f-aws-ipv6-securitygroup-support branch from 5014295 to 4b9c595 Compare March 13, 2017 17:46
@stack72 stack72 force-pushed the f-aws-ipv6-securitygroup-support branch from 4b9c595 to 1a50333 Compare March 13, 2017 18:09
@stack72 stack72 mentioned this pull request Mar 13, 2017
Closed
@stack72 stack72 modified the milestone: Terraform 0.9 Mar 13, 2017
grubernaut
grubernaut approved these changes Mar 13, 2017
View reviewed changes
Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

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

LGTM, minor questions + nits

builtin/providers/aws/resource_aws_security_group.go
@@ -723,6 +766,10 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface
if ok {
numRemoteCidrs = len(r["cidr_blocks"].([]string))
}
riRaw, ok := r["ipv6_cidr_blocks"]
if ok {
numRemoteIpv6Cidrs = len(r["ipv6_cidr_blocks"].([]string))
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't this need to be cast to []interface{} to avoid a panic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Followed the same code as cidr_blocks above and that doesn't panic so i think we are safe

builtin/providers/aws/resource_aws_security_group.go

var remoteIpv6Cidrs []string
if riRaw != nil {
remoteIpv6Cidrs = riRaw.([]string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as removeCidrs above :)

builtin/providers/aws/resource_aws_security_group.go
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

extra space here?

@stack72
provider/aws: Add support for IPv6 to aws_security_group
134cc91 
```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSecurityGroup_'                                                                     ✹ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/13 19:07:33 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (48.67s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (51.71s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (58.46s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (65.99s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (62.18s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (47.38s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (30.48s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (16.90s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (45.36s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (45.76s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (44.50s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (45.01s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (50.67s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (76.34s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (46.29s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (48.10s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (16.71s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (22.73s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (54.14s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (74.12s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (55.83s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (54.40s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (26.93s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (61.39s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (55.93s)
PASS
ok	github.com/hashicorp/terraform/builtin/providers/aws	1136.500s
```
@stack72 stack72 force-pushed the f-aws-ipv6-securitygroup-support branch from 1a50333 to 134cc91 Compare March 14, 2017 10:44
@stack72 stack72 merged commit 1994b11 into master Mar 14, 2017
@stack72 stack72 deleted the f-aws-ipv6-securitygroup-support branch March 14, 2017 11:02
stack72 added a commit that referenced this pull request Mar 14, 2017
@stack72
provider/aws: Add support for IPv6 to aws_security_group (#12655)
4130fa0 
```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSecurityGroup_'                                                                     ✹ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/13 19:07:33 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (48.67s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (51.71s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (58.46s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (65.99s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (62.18s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (47.38s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (30.48s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (16.90s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (45.36s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (45.76s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (44.50s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (45.01s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (50.67s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (76.34s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (46.29s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (48.10s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (16.71s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (22.73s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (54.14s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (74.12s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (55.83s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (54.40s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (26.93s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (61.39s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (55.93s)
PASS
ok	github.com/hashicorp/terraform/builtin/providers/aws	1136.500s
```
@mqasim1983 mqasim1983 mentioned this pull request Mar 17, 2017
Closed
@ghost
Copy link

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

@grubernaut grubernaut grubernaut approved these changes

Assignees
No one assigned
Labels
enhancement provider/aws
Projects
None yet
Milestone
Terraform 0.9
Development

Successfully merging this pull request may close these issues.
2 participants
@stack72 @grubernaut