New Resource: aws_vpc_ipv4_cidr_block_association #3723

ewbankkit · 2018-03-09T21:48:58Z

Fixes #1539.
Replaces #1568 with the implementation described here.

New resource aws_vpc_secondary_ipv4_cidr_block associates a secondary IPv4 CIDR block with a VPC.
Only the first of the rules for adding a CIDR block to a VPC is checked: The allowed block size is between a /28 netmask and /16 netmask.

Acceptance tests:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsVpcSecondaryIpv4CidrBlock_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsVpcSecondaryIpv4CidrBlock_basic -timeout 120m
=== RUN   TestAccAwsVpcSecondaryIpv4CidrBlock_basic
--- PASS: TestAccAwsVpcSecondaryIpv4CidrBlock_basic (28.41s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	28.428s

Regression tests for the aws_vpc resource:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpc_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSVpc_ -timeout 120m
=== RUN   TestAccAWSVpc_coreMismatchedDiffs
--- PASS: TestAccAWSVpc_coreMismatchedDiffs (25.28s)
=== RUN   TestAccAWSVpc_importBasic
--- PASS: TestAccAWSVpc_importBasic (29.09s)
=== RUN   TestAccAWSVpc_basic
--- PASS: TestAccAWSVpc_basic (24.59s)
=== RUN   TestAccAWSVpc_enableIpv6
--- PASS: TestAccAWSVpc_enableIpv6 (65.23s)
=== RUN   TestAccAWSVpc_dedicatedTenancy
--- PASS: TestAccAWSVpc_dedicatedTenancy (24.67s)
=== RUN   TestAccAWSVpc_tags
--- PASS: TestAccAWSVpc_tags (44.91s)
=== RUN   TestAccAWSVpc_update
--- PASS: TestAccAWSVpc_update (45.56s)
=== RUN   TestAccAWSVpc_bothDnsOptionsSet
--- PASS: TestAccAWSVpc_bothDnsOptionsSet (27.65s)
=== RUN   TestAccAWSVpc_DisabledDnsSupport
--- PASS: TestAccAWSVpc_DisabledDnsSupport (24.37s)
=== RUN   TestAccAWSVpc_classiclinkOptionSet
--- PASS: TestAccAWSVpc_classiclinkOptionSet (24.98s)
=== RUN   TestAccAWSVpc_classiclinkDnsSupportOptionSet
--- PASS: TestAccAWSVpc_classiclinkDnsSupportOptionSet (25.96s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	362.290s

ewbankkit · 2018-03-09T21:51:11Z

The aws_vpc data source still needs to be updated with secondary IPv4 CIDR blocks.

nmische · 2018-05-03T16:33:19Z

Any idea if this may get merged anytime soon? There seems to be a lot of interest based on comments in #1568, and I know my teams could use this functionality.

ewbankkit · 2018-05-03T17:53:27Z

I will work on resolving the conflicts tomorrow (hopefully).

ewbankkit · 2018-05-04T12:30:18Z

Rebased and resolved the merge conflict.
Reran acceptance tests:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsVpcSecondaryIpv4CidrBlock_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsVpcSecondaryIpv4CidrBlock_ -timeout 120m
=== RUN   TestAccAwsVpcSecondaryIpv4CidrBlock_basic
--- PASS: TestAccAwsVpcSecondaryIpv4CidrBlock_basic (29.31s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	29.320s

simonclausen · 2018-05-07T12:22:15Z

Just want to thank @stack72 and @ewbankkit for the effort, as well as give my +1 on this thing being merged, it would definitely be usable for us!

richievos · 2018-06-06T20:03:09Z

This being explicitly _ipv4_ is a good idea. It's somewhat confusing at this point that there are so many open PRs/issues/comments about this, but I could use one implementation being merged. Does anyone have a workaround for this available, outside of running this forked terraform version? I'd imagine some sort of hack around using a provisioner after the vpc is created that manually calls the AWS cidr block association API could do it.

bflad

Thanks for your work here @ewbankkit 👍 Overall I think this resource makes the most sense for this situation. I left some little feedback below. Please let us know if you have any questions or do not have time to implement the feedback.

Existing acceptance testing is passing for me as well:

make testacc TEST=./aws TESTARGS='-run=TestAccAwsVpcSecondaryIpv4CidrBlock_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsVpcSecondaryIpv4CidrBlock_basic -timeout 120m
=== RUN   TestAccAwsVpcSecondaryIpv4CidrBlock_basic
--- PASS: TestAccAwsVpcSecondaryIpv4CidrBlock_basic (25.84s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	25.888s

bflad · 2018-06-28T13:48:48Z

aws/provider.go

@@ -570,6 +570,7 @@ func Provider() terraform.ResourceProvider {
 "aws_vpc_endpoint_subnet_association": resourceAwsVpcEndpointSubnetAssociation(),
 "aws_vpc_endpoint_service": resourceAwsVpcEndpointService(),
 "aws_vpc_endpoint_service_allowed_principal": resourceAwsVpcEndpointServiceAllowedPrincipal(),
+ "aws_vpc_secondary_ipv4_cidr_block": resourceAwsVpcSecondaryIpv4CidrBlock(),


Due to the API naming, I perosnally lean towards naming this aws_vpc_ipv4_cidr_block_association. I think out of context, the word secondary implies there can only be one of these added which is not the case (even though the EC2 User Guide refers to them that way. Either way it can be confusing. 😕

bflad · 2018-06-28T13:52:33Z

aws/aws_vpc_secondary_ipv4_cidr_block.go

+ for _, cidrAssociation := range vpc.CidrBlockAssociationSet {
+ if aws.StringValue(cidrAssociation.AssociationId) == d.Id() {
+ found = true
+ d.Set("cidr_block", cidrAssociation.CidrBlock)


We should call break after this to stop looping through the CidrBlockAssociationSet

bflad · 2018-06-28T13:56:45Z

aws/aws_vpc_secondary_ipv4_cidr_block.go

+ AssociationId: aws.String(d.Id()),
+ })
+ if err != nil {
+ return fmt.Errorf("Error deleting VPC secondary IPv4 CIDR block: %s", err.Error())


We should ignore VPC not found errors here.

if isAWSErr(err, "InvalidVpcID.NotFound", "") { return nil }

bflad

Also of note, we should probably add some documentation within the aws_subnet resource that suggests using the vpc_id of this resource when creating subnets under this new block, e.g.

When working with subnets under VPC secondary CIDR blocks created with the aws_vpc_ipv4_cidr_block_association resource, it is recommended to reference its vpc_id attribute to setup proper dependency ordering.

resource "aws_subnet" "example" {
  # ... other configuration ...
  vpc_id = "${aws_vpc_ipv4_cidr_block_association.example.vpc_id}"
}

bflad · 2018-06-28T14:22:23Z

aws/aws_vpc_secondary_ipv4_cidr_block.go

+ }
+
+ d.SetId(aws.StringValue(resp.CidrBlockAssociation.AssociationId))
+


Do we need to handle this scenario described in the VPC User Guide?

When you add or remove a CIDR block, it can go through various states: associating | associated | disassociating | disassociated | failing | failed. The CIDR block is ready for you to use when it's in the associated state.

It seems like we should have waiter logic implemented here to prevent downstream errors when trying to use the CIDR block immediately in Terraform configurations.

OK, I'll address.

AzCii · 2018-06-28T15:06:28Z

I have been following #1568 for 8 months and can see it have been closed now in favor of this one.

Are there any ETA on this?

ewbankkit · 2018-06-28T17:43:20Z

I should be able to get to the suggested changes over the next couple of days.

Adds the ability to associate extra IPv4 or IPv6 CIDR Blocks with a VPC. In order to avoid getting into the same issue as security_group and security_group_rule, we added a diffSuppressFunc that stops people enabling `ipv6` for an existant AWS VPC. We added a note to the documentation to talk about this ``` % make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpc_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccAWSVpc_ -timeout 120m === RUN TestAccAWSVpc_importBasic --- PASS: TestAccAWSVpc_importBasic (54.16s) === RUN TestAccAWSVpc_basic --- PASS: TestAccAWSVpc_basic (45.26s) === RUN TestAccAWSVpc_enableIpv6 --- PASS: TestAccAWSVpc_enableIpv6 (87.40s) === RUN TestAccAWSVpc_dedicatedTenancy --- PASS: TestAccAWSVpc_dedicatedTenancy (45.51s) === RUN TestAccAWSVpc_tags --- PASS: TestAccAWSVpc_tags (84.80s) === RUN TestAccAWSVpc_update --- PASS: TestAccAWSVpc_update (97.12s) === RUN TestAccAWSVpc_bothDnsOptionsSet --- PASS: TestAccAWSVpc_bothDnsOptionsSet (19.82s) === RUN TestAccAWSVpc_DisabledDnsSupport --- PASS: TestAccAWSVpc_DisabledDnsSupport (44.93s) === RUN TestAccAWSVpc_classiclinkOptionSet --- PASS: TestAccAWSVpc_classiclinkOptionSet (46.18s) === RUN TestAccAWSVpc_classiclinkDnsSupportOptionSet --- PASS: TestAccAWSVpc_classiclinkDnsSupportOptionSet (47.51s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 572.738s ``` ``` % make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpcAssociat' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccAWSVpcAssociat -timeout 120m === RUN TestAccAWSVpcAssociateIpv4CidrBlock --- PASS: TestAccAWSVpcAssociateIpv4CidrBlock (51.48s) === RUN TestAccAWSVpcAssociateIpv6CidrBlock --- PASS: TestAccAWSVpcAssociateIpv6CidrBlock (50.16s) === RUN TestAccAWSVpcAssociateIpv4AndIpv6CidrBlock --- PASS: TestAccAWSVpcAssociateIpv4AndIpv6CidrBlock (2.13s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 103.802s ```

…lity.

…iation'.

ewbankkit · 2018-06-29T17:52:51Z

@bflad All review comments addressed. Rebased to remove conflict.
Acceptance tests:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAwsVpcIpv4CidrBlockAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsVpcIpv4CidrBlockAssociation_ -timeout 120m
=== RUN   TestAccAwsVpcIpv4CidrBlockAssociation_basic
--- PASS: TestAccAwsVpcIpv4CidrBlockAssociation_basic (56.97s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	71.440s

ewbankkit · 2018-06-29T18:02:09Z

aws/resource_aws_vpc.go

+
+// vpcDescribe returns EC2 API information about the specified VPC.
+// If the VPC doesn't exist, return nil.
+func vpcDescribe(conn *ec2.EC2, vpcId string) (*ec2.Vpc, error) {


Added vpcDescribe here to be reused in future refactorings.

bflad

Looks great @ewbankkit! Thanks so much to you and @stack72 for your work! 🚀

1 test passed (all tests)
=== RUN   TestAccAwsVpcIpv4CidrBlockAssociation_basic
--- PASS: TestAccAwsVpcIpv4CidrBlockAssociation_basic (29.45s)

bflad · 2018-07-04T12:32:01Z

This has been released in version 1.26.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-04T17:51:37Z

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!

ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 9, 2018

ewbankkit mentioned this pull request Mar 9, 2018

New Resource: aws_vpc_associate_cidr_block #1568

Closed

radeksimko added service/ec2 Issues and PRs that pertain to the ec2 service. new-resource Introduces a new resource. labels Mar 13, 2018

radeksimko changed the title ~~Add 'aws_vpc_secondary_ipv4_cidr_block' resource~~ New Resource: aws_vpc_secondary_ipv4_cidr_block Mar 23, 2018

ewbankkit force-pushed the issue-1539 branch from 733caee to 3d75f59 Compare May 4, 2018 12:29

ghost added the size/L Managed by automation to categorize the size of a PR. label May 4, 2018

bflad reviewed Jun 28, 2018

View reviewed changes

stack72 and others added 7 commits June 29, 2018 10:58

Restore original 'aws_vpc.assign_generated_ipv6_cidr_block' functiona…

3e43f34

…lity.

'aws_vpc_associate_cidr_block' -> 'aws_vpc_secondary_ipv4_cidr_block'.

839d50b

'ipv4_cidr_block' -> 'cidr_block'.

ecad19d

'aws_vpc_secondary_ipv4_cidr_block' -> 'aws_vpc_ipv4_cidr_block_assoc…

0439357

…iation'.

Minor changes after code review.

506cc15

Correctly handle VPC IPv4 CIDR block association state changes.

ff10478

ewbankkit force-pushed the issue-1539 branch from 3d75f59 to ff10478 Compare June 29, 2018 17:51

ghost added the size/L Managed by automation to categorize the size of a PR. label Jun 29, 2018

ewbankkit commented Jun 29, 2018

View reviewed changes

bflad added this to the v1.26.0 milestone Jun 29, 2018

bflad changed the title ~~New Resource: aws_vpc_secondary_ipv4_cidr_block~~ New Resource: aws_vpc_ipv4_cidr_block_association Jun 29, 2018

bflad approved these changes Jun 29, 2018

View reviewed changes

bflad merged commit da26707 into hashicorp:master Jun 29, 2018

bflad added a commit that referenced this pull request Jun 29, 2018

Update CHANGELOG for #3723

7c3c613

ewbankkit deleted the issue-1539 branch June 30, 2018 20:59

This was referenced Jun 30, 2018

d/aws_vpc: Additional IPv4 CIDR blocks #5042

Closed

Expanding VPC's via Terraform? #2144

Closed

Support additional CIDRs in aws_vpc #2841

Closed

ghost locked and limited conversation to collaborators Apr 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_vpc_ipv4_cidr_block_association #3723

New Resource: aws_vpc_ipv4_cidr_block_association #3723

ewbankkit commented Mar 9, 2018

ewbankkit commented Mar 9, 2018

nmische commented May 3, 2018

ewbankkit commented May 3, 2018

ewbankkit commented May 4, 2018

simonclausen commented May 7, 2018

richievos commented Jun 6, 2018

bflad left a comment

bflad Jun 28, 2018

ewbankkit Jun 28, 2018

bflad Jun 28, 2018

ewbankkit Jun 29, 2018

bflad Jun 28, 2018

ewbankkit Jun 29, 2018

bflad left a comment

bflad Jun 28, 2018

ewbankkit Jun 28, 2018

AzCii commented Jun 28, 2018

ewbankkit commented Jun 28, 2018

ewbankkit commented Jun 29, 2018

ewbankkit Jun 29, 2018

bflad left a comment

bflad commented Jul 4, 2018

ghost commented Apr 4, 2020

		}

		d.SetId(aws.StringValue(resp.CidrBlockAssociation.AssociationId))

New Resource: aws_vpc_ipv4_cidr_block_association #3723

New Resource: aws_vpc_ipv4_cidr_block_association #3723

Conversation

ewbankkit commented Mar 9, 2018

ewbankkit commented Mar 9, 2018

nmische commented May 3, 2018

ewbankkit commented May 3, 2018

ewbankkit commented May 4, 2018

simonclausen commented May 7, 2018

richievos commented Jun 6, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AzCii commented Jun 28, 2018

ewbankkit commented Jun 28, 2018

ewbankkit commented Jun 29, 2018

Choose a reason for hiding this comment

bflad left a comment

Choose a reason for hiding this comment

bflad commented Jul 4, 2018

ghost commented Apr 4, 2020