Merge pull request #39196 from hashicorp/b-verifiedaccess_endpoint-crash

r/aws_verifiedaccess_endpoint: fix crash when updating `load_balancer_options.subnet_ids`
hashicorp · Sep 9, 2024 · bca2796 · bca2796
2 parents 9bc0cf6 + f9def71
commit bca2796
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 15 deletions.
diff --git a/.changelog/39196.txt b/.changelog/39196.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_verifiedaccess_endpoint: fix crash when updating `load_balancer_options.subnet_ids`
+```
diff --git a/internal/service/ec2/verifiedaccess_endpoint.go b/internal/service/ec2/verifiedaccess_endpoint.go
@@ -502,8 +502,8 @@ func expandModifyVerifiedAccessEndpointLoadBalancerOptions(tfMap map[string]inte
 		apiObject.Protocol = types.VerifiedAccessEndpointProtocol(v)
 	}
 
-	if v, ok := tfMap[names.AttrSubnetIDs]; ok {
-		apiObject.SubnetIds = flex.ExpandStringValueList(v.([]interface{}))
+	if v, ok := tfMap[names.AttrSubnetIDs].(*schema.Set); ok && v.Len() > 0 {
+		apiObject.SubnetIds = flex.ExpandStringValueSet(v)
 	}
 
 	return apiObject

diff --git a/internal/service/ec2/verifiedaccess_endpoint_test.go b/internal/service/ec2/verifiedaccess_endpoint_test.go
@@ -252,6 +252,54 @@ func testAccVerifiedAccessEndpoint_policyDocument(t *testing.T, semaphore tfsync
 	})
 }
 
+// Verifies load balancer subnet ID's can be updated without a crash
+// Ref: https://github.com/hashicorp/terraform-provider-aws/issues/39186
+func testAccVerifiedAccessEndpoint_subnetIDs(t *testing.T, semaphore tfsync.Semaphore) {
+	ctx := acctest.Context(t)
+	var v types.VerifiedAccessEndpoint
+	resourceName := "aws_verifiedaccess_endpoint.test"
+	key := acctest.TLSRSAPrivateKeyPEM(t, 2048)
+	certificate := acctest.TLSRSAX509SelfSignedCertificatePEM(t, key, "example.com")
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheckVerifiedAccessSynchronize(t, semaphore)
+			acctest.PreCheck(ctx, t)
+			testAccPreCheckVerifiedAccess(ctx, t)
+		},
+		ErrorCheck:               acctest.ErrorCheck(t, names.EC2),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckVerifiedAccessEndpointDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccVerifiedAccessEndpointConfig_subnetIDs(rName, acctest.TLSPEMEscapeNewlines(key), acctest.TLSPEMEscapeNewlines(certificate)),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckVerifiedAccessEndpointExists(ctx, resourceName, &v),
+					resource.TestCheckResourceAttr(resourceName, "load_balancer_options.#", acctest.Ct1),
+					resource.TestCheckResourceAttr(resourceName, "load_balancer_options.0.subnet_ids.#", acctest.Ct1),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"endpoint_domain_prefix",
+				},
+			},
+			{
+				Config: testAccVerifiedAccessEndpointConfig_subnetIDsUpdate(rName, acctest.TLSPEMEscapeNewlines(key), acctest.TLSPEMEscapeNewlines(certificate)),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckVerifiedAccessEndpointExists(ctx, resourceName, &v),
+					resource.TestCheckResourceAttr(resourceName, "load_balancer_options.#", acctest.Ct1),
+					resource.TestCheckResourceAttr(resourceName, "load_balancer_options.0.subnet_ids.#", acctest.Ct2),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckVerifiedAccessEndpointDestroy(ctx context.Context) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Client(ctx)
@@ -298,8 +346,10 @@ func testAccCheckVerifiedAccessEndpointExists(ctx context.Context, n string, v *
 	}
 }
 
-func testAccVerifiedAccessEndpointConfig_base(rName, key, certificate string) string {
-	return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(`
+func testAccVerifiedAccessEndpointConfig_base(rName, key, certificate string, subnetCount int) string {
+	return acctest.ConfigCompose(
+		acctest.ConfigVPCWithSubnets(rName, subnetCount),
+		fmt.Sprintf(`
 resource "aws_security_group" "test" {
   name   = %[1]q
   vpc_id = aws_vpc.test.id
@@ -398,8 +448,9 @@ resource "aws_verifiedaccess_group" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_basic(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), fmt.Sprintf(`
-
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		fmt.Sprintf(`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -427,8 +478,9 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_networkInterface(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), fmt.Sprintf(`
-
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		fmt.Sprintf(`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -454,8 +506,9 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_tags1(rName, key, certificate, tagKey1, tagValue1 string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), fmt.Sprintf(`
-
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		fmt.Sprintf(`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -481,9 +534,9 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_tags2(rName, key, certificate, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), fmt.Sprintf(`
-
-
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		fmt.Sprintf(`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -508,7 +561,9 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_policyBase(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), `
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -528,7 +583,9 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 
 func testAccVerifiedAccessEndpointConfig_policyUpdate(rName, key, certificate, policyDocument string) string {
-	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate), fmt.Sprintf(`
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 1),
+		fmt.Sprintf(`
 resource "aws_verifiedaccess_endpoint" "test" {
   application_domain     = "example.com"
   attachment_type        = "vpc"
@@ -547,3 +604,61 @@ resource "aws_verifiedaccess_endpoint" "test" {
 }
 `, rName, key, certificate, policyDocument))
 }
+
+func testAccVerifiedAccessEndpointConfig_subnetIDs(rName, key, certificate string) string {
+	return acctest.ConfigCompose(
+		testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 2),
+		fmt.Sprintf(`
+resource "aws_verifiedaccess_endpoint" "test" {
+  application_domain     = "example.com"
+  attachment_type        = "vpc"
+  description            = "example"
+  domain_certificate_arn = aws_acm_certificate.test.arn
+  endpoint_domain_prefix = "example"
+  endpoint_type          = "load-balancer"
+  sse_specification {
+    customer_managed_key_enabled = false
+  }
+  load_balancer_options {
+    load_balancer_arn = aws_lb.test.arn
+    port              = 443
+    protocol          = "https"
+    subnet_ids        = [for subnet in slice(aws_subnet.test, 0, 1) : subnet.id]
+  }
+  security_group_ids       = [aws_security_group.test.id]
+  verified_access_group_id = aws_verifiedaccess_group.test.id
+
+  tags = {
+    Name = %[1]q
+  }
+}
+`, rName, key, certificate))
+}
+
+func testAccVerifiedAccessEndpointConfig_subnetIDsUpdate(rName, key, certificate string) string {
+	return acctest.ConfigCompose(testAccVerifiedAccessEndpointConfig_base(rName, key, certificate, 2), fmt.Sprintf(`
+resource "aws_verifiedaccess_endpoint" "test" {
+  application_domain     = "example.com"
+  attachment_type        = "vpc"
+  description            = "example"
+  domain_certificate_arn = aws_acm_certificate.test.arn
+  endpoint_domain_prefix = "example"
+  endpoint_type          = "load-balancer"
+  sse_specification {
+    customer_managed_key_enabled = false
+  }
+  load_balancer_options {
+    load_balancer_arn = aws_lb.test.arn
+    port              = 443
+    protocol          = "https"
+    subnet_ids        = [for subnet in aws_subnet.test : subnet.id]
+  }
+  security_group_ids       = [aws_security_group.test.id]
+  verified_access_group_id = aws_verifiedaccess_group.test.id
+
+  tags = {
+    Name = %[1]q
+  }
+}
+`, rName, key, certificate))
+}
diff --git a/internal/service/ec2/verifiedaccess_test.go b/internal/service/ec2/verifiedaccess_test.go
@@ -21,6 +21,7 @@ func TestAccVerifiedAccess_serial(t *testing.T) {
 			"tags":               testAccVerifiedAccessEndpoint_tags,
 			acctest.CtDisappears: testAccVerifiedAccessEndpoint_disappears,
 			"policyDocument":     testAccVerifiedAccessEndpoint_policyDocument,
+			"subnetIDs":          testAccVerifiedAccessEndpoint_subnetIDs,
 		},
 		"Group": {
 			acctest.CtBasic:      testAccVerifiedAccessGroup_basic,