td/aws_networkmanager_core network_policy-enhancements

.changelog/30879.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_networkmanager_core_network: Wait for the network policy to be in the `READY_TO_EXECUTE` state before executing any changes
+```

internal/service/networkmanager/core_network.go

@@ -11,6 +11,7 @@ import (
 	"github.com/aws/aws-sdk-go/private/protocol"
 	"github.com/aws/aws-sdk-go/service/networkmanager"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
+	multierror "github.com/hashicorp/go-multierror"
 	"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/id"
@@ -28,6 +29,12 @@ import (
 const (
 	// CoreNetwork is in PENDING state before AVAILABLE. No value for PENDING at the moment.
 	coreNetworkStatePending = "PENDING"
+	// Minimum valid policy version id is 1
+	minimumValidPolicyVersionID = 1
+	// 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
 )
 
 // @SDKResource("aws_networkmanager_core_network", name="Core Network")
@@ -253,7 +260,8 @@ func resourceCoreNetworkRead(ctx context.Context, d *schema.ResourceData, meta i
 
 	// getting the policy document uses a different API call
 	// policy document is also optional
-	coreNetworkPolicy, err := FindCoreNetworkPolicyByID(ctx, conn, d.Id())
+	// pass in latestPolicyVersionId to get the latest version id by default
+	coreNetworkPolicy, err := FindCoreNetworkPolicyByTwoPartKey(ctx, conn, d.Id(), latestPolicyVersionID)
 
 	if tfresource.NotFound(err) {
 		d.Set("policy_document", nil)
@@ -394,9 +402,12 @@ func FindCoreNetworkByID(ctx context.Context, conn *networkmanager.NetworkManage
 	return output.CoreNetwork, nil
 }
 
-func FindCoreNetworkPolicyByID(ctx context.Context, conn *networkmanager.NetworkManager, id string) (*networkmanager.CoreNetworkPolicy, error) {
+func FindCoreNetworkPolicyByTwoPartKey(ctx context.Context, conn *networkmanager.NetworkManager, coreNetworkID string, policyVersionID int64) (*networkmanager.CoreNetworkPolicy, error) {
 	input := &networkmanager.GetCoreNetworkPolicyInput{
-		CoreNetworkId: aws.String(id),
+		CoreNetworkId: aws.String(coreNetworkID),
+	}
+	if policyVersionID >= minimumValidPolicyVersionID {
+		input.PolicyVersionId = aws.Int64(policyVersionID)
 	}
 
 	output, err := conn.GetCoreNetworkPolicyWithContext(ctx, input)
@@ -585,30 +596,68 @@ func PutAndExecuteCoreNetworkPolicy(ctx context.Context, conn *networkmanager.Ne
 
 	policyVersionID := aws.Int64Value(output.CoreNetworkPolicy.PolicyVersionId)
 
-	// new policy documents goes from Pending generation to Ready to execute
-	_, err = tfresource.RetryWhen(ctx, 4*time.Minute,
-		func() (interface{}, error) {
-			return conn.ExecuteCoreNetworkChangeSetWithContext(ctx, &networkmanager.ExecuteCoreNetworkChangeSetInput{
-				CoreNetworkId:   aws.String(coreNetworkId),
-				PolicyVersionId: aws.Int64(policyVersionID),
-			})
-		},
-		func(err error) (bool, error) {
-			if tfawserr.ErrMessageContains(err, networkmanager.ErrCodeValidationException, "Incorrect input") {
-				return true, err
-			}
-
-			return false, err
-		},
-	)
+	if _, err := waitCoreNetworkPolicyCreated(ctx, conn, coreNetworkId, policyVersionID, waitCoreNetworkPolicyCreatedTimeInMinutes*time.Minute); err != nil {
+		return fmt.Errorf("waiting for Network Manager Core Network Policy from Core Network (%s) create: %s", coreNetworkId, err)
+	}
 
+	_, err = conn.ExecuteCoreNetworkChangeSetWithContext(ctx, &networkmanager.ExecuteCoreNetworkChangeSetInput{
+		CoreNetworkId:   aws.String(coreNetworkId),
+		PolicyVersionId: aws.Int64(policyVersionID),
+	})
 	if err != nil {
 		return fmt.Errorf("executing Network Manager Core Network (%s) change set (%d): %s", coreNetworkId, policyVersionID, err)
 	}
 
 	return nil
 }
 
+func statusCoreNetworkPolicyState(ctx context.Context, conn *networkmanager.NetworkManager, coreNetworkId string, policyVersionId int64) retry.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		output, err := FindCoreNetworkPolicyByTwoPartKey(ctx, conn, coreNetworkId, policyVersionId)
+
+		if tfresource.NotFound(err) {
+			return nil, "", nil
+		}
+
+		if err != nil {
+			return nil, "", err
+		}
+
+		return output, aws.StringValue(output.ChangeSetState), nil
+	}
+}
+
+func waitCoreNetworkPolicyCreated(ctx context.Context, conn *networkmanager.NetworkManager, coreNetworkId string, policyVersionId int64, timeout time.Duration) (*networkmanager.CoreNetworkPolicy, error) {
+	stateConf := &retry.StateChangeConf{
+		Pending: []string{networkmanager.ChangeSetStatePendingGeneration},
+		Target:  []string{networkmanager.ChangeSetStateReadyToExecute},
+		Timeout: timeout,
+		Refresh: statusCoreNetworkPolicyState(ctx, conn, coreNetworkId, policyVersionId),
+	}
+
+	outputRaw, err := stateConf.WaitForStateContext(ctx)
+
+	if output, ok := outputRaw.(*networkmanager.CoreNetworkPolicy); ok {
+		return output, err
+	}
+
+	if output, ok := outputRaw.(*networkmanager.CoreNetworkPolicy); ok {
+		if state, errors := aws.StringValue(output.ChangeSetState), output.PolicyErrors; state == networkmanager.ChangeSetStateFailedGeneration && len(errors) > 0 {
+			var errs *multierror.Error
+
+			for _, err := range errors {
+				errs = multierror.Append(errs, fmt.Errorf("%s: %s", aws.StringValue(err.ErrorCode), aws.StringValue(err.Message)))
+			}
+
+			tfresource.SetLastError(err, errs.ErrorOrNil())
+		}
+
+		return output, err
+	}
+
+	return nil, err
+}
+
 // buildCoreNetworkBasePolicyDocument returns a base policy document
 func buildCoreNetworkBasePolicyDocument(regions []interface{}) (string, error) {
 	edgeLocations := make([]*CoreNetworkEdgeLocation, len(regions))

internal/service/networkmanager/core_network_policy_attachment.go

@@ -88,7 +88,8 @@ func resourceCoreNetworkPolicyAttachmentRead(ctx context.Context, d *schema.Reso
 	d.Set("state", coreNetwork.State)
 
 	// getting the policy document uses a different API call
-	coreNetworkPolicy, err := FindCoreNetworkPolicyByID(ctx, conn, d.Id())
+	// pass in latestPolicyVersionId to get the latest version id by default
+	coreNetworkPolicy, err := FindCoreNetworkPolicyByTwoPartKey(ctx, conn, d.Id(), latestPolicyVersionID)
 
 	if tfresource.NotFound(err) {
 		d.Set("policy_document", nil)

internal/service/networkmanager/core_network_policy_attachment_test.go

@@ -134,6 +134,23 @@ func TestAccNetworkManagerCoreNetworkPolicyAttachment_vpcAttachmentMultiRegion(t
 	})
 }
 
+func TestAccNetworkManagerCoreNetworkPolicyAttachment_expectPolicyErrorInvalidASNRange(t *testing.T) {
+	ctx := acctest.Context(t)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, networkmanager.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckCoreNetworkPolicyAttachmentDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccCoreNetworkPolicyAttachmentConfig_expectPolicyErrorInvalidASNRange(),
+				ExpectError: regexp.MustCompile("INVALID_ASN_RANGE"),
+			},
+		},
+	})
+}
+
 func testAccCheckCoreNetworkPolicyAttachmentDestroy(ctx context.Context) resource.TestCheckFunc {
 	// policy document will not be reverted to empty if the attachment is deleted
 	return nil
@@ -152,7 +169,9 @@ func testAccCheckCoreNetworkPolicyAttachmentExists(ctx context.Context, n string
 
 		conn := acctest.Provider.Meta().(*conns.AWSClient).NetworkManagerConn()
 
-		_, err := tfnetworkmanager.FindCoreNetworkPolicyByID(ctx, conn, rs.Primary.ID)
+		// pass in latestPolicyVersionId to get the latest version id by default
+		const latestPolicyVersionId = -1
+		_, err := tfnetworkmanager.FindCoreNetworkPolicyByTwoPartKey(ctx, conn, rs.Primary.ID, latestPolicyVersionId)
 
 		return err
 	}
@@ -387,3 +406,32 @@ resource "aws_networkmanager_vpc_attachment" "alternate_region" {
 }
 `, acctest.Region(), acctest.AlternateRegion()))
 }
+
+func testAccCoreNetworkPolicyAttachmentConfig_expectPolicyErrorInvalidASNRange() string {
+	return fmt.Sprintf(`
+resource "aws_networkmanager_global_network" "test" {}
+
+data "aws_networkmanager_core_network_policy_document" "test" {
+  core_network_configuration {
+    asn_ranges = ["65022-65534123"] # not a valid range
+
+    edge_locations {
+      location = %[1]q
+    }
+  }
+
+  segments {
+    name = "test"
+  }
+}
+
+resource "aws_networkmanager_core_network" "test" {
+  global_network_id = aws_networkmanager_global_network.test.id
+}
+
+resource "aws_networkmanager_core_network_policy_attachment" "test" {
+  core_network_id = aws_networkmanager_core_network.test.id
+  policy_document = data.aws_networkmanager_core_network_policy_document.test.json
+}
+`, acctest.Region())
+}