Skip to content

Conversation

@yordaa-me
Copy link
Contributor

@yordaa-me yordaa-me commented May 10, 2022

Description

The proposed changes will allow both cross-account and single account ACM creation with DNS validation.

Motivation and Context

I needed to create my ACM certificates in account B but my hosted zone belongs to Account A. These changes allowed me to meet this requirement.

Breaking Changes

I believe the two providers will now always be required and need to be explicitly passed down.

In the module call, people will now need to pass the providers block with the two required providers.

  providers = {
    aws.acm = aws.account_b,
    aws.dns = aws.account_a
  }

or if they use a single account then the following block should still work

  providers = {
    aws.acm = aws,
    aws.dns = aws
  }

How Has This Been Tested?

I have tested by calling the fork with my branch

module "acm" {
  source = "git@github.com:Pod-Point/terraform-aws-acm.git?ref=patch-cross-account-provider"

  providers = {
    aws.acm = aws.<ommited>,
    aws.dns = aws.<ommited>
  }

  domain_name = var.project_domain
  zone_id     = var.pod_point_hosted_zone_id

  subject_alternative_names = var.additional_aliased_domains

  wait_for_validation                = true
  validation_allow_overwrite_records = false
}
  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@yordaa-me yordaa-me changed the title Patch cross account provider cross-account DNS and ACM resource creation May 10, 2022
@yordaa-me yordaa-me changed the title cross-account DNS and ACM resource creation feat: cross-account DNS and ACM resource creation May 10, 2022
@yordaa-me yordaa-me changed the title feat: cross-account DNS and ACM resource creation BREAKING CHANGE: cross-account DNS and ACM resource creation May 10, 2022
yordaa-me added 2 commits May 10, 2022 14:03
add changes made from the pre-commit and the recommended PR guidelines
@yordaa-me yordaa-me force-pushed the patch-cross-account-provider branch from 9f012f7 to 632029f Compare May 10, 2022 13:04
@yordaa-me yordaa-me changed the title BREAKING CHANGE: cross-account DNS and ACM resource creation feat: cross-account DNS and ACM resource creation May 10, 2022
@yordaa-me yordaa-me changed the title feat: cross-account DNS and ACM resource creation feat: Cross-account DNS and ACM resource creation May 10, 2022
the coniguration_aliases is enough to make this work without the default provider blocks
@github-actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Jun 19, 2022
@yordaa-me
Copy link
Contributor Author

Any chance this can be looked at

@github-actions github-actions bot removed the stale label Jun 21, 2022
@Fodoj
Copy link

Fodoj commented Jul 19, 2022

Would be great to see this one merged! How does it handle the case of using single account? Does the user still need to path 2 providers even if it's the same one?

@mustafa89
Copy link

@antonbabenko any chance this can be merged?

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

I am afraid there is no chance that this PR can be merged as it is now because it breaks one of the requirement for the resource module.

To achieve the same behavior we need to have some changes to the code:

  1. Module should be updated to create either ACM certificate (and validation resources) or Route53 records.
  2. Module should use a single/standard AWS provider passed into it (explicitly or implicitly).
  3. To achieve cross-account resource creation, there should be two module calls.
  4. By default, when not using cross-account resource creation, one provider is required, and the module should be backward compatible.

The example code (taken from examples/complete-dns-validation) shows this feature:

provider "aws" {
  alias = "route53"
}

provider "aws" {
  alias = "acm"
}

module "acm" {
  source = "../../"

  providers = {
    aws = aws.acm
  }

  domain_name = local.domain_name
  zone_id     = coalescelist(data.aws_route53_zone.this.*.zone_id, aws_route53_zone.this.*.zone_id)[0]

  subject_alternative_names = [
    "*.alerts.${local.domain_name}",
    "new.sub.${local.domain_name}",
    "*.${local.domain_name}",
    "alerts.${local.domain_name}",
  ]

  wait_for_validation = true

  create_route53_records = false
  validation_record_fqdns = module.route53_records.validation_route53_record_fqdns

  tags = {
    Name = local.domain_name
  }
}

# AWS provider that is allowed to manage Route53 records required by the ACM module
module "route53_records" {
  source = "../../"

  providers = {
    aws = aws.route53
  }

  domain_name = module.acm.domain_name # TODO
  zone_id     = module.acm.zone_id  # TODO

  create_certificate = false

  acm_certificate_domain_validation_options = module.acm.acm_certificate_domain_validation_options # TODO
}

PS: I made a live stream today where I started looking into it (see from 41:43). Hopefully, it helps to understand the background and the solution.

@antonbabenko
Copy link
Member

This issue has been resolved in version 4.1.0 🎉

@mputilin
Copy link

mputilin commented Sep 8, 2022

Thanks for the feature! Already updated our code - works like a charm. No need to create validation records manually anymore.

@antonbabenko
Copy link
Member

Thank you for the confirmation, @mputilin !

@yordaa-me yordaa-me deleted the patch-cross-account-provider branch September 8, 2022 09:40
@yordaa-me yordaa-me restored the patch-cross-account-provider branch September 8, 2022 09:42
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants