Merge pull request hashicorp#19517 from hashicorp/b-aws_ec2_managed_p…

…refix_list-update-crash r/aws_ec2_managed_prefix_list: Fix crash with multiple description-only changes
coveooss · May 26, 2021 · 5a3bd61 · 5a3bd61
2 parents bb23961 + 96daf27
commit 5a3bd61
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 14 deletions.
diff --git a/.changelog/19517.txt b/.changelog/19517.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_ec2_managed_prefix_list: Fix crash with multiple description-only updates
+```
diff --git a/aws/resource_aws_ec2_managed_prefix_list.go b/aws/resource_aws_ec2_managed_prefix_list.go
@@ -242,22 +242,32 @@ func resourceAwsEc2ManagedPrefixListUpdate(d *schema.ResourceData, meta interfac
 		// 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()),
-			}
+			descriptionOnlyRemovals := []*ec2.RemovePrefixListEntry{}
+			removals := []*ec2.RemovePrefixListEntry{}
+
+			for _, removeEntry := range input.RemoveEntries {
+				inAddAndRemove := false
 
-			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:]...)
+						inAddAndRemove = true
+						break
 					}
 				}
+
+				if inAddAndRemove {
+					descriptionOnlyRemovals = append(descriptionOnlyRemovals, removeEntry)
+				} else {
+					removals = append(removals, removeEntry)
+				}
 			}
 
-			if len(removalInput.RemoveEntries) > 0 {
-				_, err := conn.ModifyManagedPrefixList(removalInput)
+			if len(descriptionOnlyRemovals) > 0 {
+				_, err := conn.ModifyManagedPrefixList(&ec2.ModifyManagedPrefixListInput{
+					CurrentVersion: input.CurrentVersion,
+					PrefixListId:   aws.String(d.Id()),
+					RemoveEntries:  descriptionOnlyRemovals,
+				})
 
 				if err != nil {
 					return fmt.Errorf("error updating EC2 Managed Prefix List (%s): %w", d.Id(), err)
@@ -274,12 +284,14 @@ func resourceAwsEc2ManagedPrefixListUpdate(d *schema.ResourceData, meta interfac
 				}
 
 				input.CurrentVersion = managedPrefixList.Version
+			}
 
+			if len(removals) > 0 {
+				input.RemoveEntries = removals
+			} else {
 				// 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
-				}
+				input.RemoveEntries = nil
 			}
 		}
 

diff --git a/aws/resource_aws_ec2_managed_prefix_list_test.go b/aws/resource_aws_ec2_managed_prefix_list_test.go
@@ -170,11 +170,15 @@ func TestAccAwsEc2ManagedPrefixList_Entry_Description(t *testing.T) {
 				ResourceName: resourceName,
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccAwsEc2ManagedPrefixListExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "entry.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "entry.#", "2"),
 					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
 						"cidr":        "1.0.0.0/8",
 						"description": "description1",
 					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
+						"cidr":        "2.0.0.0/8",
+						"description": "description1",
+					}),
 					resource.TestCheckResourceAttr(resourceName, "version", "1"),
 				),
 			},
@@ -188,11 +192,15 @@ func TestAccAwsEc2ManagedPrefixList_Entry_Description(t *testing.T) {
 				ResourceName: resourceName,
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccAwsEc2ManagedPrefixListExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "entry.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "entry.#", "2"),
 					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
 						"cidr":        "1.0.0.0/8",
 						"description": "description2",
 					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "entry.*", map[string]string{
+						"cidr":        "2.0.0.0/8",
+						"description": "description2",
+					}),
 					resource.TestCheckResourceAttr(resourceName, "version", "3"), // description-only updates require two operations
 				),
 			},
@@ -416,6 +424,11 @@ resource "aws_ec2_managed_prefix_list" "test" {
     cidr        = "1.0.0.0/8"
     description = %[2]q
   }
+
+  entry {
+    cidr        = "2.0.0.0/8"
+    description = %[2]q
+  }
 }
 `, rName, description)
 }