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

[WIP]: provider/aws: Add aws_availability_zones data source #6671

Closed
wants to merge 12 commits into from

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented May 14, 2016

This PR adds a data source with a single set, available for the schema which gets populated with the availability zones to which an account has access.

I implemented it as part of the review process of data sources so thought I would open an additional PR for feedback from @apparentlymart about this implementation.

One area we need to consider is nomenclature (cc @phinze, @catsby, @mitchellh) - many of these sources will return lists, and we should take care to keep naming arity consistent - I am expecting the actual names here to change when we make that decision.

TODO: Acceptance test.

ZZelle and others added 2 commits April 21, 2016 09:43
Official OpenStack clients commonly support specifing a client
certificate/key to enable SSL client authentication when communicating
with OpenStack services. This patch enables such feature in Terraform
with new parameters and environment variables:

* 'cert' provider parameter or OS_CERT env variable to specify client
   certificate file,
* 'key' provider parameter or OS_KEY env variable to specify client
   certificate private key file.
@vancluever
Copy link
Contributor

vancluever commented May 14, 2016 via email

@apparentlymart
Copy link
Member

@jen20 my thought was that to start we'd mainly focus on data sources that return single objects... so aws_ami returns a single AMI matching given criteria, etc... because right now you can't really do much with attributes that are lists of resources in Terraform anyway.

However, this one I think makes sense because ultimately it's just a list of strings, which Terraform is pretty good at dealing with. And so calling it aws_availability_zones makes sense.

I was planning to build out aws_ami (using @vancluever's earlier work as a starting point) as an initial non-trivial data source example, establishing a pattern for how AWS Describe* actions can map on to data sources... but I was planning to wait until #6598 lands before building that, in case we make any changes to the interface before it is merged.

Provides a list of availability zones which can be used by an AWS account
---

# aws\_availability_zones
Copy link
Member

Choose a reason for hiding this comment

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

Does this second underscore also need to be escaped, or will Markdown ignore it because there's an odd number of them? (In other files I've seen all the underscores escaped and have been following that pattern, but not sure if that's actually necessary...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably. I hate markdown ;-)

@apparentlymart
Copy link
Member

Looks good to me, minor inline niggles aside. 🎺

Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
Copy link
Member

Choose a reason for hiding this comment

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

Certainly doesn't have to be in the first version of this data source, but I was thinking that when mapping Describe* calls onto data sources we'd map each of the filters onto an Optional attribute and then include them in the call.

In this case, that could be attributes regions, which would be a list of regions to constrain to, and states, allowing to filter by state.

So then I'd use it something like this:

data "aws_availability_zones" "us" {
    regions = ["us-west-1", "us-west-2", "us-east-1"]
    states = ["available", "information", "impaired"] # don't include "unavailable" AZs
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@apparentlymart I'm not sure that I see the value in this case, since the configured provider will determine which region is valid for use? In general though, I agree with this.

jtopjian and others added 10 commits May 14, 2016 20:44
provider/openstack: Support client certificates
provider/openstack: Reassociate FIP on network changes
Don't assume dev platform was built
Data-driven Terraform Configuration
This commit adds a data source with a single set, `available` for the
schema which gets populated with the availability zones to which an
account has access.
@jen20 jen20 force-pushed the f-data-source-availability-zone branch from 40968e8 to ad69168 Compare May 16, 2016 18:32
@jen20
Copy link
Contributor Author

jen20 commented May 16, 2016

I have no idea why the diff is so screwed up here - I thought I had rebased onto origin/master. I'll close this one and open a new PR for this soon.

@jen20 jen20 closed this May 16, 2016
@jen20 jen20 deleted the f-data-source-availability-zone branch May 16, 2016 18:36
@ghost
Copy link

ghost commented Apr 25, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants