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

Add certificate_settings to ignore_changes #29

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

socketbox
Copy link
Collaborator

@socketbox socketbox commented Feb 26, 2025

Similar to what was done previously for subdomains.

Tested as follows:

  1. Created branch in terraform-pbsorg.
  2. cd terraform-pbsorg/edcar
  3. terraform init
  4. terraform plan
  5. plan shows this:
Terraform will perform the following actions:

  # module.dotorg_amplify.aws_amplify_domain_association.domain[0] will be updated in-place
  ~ resource "aws_amplify_domain_association" "domain" {
        id                                  = "dgrf2d2vn26dm/edcar.pbs.org"
        # (6 unchanged attributes hidden)

      - certificate_settings {
          - certificate_verification_dns_record = "_4458e50b7235a9c621ee51bc8f91eb35.edcar.pbs.org. CNAME _3a288d5adfc493eeef938e3aad368d5b.shzbfwcggp.acm-validations.aws." -> null
          - type                                = "AMPLIFY_MANAGED" -> null
            # (1 unchanged attribute hidden)
        }

        # (1 unchanged block hidden)
    }

  # module.dotorg_amplify.aws_amplify_domain_association.domain[1] will be updated in-place
  ~ resource "aws_amplify_domain_association" "domain" {
        id                                  = "dgrf2d2vn26dm/pr.pbs.org"
        # (6 unchanged attributes hidden)

      - certificate_settings {
          - certificate_verification_dns_record = "_5b1c37044c3b15b46a1035c4564b2d8f.pr.pbs.org. CNAME _20504ff5432d691bfddecc392aa2e926.sdgjtdhdhz.acm-validations.aws." -> null
          - type                                = "AMPLIFY_MANAGED" -> null
            # (1 unchanged attribute hidden)
        }

        # (10 unchanged blocks hidden)
    }

  1. Switch to this branch in amplify module reference.
  2. terraform init
  3. terraform plan
  4. Proposed changes due to certs go away.

@socketbox socketbox marked this pull request as ready for review February 26, 2025 13:30
@socketbox socketbox requested a review from brianmlink February 26, 2025 13:31
@socketbox socketbox force-pushed the ignore_cert_settings branch 4 times, most recently from 01d249f to 1acd3c3 Compare February 26, 2025 16:38
@socketbox socketbox marked this pull request as draft February 26, 2025 17:23
@socketbox
Copy link
Collaborator Author

socketbox commented Feb 26, 2025

Taking a different tack by adding certificate_settings to the domains object that's passed in.

Update: this ultimately proved fruitless, as I was unable to actually test out if it worked by creating a new Amplify instance. The testing ultimately ended with me being unable to get past this error:

╷
│ Error: creating Amplify App (amplify-test): operation error Amplify: CreateApp, https response error StatusCode: 400, RequestID: ec5c9f36-46f2-4e54-96a8-79deb6641f1f, BadRequestException: There was an issue setting up your repository. Please try again later.({"message":"Not Found","documentation_url":"https://docs.github.com/rest/repos/webhooks#list-repository-webhooks","status":"404"})
│
│   with module.amplify_test.aws_amplify_app.app,
│   on .terraform/modules/amplify_test/main.tf line 1, in resource "aws_amplify_app" "app":
│    1: resource "aws_amplify_app" "app" {
│
╵

For what little it's worth, I've preserved the branch as pass_cert_settings_in.

@socketbox socketbox force-pushed the ignore_cert_settings branch 2 times, most recently from 13509bb to 040534a Compare February 27, 2025 18:18
@socketbox socketbox marked this pull request as ready for review February 27, 2025 18:38
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.

1 participant