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

Added api sub-domain in the list of alt names in certificates #493

Merged
merged 4 commits into from
Jul 19, 2022

Conversation

jimleroyer
Copy link
Member

@jimleroyer jimleroyer commented Jul 18, 2022

Summary | Résumé

Trying out to add a subdomain for *.api.notification.canada.ca as an alt name for our certificates, helping our friends using WebLogic as an app container, as indicated by this Freshdesk ticket.

Also removed an ignore_changes statement on a bug previously identified but that should not take place anymore with hashicorp/aws/3.0.0. Let's see how that works out.

Test instructions | Instructions pour tester la modification

  1. Head to staging or production environment with the new deployed certificate (e.g. api.notification.canada.ca).
  2. Get the certificate details.
  3. Check the Subject Alternative Name field and check if there is an entry for the new *.api.notification.canada.ca entry.

Here is a screenshot of the current certificate details without the new entry for the API sub-domain:

image

"*.document.${var.domain}"
]
validation_method = "DNS"

lifecycle {
create_before_destroy = true
# TF bug on AWS 2.0: prevents certificates from being destroyed/recreated
# https://github.com/hashicorp/terraform-provider-aws/issues/8531
ignore_changes = [subject_alternative_names]
Copy link
Member Author

@jimleroyer jimleroyer Jul 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the provided linked issue, this should have been fixed by version hashicorp/aws/3.0.0 which we switched to 18 months ago. That ignore_changes statement was introduced 20 months ago though and we might never updated this (as we didn't modify that yet).

Let's try it out and see what the plan provides.

aws/dns/acm.tf Outdated
@@ -24,15 +22,13 @@ resource "aws_acm_certificate" "notification-canada-ca-alt" {
domain_name = var.alt_domain
subject_alternative_names = [
"*.${var.alt_domain}",
"*.api.${var.domain}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this block is for the alpha.canada.ca domain so i think you need to switch this var to var.alt_domain

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right!

Copy link
Contributor

@mohdnr mohdnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change suggested but other than that LGTM since it's the same as what i did in the security-tools project

@jimleroyer jimleroyer requested a review from mohdnr July 19, 2022 16:54
Copy link
Contributor

@sastels sastels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see if it all works!

@github-actions
Copy link

Staging: dns

✅   Terraform Format: success
✅   Terraform Plan: success
✅   Conftest: success

⚠️   WARNING: resources will be destroyed by this change!

Plan: 1 to add, 0 to change, 1 to destroy
Show plan
Resource actions are indicated with the following symbols:
+/- create replacement and then destroy

Terraform will perform the following actions:

  # aws_acm_certificate.notification-canada-ca must be replaced
+/- resource "aws_acm_certificate" "notification-canada-ca" {
      ~ arn                       = "arn:aws:acm:ca-central-1:239043911459:certificate/b8ba5e08-2c0f-4d1a-afb1-e41accb271bb" -> (known after apply)
      ~ domain_validation_options = [
          - {
              - domain_name           = "*.document.staging.notification.cdssandbox.xyz"
              - resource_record_name  = "_b4c1dfe3fa5ef9427b62475bf31154f3.document.staging.notification.cdssandbox.xyz."
              - resource_record_type  = "CNAME"
              - resource_record_value = "_aa51db2fea1aaaa2486be78695309fd4.wggjkglgrm.acm-validations.aws."
            },
          - {
              - domain_name           = "*.staging.notification.cdssandbox.xyz"
              - resource_record_name  = "_5bc41be686a84b97346535d7c4d2fe4e.staging.notification.cdssandbox.xyz."
              - resource_record_type  = "CNAME"
              - resource_record_value = "_a3c8bd5843dfed42acc803da5f589492.wggjkglgrm.acm-validations.aws."
            },
          - {
              - domain_name           = "staging.notification.cdssandbox.xyz"
              - resource_record_name  = "_5bc41be686a84b97346535d7c4d2fe4e.staging.notification.cdssandbox.xyz."
              - resource_record_type  = "CNAME"
              - resource_record_value = "_a3c8bd5843dfed42acc803da5f589492.wggjkglgrm.acm-validations.aws."
            },
            # (4 unchanged elements hidden)
        ]
      ~ id                        = "arn:aws:acm:ca-central-1:239043911459:certificate/b8ba5e08-2c0f-4d1a-afb1-e41accb271bb" -> (known after apply)
      ~ status                    = "ISSUED" -> (known after apply)
      ~ subject_alternative_names = [ # forces replacement
          + "*.api.staging.notification.cdssandbox.xyz",
            # (2 unchanged elements hidden)
        ]
        tags                      = {
            "CostCenter" = "notification-canada-ca-staging"
        }
      ~ validation_emails         = [] -> (known after apply)
        # (3 unchanged attributes hidden)

      - options {
          - certificate_transparency_logging_preference = "ENABLED" -> null
        }
    }

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

Changes to Outputs:
  ~ acm_cert_name_validation           = [
      - {
          - domain_name           = "*.document.staging.notification.cdssandbox.xyz"
          - resource_record_name  = "_b4c1dfe3fa5ef9427b62475bf31154f3.document.staging.notification.cdssandbox.xyz."
          - resource_record_type  = "CNAME"
          - resource_record_value = "_aa51db2fea1aaaa2486be78695309fd4.wggjkglgrm.acm-validations.aws."
        },
      - {
          - domain_name           = "*.staging.notification.cdssandbox.xyz"
          - resource_record_name  = "_5bc41be686a84b97346535d7c4d2fe4e.staging.notification.cdssandbox.xyz."
          - resource_record_type  = "CNAME"
          - resource_record_value = "_a3c8bd5843dfed42acc803da5f589492.wggjkglgrm.acm-validations.aws."
        },
      - {
          - domain_name           = "staging.notification.cdssandbox.xyz"
          - resource_record_name  = "_5bc41be686a84b97346535d7c4d2fe4e.staging.notification.cdssandbox.xyz."
          - resource_record_type  = "CNAME"
          - resource_record_value = "_a3c8bd5843dfed42acc803da5f589492.wggjkglgrm.acm-validations.aws."
        },
        # (4 unchanged elements hidden)
    ]
  ~ aws_acm_notification_canada_ca_arn = "arn:aws:acm:ca-central-1:239043911459:certificate/b8ba5e08-2c0f-4d1a-afb1-e41accb271bb" -> (known after apply)

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: plan.tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "plan.tfplan"
Show Conftest results
WARN - plan.json - main - Missing Common Tags: ["aws_acm_certificate.assets-notification-canada-ca"]
WARN - plan.json - main - Missing Common Tags: ["aws_acm_certificate.notification-canada-ca"]

19 tests, 17 passed, 2 warnings, 0 failures, 0 exceptions

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