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

IPAM Pool CIDR Allocation Eventual Consistency #29022

Merged
3 changes: 3 additions & 0 deletions .changelog/29022.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_vpc_ipam_pool_cidr_allocation: Added support for eventual consistency on read operations after create.
```
6 changes: 5 additions & 1 deletion internal/service/ec2/ipam_pool_cidr_allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ func resourceIPAMPoolCIDRAllocationCreate(ctx context.Context, d *schema.Resourc
}
d.SetId(IPAMPoolCIDRAllocationCreateResourceID(aws.StringValue(output.IpamPoolAllocation.IpamPoolAllocationId), ipamPoolID))

if _, err := WaitIPAMPoolCIDRAllocationCreated(ctx, conn, aws.StringValue(output.IpamPoolAllocation.IpamPoolAllocationId), ipamPoolID, d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for IPAM Pool CIDR Allocation (%s) create: %s", d.Id(), err)
}

return append(diags, resourceIPAMPoolCIDRAllocationRead(ctx, d, meta)...)
}

Expand All @@ -135,7 +139,7 @@ func resourceIPAMPoolCIDRAllocationRead(ctx context.Context, d *schema.ResourceD
allocationID, poolID, err := IPAMPoolCIDRAllocationParseResourceID(d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "reading IPAM Pool CIDR Allocation (%s): %s", d.Id(), err)
return sdkdiag.AppendErrorf(diags, "parsing IPAM Pool CIDR Allocation (%s): %s", d.Id(), err)
}

allocation, err := FindIPAMPoolAllocationByTwoPartKey(ctx, conn, allocationID, poolID)
Expand Down
102 changes: 102 additions & 0 deletions internal/service/ec2/ipam_pool_cidr_allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand Down Expand Up @@ -174,6 +175,41 @@ func TestAccIPAMPoolCIDRAllocation_multiple(t *testing.T) {
})
}

func TestAccIPAMPoolCIDRAllocation_differentRegion(t *testing.T) {
ctx := acctest.Context(t)
var allocation ec2.IpamPoolAllocation
var providers []*schema.Provider
resourceName := "aws_vpc_ipam_pool_cidr_allocation.test"
cidr := "172.2.0.0/28"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(t)
acctest.PreCheckMultipleRegion(t, 2)
},
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
ProviderFactories: acctest.FactoriesAlternate(t, &providers),
CheckDestroy: testAccCheckIPAMPoolAllocationDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccIPAMPoolCIDRAllocationConfig_differentRegion(cidr),
Check: resource.ComposeTestCheckFunc(
testAccCheckIPAMPoolCIDRAllocationExistsWithProvider(ctx, resourceName, &allocation, acctest.RegionProviderFunc(acctest.AlternateRegion(), &providers)),
resource.TestCheckResourceAttr(resourceName, "cidr", cidr),
resource.TestMatchResourceAttr(resourceName, "id", regexp.MustCompile(`^ipam-pool-alloc-[\da-f]+_ipam-pool(-[\da-f]+)$`)),
resource.TestMatchResourceAttr(resourceName, "ipam_pool_allocation_id", regexp.MustCompile(`^ipam-pool-alloc-[\da-f]+$`)),
resource.TestCheckResourceAttrPair(resourceName, "ipam_pool_id", "aws_vpc_ipam_pool.test", "id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccCheckIPAMCIDRPrefix(allocation *ec2.IpamPoolAllocation, expected string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if strings.Split(aws.StringValue(allocation.Cidr), "/")[1] != expected {
Expand Down Expand Up @@ -215,6 +251,37 @@ func testAccCheckIPAMPoolCIDRAllocationExists(ctx context.Context, n string, v *
}
}

func testAccCheckIPAMPoolCIDRAllocationExistsWithProvider(ctx context.Context, n string, v *ec2.IpamPoolAllocation, providerF func() *schema.Provider) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No IPAM Pool CIDR Allocation ID is set")
}

allocationID, poolID, err := tfec2.IPAMPoolCIDRAllocationParseResourceID(rs.Primary.ID)

if err != nil {
return err
}

conn := providerF().Meta().(*conns.AWSClient).EC2Conn()

output, err := tfec2.FindIPAMPoolAllocationByTwoPartKey(ctx, conn, allocationID, poolID)

if err != nil {
return err
}

*v = *output

return nil
}
}

func testAccCheckIPAMPoolAllocationDestroy(ctx context.Context) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn()
Expand Down Expand Up @@ -336,3 +403,38 @@ resource "aws_vpc_ipam_pool_cidr_allocation" "test2" {
}
`, cidr1, cidr2))
}

func testAccIPAMPoolCIDRAllocationConfig_differentRegion(cidr string) string {
return acctest.ConfigCompose(acctest.ConfigMultipleRegionProvider(2),
fmt.Sprintf(`
resource "aws_vpc_ipam" "test" {
operating_regions {
region_name = %[2]q
}
operating_regions {
region_name = %[3]q
}
}

resource "aws_vpc_ipam_pool" "test" {
address_family = "ipv4"
ipam_scope_id = aws_vpc_ipam.test.private_default_scope_id
locale = %[3]q
}

resource "aws_vpc_ipam_pool_cidr" "test" {
ipam_pool_id = aws_vpc_ipam_pool.test.id
cidr = "172.2.0.0/24"
}

resource "aws_vpc_ipam_pool_cidr_allocation" "test" {
provider = "awsalternate"
ipam_pool_id = aws_vpc_ipam_pool.test.id
cidr = %[1]q

depends_on = [
aws_vpc_ipam_pool_cidr.test
]
}
`, cidr, acctest.Region(), acctest.AlternateRegion()))
}
21 changes: 21 additions & 0 deletions internal/service/ec2/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,27 @@ func StatusIPAMPoolCIDRState(ctx context.Context, conn *ec2.EC2, cidrBlock, pool
}
}

const (
// naming mapes to the SDK constants that exist for IPAM
IpamPoolCIDRAllocationCreateComplete = "create-complete" // nosemgrep:ci.caps2-in-const-name, ci.caps2-in-var-name, ci.caps5-in-const-name, ci.caps5-in-var-name
)

func StatusIPAMPoolCIDRAllocationState(ctx context.Context, conn *ec2.EC2, allocationID, poolID string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
output, err := FindIPAMPoolAllocationByTwoPartKey(ctx, conn, allocationID, poolID)

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

if err != nil {
return nil, "", err
}

return output, IpamPoolCIDRAllocationCreateComplete, nil
}
}

func StatusIPAMScopeState(ctx context.Context, conn *ec2.EC2, id string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
output, err := FindIPAMScopeByID(ctx, conn, id)
Expand Down
18 changes: 18 additions & 0 deletions internal/service/ec2/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -2926,6 +2926,24 @@ func WaitIPAMPoolCIDRDeleted(ctx context.Context, conn *ec2.EC2, cidrBlock, pool
return nil, err
}

func WaitIPAMPoolCIDRAllocationCreated(ctx context.Context, conn *ec2.EC2, allocationID, poolID string, timeout time.Duration) (*ec2.IpamPoolAllocation, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{},
Target: []string{IpamPoolCIDRAllocationCreateComplete},
Refresh: StatusIPAMPoolCIDRAllocationState(ctx, conn, allocationID, poolID),
Timeout: timeout,
Delay: 5 * time.Second,
}

outputRaw, err := stateConf.WaitForStateContext(ctx)

if output, ok := outputRaw.(*ec2.IpamPoolAllocation); ok {
return output, err
}

return nil, err
}

func WaitIPAMScopeCreated(ctx context.Context, conn *ec2.EC2, id string, timeout time.Duration) (*ec2.IpamScope, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{ec2.IpamScopeStateCreateInProgress},
Expand Down