Added data source for aws_route_tables

[jesseaukeman]

This allows lookup of a list of route table ids similar to the
existing functionality of the aws_subnet_ids data source.

Changes proposed in this pull request:

• add a new data source aws_route_table_ids

[bflad]

Hi @jesseaukeman, thanks for submitting this! I left you some initial feedback below. Can you please let us know if you have any questions or do not have time to implement them?

[bflad]

Sorry this is going to seem real pedantic, but can you please:

• Rename the data source aws_route_tables along with the file names
• Rename the functions to match, e.g. dataSourceAwsRouteTables() and dataSourceAwsRouteTablesRead()

This is mainly for future maintainability as the data source may be adjusted to return information other than just IDs. We unfortunately have some other _ids data sources (e.g. aws_subnet_ids) that will likely need to be renamed at some point. 😅

[bflad]

According the EC2 API Reference, vpc_id should not be a required argument. Its valid to search for route tables across multiple VPCs. Can you please:

• Change the attribute to Optional: true
• Use d.GetOk("vpc_id") instead of expecting it to always exist:

if v, ok := d.GetOk("vpc_id"); ok {
	req.Filters = buildEC2AttributeFilterList(
		map[string]string{
			"vpc-id": v.(string),
		},
	)
}

• Change d.SetId(d.Get("vpc_id").(string)) to d.SetId(resource.UniqueId())
• Ensure the documentation is updated for the attribute

[bflad]

To prevent potential panics, we should prefer to use the SDK helper aws.StringValue(routeTable.RouteTableId) instead of directly dereferencing via *

[bflad]

When using d.Set() with aggregate types (e.g. TypeList/TypeSet/TypeMap) we should perform error checking: https://www.terraform.io/docs/extend/best-practices/detecting-drift.html#error-checking-aggregate-types

if err := d.Set("ids", routeTables); err != nil {
  return fmt.Errorf("error setting ids: %s", err)
}

[jesseaukeman]

Hi @bflad thanks for the feedback. I believe I have updated as you requested. please let me know.

[bflad]

This check is currently failing:

=== RUN   TestAccDataSourceAwsRouteTables
--- FAIL: TestAccDataSourceAwsRouteTables (11.13s)
    testing.go:518: Step 1 error: Check failed: Check 1/2 error: data.aws_route_tables.selected: Attribute 'ids.#' expected "3", got "4"

If I had to venture a guess, its because VPCs include a default route table in addition to the 3 being created in the test.

[jesseaukeman]

@bflad of course you are correct. I've updated the test cases to account for that, now it succeeds in my tests.

jmpb:aws jaukeman$ TF_ACC=1 go test -v -run TestAccDataSourceAwsRouteTables
=== RUN   TestAccDataSourceAwsRouteTables
--- PASS: TestAccDataSourceAwsRouteTables (54.93s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       54.974s

[bflad]

Great work, @jesseaukeman! LGTM and I'll fix the one documentation thing on merge. 🚀

1 test passed (all tests)
=== RUN   TestAccDataSourceAwsRouteTables
--- PASS: TestAccDataSourceAwsRouteTables (15.34s)

[bflad]

This page needs a sidebar link added in website/aws.erb, will add post-merge.

[bflad]

This has been released in version 1.25.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!