Import acm certificates #5453

julienduchesne · 2018-08-05T04:11:32Z

Redo of PR #5002
Fixes #3560

Changes proposed in this pull request:

Allows the importation and modification of imported certificates through the aws_acm_certificate resource, to do so:
Add certificate_body, certficate_chain and private_key arguments for the aws_acm_certificate resource.
Verify apply-time (as describe in issue) that the arguments are all provided. Plan-time is not possible (see closed PR Import acm certificate (like IAM server certificate) #5002)

Output from acceptance testing:

TESTARGS='-run=TestAccAWSAcmCertificate_imported'  make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSAcmCertificate_imported -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSAcmCertificate_imported
--- PASS: TestAccAWSAcmCertificate_imported (39.41s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       39.425s

…-acm-certificates

eedwards-sk · 2018-09-05T22:10:59Z

What is needed to get this merged?

avoidik · 2018-09-06T10:44:04Z

@bflad could you please check?

megamorf · 2018-10-04T09:13:58Z

Can you check and merge this, please. I'm very much interested in using the feature.

joseph-stratoscale · 2018-10-18T18:48:01Z

@bflad Could you please merge this Pull Request. Thanks!

CarpathianUA · 2018-11-07T14:11:58Z

Guys, please merge the PR! It's a very useful feature!

SamBivins · 2018-11-21T17:48:02Z

👍

bgshacklett · 2018-11-27T14:02:52Z

Is there any issue holding this PR back from being merged? If so, it would be great to see some feedback.

mingdevopsapt · 2018-11-29T04:05:38Z

Until this feature go live, I find no other option to generate / maintain a PCA signed certificate without importing via command line.

Bump up and thanks.

vn-patriot · 2018-12-10T04:18:22Z

I look forwarding to see this merge

eedwards-sk · 2018-12-10T17:03:08Z

@bflad sorry to ping you on this, but I've been getting email updates on this for months and it seems nobody from hashicorp has responded other than when you first tagged the issue back in august

we'd love any update on if this can be merged, even a rejection or a status update would help

thanks!

bflad

Hi @julienduchesne and everyone watching this pull request. 👋 Very sorry for the lengthy delay before review. ACM testing is a bit of a hassle at the moment since it requires real, public domains and working against ACM per year limitations. I was able to verify this, along with a variation of the bug fix #5472. Thanks so much @julienduchesne for your work here! 🚀

--- PASS: TestAccAWSAcmCertificate_dnsValidation (12.03s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (11.90s)
--- PASS: TestAccAWSAcmCertificate_imported (10.15s)
--- PASS: TestAccAWSAcmCertificate_root (10.82s)
--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (12.27s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (11.53s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (16.44s)
--- PASS: TestAccAWSAcmCertificate_san_single (19.11s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (17.28s)
--- PASS: TestAccAWSAcmCertificate_tags (25.84s)
--- PASS: TestAccAWSAcmCertificate_wildcard (11.95s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (15.09s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (150.56s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (14.90s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (167.04s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (143.35s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (535.21s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (160.54s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (154.88s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (120.23s)

bflad · 2018-12-21T22:34:48Z

aws/resource_aws_acm_certificate.go

@@ -149,7 +210,11 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
 		if err := d.Set("validation_emails", emailValidationOptions); err != nil {
 			return resource.NonRetryableError(err)
 		}
-		d.Set("validation_method", resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions, emailValidationOptions))
+
+		if _, ok := d.GetOk("validation_method"); ok {


Found during testing: wound up switching validation_method to Computed: true (for now) instead of using this, since it breaks Terraform resource import functionality for this attribute.

bflad · 2018-12-21T23:11:13Z

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

ghost · 2019-11-01T15:38:57Z

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!

Julien Duchesne added 6 commits June 25, 2018 18:40

Implement ACM certificate creation from imported certificate

dd790f2

Implement ACM imported certificate update

a81668a

Add tests for acm certificate import

b77c1d4

Merge remote-tracking branch 'terraform-providers/master' into import…

06e33d5

…-acm-certificates

aws_acm_certificate, verify apply-time

754b98d

Add aws_acm_certificate importation doc

d431c03

ghost added the size/L Managed by automation to categorize the size of a PR. label Aug 5, 2018

bflad added enhancement Requests to existing resources that expand the functionality or scope. service/acm Issues and PRs that pertain to the acm service. labels Aug 6, 2018

bflad mentioned this pull request Dec 20, 2018

aws_api_gateway_domain_name: uploading certificates is not supported for REGIONAL. #6936

Closed

bflad approved these changes Dec 21, 2018

View reviewed changes

bflad added this to the v1.54.0 milestone Dec 21, 2018

bflad merged commit d431c03 into hashicorp:master Dec 21, 2018

bflad added a commit that referenced this pull request Dec 21, 2018

Update CHANGELOG for #5453 and #5472

f1a5863

julienduchesne deleted the import-acm-certificates branch January 18, 2019 16:10

ghost locked and limited conversation to collaborators Nov 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import acm certificates #5453

Import acm certificates #5453

julienduchesne commented Aug 5, 2018 •

edited

Loading

eedwards-sk commented Sep 5, 2018

avoidik commented Sep 6, 2018 •

edited

Loading

megamorf commented Oct 4, 2018

joseph-stratoscale commented Oct 18, 2018

CarpathianUA commented Nov 7, 2018

SamBivins commented Nov 21, 2018

bgshacklett commented Nov 27, 2018

mingdevopsapt commented Nov 29, 2018

vn-patriot commented Dec 10, 2018 •

edited

Loading

eedwards-sk commented Dec 10, 2018

bflad left a comment

bflad Dec 21, 2018

bflad commented Dec 21, 2018

ghost commented Nov 1, 2019

Import acm certificates #5453

Import acm certificates #5453

Conversation

julienduchesne commented Aug 5, 2018 • edited Loading

eedwards-sk commented Sep 5, 2018

avoidik commented Sep 6, 2018 • edited Loading

megamorf commented Oct 4, 2018

joseph-stratoscale commented Oct 18, 2018

CarpathianUA commented Nov 7, 2018

SamBivins commented Nov 21, 2018

bgshacklett commented Nov 27, 2018

mingdevopsapt commented Nov 29, 2018

vn-patriot commented Dec 10, 2018 • edited Loading

eedwards-sk commented Dec 10, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad Dec 21, 2018

Choose a reason for hiding this comment

bflad commented Dec 21, 2018

ghost commented Nov 1, 2019

julienduchesne commented Aug 5, 2018 •

edited

Loading

avoidik commented Sep 6, 2018 •

edited

Loading

vn-patriot commented Dec 10, 2018 •

edited

Loading