From b2e4c25575954fc1f28ef1f5b129ec3420c03097 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 13:42:50 +0000 Subject: [PATCH 01/11] feat(ipam): allocation read description --- internal/service/ec2/ipam_pool_cidr_allocation.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation.go b/internal/service/ec2/ipam_pool_cidr_allocation.go index 207df9925a64..9d7f82855e9e 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation.go @@ -167,6 +167,7 @@ func resourceIPAMPoolCIDRAllocationRead(ctx context.Context, d *schema.ResourceD d.Set("cidr", allocation.Cidr) d.Set("ipam_pool_allocation_id", allocation.IpamPoolAllocationId) d.Set("ipam_pool_id", poolID) + d.Set(names.AttrDescription, allocation.Description) d.Set(names.AttrResourceID, allocation.ResourceId) d.Set(names.AttrResourceOwner, allocation.ResourceOwner) d.Set(names.AttrResourceType, allocation.ResourceType) From f5ff9489ca4718b85805f841372ccaeb6c657c7a Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 13:42:59 +0000 Subject: [PATCH 02/11] test(ipam): alloc desc create, import, recreate --- .../ec2/ipam_pool_cidr_allocation_test.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation_test.go b/internal/service/ec2/ipam_pool_cidr_allocation_test.go index 7f5ebdbf46c5..f678445aba22 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation_test.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation_test.go @@ -53,6 +53,43 @@ func TestAccIPAMPoolCIDRAllocation_ipv4Basic(t *testing.T) { }) } +func TestAccIPAMPoolCIDRAllocation_ipv4Description(t *testing.T) { + ctx := acctest.Context(t) + var allocationV1, allocationV2 awstypes.IpamPoolAllocation + resourceName := "aws_vpc_ipam_pool_cidr_allocation.test" + originalDescription := "original" + updatedDescription := "updated" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckIPAMPoolAllocationDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccIPAMPoolCIDRAllocationConfig_description(originalDescription), + Check: resource.ComposeTestCheckFunc( + testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV1), + resource.TestCheckResourceAttr(resourceName, "description", originalDescription), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccIPAMPoolCIDRAllocationConfig_description(updatedDescription), + Check: resource.ComposeTestCheckFunc( + testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV2), + testAccCheckIPAMPoolCIDRAllocationRecreated(&allocationV1, &allocationV2), + resource.TestCheckResourceAttr(resourceName, "description", updatedDescription), + ), + }, + }, + }) +} + func TestAccIPAMPoolCIDRAllocation_disappears(t *testing.T) { ctx := acctest.Context(t) var allocation awstypes.IpamPoolAllocation @@ -266,6 +303,16 @@ func testAccCheckIPAMPoolCIDRAllocationExistsWithProvider(ctx context.Context, n } } +func testAccCheckIPAMPoolCIDRAllocationRecreated(before, after *awstypes.IpamPoolAllocation) resource.TestCheckFunc { + return func(s *terraform.State) error { + if before, after := aws.ToString(before.IpamPoolAllocationId), aws.ToString(after.IpamPoolAllocationId); before == after { + return fmt.Errorf("IPAM Pool Cidr Allocation (%s) not recreated", before) + } + + return nil + } +} + func testAccCheckIPAMPoolAllocationDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Client(ctx) @@ -326,6 +373,20 @@ resource "aws_vpc_ipam_pool_cidr_allocation" "test" { `, cidr)) } +func testAccIPAMPoolCIDRAllocationConfig_description(description string) string { + return acctest.ConfigCompose(testAccIPAMPoolCIDRAllocationConfig_base, fmt.Sprintf(` +resource "aws_vpc_ipam_pool_cidr_allocation" "test" { + ipam_pool_id = aws_vpc_ipam_pool.test.id + cidr = "172.2.0.0/28" + description = %[1]q + + depends_on = [ + aws_vpc_ipam_pool_cidr.test + ] +} +`, description)) +} + func testAccIPAMPoolCIDRAllocationConfig_ipv4Netmask(netmask string) string { return acctest.ConfigCompose(testAccIPAMPoolCIDRAllocationConfig_base, fmt.Sprintf(` resource "aws_vpc_ipam_pool_cidr_allocation" "test" { From fd818d2bced188be4b9c9696c0fc6f4f9b6e13e5 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 13:43:06 +0000 Subject: [PATCH 03/11] docs(ipam): alloc desc forces new rsrc --- website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown index f57b56ce7881..17e08cc67fbb 100644 --- a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown +++ b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown @@ -84,7 +84,7 @@ resource "aws_vpc_ipam" "example" { This resource supports the following arguments: * `cidr` - (Optional) The CIDR you want to assign to the pool. -* `description` - (Optional) The description for the allocation. +* `description` - (Optional, Forces new resource) The description for the allocation. * `disallowed_cidrs` - (Optional) Exclude a particular CIDR range from being returned by the pool. * `ipam_pool_id` - (Required) The ID of the pool to which you want to assign a CIDR. * `netmask_length` - (Optional) The netmask length of the CIDR you would like to allocate to the IPAM pool. Valid Values: `0-128`. From 9725c1f76bf8e9ff1042a0d48c251a4525baea19 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:15:50 +0000 Subject: [PATCH 04/11] feat(ipam): allocation read netmask --- internal/service/ec2/ipam_pool_cidr_allocation.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation.go b/internal/service/ec2/ipam_pool_cidr_allocation.go index 9d7f82855e9e..b8ca85149d3f 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "strconv" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -78,6 +79,7 @@ func resourceIPAMPoolCIDRAllocation() *schema.Resource { Type: schema.TypeInt, Optional: true, ForceNew: true, + Computed: true, ValidateFunc: validation.IntBetween(0, 128), ConflictsWith: []string{"cidr"}, }, @@ -167,6 +169,18 @@ func resourceIPAMPoolCIDRAllocationRead(ctx context.Context, d *schema.ResourceD d.Set("cidr", allocation.Cidr) d.Set("ipam_pool_allocation_id", allocation.IpamPoolAllocationId) d.Set("ipam_pool_id", poolID) + cidr := aws.ToString(allocation.Cidr) + d.Set("netmask_length", nil) + parts := strings.Split(cidr, "/") + if len(parts) == 2 { + if v, err := strconv.Atoi(parts[1]); err == nil { + d.Set("netmask_length", v) + } else { + log.Printf("[WARN] Unable to parse CIDR (%s) netmask length: %s", cidr, err) + } + } else { + log.Printf("[WARN] Invalid CIDR block format: %s", cidr) + } d.Set(names.AttrDescription, allocation.Description) d.Set(names.AttrResourceID, allocation.ResourceId) d.Set(names.AttrResourceOwner, allocation.ResourceOwner) From b080da756d87c8d5321d5ca54a99a8069f5b409f Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:16:29 +0000 Subject: [PATCH 05/11] test(ipam): alloc netmask import, recreate --- .../ec2/ipam_pool_cidr_allocation_test.go | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation_test.go b/internal/service/ec2/ipam_pool_cidr_allocation_test.go index f678445aba22..3a9c2286a27f 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation_test.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation_test.go @@ -116,9 +116,10 @@ func TestAccIPAMPoolCIDRAllocation_disappears(t *testing.T) { func TestAccIPAMPoolCIDRAllocation_ipv4BasicNetmask(t *testing.T) { ctx := acctest.Context(t) - var allocation awstypes.IpamPoolAllocation + var allocationV1, allocationV2 awstypes.IpamPoolAllocation resourceName := "aws_vpc_ipam_pool_cidr_allocation.test" - netmask := "28" + originalNetmask := "28" + updatedNetmask := "25" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -127,17 +128,24 @@ func TestAccIPAMPoolCIDRAllocation_ipv4BasicNetmask(t *testing.T) { CheckDestroy: testAccCheckIPAMPoolAllocationDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccIPAMPoolCIDRAllocationConfig_ipv4Netmask(netmask), + Config: testAccIPAMPoolCIDRAllocationConfig_ipv4Netmask(originalNetmask), Check: resource.ComposeTestCheckFunc( - testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocation), - testAccCheckIPAMCIDRPrefix(&allocation, netmask), + testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV1), + testAccCheckIPAMCIDRPrefix(&allocationV1, originalNetmask), ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"netmask_length"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccIPAMPoolCIDRAllocationConfig_ipv4Netmask(updatedNetmask), + Check: resource.ComposeTestCheckFunc( + testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV2), + testAccCheckIPAMPoolCIDRAllocationRecreated(&allocationV1, &allocationV2), + testAccCheckIPAMCIDRPrefix(&allocationV2, updatedNetmask), + ), }, }, }) From d53f23cb67a213065af496a849bdd86e45c86f23 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:16:56 +0000 Subject: [PATCH 06/11] test(ipam): alloc cidr basic netmask rstate --- internal/service/ec2/ipam_pool_cidr_allocation_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation_test.go b/internal/service/ec2/ipam_pool_cidr_allocation_test.go index 3a9c2286a27f..71beba990a70 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation_test.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation_test.go @@ -42,6 +42,7 @@ func TestAccIPAMPoolCIDRAllocation_ipv4Basic(t *testing.T) { resource.TestMatchResourceAttr(resourceName, names.AttrID, regexache.MustCompile(`^ipam-pool-alloc-[0-9a-f]+_ipam-pool(-[0-9a-f]+)$`)), resource.TestMatchResourceAttr(resourceName, "ipam_pool_allocation_id", regexache.MustCompile(`^ipam-pool-alloc-[0-9a-f]+$`)), resource.TestCheckResourceAttrPair(resourceName, "ipam_pool_id", "aws_vpc_ipam_pool.test", names.AttrID), + resource.TestCheckResourceAttr(resourceName, "netmask_length", strings.Split(cidr, "/")[1]), ), }, { From cc102c16123110036446347873aceae210d20c9c Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:17:11 +0000 Subject: [PATCH 07/11] docs(ipam): alloc netmask forces new rsrc --- website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown index 17e08cc67fbb..f2bde2182c05 100644 --- a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown +++ b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown @@ -87,7 +87,7 @@ This resource supports the following arguments: * `description` - (Optional, Forces new resource) The description for the allocation. * `disallowed_cidrs` - (Optional) Exclude a particular CIDR range from being returned by the pool. * `ipam_pool_id` - (Required) The ID of the pool to which you want to assign a CIDR. -* `netmask_length` - (Optional) The netmask length of the CIDR you would like to allocate to the IPAM pool. Valid Values: `0-128`. +* `netmask_length` - (Optional, Forces new resource) The netmask length of the CIDR you would like to allocate to the IPAM pool. Valid Values: `0-128`. ## Attribute Reference From add7493c785d421d9936fa632c9b8c7adffc6cab Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:18:00 +0000 Subject: [PATCH 08/11] docs(ipam): alloc args force new rsrc --- website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown index f2bde2182c05..6894987db503 100644 --- a/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown +++ b/website/docs/r/vpc_ipam_pool_cidr_allocation.html.markdown @@ -83,10 +83,10 @@ resource "aws_vpc_ipam" "example" { This resource supports the following arguments: -* `cidr` - (Optional) The CIDR you want to assign to the pool. +* `cidr` - (Optional, Forces new resource) The CIDR you want to assign to the pool. * `description` - (Optional, Forces new resource) The description for the allocation. -* `disallowed_cidrs` - (Optional) Exclude a particular CIDR range from being returned by the pool. -* `ipam_pool_id` - (Required) The ID of the pool to which you want to assign a CIDR. +* `disallowed_cidrs` - (Optional, Forces new resource) Exclude a particular CIDR range from being returned by the pool. +* `ipam_pool_id` - (Required, Forces new resource) The ID of the pool to which you want to assign a CIDR. * `netmask_length` - (Optional, Forces new resource) The netmask length of the CIDR you would like to allocate to the IPAM pool. Valid Values: `0-128`. ## Attribute Reference From a74f525adc730019b7dbef705602cdad9d73c738 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:27:07 +0000 Subject: [PATCH 09/11] ci(ipam): alloc changelog desc, netmask read --- .changelog/38618.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/38618.txt diff --git a/.changelog/38618.txt b/.changelog/38618.txt new file mode 100644 index 000000000000..dac5e23c027e --- /dev/null +++ b/.changelog/38618.txt @@ -0,0 +1,7 @@ +```release-note:bug +resource/aws_vpc_ipam_pool_cidr_allocation: Set `description` on Read +``` + +```release-note:bug +resource/aws_vpc_ipam_pool_cidr_allocation: Set `netmask_length` on Read +``` \ No newline at end of file From ce7093341af48b7543d3421272ac27a80c4f0bb1 Mon Sep 17 00:00:00 2001 From: GlennChia Date: Wed, 31 Jul 2024 14:51:49 +0000 Subject: [PATCH 10/11] test(ipam): alloc use names for desc --- internal/service/ec2/ipam_pool_cidr_allocation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation_test.go b/internal/service/ec2/ipam_pool_cidr_allocation_test.go index 71beba990a70..67563f3df3d7 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation_test.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation_test.go @@ -71,7 +71,7 @@ func TestAccIPAMPoolCIDRAllocation_ipv4Description(t *testing.T) { Config: testAccIPAMPoolCIDRAllocationConfig_description(originalDescription), Check: resource.ComposeTestCheckFunc( testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV1), - resource.TestCheckResourceAttr(resourceName, "description", originalDescription), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, originalDescription), ), }, { @@ -84,7 +84,7 @@ func TestAccIPAMPoolCIDRAllocation_ipv4Description(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckIPAMPoolCIDRAllocationExists(ctx, resourceName, &allocationV2), testAccCheckIPAMPoolCIDRAllocationRecreated(&allocationV1, &allocationV2), - resource.TestCheckResourceAttr(resourceName, "description", updatedDescription), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, updatedDescription), ), }, }, From 05c7db38ed0206d3fd96875f7d0efed50478b365 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 31 Jul 2024 13:37:25 -0400 Subject: [PATCH 11/11] r/aws_vpc_ipam_pool_cidr_allocation: Cosmetics. --- internal/service/ec2/ipam_pool_cidr_allocation.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/ipam_pool_cidr_allocation.go b/internal/service/ec2/ipam_pool_cidr_allocation.go index b8ca85149d3f..9953884c169c 100644 --- a/internal/service/ec2/ipam_pool_cidr_allocation.go +++ b/internal/service/ec2/ipam_pool_cidr_allocation.go @@ -166,13 +166,13 @@ func resourceIPAMPoolCIDRAllocationRead(ctx context.Context, d *schema.ResourceD return sdkdiag.AppendErrorf(diags, "reading IPAM Pool CIDR Allocation (%s): %s", d.Id(), err) } - d.Set("cidr", allocation.Cidr) + cidr := aws.ToString(allocation.Cidr) + d.Set("cidr", cidr) + d.Set(names.AttrDescription, allocation.Description) d.Set("ipam_pool_allocation_id", allocation.IpamPoolAllocationId) d.Set("ipam_pool_id", poolID) - cidr := aws.ToString(allocation.Cidr) d.Set("netmask_length", nil) - parts := strings.Split(cidr, "/") - if len(parts) == 2 { + if parts := strings.Split(cidr, "/"); len(parts) == 2 { if v, err := strconv.Atoi(parts[1]); err == nil { d.Set("netmask_length", v) } else { @@ -181,7 +181,7 @@ func resourceIPAMPoolCIDRAllocationRead(ctx context.Context, d *schema.ResourceD } else { log.Printf("[WARN] Invalid CIDR block format: %s", cidr) } - d.Set(names.AttrDescription, allocation.Description) + d.Set(names.AttrResourceID, allocation.ResourceId) d.Set(names.AttrResourceOwner, allocation.ResourceOwner) d.Set(names.AttrResourceType, allocation.ResourceType)