From 8603cfc7b996a21ecff51665a94896d7b386463e Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 20 Nov 2023 15:01:43 -0800 Subject: [PATCH 01/10] Name correction --- internal/service/networkmanager/README.md | 4 ++-- .../docs/r/networkmanager_attachment_accepter.html.markdown | 4 ++-- .../docs/r/networkmanager_connect_attachment.html.markdown | 4 ++-- website/docs/r/networkmanager_connect_peer.html.markdown | 4 ++-- .../networkmanager_site_to_site_vpn_attachment.html.markdown | 4 ++-- website/docs/r/networkmanager_vpc_attachment.html.markdown | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/service/networkmanager/README.md b/internal/service/networkmanager/README.md index 332fd02e7dd..05c5f7e46b0 100644 --- a/internal/service/networkmanager/README.md +++ b/internal/service/networkmanager/README.md @@ -1,4 +1,4 @@ -# Terraform AWS Provider NetworkManager Package +# Terraform AWS Provider Network Manager Package This area is primarily for AWS provider contributors and maintainers. For information on _using_ Terraform and the AWS provider, see the links below. @@ -6,4 +6,4 @@ This area is primarily for AWS provider contributors and maintainers. For inform * [Find out about contributing](https://hashicorp.github.io/terraform-provider-aws/#contribute) to the AWS provider! * AWS Provider Docs: [Home](https://registry.terraform.io/providers/hashicorp/aws/latest/docs) -* AWS Docs: [AWS SDK for Go NetworkManager](https://docs.aws.amazon.com/sdk-for-go/api/service/networkmanager/) +* AWS Docs: [AWS SDK for Go Network Manager](https://docs.aws.amazon.com/sdk-for-go/api/service/networkmanager/) diff --git a/website/docs/r/networkmanager_attachment_accepter.html.markdown b/website/docs/r/networkmanager_attachment_accepter.html.markdown index cb0da00a9c6..7898d3a2ff4 100644 --- a/website/docs/r/networkmanager_attachment_accepter.html.markdown +++ b/website/docs/r/networkmanager_attachment_accepter.html.markdown @@ -3,12 +3,12 @@ subcategory: "Network Manager" layout: "aws" page_title: "AWS: aws_networkmanager_attachment_accepter" description: |- - Terraform resource for managing an AWS NetworkManager Attachment Accepter. + Terraform resource for managing an AWS Network Manager Attachment Accepter. --- # Resource: aws_networkmanager_attachment_accepter -Terraform resource for managing an AWS NetworkManager Attachment Accepter. +Terraform resource for managing an AWS Network Manager Attachment Accepter. ## Example Usage diff --git a/website/docs/r/networkmanager_connect_attachment.html.markdown b/website/docs/r/networkmanager_connect_attachment.html.markdown index 10beee0dfcd..a1b842710d3 100644 --- a/website/docs/r/networkmanager_connect_attachment.html.markdown +++ b/website/docs/r/networkmanager_connect_attachment.html.markdown @@ -3,12 +3,12 @@ subcategory: "Network Manager" layout: "aws" page_title: "AWS: aws_networkmanager_connect_attachment" description: |- - Terraform resource for managing an AWS NetworkManager ConnectAttachment. + Terraform resource for managing an AWS Network Manager ConnectAttachment. --- # Resource: aws_networkmanager_connect_attachment -Terraform resource for managing an AWS NetworkManager ConnectAttachment. +Terraform resource for managing an AWS Network Manager ConnectAttachment. ## Example Usage diff --git a/website/docs/r/networkmanager_connect_peer.html.markdown b/website/docs/r/networkmanager_connect_peer.html.markdown index 91a942a9ff6..1bbea37acb5 100644 --- a/website/docs/r/networkmanager_connect_peer.html.markdown +++ b/website/docs/r/networkmanager_connect_peer.html.markdown @@ -3,12 +3,12 @@ subcategory: "Network Manager" layout: "aws" page_title: "AWS: aws_networkmanager_connect_peer" description: |- - Terraform resource for managing an AWS NetworkManager Connect Peer. + Terraform resource for managing an AWS Network Manager Connect Peer. --- # Resource: aws_networkmanager_connect_peer -Terraform resource for managing an AWS NetworkManager Connect Peer. +Terraform resource for managing an AWS Network Manager Connect Peer. ## Example Usage diff --git a/website/docs/r/networkmanager_site_to_site_vpn_attachment.html.markdown b/website/docs/r/networkmanager_site_to_site_vpn_attachment.html.markdown index ec030c53d8b..645462e72c7 100644 --- a/website/docs/r/networkmanager_site_to_site_vpn_attachment.html.markdown +++ b/website/docs/r/networkmanager_site_to_site_vpn_attachment.html.markdown @@ -3,12 +3,12 @@ subcategory: "Network Manager" layout: "aws" page_title: "AWS: aws_networkmanager_site_to_site_vpn_attachment" description: |- - Terraform resource for managing an AWS NetworkManager SiteToSiteAttachment. + Terraform resource for managing an AWS Network Manager SiteToSiteAttachment. --- # Resource: aws_networkmanager_site_to_site_vpn_attachment -Terraform resource for managing an AWS NetworkManager SiteToSiteAttachment. +Terraform resource for managing an AWS Network Manager SiteToSiteAttachment. ## Example Usage diff --git a/website/docs/r/networkmanager_vpc_attachment.html.markdown b/website/docs/r/networkmanager_vpc_attachment.html.markdown index 25448c9ff1d..bb51f368fc0 100644 --- a/website/docs/r/networkmanager_vpc_attachment.html.markdown +++ b/website/docs/r/networkmanager_vpc_attachment.html.markdown @@ -3,12 +3,12 @@ subcategory: "Network Manager" layout: "aws" page_title: "AWS: aws_networkmanager_vpc_attachment" description: |- - Terraform resource for managing an AWS NetworkManager VpcAttachment. + Terraform resource for managing an AWS Network Manager VPC Attachment. --- # Resource: aws_networkmanager_vpc_attachment -Terraform resource for managing an AWS NetworkManager VpcAttachment. +Terraform resource for managing an AWS Network Manager VPC Attachment. ## Example Usage From 272a734473de122ad391e451ead5c2e9b2933797 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 18:02:28 -0800 Subject: [PATCH 02/10] Properly handles VPC attachment when not accepted --- .../networkmanager/attachment_accepter.go | 6 +- .../service/networkmanager/vpc_attachment.go | 109 +++- .../networkmanager/vpc_attachment_test.go | 522 ++++++++++++------ ...etworkmanager_vpc_attachment.html.markdown | 5 +- 4 files changed, 463 insertions(+), 179 deletions(-) diff --git a/internal/service/networkmanager/attachment_accepter.go b/internal/service/networkmanager/attachment_accepter.go index 39d87cb94c5..64095254cd9 100644 --- a/internal/service/networkmanager/attachment_accepter.go +++ b/internal/service/networkmanager/attachment_accepter.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// AttachmentAccepter does not require AttachmentType. However, querying attachments for status updates requires knowing tyupe +// AttachmentAccepter does not require AttachmentType. However, querying attachments for status updates requires knowing type // To facilitate querying and waiters on specific attachment types, attachment_type set to required // @SDKResource("aws_networkmanager_attachment_accepter") @@ -150,8 +150,8 @@ func resourceAttachmentAccepterCreate(ctx context.Context, d *schema.ResourceDat switch attachmentType { case networkmanager.AttachmentTypeVpc: - if _, err := waitVPCAttachmentCreated(ctx, conn, attachmentID, d.Timeout(schema.TimeoutCreate)); err != nil { - return diag.Errorf("waiting for Network Manager VPC Attachment (%s) create: %s", attachmentID, err) + if _, err := waitVPCAttachmentAvailable(ctx, conn, attachmentID, d.Timeout(schema.TimeoutCreate)); err != nil { + return diag.Errorf("waiting for Network Manager VPC Attachment (%s) to be attached: %s", attachmentID, err) } case networkmanager.AttachmentTypeSiteToSiteVpn: diff --git a/internal/service/networkmanager/vpc_attachment.go b/internal/service/networkmanager/vpc_attachment.go index 8567b9e63b8..00254f3d12d 100644 --- a/internal/service/networkmanager/vpc_attachment.go +++ b/internal/service/networkmanager/vpc_attachment.go @@ -14,9 +14,11 @@ import ( "github.com/aws/aws-sdk-go/service/networkmanager" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" @@ -37,10 +39,40 @@ func ResourceVPCAttachment() *schema.Resource { StateContext: schema.ImportStatePassthroughContext, }, - CustomizeDiff: verify.SetTagsDiff, + CustomizeDiff: customdiff.All( + verify.SetTagsDiff, + func(ctx context.Context, d *schema.ResourceDiff, meta any) error { + if d.Id() == "" { + return nil + } + + if !d.HasChange("options.0.appliance_mode_support") { + return nil + } + + if state := d.Get("state").(string); state == networkmanager.AttachmentStatePendingAttachmentAcceptance { + return d.ForceNew("options.0.appliance_mode_support") + } + return nil + }, + func(ctx context.Context, d *schema.ResourceDiff, meta any) error { + if d.Id() == "" { + return nil + } + + if !d.HasChange("options.0.ipv6_support") { + return nil + } + + if state := d.Get("state").(string); state == networkmanager.AttachmentStatePendingAttachmentAcceptance { + return d.ForceNew("options.0.ipv6_support") + } + return nil + }, + ), Timeouts: &schema.ResourceTimeout{ - Create: schema.DefaultTimeout(10 * time.Minute), + Create: schema.DefaultTimeout(15 * time.Minute), Update: schema.DefaultTimeout(10 * time.Minute), Delete: schema.DefaultTimeout(10 * time.Minute), }, @@ -253,6 +285,8 @@ func resourceVPCAttachmentUpdate(ctx context.Context, d *schema.ResourceData, me } func resourceVPCAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) // If ResourceAttachmentAccepter is used, then VPC Attachment state @@ -260,7 +294,7 @@ func resourceVPCAttachmentDelete(ctx context.Context, d *schema.ResourceData, me output, sErr := FindVPCAttachmentByID(ctx, conn, d.Id()) if tfawserr.ErrCodeEquals(sErr, networkmanager.ErrCodeResourceNotFoundException) { - return nil + return diags } if sErr != nil { @@ -270,7 +304,16 @@ func resourceVPCAttachmentDelete(ctx context.Context, d *schema.ResourceData, me d.Set("state", output.Attachment.State) if state := d.Get("state").(string); state == networkmanager.AttachmentStatePendingAttachmentAcceptance || state == networkmanager.AttachmentStatePendingTagAcceptance { - return diag.Errorf("cannot delete Network Manager VPC Attachment (%s) in %s state", d.Id(), state) + _, err := conn.RejectAttachmentWithContext(ctx, &networkmanager.RejectAttachmentInput{ + AttachmentId: aws.String(d.Id()), + }) + if err != nil { + return sdkdiag.AppendErrorf(diags, "detaching Network Manager VPC Attachment (%s): %s", d.Id(), err) + } + + if _, err := waitVPCAttachmenRejected(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for Network Manager VPC Attachment (%s) to be detached: %s", d.Id(), err) + } } log.Printf("[DEBUG] Deleting Network Manager VPC Attachment: %s", d.Id()) @@ -279,18 +322,18 @@ func resourceVPCAttachmentDelete(ctx context.Context, d *schema.ResourceData, me }) if tfawserr.ErrCodeEquals(err, networkmanager.ErrCodeResourceNotFoundException) { - return nil + return diags } if err != nil { - return diag.Errorf("deleting Network Manager VPC Attachment (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting Network Manager VPC Attachment (%s): %s", d.Id(), err) } if _, err := waitVPCAttachmentDeleted(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { - return diag.Errorf("waiting for Network Manager VPC Attachment (%s) delete: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "waiting for Network Manager VPC Attachment (%s) delete: %s", d.Id(), err) } - return nil + return diags } func FindVPCAttachmentByID(ctx context.Context, conn *networkmanager.NetworkManager, id string) (*networkmanager.VpcAttachment, error) { @@ -336,8 +379,54 @@ func StatusVPCAttachmentState(ctx context.Context, conn *networkmanager.NetworkM func waitVPCAttachmentCreated(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam stateConf := &retry.StateChangeConf{ - Pending: []string{networkmanager.AttachmentStateCreating, networkmanager.AttachmentStatePendingNetworkUpdate}, - Target: []string{networkmanager.AttachmentStateAvailable, networkmanager.AttachmentStatePendingAttachmentAcceptance}, + Pending: []string{ + networkmanager.AttachmentStateCreating, + networkmanager.AttachmentStatePendingNetworkUpdate, + }, + Target: []string{ + networkmanager.AttachmentStateAvailable, + networkmanager.AttachmentStatePendingAttachmentAcceptance, + }, + Timeout: timeout, + Refresh: StatusVPCAttachmentState(ctx, conn, id), + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*networkmanager.VpcAttachment); ok { + return output, err + } + + return nil, err +} + +func waitVPCAttachmentAvailable(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam + stateConf := &retry.StateChangeConf{ + Pending: []string{ + networkmanager.AttachmentStateCreating, + networkmanager.AttachmentStatePendingNetworkUpdate, + networkmanager.AttachmentStatePendingAttachmentAcceptance, + }, + Target: []string{ + networkmanager.AttachmentStateAvailable, + }, + Timeout: timeout, + Refresh: StatusVPCAttachmentState(ctx, conn, id), + } + + outputRaw, err := stateConf.WaitForStateContext(ctx) + + if output, ok := outputRaw.(*networkmanager.VpcAttachment); ok { + return output, err + } + + return nil, err +} + +func waitVPCAttachmenRejected(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam + stateConf := &retry.StateChangeConf{ + Pending: []string{networkmanager.AttachmentStatePendingAttachmentAcceptance, networkmanager.AttachmentStateAvailable}, + Target: []string{networkmanager.AttachmentStateRejected}, Timeout: timeout, Refresh: StatusVPCAttachmentState(ctx, conn, id), } diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index 6aa0ad8f6c8..e205d9acb8f 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/YakDriver/regexache" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/networkmanager" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -20,11 +21,148 @@ import ( ) func TestAccNetworkManagerVPCAttachment_basic(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + coreNetworkResourceName = "aws_networkmanager_core_network.test" + vpcResourceName = "aws_vpc.test" + ) + + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + expectedState string + }{ + "acceptance_required": { + acceptanceRequired: true, + expectedState: networkmanager.AttachmentStatePendingAttachmentAcceptance, + }, + + "acceptance_not_required": { + acceptanceRequired: false, + expectedState: networkmanager.AttachmentStateAvailable, + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v networkmanager.VpcAttachment + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_basic(rName, testcase.acceptanceRequired), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v), + acctest.MatchResourceAttrGlobalARN(resourceName, "arn", "networkmanager", regexache.MustCompile(`attachment/.+`)), + resource.TestCheckResourceAttr(resourceName, "attachment_policy_rule_number", "1"), + resource.TestCheckResourceAttr(resourceName, "attachment_type", "VPC"), + resource.TestCheckResourceAttrPair(resourceName, "core_network_arn", coreNetworkResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "core_network_id", coreNetworkResourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "edge_location", acctest.Region()), + resource.TestCheckResourceAttr(resourceName, "options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + acctest.CheckResourceAttrAccountID(resourceName, "owner_account_id"), + resource.TestCheckResourceAttrPair(resourceName, "resource_arn", vpcResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "segment_name", "shared"), + resource.TestCheckResourceAttr(resourceName, "state", testcase.expectedState), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttrPair(resourceName, "vpc_arn", vpcResourceName, "arn"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) + } +} + +func TestAccNetworkManagerVPCAttachment_Attached_basic(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + coreNetworkResourceName = "aws_networkmanager_core_network.test" + vpcResourceName = "aws_vpc.test" + ) + + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + }{ + "acceptance_required": { + acceptanceRequired: true, + }, + + "acceptance_not_required": { + acceptanceRequired: false, + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v networkmanager.VpcAttachment + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_Attached_basic(rName, testcase.acceptanceRequired), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v), + acctest.MatchResourceAttrGlobalARN(resourceName, "arn", "networkmanager", regexache.MustCompile(`attachment/.+`)), + resource.TestCheckResourceAttr(resourceName, "attachment_policy_rule_number", "1"), + resource.TestCheckResourceAttr(resourceName, "attachment_type", "VPC"), + resource.TestCheckResourceAttrPair(resourceName, "core_network_arn", coreNetworkResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "core_network_id", coreNetworkResourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "edge_location", acctest.Region()), + resource.TestCheckResourceAttr(resourceName, "options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + acctest.CheckResourceAttrAccountID(resourceName, "owner_account_id"), + resource.TestCheckResourceAttrPair(resourceName, "resource_arn", vpcResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "segment_name", "shared"), + resource.TestCheckResourceAttrSet(resourceName, "state"), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttrPair(resourceName, "vpc_arn", vpcResourceName, "arn"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) + } +} + +func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { ctx := acctest.Context(t) var v networkmanager.VpcAttachment resourceName := "aws_networkmanager_vpc_attachment.test" - coreNetworkResourceName := "aws_networkmanager_core_network.test" - vpcResourceName := "aws_vpc.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -34,37 +172,18 @@ func TestAccNetworkManagerVPCAttachment_basic(t *testing.T) { CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccVPCAttachmentConfig_basic(rName), - Check: resource.ComposeAggregateTestCheckFunc( + Config: testAccVPCAttachmentConfig_basic(rName, false), + Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v), - acctest.MatchResourceAttrGlobalARN(resourceName, "arn", "networkmanager", regexache.MustCompile(`attachment/.+`)), - resource.TestCheckResourceAttr(resourceName, "attachment_policy_rule_number", "0"), - resource.TestCheckResourceAttr(resourceName, "attachment_type", "VPC"), - resource.TestCheckResourceAttrPair(resourceName, "core_network_arn", coreNetworkResourceName, "arn"), - resource.TestCheckResourceAttrSet(resourceName, "core_network_id"), - resource.TestCheckResourceAttr(resourceName, "edge_location", acctest.Region()), - resource.TestCheckResourceAttr(resourceName, "options.#", "1"), - resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), - resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), - acctest.CheckResourceAttrAccountID(resourceName, "owner_account_id"), - resource.TestCheckResourceAttrPair(resourceName, "resource_arn", vpcResourceName, "arn"), - resource.TestCheckResourceAttr(resourceName, "segment_name", ""), - resource.TestCheckResourceAttrSet(resourceName, "state"), - resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), - resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttrPair(resourceName, "vpc_arn", vpcResourceName, "arn"), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ExpectNonEmptyPlan: true, }, }, }) } -func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { +func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { ctx := acctest.Context(t) var v networkmanager.VpcAttachment resourceName := "aws_networkmanager_vpc_attachment.test" @@ -77,7 +196,7 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccVPCAttachmentConfig_basic(rName), + Config: testAccVPCAttachmentConfig_Attached_basic(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), @@ -135,58 +254,94 @@ func TestAccNetworkManagerVPCAttachment_tags(t *testing.T) { } func TestAccNetworkManagerVPCAttachment_update(t *testing.T) { - ctx := acctest.Context(t) - var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + coreNetworkResourceName = "aws_networkmanager_core_network.test" + vpcResourceName = "aws_vpc.test" + ) + + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + expectedState string + expectRecreation bool + }{ + "acceptance_required": { + acceptanceRequired: true, + expectedState: networkmanager.AttachmentStatePendingAttachmentAcceptance, + expectRecreation: true, + }, - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), - Steps: []resource.TestStep{ - { - Config: testAccVPCAttachmentConfig_updates(rName, 2, true, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckVPCAttachmentExists(ctx, resourceName, &v), - resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), - resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "true"), - resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), - ), - }, - { - Config: testAccVPCAttachmentConfig_updates(rName, 1, false, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "1"), - resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), - resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), - ), - }, - { - Config: testAccVPCAttachmentConfig_updates(rName, 2, false, false), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), - resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), - resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), - ), - }, - // Cannot currently update ipv6 on its own, must also update subnet_arn - { - Config: testAccVPCAttachmentConfig_updates(rName, 2, true, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), - resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "true"), - resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, + "acceptance_not_required": { + acceptanceRequired: false, + expectedState: networkmanager.AttachmentStateAvailable, + expectRecreation: false, }, - }) + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v1, v2, v3, v4 networkmanager.VpcAttachment + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_updates(rName, testcase.acceptanceRequired, 2, true, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v1), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "true"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + ), + }, + { + Config: testAccVPCAttachmentConfig_updates(rName, testcase.acceptanceRequired, 1, false, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v2), + testAccCheckVPCAttachmentRecreated(&v1, &v2, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "1"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), + ), + }, + { + Config: testAccVPCAttachmentConfig_updates(rName, testcase.acceptanceRequired, 2, false, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v3), + testAccCheckVPCAttachmentRecreated(&v2, &v3, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + ), + }, + { + Config: testAccVPCAttachmentConfig_updates(rName, testcase.acceptanceRequired, 2, false, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v4), + testAccCheckVPCAttachmentRecreated(&v3, &v4, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) + } } func testAccCheckVPCAttachmentExists(ctx context.Context, n string, v *networkmanager.VpcAttachment) resource.TestCheckFunc { @@ -240,8 +395,127 @@ func testAccCheckVPCAttachmentDestroy(ctx context.Context) resource.TestCheckFun } } -func testAccVPCAttachmentConfig_base(rName string) string { - return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(` +func testAccCheckVPCAttachmentRecreated(v1, v2 *networkmanager.VpcAttachment, expectRecreation bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + return testAccCheckAttachmentRecreated(v1.Attachment, v2.Attachment, expectRecreation) + } +} + +func testAccCheckAttachmentRecreated(v1, v2 *networkmanager.Attachment, expectRecreation bool) error { + v1CreatedAt := aws.TimeValue(v1.CreatedAt) + v2CreatedAt := aws.TimeValue(v2.CreatedAt) + cmp := v1CreatedAt.Compare(v2CreatedAt) + if expectRecreation && cmp != -1 { + return fmt.Errorf("Attachment not recreated: v1.CreatedAt=%q, v2.CreatedAt=%q", v1CreatedAt, v2CreatedAt) + } else if !expectRecreation && cmp != 0 { + return fmt.Errorf("Attachment recreated: v1.CreatedAt=%q, v2.CreatedAt=%q", v1CreatedAt, v2CreatedAt) + } + return nil +} + +func testAccVPCAttachmentConfig_basic(rName string, requireAcceptance bool) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, requireAcceptance), ` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = aws_subnet.test[*].arn + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn +} +`) +} + +func testAccVPCAttachmentConfig_Attached_basic(rName string, requireAcceptance bool) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, requireAcceptance), ` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = aws_subnet.test[*].arn + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn +} + +resource "aws_networkmanager_attachment_accepter" "test" { + attachment_id = aws_networkmanager_vpc_attachment.test.id + attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type +} +`) +} + +func testAccVPCAttachmentConfig_tags1(rName, tagKey1, tagValue1 string) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, false), + fmt.Sprintf(` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = [aws_subnet.test[0].arn] + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn + + tags = { + %[1]q = %[2]q + } +} +`, tagKey1, tagValue1)) +} + +func testAccVPCAttachmentConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, false), + fmt.Sprintf(` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = [aws_subnet.test[0].arn] + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn + + tags = { + %[1]q = %[2]q + %[3]q = %[4]q + } +} +`, tagKey1, tagValue1, tagKey2, tagValue2)) +} + +func testAccVPCAttachmentConfig_updates(rName string, requireAcceptance bool, nSubnets int, applianceModeSupport, ipv6Support bool) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, requireAcceptance), + fmt.Sprintf(` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = slice(aws_subnet.test[*].arn, 0, %[2]d) + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn + + options { + appliance_mode_support = %[3]t + ipv6_support = %[4]t + } +} +`, rName, nSubnets, applianceModeSupport, ipv6Support)) +} + +func testAccVPCAttachmentConfig_Attached_updates(rName string, requireAcceptance bool, nSubnets int, applianceModeSupport, ipv6Support bool) string { + return acctest.ConfigCompose( + testAccVPCAttachmentConfig_base(rName, requireAcceptance), + fmt.Sprintf(` +resource "aws_networkmanager_vpc_attachment" "test" { + subnet_arns = slice(aws_subnet.test[*].arn, 0, %[2]d) + core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id + vpc_arn = aws_vpc.test.arn + + options { + appliance_mode_support = %[3]t + ipv6_support = %[4]t + } +} + +resource "aws_networkmanager_attachment_accepter" "test" { + attachment_id = aws_networkmanager_vpc_attachment.test.id + attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type +} +`, rName, nSubnets, applianceModeSupport, ipv6Support)) +} + +func testAccVPCAttachmentConfig_base(rName string, requireAcceptance bool) string { + return acctest.ConfigCompose( + acctest.ConfigAvailableAZsNoOptIn(), + fmt.Sprintf(` data "aws_region" "current" {} resource "aws_vpc" "test" { @@ -301,7 +575,7 @@ data "aws_networkmanager_core_network_policy_document" "test" { segments { name = "shared" description = "SegmentForSharedServices" - require_attachment_acceptance = true + require_attachment_acceptance = %[2]t } segment_actions { @@ -312,14 +586,10 @@ data "aws_networkmanager_core_network_policy_document" "test" { } attachment_policies { - rule_number = 1 - condition_logic = "or" + rule_number = 1 conditions { - type = "tag-value" - operator = "equals" - key = "segment" - value = "shared" + type = "any" } action { @@ -328,83 +598,5 @@ data "aws_networkmanager_core_network_policy_document" "test" { } } } -`, rName)) -} - -func testAccVPCAttachmentConfig_basic(rName string) string { - return acctest.ConfigCompose(testAccVPCAttachmentConfig_base(rName), ` -resource "aws_networkmanager_vpc_attachment" "test" { - subnet_arns = aws_subnet.test[*].arn - core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id - vpc_arn = aws_vpc.test.arn -} - -resource "aws_networkmanager_attachment_accepter" "test" { - attachment_id = aws_networkmanager_vpc_attachment.test.id - attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type -} -`) -} - -func testAccVPCAttachmentConfig_tags1(rName, tagKey1, tagValue1 string) string { - return acctest.ConfigCompose(testAccVPCAttachmentConfig_base(rName), fmt.Sprintf(` -resource "aws_networkmanager_vpc_attachment" "test" { - subnet_arns = [aws_subnet.test[0].arn] - core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id - vpc_arn = aws_vpc.test.arn - - tags = { - %[1]q = %[2]q - } -} - -resource "aws_networkmanager_attachment_accepter" "test" { - attachment_id = aws_networkmanager_vpc_attachment.test.id - attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type -} -`, tagKey1, tagValue1)) -} - -func testAccVPCAttachmentConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { - return acctest.ConfigCompose(testAccVPCAttachmentConfig_base(rName), fmt.Sprintf(` -resource "aws_networkmanager_vpc_attachment" "test" { - subnet_arns = [aws_subnet.test[0].arn] - core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id - vpc_arn = aws_vpc.test.arn - - tags = { - %[1]q = %[2]q - %[3]q = %[4]q - } -} - -resource "aws_networkmanager_attachment_accepter" "test" { - attachment_id = aws_networkmanager_vpc_attachment.test.id - attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type -} -`, tagKey1, tagValue1, tagKey2, tagValue2)) -} - -func testAccVPCAttachmentConfig_updates(rName string, nSubnets int, applianceModeSupport, ipv6Support bool) string { - return acctest.ConfigCompose(testAccVPCAttachmentConfig_base(rName), fmt.Sprintf(` -resource "aws_networkmanager_vpc_attachment" "test" { - subnet_arns = slice(aws_subnet.test[*].arn, 0, %[2]d) - core_network_id = aws_networkmanager_core_network_policy_attachment.test.core_network_id - vpc_arn = aws_vpc.test.arn - - options { - appliance_mode_support = %[3]t - ipv6_support = %[4]t - } - - tags = { - Name = %[1]q - } -} - -resource "aws_networkmanager_attachment_accepter" "test" { - attachment_id = aws_networkmanager_vpc_attachment.test.id - attachment_type = aws_networkmanager_vpc_attachment.test.attachment_type -} -`, rName, nSubnets, applianceModeSupport, ipv6Support)) +`, rName, requireAcceptance)) } diff --git a/website/docs/r/networkmanager_vpc_attachment.html.markdown b/website/docs/r/networkmanager_vpc_attachment.html.markdown index bb51f368fc0..ccdd14efbc1 100644 --- a/website/docs/r/networkmanager_vpc_attachment.html.markdown +++ b/website/docs/r/networkmanager_vpc_attachment.html.markdown @@ -37,8 +37,11 @@ The following arguments are optional: ### options -* `appliance_mode_support` - (Optional) Indicates whether appliance mode is supported. If enabled, traffic flow between a source and destination use the same Availability Zone for the VPC attachment for the lifetime of that flow. +* `appliance_mode_support` - (Optional) Indicates whether appliance mode is supported. + If enabled, traffic flow between a source and destination use the same Availability Zone for the VPC attachment for the lifetime of that flow. + If the VPC attachment is pending acceptance, changing this value will recreate the resource. * `ipv6_support` - (Optional) Indicates whether IPv6 is supported. + If the VPC attachment is pending acceptance, changing this value will recreate the resource. ## Attribute Reference From f68581f20988787d687e2ff352d5761ec1c59df4 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 22 Nov 2023 10:50:07 -0800 Subject: [PATCH 03/10] Deletes VPC attachment when accepter deleted --- .../networkmanager/attachment_accepter.go | 31 ++++- .../service/networkmanager/core_network.go | 2 +- .../networkmanager/vpc_attachment_test.go | 118 ++++++++++++++---- 3 files changed, 128 insertions(+), 23 deletions(-) diff --git a/internal/service/networkmanager/attachment_accepter.go b/internal/service/networkmanager/attachment_accepter.go index 64095254cd9..d9528fe65c7 100644 --- a/internal/service/networkmanager/attachment_accepter.go +++ b/internal/service/networkmanager/attachment_accepter.go @@ -10,10 +10,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/networkmanager" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) @@ -25,7 +27,7 @@ func ResourceAttachmentAccepter() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceAttachmentAccepterCreate, ReadWithoutTimeout: resourceAttachmentAccepterRead, - DeleteWithoutTimeout: schema.NoopContext, + DeleteWithoutTimeout: resourceAttachmentAccepterDelete, Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(10 * time.Minute), @@ -252,3 +254,30 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, return nil } + +func resourceAttachmentAccepterDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) + + switch d.Get("attachment_type") { + case networkmanager.AttachmentTypeVpc: + _, err := conn.DeleteAttachmentWithContext(ctx, &networkmanager.DeleteAttachmentInput{ + AttachmentId: aws.String(d.Id()), + }) + + if tfawserr.ErrCodeEquals(err, networkmanager.ErrCodeResourceNotFoundException) { + return diags + } + + if err != nil { + return sdkdiag.AppendErrorf(diags, "deleting Network Manager VPC Attachment (%s): %s", d.Id(), err) + } + + if _, err := waitVPCAttachmentDeleted(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for Network Manager VPC Attachment (%s) delete: %s", d.Id(), err) + } + } + + return diags +} diff --git a/internal/service/networkmanager/core_network.go b/internal/service/networkmanager/core_network.go index c74e3a39b14..24b11f9afb0 100644 --- a/internal/service/networkmanager/core_network.go +++ b/internal/service/networkmanager/core_network.go @@ -36,7 +36,7 @@ const ( // Using the following in the FindCoreNetworkPolicyByID function will default to get the latest policy version latestPolicyVersionID = -1 // Wait time value for core network policy - the default update for the core network policy of 30 minutes is excessive - waitCoreNetworkPolicyCreatedTimeInMinutes = 4 + waitCoreNetworkPolicyCreatedTimeInMinutes = 5 ) // @SDKResource("aws_networkmanager_core_network", name="Core Network") diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index e205d9acb8f..54e16a83d6e 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -160,33 +160,109 @@ func TestAccNetworkManagerVPCAttachment_Attached_basic(t *testing.T) { } func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { - ctx := acctest.Context(t) - var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + coreNetworkResourceName = "aws_networkmanager_core_network.test" + vpcResourceName = "aws_vpc.test" + ) - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), - Steps: []resource.TestStep{ - { - Config: testAccVPCAttachmentConfig_basic(rName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckVPCAttachmentExists(ctx, resourceName, &v), - acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), - ), - ExpectNonEmptyPlan: true, - }, + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + }{ + "acceptance_required": { + acceptanceRequired: true, }, - }) + + "acceptance_not_required": { + acceptanceRequired: false, + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v networkmanager.VpcAttachment + resourceName := "aws_networkmanager_vpc_attachment.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_basic(rName, testcase.acceptanceRequired), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) + }) + } } func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + coreNetworkResourceName = "aws_networkmanager_core_network.test" + vpcResourceName = "aws_vpc.test" + ) + + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + }{ + "acceptance_required": { + acceptanceRequired: true, + }, + + "acceptance_not_required": { + acceptanceRequired: false, + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v networkmanager.VpcAttachment + resourceName := "aws_networkmanager_vpc_attachment.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_Attached_basic(rName, testcase.acceptanceRequired), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) + }) + } +} + +func TestAccNetworkManagerVPCAttachment_Attached_disappearsAccepter(t *testing.T) { ctx := acctest.Context(t) var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" + resourceName := "aws_networkmanager_attachment_accepter.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -199,7 +275,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { Config: testAccVPCAttachmentConfig_Attached_basic(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v), - acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceAttachmentAccepter(), resourceName), ), ExpectNonEmptyPlan: true, }, From 32e13fab9dc5a1a1b41769e7a6747cb81ee08065 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 22 Nov 2023 17:07:23 -0800 Subject: [PATCH 04/10] Adds plan checks for `disappears` tests --- .../networkmanager/vpc_attachment_test.go | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index 54e16a83d6e..f893c947cba 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go/service/networkmanager" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -161,9 +162,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_basic(t *testing.T) { func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { const ( - resourceName = "aws_networkmanager_vpc_attachment.test" - coreNetworkResourceName = "aws_networkmanager_core_network.test" - vpcResourceName = "aws_vpc.test" + resourceName = "aws_networkmanager_vpc_attachment.test" ) t.Parallel() @@ -186,7 +185,6 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := acctest.Context(t) var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -202,6 +200,11 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), ), ExpectNonEmptyPlan: true, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate), + }, + }, }, }, }) @@ -211,9 +214,8 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { const ( - resourceName = "aws_networkmanager_vpc_attachment.test" - coreNetworkResourceName = "aws_networkmanager_core_network.test" - vpcResourceName = "aws_vpc.test" + resourceName = "aws_networkmanager_vpc_attachment.test" + attachmentResourceName = "aws_networkmanager_attachment_accepter.test" ) t.Parallel() @@ -236,7 +238,6 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := acctest.Context(t) var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -252,6 +253,12 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceVPCAttachment(), resourceName), ), ExpectNonEmptyPlan: true, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate), + plancheck.ExpectResourceAction(attachmentResourceName, plancheck.ResourceActionCreate), + }, + }, }, }, }) @@ -260,9 +267,13 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { } func TestAccNetworkManagerVPCAttachment_Attached_disappearsAccepter(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + attachmentResourceName = "aws_networkmanager_attachment_accepter.test" + ) + ctx := acctest.Context(t) var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_attachment_accepter.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -278,15 +289,24 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappearsAccepter(t *testing.T acctest.CheckResourceDisappears(ctx, acctest.Provider, tfnetworkmanager.ResourceAttachmentAccepter(), resourceName), ), ExpectNonEmptyPlan: true, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate), + plancheck.ExpectResourceAction(attachmentResourceName, plancheck.ResourceActionCreate), + }, + }, }, }, }) } func TestAccNetworkManagerVPCAttachment_tags(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + ) + ctx := acctest.Context(t) var v networkmanager.VpcAttachment - resourceName := "aws_networkmanager_vpc_attachment.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ @@ -331,9 +351,7 @@ func TestAccNetworkManagerVPCAttachment_tags(t *testing.T) { func TestAccNetworkManagerVPCAttachment_update(t *testing.T) { const ( - resourceName = "aws_networkmanager_vpc_attachment.test" - coreNetworkResourceName = "aws_networkmanager_core_network.test" - vpcResourceName = "aws_vpc.test" + resourceName = "aws_networkmanager_vpc_attachment.test" ) t.Parallel() From 42379418532c76ebee785e5dc8ad82eb50165a6a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 23 Nov 2023 09:53:05 -0800 Subject: [PATCH 05/10] Linting fixes --- .../networkmanager/attachment_accepter.go | 2 +- .../service/networkmanager/vpc_attachment.go | 8 +- .../networkmanager/vpc_attachment_test.go | 99 ++++++++++++++++++- 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/internal/service/networkmanager/attachment_accepter.go b/internal/service/networkmanager/attachment_accepter.go index d9528fe65c7..4973a30e3a8 100644 --- a/internal/service/networkmanager/attachment_accepter.go +++ b/internal/service/networkmanager/attachment_accepter.go @@ -30,7 +30,7 @@ func ResourceAttachmentAccepter() *schema.Resource { DeleteWithoutTimeout: resourceAttachmentAccepterDelete, Timeouts: &schema.ResourceTimeout{ - Create: schema.DefaultTimeout(10 * time.Minute), + Create: schema.DefaultTimeout(15 * time.Minute), }, Schema: map[string]*schema.Schema{ diff --git a/internal/service/networkmanager/vpc_attachment.go b/internal/service/networkmanager/vpc_attachment.go index 00254f3d12d..f6bf054ceeb 100644 --- a/internal/service/networkmanager/vpc_attachment.go +++ b/internal/service/networkmanager/vpc_attachment.go @@ -377,7 +377,7 @@ func StatusVPCAttachmentState(ctx context.Context, conn *networkmanager.NetworkM } } -func waitVPCAttachmentCreated(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam +func waitVPCAttachmentCreated(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ networkmanager.AttachmentStateCreating, @@ -400,7 +400,7 @@ func waitVPCAttachmentCreated(ctx context.Context, conn *networkmanager.NetworkM return nil, err } -func waitVPCAttachmentAvailable(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam +func waitVPCAttachmentAvailable(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { stateConf := &retry.StateChangeConf{ Pending: []string{ networkmanager.AttachmentStateCreating, @@ -423,7 +423,7 @@ func waitVPCAttachmentAvailable(ctx context.Context, conn *networkmanager.Networ return nil, err } -func waitVPCAttachmenRejected(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam +func waitVPCAttachmenRejected(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { stateConf := &retry.StateChangeConf{ Pending: []string{networkmanager.AttachmentStatePendingAttachmentAcceptance, networkmanager.AttachmentStateAvailable}, Target: []string{networkmanager.AttachmentStateRejected}, @@ -440,7 +440,7 @@ func waitVPCAttachmenRejected(ctx context.Context, conn *networkmanager.NetworkM return nil, err } -func waitVPCAttachmentDeleted(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { +func waitVPCAttachmentDeleted(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.VpcAttachment, error) { //nolint:unparam stateConf := &retry.StateChangeConf{ Pending: []string{networkmanager.AttachmentStateDeleting}, Target: []string{}, diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index f893c947cba..7012a203db9 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -45,7 +45,7 @@ func TestAccNetworkManagerVPCAttachment_basic(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest // false positive testcase := testcase t.Run(name, func(t *testing.T) { @@ -113,7 +113,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_basic(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest // false positive testcase := testcase t.Run(name, func(t *testing.T) { @@ -179,7 +179,7 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest // false positive testcase := testcase t.Run(name, func(t *testing.T) { @@ -232,7 +232,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest // false positive testcase := testcase t.Run(name, func(t *testing.T) { @@ -374,7 +374,7 @@ func TestAccNetworkManagerVPCAttachment_update(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest // false positive testcase := testcase t.Run(name, func(t *testing.T) { @@ -438,6 +438,95 @@ func TestAccNetworkManagerVPCAttachment_update(t *testing.T) { } } +func TestAccNetworkManagerVPCAttachment_Attached_update(t *testing.T) { + const ( + resourceName = "aws_networkmanager_vpc_attachment.test" + ) + + t.Parallel() + + testcases := map[string]struct { + acceptanceRequired bool + expectedState string + expectRecreation bool + }{ + "acceptance_required": { + acceptanceRequired: true, + expectedState: networkmanager.AttachmentStatePendingAttachmentAcceptance, + expectRecreation: true, + }, + + "acceptance_not_required": { + acceptanceRequired: false, + expectedState: networkmanager.AttachmentStateAvailable, + expectRecreation: false, + }, + } + + for name, testcase := range testcases { //nolint:paralleltest // false positive + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v1, v2, v3, v4 networkmanager.VpcAttachment + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkmanager.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 2, true, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v1), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "true"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + ), + }, + { + Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 1, false, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v2), + testAccCheckVPCAttachmentRecreated(&v1, &v2, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "1"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), + ), + }, + { + Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 2, false, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v3), + testAccCheckVPCAttachmentRecreated(&v2, &v3, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), + ), + }, + { + Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 2, false, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckVPCAttachmentExists(ctx, resourceName, &v4), + testAccCheckVPCAttachmentRecreated(&v3, &v4, testcase.expectRecreation), + resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), + resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), + resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) + } +} + func testAccCheckVPCAttachmentExists(ctx context.Context, n string, v *networkmanager.VpcAttachment) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From 9fc9fa91b07f338f92ff1eb4cd0960301a52160f Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 23 Nov 2023 09:54:26 -0800 Subject: [PATCH 06/10] Adds timing for Core Network retries --- internal/service/networkmanager/core_network.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/service/networkmanager/core_network.go b/internal/service/networkmanager/core_network.go index 24b11f9afb0..40d254e7638 100644 --- a/internal/service/networkmanager/core_network.go +++ b/internal/service/networkmanager/core_network.go @@ -436,10 +436,12 @@ func waitCoreNetworkUpdated(ctx context.Context, conn *networkmanager.NetworkMan func waitCoreNetworkDeleted(ctx context.Context, conn *networkmanager.NetworkManager, id string, timeout time.Duration) (*networkmanager.CoreNetwork, error) { stateConf := &retry.StateChangeConf{ - Pending: []string{networkmanager.CoreNetworkStateDeleting}, - Target: []string{}, - Timeout: timeout, - Refresh: statusCoreNetworkState(ctx, conn, id), + Pending: []string{networkmanager.CoreNetworkStateDeleting}, + Target: []string{}, + Timeout: timeout, + Delay: 5 * time.Minute, + MinTimeout: 10 * time.Second, + Refresh: statusCoreNetworkState(ctx, conn, id), } outputRaw, err := stateConf.WaitForStateContext(ctx) From 96e1dd9ef808d9abe266118d008923ad7024a286 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 23 Nov 2023 10:13:36 -0800 Subject: [PATCH 07/10] Updates diagnostics handling --- .../networkmanager/attachment_accepter.go | 16 ++++++++++------ .../service/networkmanager/vpc_attachment.go | 14 ++++++++++---- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/internal/service/networkmanager/attachment_accepter.go b/internal/service/networkmanager/attachment_accepter.go index 4973a30e3a8..ffc72827450 100644 --- a/internal/service/networkmanager/attachment_accepter.go +++ b/internal/service/networkmanager/attachment_accepter.go @@ -84,6 +84,8 @@ func ResourceAttachmentAccepter() *schema.Resource { } func resourceAttachmentAccepterCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) var state string @@ -173,10 +175,12 @@ func resourceAttachmentAccepterCreate(ctx context.Context, d *schema.ResourceDat } } - return resourceAttachmentAccepterRead(ctx, d, meta) + return append(diags, resourceAttachmentAccepterRead(ctx, d, meta)...) } func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) var a *networkmanager.Attachment @@ -188,7 +192,7 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Network Manager VPC Attachment %s not found, removing from state", d.Id()) d.SetId("") - return nil + return diags } if err != nil { @@ -203,7 +207,7 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Network Manager Site To Site VPN Attachment %s not found, removing from state", d.Id()) d.SetId("") - return nil + return diags } if err != nil { @@ -218,7 +222,7 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Network Manager Connect Attachment %s not found, removing from state", d.Id()) d.SetId("") - return nil + return diags } if err != nil { @@ -233,7 +237,7 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Network Manager Transit Gateway Route Table Attachment %s not found, removing from state", d.Id()) d.SetId("") - return nil + return diags } if err != nil { @@ -252,7 +256,7 @@ func resourceAttachmentAccepterRead(ctx context.Context, d *schema.ResourceData, d.Set("segment_name", a.SegmentName) d.Set("state", a.State) - return nil + return diags } func resourceAttachmentAccepterDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { diff --git a/internal/service/networkmanager/vpc_attachment.go b/internal/service/networkmanager/vpc_attachment.go index f6bf054ceeb..6d58c5a35b6 100644 --- a/internal/service/networkmanager/vpc_attachment.go +++ b/internal/service/networkmanager/vpc_attachment.go @@ -158,6 +158,8 @@ func ResourceVPCAttachment() *schema.Resource { } func resourceVPCAttachmentCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) coreNetworkID := d.Get("core_network_id").(string) @@ -186,10 +188,12 @@ func resourceVPCAttachmentCreate(ctx context.Context, d *schema.ResourceData, me return diag.Errorf("waiting for Network Manager VPC Attachment (%s) create: %s", d.Id(), err) } - return resourceVPCAttachmentRead(ctx, d, meta) + return append(diags, resourceVPCAttachmentRead(ctx, d, meta)...) } func resourceVPCAttachmentRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) vpcAttachment, err := FindVPCAttachmentByID(ctx, conn, d.Id()) @@ -197,7 +201,7 @@ func resourceVPCAttachmentRead(ctx context.Context, d *schema.ResourceData, meta if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Network Manager VPC Attachment %s not found, removing from state", d.Id()) d.SetId("") - return nil + return diags } if err != nil { @@ -233,10 +237,12 @@ func resourceVPCAttachmentRead(ctx context.Context, d *schema.ResourceData, meta setTagsOut(ctx, a.Tags) - return nil + return diags } func resourceVPCAttachmentUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).NetworkManagerConn(ctx) if d.HasChangesExcept("tags", "tags_all") { @@ -281,7 +287,7 @@ func resourceVPCAttachmentUpdate(ctx context.Context, d *schema.ResourceData, me } } - return resourceVPCAttachmentRead(ctx, d, meta) + return append(diags, resourceVPCAttachmentRead(ctx, d, meta)...) } func resourceVPCAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { From d1d669350a82cfa90225433c22f9f2b34108b065 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 23 Nov 2023 10:26:11 -0800 Subject: [PATCH 08/10] Updates CHANGELOG --- .changelog/34547.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changelog/34547.txt diff --git a/.changelog/34547.txt b/.changelog/34547.txt new file mode 100644 index 00000000000..0a91d8c24d3 --- /dev/null +++ b/.changelog/34547.txt @@ -0,0 +1,11 @@ +```release-note:bug +resource/aws_networkmanager_vpc_attachment: Fixes error where VPC Attachments waiting for acceptance could not be deleted +``` + +```release-note:bug +resource/aws_networkmanager_vpc_attachment: Fixes error when modifying `options` fields while waiting for acceptance +``` + +```release-note:bug +resource/aws_networkmanager_attachment_accepter: Now revokes attachment on deletion for VPC Attachments +``` From 9ea66a8854692c92c077d75559721d91bd49d00c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 24 Nov 2023 10:33:50 -0800 Subject: [PATCH 09/10] Semgrep fixes --- internal/service/networkmanager/vpc_attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index 7012a203db9..6e07c2a8fcd 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -212,7 +212,7 @@ func TestAccNetworkManagerVPCAttachment_disappears(t *testing.T) { } } -func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { +func TestAccNetworkManagerVPCAttachment_Attached_disappears(t *testing.T) { // nosemgrep:ci.acceptance-test-naming-parent-disappears const ( resourceName = "aws_networkmanager_vpc_attachment.test" attachmentResourceName = "aws_networkmanager_attachment_accepter.test" From 9b435a1ed19e56fea1b8aec3a149f2da79e6102b Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 28 Nov 2023 16:20:38 -0800 Subject: [PATCH 10/10] Removes unneeded check --- internal/service/networkmanager/vpc_attachment_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/service/networkmanager/vpc_attachment_test.go b/internal/service/networkmanager/vpc_attachment_test.go index 6e07c2a8fcd..71d90b9934e 100644 --- a/internal/service/networkmanager/vpc_attachment_test.go +++ b/internal/service/networkmanager/vpc_attachment_test.go @@ -448,18 +448,15 @@ func TestAccNetworkManagerVPCAttachment_Attached_update(t *testing.T) { testcases := map[string]struct { acceptanceRequired bool expectedState string - expectRecreation bool }{ "acceptance_required": { acceptanceRequired: true, expectedState: networkmanager.AttachmentStatePendingAttachmentAcceptance, - expectRecreation: true, }, "acceptance_not_required": { acceptanceRequired: false, expectedState: networkmanager.AttachmentStateAvailable, - expectRecreation: false, }, } @@ -490,7 +487,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_update(t *testing.T) { Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 1, false, true), Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v2), - testAccCheckVPCAttachmentRecreated(&v1, &v2, testcase.expectRecreation), + testAccCheckVPCAttachmentRecreated(&v1, &v2, false), resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "1"), resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"), @@ -500,7 +497,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_update(t *testing.T) { Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 2, false, false), Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v3), - testAccCheckVPCAttachmentRecreated(&v2, &v3, testcase.expectRecreation), + testAccCheckVPCAttachmentRecreated(&v2, &v3, false), resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "false"), @@ -510,7 +507,7 @@ func TestAccNetworkManagerVPCAttachment_Attached_update(t *testing.T) { Config: testAccVPCAttachmentConfig_Attached_updates(rName, testcase.acceptanceRequired, 2, false, true), Check: resource.ComposeTestCheckFunc( testAccCheckVPCAttachmentExists(ctx, resourceName, &v4), - testAccCheckVPCAttachmentRecreated(&v3, &v4, testcase.expectRecreation), + testAccCheckVPCAttachmentRecreated(&v3, &v4, false), resource.TestCheckResourceAttr(resourceName, "subnet_arns.#", "2"), resource.TestCheckResourceAttr(resourceName, "options.0.appliance_mode_support", "false"), resource.TestCheckResourceAttr(resourceName, "options.0.ipv6_support", "true"),