provider/aws: data source for AWS Route Table #10301

mathieuherbert · 2016-11-22T21:18:53Z

Hi,

I added an AWS provider, aws_route_table.
We can retrieve, for example a route table from a subnet id and add a route with this route table

variable "subnet_id" {}

data "aws_route_table" "selected" {
  subnet_id = "${var.subnet_id}"
}

resource "aws_route" "route" {
  route_table_id = "${data.aws_route_table.selected.id}"
  destination_cidr_block = "10.0.1.0/22"
  vpc_peering_connection_id = "pcx-45ff3dc1"
}

Tests:

/opt/gopath/src/github.com/hashicorp/terraform$ TF_ACC=true go test -trace /tmp/toto.txt -v github.com/hashicorp/terraform/builtin/providers/aws -run ^TestAccDataSourceAwsRouteTable$
=== RUN   TestAccDataSourceAwsRouteTable
--- PASS: TestAccDataSourceAwsRouteTable (46.92s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	46.966s

kwilczynski · 2016-11-22T22:37:52Z

builtin/providers/aws/data_source_aws_route_table.go

+		Read: dataSourceAwsRouteTableRead,
+
+		Schema: map[string]*schema.Schema{
+			"subnet_id": &schema.Schema{


As of Go 1.7+ the explicit type is no longer needed here, so &schema.Schema can be removed.

kwilczynski · 2016-11-22T22:41:30Z

builtin/providers/aws/data_source_aws_route_table.go

+		Schema: map[string]*schema.Schema{
+			"subnet_id": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,


I am not sure if this should be optional? It seem that rest of the attributes are also optional, is this desirable?

Is this ever going to be empty? Given the first req.Filters getting some minimal filters.

I think we can get a Route Table with

subnet_id

tags

vpc_id and tags

filters
What do you think ?

kwilczynski · 2016-11-22T22:42:34Z

builtin/providers/aws/data_source_aws_route_table.go

+	rt := resp.RouteTables[0]
+
+	d.SetId(*rt.RouteTableId)
+	d.Set("id", *rt.RouteTableId)


The d.SetId() would set id, there is no need to have it explicitly set.

kwilczynski · 2016-11-22T22:43:36Z

builtin/providers/aws/data_source_aws_route_table.go

+	req.Filters = append(req.Filters, buildEC2CustomFilterList(
+		d.Get("filter").(*schema.Set),
+	)...)
+	if len(req.Filters) == 0 {


Is this ever going to be empty? Given the first req.Filters getting some minimal filters.

I add an error if there is no filter

I see, so you want to use the sheer fact that buildEC2AttributeFilterList would return empty slice given that values in the map are empty. Which is fine, albeit it can be make a whole lot simpler should you break even before getting to this helper. You can do this as follows:

vpcId, vpcIdOk := d.GetOk("vpc_id") subnetId, subnetIdOk := d.GetOk("subnet_id") if !vpcIdOk && !subnetIdOk { return fmt.Errorf("One of vpc_id or subnet_id must be assigned") } req.Filters = buildEC2AttributeFilterList( map[string]string{ "vpc-id": vpcId.(string), "association.subnet-id": subnetId.(string), }, )

mathieuherbert · 2016-11-23T07:36:27Z

Hi @kwilczynski,
Thank you for your review :)
I think we can get a Route Table with

subnet_id
tags
vpc_id and tags
filters
The subnet is just one of the possibilities. What do you think ?

kwilczynski · 2016-11-23T10:46:31Z

builtin/providers/aws/data_source_aws_route_table.go

+			},
+			"filter": ec2CustomFiltersSchema(),
+
+			"id": {


This is not needed (as per the earlier comment, you do not to set id explicitly - this is done via the d.SetId().

kwilczynski · 2016-11-23T10:49:14Z

@mathieuherbert hi there! Thank you for your updates!

I see, you are allowing to set either and then narrow the result down with either tags or filters. Which is fine, but in such a case I would add a check whether at least one is set, and issue an appropriate message should that not be the case.

kwilczynski · 2016-11-23T10:57:49Z

@mathieuherbert hi there again!

I was wondering, how would you feel about adding associations and routes too? It would be an additional computed attribute (a sub-schema, etc.). Would it be something you could consider? It would make this data source more feature complete.

mathieuherbert · 2016-11-23T21:04:21Z

Hi @kwilczynski,

So now, I export every routesand associations from the route table. I hope it is the good way. I take some code from aws_route_table resource.

kwilczynski · 2016-11-24T20:49:33Z

builtin/providers/aws/data_source_aws_route_table.go

+	d.Set("vpc_id", rt.VpcId)
+	d.Set("tags", tagsToMap(rt.Tags))
+
+	routes := make([]map[string]interface{}, 0, len(rt.Routes))


Very nice! I think it might be better served as a function.

kwilczynski · 2016-11-24T20:53:58Z

builtin/providers/aws/data_source_aws_route_table.go

+	}
+	d.Set("routes", routes)
+
+	associations := make([]map[string]interface{}, 0, len(rt.Associations))


Same as above, if we turn this into a function it would be great.

kwilczynski · 2016-11-24T21:13:15Z

builtin/providers/aws/data_source_aws_route_table.go

+		return err
+	}
+	if resp == nil || len(resp.RouteTables) == 0 {
+		return fmt.Errorf("no matching Route Table found")


Check the Amazon AMI data source for an idea of more user-friendly error messages.

kwilczynski · 2016-11-24T21:15:31Z

builtin/providers/aws/data_source_aws_route_table.go

+
+		routes = append(routes, m)
+	}
+	d.Set("routes", routes)


When storing complex values, if would recommend adding an error check, for example:

if err := d.Set("routes", routes); err != nil { return err }

mathieuherbert · 2016-11-24T21:39:40Z

@kwilczynski , thank you very much for your help, it is very interesting !

kwilczynski · 2016-11-24T22:55:27Z

@mathieuherbert thank you for working on it!

mathieuherbert · 2016-11-28T15:10:33Z

@kwilczynski , do you have some comments on my last commit please ? Thanks !

kwilczynski · 2016-11-28T16:21:00Z

builtin/providers/aws/data_source_aws_route_table.go

+
+	d.SetId(*rt.RouteTableId)
+	d.Set("vpc_id", rt.VpcId)
+	d.Set("tags", tagsToMap(rt.Tags))


if err := d.Set("tags", tagsToMap(rt.Tags)); err != nil { return err }

kwilczynski · 2016-11-28T16:23:29Z

builtin/providers/aws/data_source_aws_route_table.go

+	associations := make([]map[string]interface{}, 0, len(ec2Assocations))
+	// Loop through the routes and add them to the set
+	for _, a := range ec2Assocations {
+


No need for the extra line.

kwilczynski · 2016-11-28T16:24:18Z

@mathieuherbert hi there! Looks good! Two minor comments, and then I think ready to go.

stack72 · 2016-11-28T18:07:03Z

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccDataSourceAwsRouteTable'          ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/11/28 18:02:00 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccDataSourceAwsRouteTable -timeout 120m
=== RUN   TestAccDataSourceAwsRouteTable
--- PASS: TestAccDataSourceAwsRouteTable (21.16s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	21.199s

Massive thanks to @kwilczynski for all the work here!

stack72 · 2016-11-28T18:08:28Z

Merged manually :)

swestcott · 2016-12-15T00:00:37Z

website/source/docs/providers/aws/d/route_table.html.markdown

+
+* `filter` - (Optional) Custom filter block as described below.
+
+* `id` - (Optional) The id of the specific Route Table to retrieve.


Was this parameter implemented? I can't see any reference to it in data_source_aws_route_table.go and get this when I try to use it,

Errors: * data.aws_route_table.selected: : invalid or unknown key: id

ghost · 2020-04-18T02:26:33Z

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Mathieu Herbert added 2 commits November 22, 2016 22:16

provider/aws: data source for AWS Route Table

ffdff82

provider/aws: data source for AWS Route Table

ae01555

kwilczynski reviewed Nov 22, 2016

View reviewed changes

remove &schema.Schema and set id

dd32d43

stack72 added enhancement new-data-source provider/aws labels Nov 23, 2016

kwilczynski reviewed Nov 23, 2016

View reviewed changes

add routes and associations support

6482fc6

kwilczynski reviewed Nov 24, 2016

View reviewed changes

add some functions and modify error messages

b403815

kwilczynski reviewed Nov 28, 2016

View reviewed changes

stack72 closed this Nov 28, 2016

mathieuherbert deleted the routetable-datasource branch November 28, 2016 20:21

swestcott reviewed Dec 15, 2016

View reviewed changes

ghost locked and limited conversation to collaborators Apr 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: data source for AWS Route Table #10301

provider/aws: data source for AWS Route Table #10301

mathieuherbert commented Nov 22, 2016

kwilczynski Nov 22, 2016

kwilczynski Nov 22, 2016

kwilczynski Nov 22, 2016

mathieuherbert Nov 23, 2016

kwilczynski Nov 22, 2016

kwilczynski Nov 22, 2016

mathieuherbert Nov 23, 2016

kwilczynski Nov 24, 2016

mathieuherbert commented Nov 23, 2016

kwilczynski Nov 23, 2016

kwilczynski commented Nov 23, 2016

kwilczynski commented Nov 23, 2016

mathieuherbert commented Nov 23, 2016

kwilczynski Nov 24, 2016

kwilczynski Nov 24, 2016

kwilczynski Nov 24, 2016

kwilczynski Nov 24, 2016

mathieuherbert commented Nov 24, 2016

kwilczynski commented Nov 24, 2016

mathieuherbert commented Nov 28, 2016

kwilczynski Nov 28, 2016

kwilczynski Nov 28, 2016

kwilczynski commented Nov 28, 2016

stack72 commented Nov 28, 2016

stack72 commented Nov 28, 2016

swestcott Dec 15, 2016

ghost commented Apr 18, 2020


		* `filter` - (Optional) Custom filter block as described below.

		* `id` - (Optional) The id of the specific Route Table to retrieve.

provider/aws: data source for AWS Route Table #10301

provider/aws: data source for AWS Route Table #10301

Conversation

mathieuherbert commented Nov 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathieuherbert commented Nov 23, 2016

Choose a reason for hiding this comment

kwilczynski commented Nov 23, 2016

kwilczynski commented Nov 23, 2016

mathieuherbert commented Nov 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathieuherbert commented Nov 24, 2016

kwilczynski commented Nov 24, 2016

mathieuherbert commented Nov 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwilczynski commented Nov 28, 2016

stack72 commented Nov 28, 2016

stack72 commented Nov 28, 2016

Choose a reason for hiding this comment

ghost commented Apr 18, 2020