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

New Data Source: aws_dx_connection #17852

Merged
merged 32 commits into from
Aug 25, 2021
Merged

New Data Source: aws_dx_connection #17852

merged 32 commits into from
Aug 25, 2021

Conversation

yzguy
Copy link

@yzguy yzguy commented Feb 27, 2021

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #18400.
Closes #10472.
Closes #17853.

Output from acceptance testing:

-> make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection -timeout 120m
=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (23.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	25.404s

@yzguy yzguy requested a review from a team as a code owner February 27, 2021 03:14
@ghost ghost added size/M Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/directconnect Issues and PRs that pertain to the directconnect service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Feb 27, 2021
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Feb 27, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @yzguy 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@ewbankkit
Copy link
Contributor

@ewbankkit
Copy link
Contributor

@yzguy Thanks for the contribution 👏.
There is a stale PR #9705 that adds the same data source.
Could you please add the additional attributes from that PR?

@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Feb 28, 2021
@yzguy
Copy link
Author

yzguy commented Mar 1, 2021

@ewbankkit Absolutely! Will update PR soon.

@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Mar 1, 2021
@yzguy
Copy link
Author

yzguy commented Mar 1, 2021

Couple things I believe I did right, but am unsure.

  1. You can search by ID or Name. If you used both, it would search for the connection with the specific ID, then only continue if it found the one with the specified name. If it didn't it would error out. Not sure if it's necessary to actually make them mutually exclusive.

  2. It looks like AWS does not allow filtering on DirectConnect connections so I left this out, but I'm unsure if searching by tags would still be a good idea.

  3. A couple of the fields (vlan, partner_name, provider_name, lag_id) are not present in the JSON response from AWS all the time, and I believe depending on if it's a Hosted versus Dedicated connection they will show up, or if you have 2 or more connections in LAG. I don't have access to an environment that uses hosted connections, only dedicated. Therefore I have made them only added when they are present in the JSON.

I referenced @robh007 #9705 and @beezly branch and want to make sure to give credit as some parts were copy+paste.

@ewbankkit
Copy link
Contributor

@yzguy The better way here I think would be to add an additional data source aws_dx_connections where connections can be filtered by name or tags. This DS would output just a set of IDs. The aws_dx_connection would have the one required attribute connection_id and output all the other connection attributes.
You can look at the aws_apigatewayv2_apis and aws_apigatewayv2_api data sources for an example for how to approach this.
BTW you don't need the != nil check for fields missing from the API response, d.Set() handles nil input.

Let me know if this is not clear or if you are unable to make progress.
Thanks.

@yzguy
Copy link
Author

yzguy commented Mar 12, 2021

-> make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection -timeout 120m
^[[A=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== RUN   TestAccDataSourceAwsDxConnections_Name
--- PASS: TestAccDataSourceAwsDxConnections_Name (21.49s)
=== RUN   TestAccDataSourceAwsDxConnections_Tags
--- PASS: TestAccDataSourceAwsDxConnections_Tags (19.61s)
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (19.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	63.153s

@yzguy
Copy link
Author

yzguy commented Mar 12, 2021

@ewbankkit I was able to get back to this after, completing the changes you suggested. Using the API Gateway v2 Datasource as a reference.

Couple things I'm not sure about are on data.aws_dx_connection there are a bunch of attributes that are not returned by aws_dx_connection, so I'm not checking they match in the test. I'm not sure best way to go about that, perhaps just checking that the attributes are set on the data source.

Other thing is that I pass in the Bandwidth and Locations in the tests, and that works, but I believe because both are us-east-1 resources, and that's my default region that works fine, but we can't be 100% certain that the tests run in us-east-1, or that they would be forever. I believe maybe a better choice would be to have perhaps a map of locations per regions, then just randomly select one for whatever region the tests run it.

@ewbankkit
Copy link
Contributor

@yzguy The location/bandwidth issue will be resolvable once #9735, aws_dx_location(s) data sources, has been merged.
Don't worry about checking for specific values of any of those attributes, just checking that they are set is sufficient.
There are also some formatting issues with the acceptance test configurations.
Thanks.

@ewbankkit
Copy link
Contributor

ewbankkit commented Mar 25, 2021

@yzguy Could you please add "Closes #18400" to the initial comment; Thanks.

@yzguy
Copy link
Author

yzguy commented Mar 25, 2021

@ewbankkit Done!

@github-actions github-actions bot removed provider Pertains to the provider itself, rather than any interaction with AWS. documentation Introduces or discusses updates to documentation. size/XL Managed by automation to categorize the size of a PR. labels Aug 5, 2021
ewbankkit and others added 10 commits August 24, 2021 15:25
Acceptance test output:

% make testacc TESTARGS='-run=TestAccAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxConnection_ -timeout 180m
=== RUN   TestAccAwsDxConnection_basic
=== PAUSE TestAccAwsDxConnection_basic
=== RUN   TestAccAwsDxConnection_disappears
=== PAUSE TestAccAwsDxConnection_disappears
=== RUN   TestAccAwsDxConnection_Tags
=== PAUSE TestAccAwsDxConnection_Tags
=== CONT  TestAccAwsDxConnection_basic
=== CONT  TestAccAwsDxConnection_Tags
=== CONT  TestAccAwsDxConnection_disappears
--- PASS: TestAccAwsDxConnection_disappears (24.65s)
--- PASS: TestAccAwsDxConnection_basic (30.83s)
--- PASS: TestAccAwsDxConnection_Tags (68.13s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	74.122s
Acceptance test output:

% make testacc TESTARGS='-run=TestAccAwsDxConnection_\|TestAccAwsDxLag_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxConnection_\|TestAccAwsDxLag_ -timeout 180m
=== RUN   TestAccAwsDxConnection_basic
=== PAUSE TestAccAwsDxConnection_basic
=== RUN   TestAccAwsDxConnection_disappears
=== PAUSE TestAccAwsDxConnection_disappears
=== RUN   TestAccAwsDxConnection_Tags
=== PAUSE TestAccAwsDxConnection_Tags
=== RUN   TestAccAwsDxLag_basic
=== PAUSE TestAccAwsDxLag_basic
=== RUN   TestAccAwsDxLag_disappears
=== PAUSE TestAccAwsDxLag_disappears
=== RUN   TestAccAwsDxLag_Tags
=== PAUSE TestAccAwsDxLag_Tags
=== CONT  TestAccAwsDxConnection_basic
=== CONT  TestAccAwsDxLag_disappears
=== CONT  TestAccAwsDxLag_Tags
=== CONT  TestAccAwsDxConnection_disappears
=== CONT  TestAccAwsDxConnection_Tags
=== CONT  TestAccAwsDxLag_basic
--- PASS: TestAccAwsDxLag_disappears (19.44s)
--- PASS: TestAccAwsDxConnection_disappears (19.49s)
--- PASS: TestAccAwsDxConnection_basic (23.77s)
--- PASS: TestAccAwsDxLag_basic (36.78s)
--- PASS: TestAccAwsDxConnection_Tags (52.49s)
--- PASS: TestAccAwsDxLag_Tags (53.92s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       57.070s
Acceptance test output:

% make testacc TESTARGS='-run=TestAccAWSDxConnectionAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDxConnectionAssociation_ -timeout 180m
=== RUN   TestAccAWSDxConnectionAssociation_basic
=== PAUSE TestAccAWSDxConnectionAssociation_basic
=== RUN   TestAccAWSDxConnectionAssociation_multiConns
=== PAUSE TestAccAWSDxConnectionAssociation_multiConns
=== CONT  TestAccAWSDxConnectionAssociation_basic
=== CONT  TestAccAWSDxConnectionAssociation_multiConns
--- PASS: TestAccAWSDxConnectionAssociation_basic (25.72s)
--- PASS: TestAccAWSDxConnectionAssociation_multiConns (26.54s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       34.593s
r/aws_dx_lag: Add 'owner_account_id' attribute.

Acceptance test output:

% make testacc TESTARGS='-run=TestAccAwsDxConnection_basic\|TestAccAwsDxLag_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxConnection_basic\|TestAccAwsDxLag_basic -timeout 180m
=== RUN   TestAccAwsDxConnection_basic
=== PAUSE TestAccAwsDxConnection_basic
=== RUN   TestAccAwsDxLag_basic
=== PAUSE TestAccAwsDxLag_basic
=== CONT  TestAccAwsDxConnection_basic
=== CONT  TestAccAwsDxLag_basic
--- PASS: TestAccAwsDxConnection_basic (19.66s)
--- PASS: TestAccAwsDxLag_basic (33.61s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       36.900s
Acceptance test output:

% make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection_ -timeout 180m
=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (16.58s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       21.036s
r/aws_dx_lag: Add 'provider_name' argument.

Acceptance test output:

% make testacc TESTARGS='-run=TestAccAwsDxConnection_basic\|TestAccAwsDxConnection_ProviderName'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxConnection_basic\|TestAccAwsDxConnection_ProviderName -timeout 180m
=== RUN   TestAccAwsDxConnection_basic
=== PAUSE TestAccAwsDxConnection_basic
=== RUN   TestAccAwsDxConnection_ProviderName
=== PAUSE TestAccAwsDxConnection_ProviderName
=== CONT  TestAccAwsDxConnection_basic
=== CONT  TestAccAwsDxConnection_ProviderName
--- PASS: TestAccAwsDxConnection_basic (23.95s)
--- PASS: TestAccAwsDxConnection_ProviderName (26.53s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       32.586s
% make testacc TESTARGS='-run=TestAccAwsDxLag_basic\|TestAccAwsDxLag_ProviderName'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxLag_basic\|TestAccAwsDxLag_ProviderName -timeout 180m
=== RUN   TestAccAwsDxLag_basic
=== PAUSE TestAccAwsDxLag_basic
=== RUN   TestAccAwsDxLag_ProviderName
=== PAUSE TestAccAwsDxLag_ProviderName
=== CONT  TestAccAwsDxLag_basic
=== CONT  TestAccAwsDxLag_ProviderName
--- PASS: TestAccAwsDxLag_ProviderName (24.24s)
--- PASS: TestAccAwsDxLag_basic (33.67s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       39.380s
Acceptance test output:

% make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection_ -timeout 180m
=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (24.67s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       29.105s
@github-actions github-actions bot added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/location Issues and PRs that pertain to the location service. and removed size/M Managed by automation to categorize the size of a PR. labels Aug 25, 2021
@ewbankkit ewbankkit added new-data-source Introduces a new data source. and removed service/location Issues and PRs that pertain to the location service. labels Aug 25, 2021
@github-actions github-actions bot added the service/location Issues and PRs that pertain to the location service. label Aug 25, 2021
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

Commercial
% make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection_ -timeout 180m
=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (24.67s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       29.105s
GovCloud
% make testacc TESTARGS='-run=TestAccDataSourceAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsDxConnection_ -timeout 180m
=== RUN   TestAccDataSourceAwsDxConnection_basic
=== PAUSE TestAccDataSourceAwsDxConnection_basic
=== CONT  TestAccDataSourceAwsDxConnection_basic
--- PASS: TestAccDataSourceAwsDxConnection_basic (31.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	36.544s

@ewbankkit ewbankkit removed the service/location Issues and PRs that pertain to the location service. label Aug 25, 2021
@ewbankkit
Copy link
Contributor

@yzguy Thanks for the contribution 🎉 👏.
I went ahead and added some additional attributes to the data source.

@ewbankkit ewbankkit merged commit 709e4a0 into hashicorp:main Aug 25, 2021
@github-actions github-actions bot added this to the v3.56.0 milestone Aug 25, 2021
@yzguy yzguy deleted the data_source_aws_dx_connection branch August 25, 2021 21:18
@github-actions
Copy link

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-data-source Introduces a new data source. provider Pertains to the provider itself, rather than any interaction with AWS. service/directconnect Issues and PRs that pertain to the directconnect service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
2 participants