provider/aws: Allow filtering of aws_subnet_ids by tags

[tomelliff]

This is the minimal amount of work needed to be able to create a list of a subset of subnet IDs in a VPC, allowing people to loop through them easily when creating EC2 instances or provide a list straight to an ELB.

As mentioned in the docs updated this allows for configuration like this:

data "aws_subnet_ids" "private" {
  vpc_id = "${var.vpc_id}"
  tags {
    Tier = "Private"
  }
}

resource "aws_instance" "app" {
  count         = "3"
  ami           = "${var.ami}"
  instance_type = "t2.micro"
  subnet_id     = "${element(data.aws_subnet_ids.private.ids, count.index)}"
}

Which in my brief testing led to plans of the following:

+ aws_instance.app.0
    ami:                         "ami-971238f1"
    associate_public_ip_address: "<computed>"
    availability_zone:           "<computed>"
    ebs_block_device.#:          "<computed>"
    ephemeral_block_device.#:    "<computed>"
    instance_state:              "<computed>"
    instance_type:               "t2.micro"
    ipv6_addresses.#:            "<computed>"
    key_name:                    "<computed>"
    network_interface_id:        "<computed>"
    placement_group:             "<computed>"
    private_dns:                 "<computed>"
    private_ip:                  "<computed>"
    public_dns:                  "<computed>"
    public_ip:                   "<computed>"
    root_block_device.#:         "<computed>"
    security_groups.#:           "<computed>"
    source_dest_check:           "true"
    subnet_id:                   "subnet-669ac310"
    tenancy:                     "<computed>"
    vpc_security_group_ids.#:    "<computed>"

+ aws_instance.app.1
    ami:                         "ami-971238f1"
    associate_public_ip_address: "<computed>"
    availability_zone:           "<computed>"
    ebs_block_device.#:          "<computed>"
    ephemeral_block_device.#:    "<computed>"
    instance_state:              "<computed>"
    instance_type:               "t2.micro"
    ipv6_addresses.#:            "<computed>"
    key_name:                    "<computed>"
    network_interface_id:        "<computed>"
    placement_group:             "<computed>"
    private_dns:                 "<computed>"
    private_ip:                  "<computed>"
    public_dns:                  "<computed>"
    public_ip:                   "<computed>"
    root_block_device.#:         "<computed>"
    security_groups.#:           "<computed>"
    source_dest_check:           "true"
    subnet_id:                   "subnet-a4aa8dc0"
    tenancy:                     "<computed>"
    vpc_security_group_ids.#:    "<computed>"

+ aws_instance.app.2
    ami:                         "ami-971238f1"
    associate_public_ip_address: "<computed>"
    availability_zone:           "<computed>"
    ebs_block_device.#:          "<computed>"
    ephemeral_block_device.#:    "<computed>"
    instance_state:              "<computed>"
    instance_type:               "t2.micro"
    ipv6_addresses.#:            "<computed>"
    key_name:                    "<computed>"
    network_interface_id:        "<computed>"
    placement_group:             "<computed>"
    private_dns:                 "<computed>"
    private_ip:                  "<computed>"
    public_dns:                  "<computed>"
    public_ip:                   "<computed>"
    root_block_device.#:         "<computed>"
    security_groups.#:           "<computed>"
    source_dest_check:           "true"
    subnet_id:                   "subnet-9b5c3cc3"
    tenancy:                     "<computed>"
    vpc_security_group_ids.#:    "<computed>"


Plan: 3 to add, 0 to change, 0 to destroy.

The order appears to be stable every time I plan but to be honest I'm not sure exactly what the ordering is that's coming back from AWS' API. As long as it's stable I don't actually care what order my instances are placed in the subnets although I imagine that some people may have a requirement that different filters lead to a stable order of AZs but I've deliberately left that out of scope for now.

I've also just added filtering by tags but any of the filters on the singular subnet data source should be usable. I can add these if people want them or we can do that in a future PR if people want that functionality at a later date.

Testing looping through multiple times and also that sort order is stable:

$ for i in 1 2 3; do terraform plan | grep subnet_id; done
data.aws_subnet_ids.private: Refreshing state...
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"
data.aws_subnet_ids.private: Refreshing state...
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"
data.aws_subnet_ids.private: Refreshing state...
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"
    subnet_id:                   "subnet-679ac311"
    subnet_id:                   "subnet-9a5c3cc2"
    subnet_id:                   "subnet-adaa8dc9"

Acceptance tests:

$ make testacc TEST=./builtin/providers/aws TESTARGS="-run=TestAccDataSourceAwsSubnetIDs"
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/25 13:40:09 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccDataSourceAwsSubnetIDs -timeout 120m
=== RUN   TestAccDataSourceAwsSubnetIDs
--- PASS: TestAccDataSourceAwsSubnetIDs (108.30s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	108.322s

[tomelliff]

I'm not a fan of the duplicated resources in the acceptance test and asked a question about that on gitter (it happens in a lot of other places but not all of them, for example the singular subnet data source does it like this for ip6 but not for ip4 tests) but when I tried removing the config without data sources step it failed and I don't know why with my limited exposure to the test framework.

Could anyone explain why it's needed?

[grubernaut]

Hey @tomelliff, thanks for the contribution!

Data sources need to be evaluated after the actual resources are created. Usually this isn't a concern, as the data source configuration contains interpolated values from the actual resource creation, so Terraform graphs the resources accordingly. However, there are cases, like this one for example, where the data source isn't configured with interpolations on the dependent resource. This can sometimes be solved via adding depends_on on the data source, but we've also experienced some core bugs there. Separating the data source addition into two separate Terraform Runs guarantees the parent resource is created before the data source is refreshed.

[grubernaut]

$ make testacc TEST=./builtin/providers/aws TESTARGS="-run=TestAccDataSourceAwsSubnetIDs" 
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/25 17:31:16 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccDataSourceAwsSubnetIDs -timeout 120m
=== RUN   TestAccDataSourceAwsSubnetIDs
--- PASS: TestAccDataSourceAwsSubnetIDs (53.81s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    53.852s

[grubernaut]

LGTM, thanks!

[tomelliff]

Ah fair enough, hadn't spotted that somehow. Thanks for that!

[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 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.