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

provider/aws: Add 'aws_vpc_peering_connection' data source #10913

Merged
merged 3 commits into from
Dec 30, 2016
Merged

provider/aws: Add 'aws_vpc_peering_connection' data source #10913

merged 3 commits into from
Dec 30, 2016

Conversation

ewbankkit
Copy link
Contributor

Provides details about a specific VPC peering connection.
I tried to keep the semantics of the attributes the same as for the aws_vpc_peering_connection resource.
Acceptance test:

make testacc TEST=./builtin/providers/aws/ TESTARGS='-run=TestAccDataSourceAwsVpcPeeringConnection_'

@@ -450,7 +451,7 @@ func init() {
" no session name is passed to the AssumeRole call.",

"assume_role_external_id": "The external ID to use when assuming the role. If omitted," +
" no external ID is passed to the AssumeRole call.",
" no external ID is passed to t he AssumeRole call.",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this modification is a mistake :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I'll revert it.


req := &ec2.DescribeVpcPeeringConnectionsInput{}

if id := d.Get("id"); id != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

d.GetOk("id") wouldn't be better ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.

@kwilczynski
Copy link
Contributor

@ewbankkit hi there! Thank you for this.

Would you consider adding support for the requester and accepter blocks to expose configuration that is available there, what do you think?

Read: dataSourceAwsVpcPeeringConnectionRead,

Schema: map[string]*schema.Schema{
"id": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

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

From Go 1.7 onwards type is derived, therefore there is no need to add schema.Schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.

@ewbankkit
Copy link
Contributor Author

@kwilczynski Yes, I'll support for the requester and accepter blocks as per the resource.
No support for IPv6 yet as that seems like a more systemic change affecting many resources and data sources.

@stack72
Copy link
Contributor

stack72 commented Dec 30, 2016

LGTM! Thanks for the work here @ewbankkit and the review @kwilczynski

I agree on not supporting IPV6 just now

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccDataSourceAwsVpcPeeringConnection_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/30 00:28:37 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccDataSourceAwsVpcPeeringConnection_ -timeout 120m
=== RUN   TestAccDataSourceAwsVpcPeeringConnection_basic
--- PASS: TestAccDataSourceAwsVpcPeeringConnection_basic (49.71s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	49.737s

@stack72 stack72 merged commit e8f4d00 into hashicorp:master Dec 30, 2016
Mongey pushed a commit to Mongey/terraform that referenced this pull request Jan 6, 2017
…#10913)

* Add 'aws_vpc_peering_connection' data source.

* Changes after code review.

* Add 'accepter' and 'requester' blocks to aws_vpc_peering_connection data source output attributes.
@ewbankkit ewbankkit deleted the aws_vpc_peering_connection-data-source branch January 7, 2017 12:38
@ghost
Copy link

ghost commented Apr 18, 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 18, 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.

5 participants