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

[feature] aws-acm-certificate module compatible with TF AWS Provider >3.0 #321

Merged
merged 2 commits into from
Jul 7, 2021

Conversation

mbarrien
Copy link
Contributor

@mbarrien mbarrien commented Jul 6, 2021

Adds a new aws-acm-certificate module, intended to replace aws-acm-cert for Terraform AWS Provider >=3.0.

Users of TF AWS Provider <3.0 must continue using aws-acm-cert module, which is now officially deprecated; it should be deleted once all uses of it are done.

Although cloudposse provides their own module https://github.com/cloudposse/terraform-aws-acm-request-certificate it does not have some features that we use, such as verifying subject alternative names on different Route 53 zones than the one used for the main domain, so we will for now continue maintaining our own.

The following changes are made:

  • Removal of hacks cert_subject_alternative_names_count and cert_subject_alternative_names_order
  • Replacing individual project/env/service/owner inputs with a single tags input that takes those as a map
  • Using a for_each map instead of an indexed count for creating the validation Route 53 record (this is the change that makes it so we can't just do a drop in replacement)

A migration of existing certificates to this module must either map the Route 53 records to the new Terraform state location (since it switches from a count to a for_each), or force recreation. In addition, the underlying provider changes the domain_validation_options of aws_acm_certificate from a list to a set, and does this in a backwards incompatible way such that naive applies (even if not creating Route 53 records) will break.

See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/guides/version-3-upgrade#resource-aws_acm_certificate for more details.

Implementing the migration (i.e. identifying where the migration needs to be done, and scripting Terraform state moves and Terraform applies to the new state name, across existing resources even if the reference to aws-acm-cert is deeply nested inside modules) is left as a future exercise. This would either require a bunch of state migrations, or possibly deletion and recreation of the ACM certificate.

Full diff between the 2 similar modules continues below:

diff aws-acm-cert/README.md aws-acm-certificate/README.md
1,3c1
< # AWS ACM Cert
<
< **_DEPRECATED: Use aws-acm-certificate if using Terraform AWS Provider >3.0._**
---
> # AWS ACM Certificate
13c11
<   source = "github.com/chanzuckerberg/cztack//aws-acm-cert?ref=v0.36.0"
---
>   source = "github.com/chanzuckerberg/cztack//aws-acm-certificate?ref=v0.40.0"
17c15
<
---
>
20,22d17
<
<   # a map of alternative : route53_zone_id
<   cert_subject_alternative_names = "${map(..)}"
23a19,20
>   # an optional map of alternative : route53_zone_id
>   cert_subject_alternative_names = {"foobar.com" = data.aws_route53_zone.foo.id}
25,29c22,28
<   # variables for tags
<   owner   = "..."
<   project = "..."
<   env     = "..."
<   service = "..."
---
>   # optional variable for tags
>   tags = {
>     project = "...",
>     env     = "...",
>     service = "...",
>     owner   = "..."
>   }
38c37
< | <a name="requirement_aws"></a> [aws](#requirement\_aws) | < 3.0.0 |
---
> | <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 3.0.0 |
44c43
< | <a name="provider_aws"></a> [aws](#provider\_aws) | < 3.0.0 |
---
> | <a name="provider_aws"></a> [aws](#provider\_aws) | >= 3.0.0 |
62d60
< | <a name="input_allow_validation_record_overwrite"></a> [allow\_validation\_record\_overwrite](#input\_allow\_validation\_record\_overwrite) | Allow the overwrite of validation records. This is needed if you are creating certificates in multiple regions. | `string` | `true` | no |
66,71c64
< | <a name="input_cert_subject_alternative_names_count"></a> [cert\_subject\_alternative\_names\_count](#input\_cert\_subject\_alternative\_names\_count) | The size of var.cert\_subject\_alternative\_names. Since var.cert\_subject\_alternative\_names can have dynamic keys/values we must hint terraform on its size. If you have no SANs then this should be 0. | `number` | `0` | no |
< | <a name="input_env"></a> [env](#input\_env) | Env for tagging and naming. See [doc](../README.md#consistent-tagging). | `string` | n/a | yes |
< | <a name="input_owner"></a> [owner](#input\_owner) | Owner for tagging and naming. See [doc](../README.md#consistent-tagging). | `string` | n/a | yes |
< | <a name="input_project"></a> [project](#input\_project) | Project for tagging and naming. See [doc](../README.md#consistent-tagging) | `string` | n/a | yes |
< | <a name="input_service"></a> [service](#input\_service) | Service for tagging and naming. See [doc](../README.md#consistent-tagging). | `string` | n/a | yes |
< | <a name="input_subject_alternative_names_order"></a> [subject\_alternative\_names\_order](#input\_subject\_alternative\_names\_order) | Order to list the subject alternative names in the ACM cert. Workaround for https://github.com/terraform-providers/terraform-provider-aws/issues/8531 | `list(string)` | `null` | no |
---
> | <a name="input_tags"></a> [tags](#input\_tags) | Tags to apply to certificate | `map(string)` | `{}` | no |
diff aws-acm-cert/main.tf aws-acm-certificate/main.tf
1,12d0
< locals {
<   tags = {
<     project   = var.project
<     env       = var.env
<     service   = var.service
<     owner     = var.owner
<     managedBy = "terraform"
<   }
<
<   cert_validation_count = var.cert_subject_alternative_names_count + 1
< }
<
15c3
<   subject_alternative_names = var.subject_alternative_names_order == null ? keys(var.cert_subject_alternative_names) : var.subject_alternative_names_order
---
>   subject_alternative_names = keys(var.cert_subject_alternative_names)
17c5
<   tags                      = local.tags
---
>   tags                      = var.tags
24d11
< # https://www.terraform.io/docs/providers/aws/r/acm_certificate_validation.html
26c13,19
<   count = local.cert_validation_count
---
>   for_each = {
>     for dvo in aws_acm_certificate.cert.domain_validation_options : dvo.domain_name => {
>       name   = dvo.resource_record_name
>       record = dvo.resource_record_value
>       type   = dvo.resource_record_type
>     }
>   }
28,31c21,24
<   name    = lookup(aws_acm_certificate.cert.domain_validation_options[count.index], "resource_record_name")
<   type    = lookup(aws_acm_certificate.cert.domain_validation_options[count.index], "resource_record_type")
<   zone_id = lookup(var.cert_subject_alternative_names, lookup(aws_acm_certificate.cert.domain_validation_options[count.index], "domain_name"), var.aws_route53_zone_id)
<   records = [lookup(aws_acm_certificate.cert.domain_validation_options[count.index], "resource_record_value")]
---
>   name    = each.value.name
>   type    = each.value.type
>   zone_id = lookup(var.cert_subject_alternative_names, each.key, var.aws_route53_zone_id)
>   records = [each.value.record]
34c27
<   allow_overwrite = var.allow_validation_record_overwrite
---
>   allow_overwrite = true # Needed if making cert in multiple regions, and for AWS Provider 3.0 conversion
39c32
<   validation_record_fqdns = aws_route53_record.cert_validation.*.fqdn
---
>   validation_record_fqdns = [for record in aws_route53_record.cert_validation : record.fqdn]
diff aws-acm-cert/module_test.go aws-acm-certificate/module_test.go
11c11
< func TestAWSACMCertInit(t *testing.T) {
---
> func TestAWSACMCertificateInit(t *testing.T) {
18c18
< func TestAWSACMCertDefaults(t *testing.T) {
---
> func TestAWSACMCertificateDefaults(t *testing.T) {
39,46c39,59
< 			return tftest.Options(
< 				tftest.DefaultRegion,
< 				map[string]interface{}{
< 					"cert_domain_name":                     certDomainName,
< 					"aws_route53_zone_id":                  route53ZoneID,
< 					"validation_record_ttl":                5,
< 					"cert_subject_alternative_names":       alternativeNames,
< 					"cert_subject_alternative_names_count": len(alternativeNames),
---
> 			tags := map[string]string{
> 				"project":   tftest.UniqueID(),
> 				"env":       tftest.UniqueID(),
> 				"service":   tftest.UniqueID(),
> 				"owner":     tftest.UniqueID(),
> 				"managedBy": "terraform",
> 			}
>
> 			vars := map[string]interface{}{
> 				"cert_domain_name":               certDomainName,
> 				"aws_route53_zone_id":            route53ZoneID,
> 				"validation_record_ttl":          5,
> 				"cert_subject_alternative_names": alternativeNames,
> 				"tags":                           tags,
> 			}
>
> 			options := &terraform.Options{
> 				TerraformDir: ".",
>
> 				EnvVars: map[string]string{
> 					"AWS_DEFAULT_REGION": tftest.DefaultRegion,
48c61,65
< 			)
---
>
> 				Vars: vars,
> 			}
>
> 			return options
diff aws-acm-cert/terraform.tf aws-acm-certificate/terraform.tf
3c3,4
<     aws = "< 3.0.0"
---
>     # aws_acm_certificate changed API in 3.0.0
>     aws = ">= 3.0.0"
diff aws-acm-cert/variables.tf aws-acm-certificate/variables.tf
21,56c21,24
< variable "project" {
<   type        = string
<   description = "Project for tagging and naming. See [doc](../README.md#consistent-tagging)"
< }
<
< variable "env" {
<   type        = string
<   description = "Env for tagging and naming. See [doc](../README.md#consistent-tagging)."
< }
<
< variable "service" {
<   type        = string
<   description = "Service for tagging and naming. See [doc](../README.md#consistent-tagging)."
< }
<
< variable "owner" {
<   type        = string
<   description = "Owner for tagging and naming. See [doc](../README.md#consistent-tagging)."
< }
<
< variable "allow_validation_record_overwrite" {
<   type        = string
<   description = "Allow the overwrite of validation records. This is needed if you are creating certificates in multiple regions."
<   default     = true
< }
<
< variable "subject_alternative_names_order" {
<   type        = list(string)
<   description = "Order to list the subject alternative names in the ACM cert. Workaround for https://github.com/terraform-providers/terraform-provider-aws/issues/8531"
<   default     = null
< }
<
< variable "cert_subject_alternative_names_count" {
<   type        = number
<   description = "The size of var.cert_subject_alternative_names. Since var.cert_subject_alternative_names can have dynamic keys/values we must hint terraform on its size. If you have no SANs then this should be 0."
<   default     = 0
---
> variable tags {
>   type        = map(string)
>   description = "Tags to apply to certificate"
>   default     = {}

@mbarrien mbarrien requested review from edulop91 and srm78 July 6, 2021 06:16
@mbarrien mbarrien requested a review from a team as a code owner July 6, 2021 06:16
@mbarrien mbarrien force-pushed the mbarrien/aws-acm-cert-tf-aws-3.0 branch from b5f576e to d230695 Compare July 6, 2021 07:09
aws-acm-certificate/outputs.tf Show resolved Hide resolved
aws-acm-certificate/outputs.tf Show resolved Hide resolved
aws-acm-certificate/variables.tf Show resolved Hide resolved
aws-acm-certificate/variables.tf Show resolved Hide resolved
@mbarrien mbarrien force-pushed the mbarrien/aws-acm-cert-tf-aws-3.0 branch from fcb46f1 to 0f9f810 Compare July 7, 2021 09:14
@mbarrien mbarrien merged commit d49054c into main Jul 7, 2021
@mbarrien mbarrien deleted the mbarrien/aws-acm-cert-tf-aws-3.0 branch July 7, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants