From 33406fe1e2a65e133cb152b1703240643256f52b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 12:57:36 -0500 Subject: [PATCH 1/8] ipam_pool: Fix publicly_advertisable bug --- internal/service/ec2/ipam_pool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/ec2/ipam_pool.go b/internal/service/ec2/ipam_pool.go index 6f674203e0a..138e10398c0 100644 --- a/internal/service/ec2/ipam_pool.go +++ b/internal/service/ec2/ipam_pool.go @@ -204,9 +204,9 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in if v, ok := d.GetOk("public_ip_source"); ok { input.PublicIpSource = awstypes.IpamPoolPublicIpSource(v.(string)) } - // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip. - // The request can only contain PubliclyAdvertisable if the AddressFamily is IPv6 and PublicIpSource is byoip. - if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic { + // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip (either '' or 'byoip'). + // The request can't contain PubliclyAdvertisable if PublicIpSource is 'amazon'. + if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic && input.PublicIpSource != awstypes.IpamPoolPublicIpSourceAmazon { input.PubliclyAdvertisable = aws.Bool(d.Get("publicly_advertisable").(bool)) } From 2b2fe389f5b109e505026647f98e4e95c8d76292 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:00:40 -0500 Subject: [PATCH 2/8] Add changelog --- .changelog/40042.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/40042.txt diff --git a/.changelog/40042.txt b/.changelog/40042.txt new file mode 100644 index 00000000000..bcc27c3e424 --- /dev/null +++ b/.changelog/40042.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_vpc_ipam_pool: Fix bug when `public_ip_source = "amazon"`: `The request can only contain PubliclyAdvertisable if the AddressFamily is IPv6 and PublicIpSource is byoip.` +``` From 9208140788593832683d8f2d00ea15f97dc84a06 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:01:18 -0500 Subject: [PATCH 3/8] ipam_pool: Add test for public_ip_source = amazon --- internal/service/ec2/ipam_pool_test.go | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index 4833936b162..c457bc0dfd5 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -132,6 +132,36 @@ func TestAccIPAMPool_ipv6Basic(t *testing.T) { }) } +func TestAccIPAMPool_ipv6PublicIPAmazon(t *testing.T) { + ctx := acctest.Context(t) + var pool awstypes.IpamPool + resourceName := "aws_vpc_ipam_pool.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckIPAMPoolDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccIPAMPoolConfig_ipv6PublicIPAmazon, + Check: resource.ComposeTestCheckFunc( + testAccCheckIPAMPoolExists(ctx, resourceName, &pool), + resource.TestCheckResourceAttr(resourceName, "address_family", "ipv6"), + resource.TestCheckResourceAttr(resourceName, "public_ip_source", "amazon"), + resource.TestCheckResourceAttr(resourceName, "aws_service", "ec2"), + resource.TestCheckResourceAttr(resourceName, "publicly_advertisable", acctest.CtFalse), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccIPAMPool_ipv6Contiguous(t *testing.T) { ctx := acctest.Context(t) var pool awstypes.IpamPool @@ -368,6 +398,16 @@ resource "aws_vpc_ipam_pool" "test" { } `) +var testAccIPAMPoolConfig_ipv6PublicIPAmazon = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` +resource "aws_vpc_ipam_pool" "test" { + address_family = "ipv6" + ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id + locale = data.aws_region.current.name + public_ip_source = "amazon" + aws_service = "ec2" +} +`) + var testAccIPAMPoolConfig_ipv6Contiguous = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` resource "aws_vpc_ipam_pool" "test" { address_family = "ipv6" From 37b97bf22f78232b5f405a66a37c22ee35b6e4ac Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:23:05 -0500 Subject: [PATCH 4/8] ipam_pool: Move declaration to when used --- internal/service/ec2/ipam_pool.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/ipam_pool.go b/internal/service/ec2/ipam_pool.go index 138e10398c0..e8f813e2356 100644 --- a/internal/service/ec2/ipam_pool.go +++ b/internal/service/ec2/ipam_pool.go @@ -155,11 +155,6 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in conn := meta.(*conns.AWSClient).EC2Client(ctx) scopeID := d.Get("ipam_scope_id").(string) - scope, err := findIPAMScopeByID(ctx, conn, scopeID) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IPAM Scope (%s): %s", scopeID, err) - } addressFamily := awstypes.AddressFamily(d.Get("address_family").(string)) input := &ec2.CreateIpamPoolInput{ @@ -204,6 +199,12 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in if v, ok := d.GetOk("public_ip_source"); ok { input.PublicIpSource = awstypes.IpamPoolPublicIpSource(v.(string)) } + + scope, err := findIPAMScopeByID(ctx, conn, scopeID) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading IPAM Scope (%s): %s", scopeID, err) + } + // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip (either '' or 'byoip'). // The request can't contain PubliclyAdvertisable if PublicIpSource is 'amazon'. if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic && input.PublicIpSource != awstypes.IpamPoolPublicIpSourceAmazon { From 418c45cc755e00632b519939801c69e8d8e7518d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:27:38 -0500 Subject: [PATCH 5/8] docs/ipam_pool: Add clarification --- website/docs/r/vpc_ipam_pool.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/vpc_ipam_pool.html.markdown b/website/docs/r/vpc_ipam_pool.html.markdown index dd56e5c1a30..8238167aa01 100644 --- a/website/docs/r/vpc_ipam_pool.html.markdown +++ b/website/docs/r/vpc_ipam_pool.html.markdown @@ -81,7 +81,7 @@ within the CIDR range in the pool. * `description` - (Optional) A description for the IPAM pool. * `ipam_scope_id` - (Required) The ID of the scope in which you would like to create the IPAM pool. * `locale` - (Optional) The locale in which you would like to create the IPAM pool. Locale is the Region where you want to make an IPAM pool available for allocations. You can only create pools with locales that match the operating Regions of the IPAM. You can only create VPCs from a pool whose locale matches the VPC's Region. Possible values: Any AWS region, such as `us-east-1`. -* `publicly_advertisable` - (Optional) Defines whether or not IPv6 pool space is publicly advertisable over the internet. This argument is required if `address_family = "ipv6"` and `public_ip_source = "byoip"`, default is `false`. This option is not available for IPv4 pool space or if `public_ip_source = "amazon"`. +* `publicly_advertisable` - (Optional) Defines whether or not IPv6 pool space is publicly advertisable over the internet. This argument is required if `address_family = "ipv6"` and `public_ip_source = "byoip"`, default is `false`. This option is not available for IPv4 pool space or if `public_ip_source = "amazon"`. Setting this argument to `true` when it is not available may result in erroneous differences being reported. * `public_ip_source` - (Optional) The IP address source for pools in the public scope. Only used for provisioning IP address CIDRs to pools in the public scope. Valid values are `byoip` or `amazon`. Default is `byoip`. * `source_ipam_pool_id` - (Optional) The ID of the source IPAM pool. Use this argument to create a child pool within an existing pool. * `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. From b2dd8324d01eb02b9c3d9f641b888282179a110c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:28:22 -0500 Subject: [PATCH 6/8] tests/ipam_pool: Fix format --- internal/service/ec2/ipam_pool_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index c457bc0dfd5..bbfdb169f50 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -400,11 +400,11 @@ resource "aws_vpc_ipam_pool" "test" { var testAccIPAMPoolConfig_ipv6PublicIPAmazon = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` resource "aws_vpc_ipam_pool" "test" { - address_family = "ipv6" - ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id - locale = data.aws_region.current.name - public_ip_source = "amazon" - aws_service = "ec2" + address_family = "ipv6" + ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id + locale = data.aws_region.current.name + public_ip_source = "amazon" + aws_service = "ec2" } `) From f95b677c6abcaadc6fa777da86b0d6b36a0f5b67 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:31:18 -0500 Subject: [PATCH 7/8] tests/ipam_pool: Fix format --- internal/service/ec2/ipam_pool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index bbfdb169f50..b1f2852cae5 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -404,7 +404,7 @@ resource "aws_vpc_ipam_pool" "test" { ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id locale = data.aws_region.current.name public_ip_source = "amazon" - aws_service = "ec2" + aws_service = "ec2" } `) From 619fecb91325019323723c93bd680fca6adff7c3 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:50:21 -0500 Subject: [PATCH 8/8] Lower memory use of golangci, avoid OOM doom --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 925ad7e64cb..7984d4ee338 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -100,4 +100,4 @@ jobs: # Trigger garbage collection more frequently to reduce the likelihood # of OOM errors. Higher values mean it runs faster but more likely to OOM, exit 137. # ref: https://golangci-lint.run/product/performance/ - GOGC: "150" # 100 is the default value + GOGC: "140" # 100 is the default value