Add validation_domain parameter to aws_acm_certificate

[scottwinkler]

Adding an optional input parameter that is present in the sdk but missing from the terraform resource. This allows a user to specify an alternate validation_domain for EMAIL approval.

Closes #3851.

[telepath]

Works like a charm, thanks!

[scottwinkler]

Thanks @telepath, glad this works for you. This solves issue #3851

[telepath]

Ok, yesterday it looked good, today it doesn't.
I'm not too fluent in go. Is the validation domain set for all alternative names, or just the domain name? Currently I have a case where the domain name works, but the alternative names are all wrong.

[scottwinkler]

@telepath nice observation! I never set the domain validation for the subject alternative names because the certificate is issued as long as the domain name is validated. I have tested it with several subject alternative names, and it issues fine for me, even though the alternative names are wrong like you said. I looked at your issue again and it looks like you want to set the validation domain to be the same for all the subject alternative names, is that correct? There are two ways to fix this problem: 1) change the input to match the cloudformation/cli so that you need to specify a list of DomainName and ValidationDomain objects in a domain_validation_options field (which name conflicts with an existing, computed field... but this computed field doesn't do much of anything anyways so we can get rid of it) or 2) keep the inputs like they are and if a validation_domain is set, loop through each of the subject alternative names and set the validationdomain to what was provided. I think option two would probably be best, what do you think?

[telepath]

Option 2 would be good.

Option one would be more complete, but difficult to assemble in terraform (im using a simple list of domains as input, that would not work to assemble a key-value string).

Maybe both? I suspect the most cases for this could be handled with option 2, but if more complexity is needed, the option string for option 1 could still be assembled.

I'm not sure why the behaviour differs here, maybe because some subdomains are delegated zones.

[telepath]

But if I remember correctly, weren't the domain_validation_options used for automatic dns validation somewhere?

[scottwinkler]

@telepath I fixed it. Below is an example how to specify the domain_validaton_options. The output has changed as well, because of the name collision i talked about earlier. I changed the output that used to be called domain_validation_options to "certificate_details" which is a list of objects with the following variables; domain_name, resource_record_name, resource_record_type, resource_record_value, validation_domain, validation_emails, validation_method. So some documentation will have to be changed but it works fine

resource "aws_acm_certificate" "cert" {
  domain_name               = "mytest.rd.example.io"
  validation_method         = "EMAIL"
  subject_alternative_names = ["app1.mytest.rd.example.io", "app2.mytest.rd.example.io"]

  domain_validation_options = [
    {
      domain_name       = "mytest.rd.example.io"
      validation_domain = "example.io"
    },
    {
      domain_name       = "app1.mytest.rd.elliemae.io"
      validation_domain = "example.io"
    },
  ]
}

[telepath]

Ok, thanks!
This is certainly the most complete solution.
I'll have to think about how to use this, at the moment my input is a list of subdomains, I'm not sure how to generate these options... But at least its possible now.
👍

[scottwinkler]

if you know the length of the list is always going to be the same, then you can hard code it: list(map("domain_name","${var.subject_alternative_names[0],"validation_domain","${var.validation_domain}",map("domain_name",${var.subject_alternative_names[1],"validation_domain","${var.validation_domain}"]) ... not the prettiest solution, but straightforward enough. A more general solution working with arbitrary list lengths would be a good stackoverflow question because I can't think how to do it on the top of my head. maybe something to do with null_resources or templates and doing some trickery with the count parameter? You can't pass a list of maps as an input variable to modules, so there aren't any examples I could find of people doing this.

[telepath]

Yes, this is a normal limitation of terraform.
I've had the thought about null resources as well, but defaulted to the universal solution for these cases: Generating the terraform file with ansible. Now it works perfectly, I'll just have to wait for this to be released so I can integrate it in a pipeline :)

[bflad]

Thanks for this contribution, @scottwinkler. I left some initial review comments on why these changes would prevent us from moving forward with this PR as it breaks backwards compatibility. Please let us know if you have any questions.

[bflad]

We still need to set the old attributes if we are deprecating them. Have you run the acceptance tests?

[scottwinkler]

all acceptance tests are now passing

OSXSWINKMBP15:terraform-provider-aws swinkler$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAcmCertificate_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAcmCertificate_ -timeout 120m
=== RUN   TestAccAWSAcmCertificate_emailValidation
--- PASS: TestAccAWSAcmCertificate_emailValidation (21.98s)
=== RUN   TestAccAWSAcmCertificate_dnsValidation
--- PASS: TestAccAWSAcmCertificate_dnsValidation (22.93s)
=== RUN   TestAccAWSAcmCertificate_root
--- PASS: TestAccAWSAcmCertificate_root (42.27s)
=== RUN   TestAccAWSAcmCertificate_rootAndWildcardSan
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (36.57s)
=== RUN   TestAccAWSAcmCertificate_san_single
--- PASS: TestAccAWSAcmCertificate_san_single (36.50s)
=== RUN   TestAccAWSAcmCertificate_san_multiple
--- PASS: TestAccAWSAcmCertificate_san_multiple (23.27s)
=== RUN   TestAccAWSAcmCertificate_wildcard
--- PASS: TestAccAWSAcmCertificate_wildcard (20.59s)
=== RUN   TestAccAWSAcmCertificate_wildcardAndRootSan
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (22.20s)
=== RUN   TestAccAWSAcmCertificate_tags
--- PASS: TestAccAWSAcmCertificate_tags (78.62s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	304.968s

[telepath]

I cannot get DNS validation to work with these changes:
I tried this with terraform 0.11.7 and the aws plugin build from current master and Scotts changes merged.

data "aws_route53_zone" "domain" {
  name = "subdomain.domain.com."
}

resource "aws_acm_certificate" "domain" {
  domain_name = "*.subdomain.domain.com"
  validation_method = "DNS"
  domain_validation_options = [
    {
      domain_name       = "*.subdomain.domain.com"
      validation_domain = "domain.com"
    }
  ]
}

resource "aws_route53_record" "domain_validation" {
  name = "${aws_acm_certificate.domain.certificate_details.0.resource_record_name}"
  type = "${aws_acm_certificate.domain.certificate_details.0.resource_record_type}"
  zone_id = "${data.aws_route53_zone.domain.id}"
  records = ["${aws_acm_certificate.domain.certificate_details.0.resource_record_value}"]
  ttl = 60
}

resource "aws_acm_certificate_validation" "domain" {
  certificate_arn = "${aws_acm_certificate.domain.arn}"
  validation_record_fqdns = ["${aws_route53_record.domain_validation.fqdn}"]
}

~/Documents/acm (master *)$ ~/.local/share/go/bin/terraform apply --target aws_route53_record.domain_validation --target aws_acm_certificate_validation.domain                                                                                                           
data.aws_route53_zone.domain: Refreshing state...

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + aws_acm_certificate.domain
      id:                                            <computed>
      arn:                                           <computed>
      certificate_details.#:                         <computed>
      domain_name:                                   "*.subdomain.domain.com"
      domain_validation_options.#:                   "1"
      domain_validation_options.0.domain_name:       "*.subdomain.domain.com"
      domain_validation_options.0.validation_domain: "domain.com"
      validation_method:                             "DNS"

  + aws_acm_certificate_validation.domain
      id:                                            <computed>
      certificate_arn:                               "${aws_acm_certificate.domain.arn}"
      validation_record_fqdns.#:                     <computed>

  + aws_route53_record.domain_validation
      id:                                            <computed>
      allow_overwrite:                               "true"
      fqdn:                                          <computed>
      name:                                          "${aws_acm_certificate.domain.certificate_details.0.resource_record_name}"
      records.#:                                     <computed>
      ttl:                                           "60"
      type:                                          "${aws_acm_certificate.domain.certificate_details.0.resource_record_type}"
      zone_id:                                       "XXXXXXXXXXXXXX"


Plan: 3 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_acm_certificate.domain: Creating...
  arn:                                           "" => "<computed>"
  certificate_details.#:                         "" => "<computed>"
  domain_name:                                   "" => "*.subdomain.domain.com"
  domain_validation_options.#:                   "" => "1"
  domain_validation_options.0.domain_name:       "" => "*.subdomain.domain.com"
  domain_validation_options.0.validation_domain: "" => "domain.com"
  validation_method:                             "" => "DNS"

Error: Error applying plan:

1 error(s) occurred:

* aws_acm_certificate.domain: 1 error(s) occurred:

* aws_acm_certificate.domain: unexpected EOF

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.


panic: runtime error: invalid memory address or nil pointer dereference
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1ac5c62]
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: goroutine 172 [running]:
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: github.com/terraform-providers/terraform-provider-aws/aws.convertCertificateDetails(0xc420a31560, 0x2aa65ac, 0x19, 0x21e5fc0, 0xc4202e4d00, 0x0)
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:     /home/user/.local/share/go/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_acm_certificate.go:261 +0x3c2
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsAcmCertificateRead.func1(0x43c0bf)
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:     /home/user/.local/share/go/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_acm_certificate.go:185 +0x3d9
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource.Retry.func1(0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:     /home/user/.local/share/go/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource/wait.go:22 +0x75
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource.(*StateChangeConf).WaitForState.func1(0xc420512480, 0xc420208690, 0xc420573080, 0xc42056ecc0, 0xc420317480, 0xc420317478)
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:     /home/user/.local/share/go/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource/state.go:103 +0x4e0
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws: created by github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource.(*StateChangeConf).WaitForState
2018-04-24T11:52:57.477+0200 [DEBUG] plugin.terraform-provider-aws:     /home/user/.local/share/go/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/resource/state.go:80 +0x1d0
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalWriteState
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalApplyProvisioners
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalIf
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalWriteState
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalWriteDiff
2018/04/24 11:52:57 [TRACE] root: eval: *terraform.EvalApplyPost
2018/04/24 11:52:57 [ERROR] root: eval: *terraform.EvalApplyPost, err: 1 error(s) occurred:

* aws_acm_certificate.domain: unexpected EOF
2018/04/24 11:52:57 [ERROR] root: eval: *terraform.EvalSequence, err: 1 error(s) occurred:

* aws_acm_certificate.domain: unexpected EOF
2018/04/24 11:52:57 [TRACE] [walkApply] Exiting eval tree: aws_acm_certificate.domain
2018/04/24 11:52:57 [TRACE] dag/walk: upstream errored, not walking "aws_route53_record.domain_validation"
2018/04/24 11:52:57 [TRACE] dag/walk: upstream errored, not walking "aws_acm_certificate_validation.domain"
2018/04/24 11:52:57 [TRACE] dag/walk: upstream errored, not walking "provider.aws (close)"
2018/04/24 11:52:57 [TRACE] dag/walk: upstream errored, not walking "meta.count-boundary (count boundary fixup)"
2018-04-24T11:52:57.484+0200 [DEBUG] plugin: plugin process exited: path=/home/user/.local/share/go/bin/terraform-provider-aws
2018/04/24 11:52:57 [TRACE] dag/walk: upstream errored, not walking "root"
2018/04/24 11:52:57 [TRACE] Preserving existing state lineage "16219f50-c9a0-9e9f-69fb-c9d36b54aa49"
2018/04/24 11:52:57 [TRACE] Preserving existing state lineage "16219f50-c9a0-9e9f-69fb-c9d36b54aa49"
2018/04/24 11:52:57 [TRACE] Preserving existing state lineage "16219f50-c9a0-9e9f-69fb-c9d36b54aa49"
2018/04/24 11:52:57 [DEBUG] Uploading remote state to S3: {
  Body: buffer(0xc42007e510),
  Bucket: "somes3bucket",
  ContentLength: 3039,
  ContentType: "application/json",
  Key: "acm/terraform.tfstate"
}
2018/04/24 11:52:57 [DEBUG] plugin: waiting for all plugin processes to complete...
2018-04-24T11:52:57.746+0200 [WARN ] plugin: error closing client during Kill: err="connection is shut down"



!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

Terraform crashed! This is always indicative of a bug within Terraform.
A crash log has been placed at "crash.log" relative to your current
working directory. It would be immensely helpful if you could please
report the crash with Terraform[1] so that we can fix this.

When reporting bugs, please include your terraform version. That
information is available on the first line of crash.log. You can also
get it by running 'terraform --version' on the command line.

[1]: https://github.com/hashicorp/terraform/issues

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!
~/Documents/acm (master *)$ ~/.local/share/go/bin/terraform -v                                                                                                                                                                                                                   
Terraform v0.11.7
+ provider.aws (unversioned)

[scottwinkler]

@telepath I was not able to replicate the crash. It looks like it is failing on the following line 260 of resource_aws_acm_certificate.go if err := d.Set("domain_validation_options", domainValidationOptionsInput); err != nil { which might have happened if the terraform state got in a weird state, which is very likely since there has been a number of changes to the resource in the last few weeks. I would recommend removing any reference to this resource in your terraform.tfstate file, then rebuild the provider to ensure you are indeed using the latest build, and try again. Alternatively it could be a one off issue that could fix itself if you simply did another terraform apply, in which case this should be made to throw a retryable error instead of a nonretryable error on line 261 of resource_aws_acm_certificate.go. Please let me know if you experience this problem again, and I can try to work with your further to identify the issue.

Below is the configuration I used to test the resource and I did not encounter any errors on creation or destruction.

provider "aws" {
  profile = "em-publiccloud-dev"
  region  = "us-east-1"
}

data "aws_route53_zone" "domain" {
  name = "top.publiccloud.rd.elliemae.io."
}

resource "aws_acm_certificate" "domain" {
  domain_name       = "*.top.publiccloud.rd.elliemae.io"
  validation_method = "DNS"

  domain_validation_options = [
    {
      domain_name       = "*.top.publiccloud.rd.elliemae.io"
      validation_domain = "elliemae.io"
    },
  ]
}

resource "aws_route53_record" "domain_validation" {
  name    = "${aws_acm_certificate.domain.certificate_details.0.resource_record_name}"
  type    = "${aws_acm_certificate.domain.certificate_details.0.resource_record_type}"
  zone_id = "${data.aws_route53_zone.domain.id}"
  records = ["${aws_acm_certificate.domain.certificate_details.0.resource_record_value}"]
  ttl     = 60
}

resource "aws_acm_certificate_validation" "domain" {
  certificate_arn         = "${aws_acm_certificate.domain.arn}"
  validation_record_fqdns = ["${aws_route53_record.domain_validation.fqdn}"]
}

[telepath]

Is there still something missing for this to be merged?

[scottwinkler]

I changed the name of the conflicting attribute from domain_validation_options to validation_options. The original domain_validation_options is left as is for backwards compatibility, but is marked as deprecated. Certificate details is now the data struct that holds all the computed information. It passes all tests and works fine. I also merged all the latest changes with my old code so it is completely up to date.

[FreekingDean]

@bflad Any update on this being ship-able? This would be super helpful to our team as well!

[benh57]

Bump -- we could also use this

[intxEmeka]

Hello, sorry to bump an old thread but is this working now?

[scottwinkler]

@intxEmeka its been so long that i would have to fix some merge conflicts to get it in working shape again, but if I could get some assurance that this will actually get merged if i make the changes then I will do it.

[bflad]

After #14199, I believe the resource changes should reduce down to the following (removing all the deprecations, ExpectNonEmptyPlan: true, and other new things):

• Marking the domain_validation_options configuration block attribute as Optional: true
• Adding a new validation_domain attribute in the domain_validation_options configuration block with a schema such as:

"validation_domain": {
	Type:     schema.TypeString,
	Optional: true,
	Computed: true,
	ForceNew: true,
},

• Appropriately getting and setting that new attribute against the API

The new CustomizeDiff handling may require updates as well or if Terraform begins having provider produced inconsistent results after apply errors, we may need to document that when adding this configuration that all domain_validation_options configuration blocks must be specified upfront.

[jgspratt]

Is this being actively worked on?

[philicious]

I would also like to express interest in this !
the only way to make use of a validation_domain at the moment is to use the CLI or other language SDKs like e.g. python. However I'd love to keep everything in terraform and automated

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[ewbankkit]

@scottwinkler Thanks for the contribution 🎉 👏.
In order to maintain backwards compatibility and keep the implementation simple we have gone with a new argument validation_option (of which multiple can be specified).

[ewbankkit]

LGTM 🚀.

% ACM_CERTIFICATE_ROOT_DOMAIN=xxxxxxxx make testacc TESTS=TestAccACMCertificate_emailValidation PKG=acm  
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/acm/... -v -count 1 -parallel 20 -run='TestAccACMCertificate_emailValidation'  -timeout 180m
=== RUN   TestAccACMCertificate_emailValidation
=== PAUSE TestAccACMCertificate_emailValidation
=== CONT  TestAccACMCertificate_emailValidation
--- PASS: TestAccACMCertificate_emailValidation (23.21s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/acm	27.081s
% ACM_CERTIFICATE_ROOT_DOMAIN=xxxxxxxx make testacc TESTS=TestAccACMCertificate_dnsValidation PKG=acm
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/acm/... -v -count 1 -parallel 20 -run='TestAccACMCertificate_dnsValidation'  -timeout 180m
=== RUN   TestAccACMCertificate_dnsValidation
=== PAUSE TestAccACMCertificate_dnsValidation
=== CONT  TestAccACMCertificate_dnsValidation
--- PASS: TestAccACMCertificate_dnsValidation (23.85s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/acm	27.797s
% ACM_CERTIFICATE_ROOT_DOMAIN=xxxxxxxx make testacc TESTS=TestAccACMCertificate_root PKG=acm         
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/acm/... -v -count 1 -parallel 20 -run='TestAccACMCertificate_root'  -timeout 180m
=== RUN   TestAccACMCertificate_root
=== PAUSE TestAccACMCertificate_root
=== RUN   TestAccACMCertificate_rootAndWildcardSan
=== PAUSE TestAccACMCertificate_rootAndWildcardSan
=== CONT  TestAccACMCertificate_root
=== CONT  TestAccACMCertificate_rootAndWildcardSan
--- PASS: TestAccACMCertificate_rootAndWildcardSan (24.17s)
--- PASS: TestAccACMCertificate_root (24.23s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/acm	28.255s
% ACM_CERTIFICATE_ROOT_DOMAIN=xxxxxxxx make testacc TESTS=TestAccACMCertificate_validationOptions PKG=acm
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/acm/... -v -count 1 -parallel 20 -run='TestAccACMCertificate_validationOptions'  -timeout 180m
=== RUN   TestAccACMCertificate_validationOptions
=== PAUSE TestAccACMCertificate_validationOptions
=== CONT  TestAccACMCertificate_validationOptions
--- PASS: TestAccACMCertificate_validationOptions (22.18s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/acm	25.957s

[philicious]

@ewbankkit thanks for merging !! And thanks @scottwinkler 👍🏼

[github-actions]

This functionality has been released in v4.12.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]

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.