Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/aws_ec2_managed_prefix_list: Prevent entry description update errors #19095

Merged
merged 2 commits into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/19095.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ec2_managed_prefix_list: Prevent `entry` `description` update errors
```
49 changes: 49 additions & 0 deletions aws/resource_aws_ec2_managed_prefix_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
70 changes: 65 additions & 5 deletions aws/resource_aws_ec2_managed_prefix_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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")
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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" {
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/ec2_managed_prefix_list.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down