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

d/aws_prefix_list: add support for managed prefix lists #14110

81 changes: 67 additions & 14 deletions aws/data_source_aws_prefix_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package aws
import (
"fmt"
"log"
"sort"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"

"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func dataSourceAwsPrefixList() *schema.Resource {
Expand All @@ -29,47 +32,97 @@ func dataSourceAwsPrefixList() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
},
"filter": dataSourceFiltersSchema(),
"owner_id": {
Type: schema.TypeString,
Computed: true,
},
"address_family": {
Type: schema.TypeString,
Computed: true,
},
"arn": {
Type: schema.TypeString,
Computed: true,
},
"max_entries": {
Type: schema.TypeInt,
Computed: true,
},
"tags": tagsSchemaComputed(),
},
}
}

func dataSourceAwsPrefixListRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

filters, filtersOk := d.GetOk("filter")

req := &ec2.DescribePrefixListsInput{}
req := &ec2.DescribeManagedPrefixListsInput{}
Copy link
Contributor

@bflad bflad Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a very forward comment here since it will dramatically change the implementation -- rather than change the API call associated with the existing data source, which can cause issues in environments that utilize restrictive IAM permissions or with AWS "compatible" APIs that may not support the new API call, the preference would be to create a separate data source so operators can choose the appropriate solution. 👍

It may be easiest to close this pull request and separately submit a pull request with just a new aws_ec2_managed_prefix_list data source that for now just tests AWS managed prefix lists. Once #14068 is merged, we can add any remaining functionality to the new data source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bflad,

may not support the new API call,

I hadn't considered this detail, and it makes the right course clear in retrospect. Indeed, the managed prefix list API-s require a different set of IAM actions.

Just to confirm, is aws_ec2_managed_prefix_list (as well as aws_ec2_managed_prefix_list_entry in the resource PR) the preferred nomenclature?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please, and thank you regarding the naming! We can separately discuss the merits of an _entry resource in the other pull request.

if filtersOk {
req.Filters = buildAwsDataSourceFilters(filters.(*schema.Set))
}
if prefixListID := d.Get("prefix_list_id"); prefixListID != "" {
req.PrefixListIds = aws.StringSlice([]string{prefixListID.(string)})
}
req.Filters = buildEC2AttributeFilterList(
map[string]string{
"prefix-list-name": d.Get("name").(string),
},
)
if prefixListName := d.Get("name"); prefixListName.(string) != "" {
req.Filters = append(req.Filters, &ec2.Filter{
Name: aws.String("prefix-list-name"),
Values: aws.StringSlice([]string{prefixListName.(string)}),
})
}

log.Printf("[DEBUG] Reading Prefix List: %s", req)
resp, err := conn.DescribePrefixLists(req)
if err != nil {
resp, err := conn.DescribeManagedPrefixLists(req)
switch {
case err != nil:
return err
}
if resp == nil || len(resp.PrefixLists) == 0 {
case resp == nil || len(resp.PrefixLists) == 0:
return fmt.Errorf("no matching prefix list found; the prefix list ID or name may be invalid or not exist in the current region")
case len(resp.PrefixLists) > 1:
return fmt.Errorf("more than one prefix list matched the given set of criteria")
}

pl := resp.PrefixLists[0]

d.SetId(*pl.PrefixListId)
d.Set("name", pl.PrefixListName)

cidrs := make([]string, len(pl.Cidrs))
for i, v := range pl.Cidrs {
cidrs[i] = *v
getEntriesInput := ec2.GetManagedPrefixListEntriesInput{
PrefixListId: pl.PrefixListId,
}

cidrs := []string(nil)

err = conn.GetManagedPrefixListEntriesPages(
&getEntriesInput, func(output *ec2.GetManagedPrefixListEntriesOutput, last bool) bool {
for _, entry := range output.Entries {
cidrs = append(cidrs, aws.StringValue(entry.Cidr))
}
return true
})
if err != nil {
return fmt.Errorf("failed to get entries of prefix list %s: %s", *pl.PrefixListId, err)
}

sort.Strings(cidrs)

if err := d.Set("cidr_blocks", cidrs); err != nil {
return fmt.Errorf("failed to set cidr blocks of prefix list %s: %s", d.Id(), err)
}

d.Set("owner_id", pl.OwnerId)
d.Set("address_family", pl.AddressFamily)
d.Set("arn", pl.PrefixListArn)

if actual := aws.Int64Value(pl.MaxEntries); actual > 0 {
d.Set("max_entries", actual)
}

if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(pl.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("failed to set tags of prefix list %s: %s", d.Id(), err)
}
d.Set("cidr_blocks", cidrs)

return nil
}
119 changes: 119 additions & 0 deletions aws/data_source_aws_prefix_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"strconv"
"testing"

Expand Down Expand Up @@ -69,6 +70,26 @@ func testAccDataSourceAwsPrefixListCheck(name string) resource.TestCheckFunc {
return fmt.Errorf("cidr_blocks seem suspiciously low: %d", cidrBlockSize)
}

if actual := attr["owner_id"]; actual != "AWS" {
return fmt.Errorf("bad owner_id %s", actual)
}

if actual := attr["address_family"]; actual != "IPv4" {
return fmt.Errorf("bad address_family %s", actual)
}

if actual := attr["arn"]; actual != "arn:aws:ec2:us-west-2:aws:prefix-list/pl-68a54001" {
return fmt.Errorf("bad arn %s", actual)
}

if actual := attr["max_entries"]; actual != "" {
return fmt.Errorf("unexpected max_entries %s", actual)
}

if attr["tags.%"] != "0" {
return fmt.Errorf("expected 0 tags")
}

return nil
}
}
Expand Down Expand Up @@ -98,3 +119,101 @@ data "aws_prefix_list" "s3_by_id" {
}
}
`

func TestAccDataSourceAwsPrefixList_matchesTooMany(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAwsPrefixListConfig_matchesTooMany,
ExpectError: regexp.MustCompile(`more than one prefix list matched the given set of criteria`),
},
},
})
}

const testAccDataSourceAwsPrefixListConfig_matchesTooMany = `
data "aws_prefix_list" "test" {}
`

func TestAccDataSourceAwsPrefixList_nameDoesNotOverrideFilter(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
// The vanilla DescribePrefixLists API only supports filtering by
// id and name. In this case, the `name` attribute and `prefix-list-id`
// filter have been set up such that they conflict, thus proving
// that both criteria took effect.
Config: testAccDataSourceAwsPrefixListConfig_nameDoesNotOverrideFilter,
ExpectError: regexp.MustCompile(`no matching prefix list found`),
},
},
})
}

const testAccDataSourceAwsPrefixListConfig_nameDoesNotOverrideFilter = `
data "aws_prefix_list" "test" {
name = "com.amazonaws.us-west-2.s3"
filter {
name = "prefix-list-id"
values = ["pl-00a54069"] # com.amazonaws.us-west-2.dynamodb
}
}
`

func TestAccDataSourceAwsPrefixList_managedPrefixList(t *testing.T) {
resourceName := "aws_prefix_list.test"
dataSourceName := "data.aws_prefix_list.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSPrefixListDestroy,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAwsPrefixListConfig_managedPrefixList,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(resourceName, "id", dataSourceName, "id"),
resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "name"),
resource.TestCheckResourceAttrPair(resourceName, "arn", dataSourceName, "arn"),
resource.TestCheckResourceAttrPair(resourceName, "owner_id", dataSourceName, "owner_id"),
testAccCheckResourceAttrAccountID(dataSourceName, "owner_id"),
resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "name"),
resource.TestCheckResourceAttrPair(resourceName, "address_family", dataSourceName, "address_family"),
resource.TestCheckResourceAttrPair(resourceName, "max_entries", dataSourceName, "max_entries"),
resource.TestCheckResourceAttr(dataSourceName, "cidr_blocks.#", "2"),
resource.TestCheckResourceAttr(dataSourceName, "cidr_blocks.0", "1.0.0.0/8"),
resource.TestCheckResourceAttr(dataSourceName, "cidr_blocks.1", "2.0.0.0/8"),
resource.TestCheckResourceAttr(dataSourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(dataSourceName, "tags.Key1", "Value1"),
resource.TestCheckResourceAttr(dataSourceName, "tags.Key2", "Value2"),
),
},
},
})
}

const testAccDataSourceAwsPrefixListConfig_managedPrefixList = `
resource "aws_prefix_list" "test" {
name = "tf-test-acc"
max_entries = 5
address_family = "IPv4"
entry {
cidr_block = "1.0.0.0/8"
}
entry {
cidr_block = "2.0.0.0/8"
}
tags = {
Key1 = "Value1"
Key2 = "Value2"
}
}

data "aws_prefix_list" "test" {
prefix_list_id = aws_prefix_list.test.id
}
`
2 changes: 2 additions & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,8 @@ func Provider() terraform.ResourceProvider {
"aws_organizations_policy_attachment": resourceAwsOrganizationsPolicyAttachment(),
"aws_organizations_organizational_unit": resourceAwsOrganizationsOrganizationalUnit(),
"aws_placement_group": resourceAwsPlacementGroup(),
"aws_prefix_list": resourceAwsPrefixList(),
"aws_prefix_list_entry": resourceAwsPrefixListEntry(),
"aws_proxy_protocol_policy": resourceAwsProxyProtocolPolicy(),
"aws_qldb_ledger": resourceAwsQLDBLedger(),
"aws_quicksight_group": resourceAwsQuickSightGroup(),
Expand Down
Loading