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.

Sign up for GitHub

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

Jump to bottom

Remove terraform-provider-tls dependency #10023

Closed
bflad opened this issue Sep 6, 2019 · 3 comments · Fixed by #10434
Closed

Remove terraform-provider-tls dependency #10023

bflad opened this issue Sep 6, 2019 · 3 comments · Fixed by #10434
Labels
provider Pertains to the provider itself, rather than any interaction with AWS. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Milestone
v2.32.0

Comments

@bflad
Copy link
Contributor

bflad commented Sep 6, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

We have a few reasons to migrate away from importing the Terraform TLS Provider code directly:

  • Terraform Providers are not intended to be used as libraries. Our usage here is a vestige of when all provider code lived in the same Terraform repository.
  • Since the Terraform TLS Provider is versioned at v2.1.2 due to its resultant binary, the strict validation of Go proxy servers to disallow +incompatible means its not possible for this dependency to be cached. While we do currently vendor dependencies, we would like to move away from that model now that public Go proxies are becoming more prevalent and defaulting to being enabled
  • The parts of the provider functionality we generally use (RSA key and certificate generation) should not be difficult to reimplement in our own helper functions

Hard requirements:

  • tls_private_key resource replacement:
    • Algorithms: RSA2048, RSA4096
    • Terraform attributes used: private_key_pem
  • tls_self_signed_cert resource replacement:
    • Allowed uses: digital_signature, key_encipherment, server_auth
    • Subject Common Name: example.com, {generated}.com, {generated} (so, effectively just need to be able to pass any common name)
    • Subject Organization: ACME Examples, Inc
    • Terraform attributes used: cert_pem

Soft requirements:

Only for aws/resource_aws_api_gateway_base_path_mapping_test.go and aws/resource_aws_api_gateway_domain_name_test.go (for functionality which is already deprecated in API Gateway):

  • tls_cert_request resource replacement
  • tls_locally_signed_cert resource replacement

References
@bflad bflad added technical-debt Addresses areas of the codebase that need refactoring or redesign. provider Pertains to the provider itself, rather than any interaction with AWS. labels Sep 6, 2019
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Sep 6, 2019
bflad added a commit that referenced this issue Sep 6, 2019
@bflad
provider: Implement replacement functions for tls_private_key and tls…
b33af5c 
…_self_signed_certificate test configuration resources

Reference: #10023
bflad added a commit that referenced this issue Sep 6, 2019
@bflad
data-source/aws_acm_certificate: Switch from terraform-provider-tls t…
8e7a3a3 
…o internal functions

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSAcmCertificateDataSource_KeyTypes (18.45s)
```
@bflad bflad mentioned this issue Sep 6, 2019
Merged
@bflad
Copy link
Contributor Author

bflad commented Sep 6, 2019

Proof of concept: #10024

@bflad bflad added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 6, 2019
@bflad bflad mentioned this issue Sep 26, 2019
Closed
1 task
bflad added a commit that referenced this issue Sep 27, 2019
@bflad
tests/data-source/aws_lb_listener: Use internal implementation for TL…
7d8103b 
…S key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccDataSourceAWSLBListener_https (217.29s)
```
@bflad bflad mentioned this issue Sep 27, 2019
Merged
bflad added a commit that referenced this issue Sep 28, 2019
@bflad
tests/resource/aws_acm_certificate: Use internal implementation for T…
9707dd5 
…LS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (14.99s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (23.89s)
```
@bflad bflad mentioned this issue Sep 28, 2019
Merged
bflad added a commit that referenced this issue Sep 28, 2019
@bflad
tests/resource/aws_cloudfront_distribution: Use internal implementati…
ffd489c 
…on for TLS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1281.11s)
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_ConflictsWithCloudFrontDefaultCertificate (1284.54s)
```
@bflad bflad mentioned this issue Sep 28, 2019
Merged
bflad added a commit that referenced this issue Sep 28, 2019
@bflad
tests/resource/aws_ec2_client_vpn_network_association: Use internal i…
c558b34 
…mplementation for TLS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAwsEc2ClientVpnNetworkAssociation_basic (549.92s)
--- PASS: TestAccAwsEc2ClientVpnNetworkAssociation_disappears (581.03s)
```
bflad added a commit that referenced this issue Sep 28, 2019
@bflad
tests/resource/aws_ec2_client_vpn_endpoint: Use internal implementati…
40ce50b 
…on for TLS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAwsEc2ClientVpnEndpoint_disappears (18.01s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_basic (17.22s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_splitTunnel (29.09s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_withDNSServers (29.77s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_withLogGroup (36.80s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_tags (39.35s)
--- PASS: TestAccAwsEc2ClientVpnEndpoint_msAD (1761.91s)
```
This was referenced Sep 28, 2019
Merged
Merged
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
tests/resource/aws_elb: Use internal implementation for TLS key/certi…
5d2c45d 
…ficate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSELB_Listener_SSLCertificateID_IAMServerCertificate (38.75s)
```
@bflad bflad mentioned this issue Sep 29, 2019
Merged
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
tests/resource/aws_load_balancer_backend_server_policy: Use internal …
3e444a4 
…implementation for TLS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSLoadBalancerBackendServerPolicy_basic (70.37s)
```
@bflad bflad mentioned this issue Sep 29, 2019
Merged
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
resource/aws_lb_listener_certificate: Retry CertificateNotFound error…
ab43e80 
…s on creation, use internal implementation for test TLS key/certificate

Reference: #10023

Fixes the following issue discovered during acceptance testing due to IAM eventual consistency:

```
--- FAIL: TestAccAwsLbListenerCertificate_multiple (326.35s)
    testing.go:569: Step 1 error: errors during apply:

        Error: Error creating LB Listener Certificate: CertificateNotFound: Certificate 'arn:aws:iam::187416307283:server-certificate/tf-acc-test-3065966466410433081-additional-3' not found
```

This also refactors the testing configurations so they can share a common base configuration.

Output from acceptance testing:

```
--- PASS: TestAccAwsLbListenerCertificate_basic (186.16s)
--- PASS: TestAccAwsLbListenerCertificate_multiple (271.64s)
```
@bflad bflad mentioned this issue Sep 29, 2019
Merged
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
tests/resource/aws_listener_rule: Use internal implementation for TLS…
99ab002 
… key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSLBListenerRule_oidc (198.18s)
--- PASS: TestAccAWSLBListenerRule_Action_Order (202.37s)
--- PASS: TestAccAWSLBListenerRule_Action_Order_Recreates (243.71s)
--- PASS: TestAccAWSLBListenerRule_cognito (243.72s)
```
@bflad bflad mentioned this issue Sep 29, 2019
Merged
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
tests/resource/aws_lb_listener: Use internal implementation for TLS k…
b4c02f9 
…ey/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSLBListener_https (193.49s)
--- PASS: TestAccAWSLBListener_DefaultAction_Order_Recreates (202.01s)
--- PASS: TestAccAWSLBListener_cognito (202.92s)
--- PASS: TestAccAWSLBListener_oidc (214.41s)
--- PASS: TestAccAWSLBListener_DefaultAction_Order (223.52s)
--- PASS: TestAccAWSLBListener_Protocol_Tls (340.53s)
```
bflad added a commit that referenced this issue Sep 29, 2019
@bflad
tests/service/apigateway: Use internal implementation for TLS key/cer…
80585bb 
…tificate

Reference: #10023

Switches `aws_api_gateway_base_path_mapping` testing to the simplest `aws_api_gateway_domain_name` configuration with `regional_certificate_arn`. Requires new environment variables for `certificate_name` testing as CloudFront now strictly requires a publicly trusted key/certificate:

```
--- FAIL: TestAccAWSAPIGatewayDomainName_CertificateName (5.58s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error creating API Gateway Domain Name: BadRequestException: The certificate that is attached to your distribution was not issued by a trusted Certificate Authority. For more details, see: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/CNAMEs.html#alternate-domain-names-requirements (Service: AmazonCloudFront; Status Code: 400; Error Code: InvalidViewerCertificate; Request ID: e052f947-e261-11e9-a70c-b3beead7f798)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSAPIGatewayBasePathMapping_basic (103.50s)
--- PASS: TestAccAWSAPIGatewayBasePathMapping_BasePath_Empty (61.24s)

--- SKIP: TestAccAWSAPIGatewayDomainName_CertificateArn (0.00s)
--- SKIP: TestAccAWSAPIGatewayDomainName_CertificateName (0.00s)
--- SKIP: TestAccAWSAPIGatewayDomainName_RegionalCertificateName (0.00s)
--- PASS: TestAccAWSAPIGatewayDomainName_RegionalCertificateArn (20.45s)
--- PASS: TestAccAWSAPIGatewayDomainName_SecurityPolicy (104.68s)
```
bflad added a commit that referenced this issue Sep 30, 2019
@bflad
tests/resource/aws_lb_ssl_negotiation_policy: Use internal implementa…
1005e17 
…tion for TLS key/certificate

Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSLBSSLNegotiationPolicy_disappears (33.49s)
--- PASS: TestAccAWSLBSSLNegotiationPolicy_basic (40.18s)
```
This was referenced Sep 30, 2019
Merged
Merged
bflad added a commit that referenced this issue Sep 30, 2019
@bflad
tests/service/apigateway: Use internal implementation for TLS key/cer…
da73396 
…tificate

Reference: #10023

Switches `aws_api_gateway_base_path_mapping` testing to the simplest `aws_api_gateway_domain_name` configuration with `regional_certificate_arn`. Requires new environment variables for `certificate_name` testing as CloudFront now strictly requires a publicly trusted key/certificate:

```
--- FAIL: TestAccAWSAPIGatewayDomainName_CertificateName (5.58s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error creating API Gateway Domain Name: BadRequestException: The certificate that is attached to your distribution was not issued by a trusted Certificate Authority. For more details, see: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/CNAMEs.html#alternate-domain-names-requirements (Service: AmazonCloudFront; Status Code: 400; Error Code: InvalidViewerCertificate; Request ID: e052f947-e261-11e9-a70c-b3beead7f798)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSAPIGatewayBasePathMapping_basic (103.50s)
--- PASS: TestAccAWSAPIGatewayBasePathMapping_BasePath_Empty (61.24s)

--- SKIP: TestAccAWSAPIGatewayDomainName_CertificateArn (0.00s)
--- SKIP: TestAccAWSAPIGatewayDomainName_CertificateName (0.00s)
--- SKIP: TestAccAWSAPIGatewayDomainName_RegionalCertificateName (0.00s)
--- PASS: TestAccAWSAPIGatewayDomainName_RegionalCertificateArn (20.45s)
--- PASS: TestAccAWSAPIGatewayDomainName_SecurityPolicy (104.68s)
```
@bflad bflad mentioned this issue Sep 30, 2019
Merged
bflad added a commit that referenced this issue Sep 30, 2019
@bflad
tests/service/iam: Use internal implementation for TLS key/certificate
2c6715c 
Reference: #10023

Output from acceptance testing:

```
--- PASS: TestAccAWSDataSourceIAMServerCertificate_matchNamePrefix (3.55s)
--- PASS: TestAccAWSIAMServerCertificate_disappears (9.30s)
--- PASS: TestAccAWSIAMServerCertificate_name_prefix (10.17s)
--- PASS: TestAccAWSIAMServerCertificate_basic (11.28s)
--- PASS: TestAccAWSDataSourceIAMServerCertificate_basic (12.65s)
--- PASS: TestAccAWSDataSourceIAMServerCertificate_path (12.71s)
--- PASS: TestAccAWSIAMServerCertificate_file (17.00s)
```
@bflad bflad mentioned this issue Sep 30, 2019
Merged
@ewbankkit ewbankkit mentioned this issue Oct 2, 2019
Merged
bflad added a commit that referenced this issue Oct 9, 2019
@bflad
deps: Remove github.com/terraform-providers/terraform-provider-tls
0a90005 
Reference: #10023

Updated via:

```console
$ go mod tidy
$ go mod vendor
```

Output from acceptance testing:

```
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (14.46s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (23.05s)
```
@bflad bflad mentioned this issue Oct 9, 2019
Merged
@bflad bflad added this to the v2.32.0 milestone Oct 9, 2019
@ghost
Copy link

ghost commented Oct 10, 2019

This has been released in version 2.32.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 for triage. Thanks!

@ghost
Copy link

ghost commented Nov 9, 2019

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!

@ghost ghost locked and limited conversation to collaborators Nov 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
provider Pertains to the provider itself, rather than any interaction with AWS. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v2.32.0
1 participant
@bflad