resource/aws_ec2_managed_prefix_list: Prevent entry description updat…

…e errors (#19095) * resource/aws_ec2_managed_prefix_list: Prevent entry description update errors Previously: ``` === CONT TestAccAwsEc2ManagedPrefixList_Entry_Description resource_aws_ec2_managed_prefix_list_test.go:162: Step 3/3 error: Error running apply: exit status 1 Error: error updating EC2 Managed Prefix List (pl-07236fe64b781b5b3): InvalidParameterValue: Request cannot contain Cidr 1.0.0.0/8 in both AddPrefixListEntries and RemovePrefixListEntries status code: 400, request id: d3904b6e-f434-4610-b6e7-923bd582ef70 on terraform_plugin_test.tf line 2, in resource "aws_ec2_managed_prefix_list" "test": 2: resource "aws_ec2_managed_prefix_list" "test" { --- FAIL: TestAccAwsEc2ManagedPrefixList_Entry_Description (31.81s) ``` Output from acceptance testing: ``` --- PASS: TestAccAwsEc2ManagedPrefixList_disappears (21.48s) --- PASS: TestAccAwsEc2ManagedPrefixList_AddressFamily_IPv6 (28.02s) --- PASS: TestAccAwsEc2ManagedPrefixList_basic (28.21s) --- PASS: TestAccAwsEc2ManagedPrefixList_Name (34.87s) --- PASS: TestAccAwsEc2ManagedPrefixList_Entry_Description (41.21s) --- PASS: TestAccAwsEc2ManagedPrefixList_Entry_Cidr (42.44s) --- PASS: TestAccAwsEc2ManagedPrefixList_Tags (50.13s) ``` * Update CHANGELOG for #19095
hashicorp · Apr 27, 2021 · a28a210 · a28a210
1 parent 053a94a
commit a28a210
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 6 deletions.
diff --git a/.changelog/19095.txt b/.changelog/19095.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_ec2_managed_prefix_list: Prevent `entry` `description` update errors
+```
diff --git a/aws/resource_aws_ec2_managed_prefix_list.go b/aws/resource_aws_ec2_managed_prefix_list.go
@@ -234,6 +234,55 @@ func resourceAwsEc2ManagedPrefixListUpdate(d *schema.ResourceData, meta interfac
 			wait = true
 		}
 
+		// Prevent the following error on description-only updates:
+		//   InvalidParameterValue: Request cannot contain Cidr #.#.#.#/# in both AddPrefixListEntries and RemovePrefixListEntries
+		// Attempting to just delete the RemoveEntries item causes:
+		//   InvalidRequest: The request received was invalid.
+		// Therefore it seems we must issue two ModifyManagedPrefixList calls,
+		// one with a collection of all description-only removals and the
+		// second one will add them all back.
+		if len(input.AddEntries) > 0 && len(input.RemoveEntries) > 0 {
+			removalInput := &ec2.ModifyManagedPrefixListInput{
+				CurrentVersion: input.CurrentVersion,
+				PrefixListId:   aws.String(d.Id()),
+			}
+
+			for idx, removeEntry := range input.RemoveEntries {
+				for _, addEntry := range input.AddEntries {
+					if aws.StringValue(addEntry.Cidr) == aws.StringValue(removeEntry.Cidr) {
+						removalInput.RemoveEntries = append(removalInput.RemoveEntries, input.RemoveEntries[idx])
+						input.RemoveEntries = append(input.RemoveEntries[:idx], input.RemoveEntries[idx+1:]...)
+					}
+				}
+			}
+
+			if len(removalInput.RemoveEntries) > 0 {
+				_, err := conn.ModifyManagedPrefixList(removalInput)
+
+				if err != nil {
+					return fmt.Errorf("error updating EC2 Managed Prefix List (%s): %w", d.Id(), err)
+				}
+
+				managedPrefixList, err := waiter.ManagedPrefixListModified(conn, d.Id())
+
+				if err != nil {
+					return fmt.Errorf("error waiting for EC2 Managed Prefix List (%s) update: %w", d.Id(), err)
+				}
+
+				if managedPrefixList == nil {
+					return fmt.Errorf("error waiting for EC2 Managed Prefix List (%s) update: empty response", d.Id())
+				}
+
+				input.CurrentVersion = managedPrefixList.Version
+
+				// Prevent this error if RemoveEntries is list with no elements after removals:
+				//   InvalidRequest: The request received was invalid.
+				if len(input.RemoveEntries) == 0 {
+					input.RemoveEntries = nil
+				}
+			}
+		}
+
 		_, err := conn.ModifyManagedPrefixList(input)
 
 		if err != nil {

diff --git a/aws/resource_aws_ec2_managed_prefix_list_test.go b/aws/resource_aws_ec2_managed_prefix_list_test.go
@@ -97,7 +97,7 @@ func TestAccAwsEc2ManagedPrefixList_AddressFamily_IPv6(t *testing.T) {
 	})
 }
 
-func TestAccAwsEc2ManagedPrefixList_Entry(t *testing.T) {
+func TestAccAwsEc2ManagedPrefixList_Entry_Cidr(t *testing.T) {
 	resourceName := "aws_ec2_managed_prefix_list.test"
 	rName := acctest.RandomWithPrefix("tf-acc-test")
 
@@ -108,7 +108,7 @@ func TestAccAwsEc2ManagedPrefixList_Entry(t *testing.T) {
 		CheckDestroy: testAccCheckAwsEc2ManagedPrefixListDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry1(rName),
+				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry_Cidr1(rName),
 				ResourceName: resourceName,
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccAwsEc2ManagedPrefixListExists(resourceName),
@@ -130,7 +130,7 @@ func TestAccAwsEc2ManagedPrefixList_Entry(t *testing.T) {
 				ImportStateVerify: true,
 			},
 			{
-				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry2(rName),
+				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry_Cidr2(rName),
 				ResourceName: resourceName,
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccAwsEc2ManagedPrefixListExists(resourceName),
@@ -155,6 +155,51 @@ func TestAccAwsEc2ManagedPrefixList_Entry(t *testing.T) {
 	})
 }
 
+func TestAccAwsEc2ManagedPrefixList_Entry_Description(t *testing.T) {
+	resourceName := "aws_ec2_managed_prefix_list.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccPreCheckEc2ManagedPrefixList(t) },
+		ErrorCheck:   testAccErrorCheck(t, ec2.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAwsEc2ManagedPrefixListDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry_Description(rName, "description1"),
+				ResourceName: resourceName,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccAwsEc2ManagedPrefixListExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "entry.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
+						"cidr":        "1.0.0.0/8",
+						"description": "description1",
+					}),
+					resource.TestCheckResourceAttr(resourceName, "version", "1"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config:       testAccAwsEc2ManagedPrefixListConfig_Entry_Description(rName, "description2"),
+				ResourceName: resourceName,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccAwsEc2ManagedPrefixListExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "entry.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
+						"cidr":        "1.0.0.0/8",
+						"description": "description2",
+					}),
+					resource.TestCheckResourceAttr(resourceName, "version", "3"), // description-only updates require two operations
+				),
+			},
+		},
+	})
+}
+
 func TestAccAwsEc2ManagedPrefixList_Name(t *testing.T) {
 	resourceName := "aws_ec2_managed_prefix_list.test"
 	rName1 := acctest.RandomWithPrefix("tf-acc-test")
@@ -320,7 +365,7 @@ resource "aws_ec2_managed_prefix_list" "test" {
 `, rName, addressFamily)
 }
 
-func testAccAwsEc2ManagedPrefixListConfig_Entry1(rName string) string {
+func testAccAwsEc2ManagedPrefixListConfig_Entry_Cidr1(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_ec2_managed_prefix_list" "test" {
   address_family = "IPv4"
@@ -340,7 +385,7 @@ resource "aws_ec2_managed_prefix_list" "test" {
 `, rName)
 }
 
-func testAccAwsEc2ManagedPrefixListConfig_Entry2(rName string) string {
+func testAccAwsEc2ManagedPrefixListConfig_Entry_Cidr2(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_ec2_managed_prefix_list" "test" {
   address_family = "IPv4"
@@ -360,6 +405,21 @@ resource "aws_ec2_managed_prefix_list" "test" {
 `, rName)
 }
 
+func testAccAwsEc2ManagedPrefixListConfig_Entry_Description(rName string, description string) string {
+	return fmt.Sprintf(`
+resource "aws_ec2_managed_prefix_list" "test" {
+  address_family = "IPv4"
+  max_entries    = 5
+  name           = %[1]q
+
+  entry {
+    cidr        = "1.0.0.0/8"
+    description = %[2]q
+  }
+}
+`, rName, description)
+}
+
 func testAccAwsEc2ManagedPrefixListConfig_Name(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_ec2_managed_prefix_list" "test" {

diff --git a/website/docs/r/ec2_managed_prefix_list.html.markdown b/website/docs/r/ec2_managed_prefix_list.html.markdown
@@ -55,7 +55,7 @@ The following arguments are supported:
 ### `entry`
 
 * `cidr` - (Required) CIDR block of this entry.
-* `description` - (Optional) Description of this entry.
+* `description` - (Optional) Description of this entry. Due to API limitations, updating only the description of an existing entry requires temporarily removing and re-adding the entry.
 
 ## Attributes Reference