New Datasource: aws_transfer_server

[atsushi-ishibashi]

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #7976

Changes proposed in this pull request:

• Data source: aws_transfer_server

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccDataSourceAwsTransferServer_'
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccDataSourceAwsTransferServer_ -timeout 120m
=== RUN   TestAccDataSourceAwsTransferServer_basic
=== PAUSE TestAccDataSourceAwsTransferServer_basic
=== RUN   TestAccDataSourceAwsTransferServer_service_managed
=== PAUSE TestAccDataSourceAwsTransferServer_service_managed
=== RUN   TestAccDataSourceAwsTransferServer_apigateway
=== PAUSE TestAccDataSourceAwsTransferServer_apigateway
=== CONT  TestAccDataSourceAwsTransferServer_basic
=== CONT  TestAccDataSourceAwsTransferServer_service_managed
=== CONT  TestAccDataSourceAwsTransferServer_apigateway
--- PASS: TestAccDataSourceAwsTransferServer_basic (52.01s)
--- PASS: TestAccDataSourceAwsTransferServer_service_managed (54.28s)
--- PASS: TestAccDataSourceAwsTransferServer_apigateway (65.28s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	65.333s

[bflad]

Some initial items below but otherwise looking great. Thanks so much @atsushi-ishibashi 🥇

[bflad]

Since invocation_role and the below attributes cannot be configured to lookup the Transfer server, Optional: true should be removed from them 👍

[bflad]

Currently, Terraform data sources should always error when their associated resource is not found, so this handling should be removed.

[bflad]

We should provide error context for operators and code maintainers here, e.g.

Suggested change

	return err
	return fmt.Errorf("error describing Transfer Server (%s): %s", serverID, err)

[bflad]

We can remove the custom function by using resource.TestCheckResourceAttrPair(), e.g.

Suggested change

	testAccDataSourceAwsTransferServerCheck(datasourceName, resourceName),
	resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"),
	resource.TestCheckResourceAttrPair(datasourceName, "endpoint", resourceName, "endpoint"),
	resource.TestCheckResourceAttrPair(datasourceName, "identity_provider_type", resourceName, "identity_provider_type"),
	resource.TestCheckResourceAttrPair(datasourceName, "invocation_role", resourceName, "invocation_role"),
	resource.TestCheckResourceAttrPair(datasourceName, "logging_role", resourceName, "logging_role"),
	resource.TestCheckResourceAttrPair(datasourceName, "url", resourceName, "url"),

Along with simplifying the code, we prefer using this upstream function since it also has some special handling in the upcoming Terraform 0.12 Provider SDK to ignore differences between missing values and zero-values. 😄

[StephenKing]

Thanks for working on this. What about filters?

[bflad]

@StephenKing if you are referring to the filter abilities offered by many of the EC2 data sources, it exists because the EC2 API directly offers those filtering options in their Describe* API calls. In the case of the Transfer service, it looks like we would have to loop through all the Transfer Servers to manually filter via the ListServers API call. I'd recommend creating a separate GitHub feature request for that support since this is going to be merged as a good first implementation of the data source.

[bflad]

LGTM, thanks @atsushi-ishibashi 🚀 (TravisCI failures related to #7993)

--- PASS: TestAccDataSourceAwsTransferServer_basic (11.79s)
--- PASS: TestAccDataSourceAwsTransferServer_service_managed (13.22s)
--- PASS: TestAccDataSourceAwsTransferServer_apigateway (19.66s)

[bflad]

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

[StephenKing]

I see. As the hostname parameter isn't supported by TF atm., there is actually nothing to filter..

[jvanwagner]

@atsushi-ishibashi @bflad is there a way to connect this Transfer Server as I can in the console using the endpoint type of Public or VPC?

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!