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.

Sign up for GitHub

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

Jump to bottom

feat: Allow multiple domains in a single certificate #149

Merged
merged 5 commits into from
Aug 13, 2024
Merged

feat: Allow multiple domains in a single certificate #149

merged 5 commits into from
Aug 13, 2024

Conversation

alemarmed
Copy link
Contributor

This is a follow up of #137
Credits to @amontalban for the work, I made requested changes and tested the feature.

Description

This PR allows creating one ACM certificate for multiple domains, which, is useful when using the certificate for CloudFront that only allows one certificate per distribution.

Motivation and Context

CloudFront does not support multiple ACM certificates, like ALB. Therefore, if you need to support multiple domains in a single CloudFront distribution you would have to create the certificate manually because this module does not support it.

Breaking Changes

This change should be backward compatible as I added a zones var containing a map with domains and their Route53 zone ID so the validation records are created in the correct Route53 zone.

Additionally, I have updated the tests in examples/complete-dns-validation to allow variables so it was easier for me (and others) to test with my test domains.

How Has This Been Tested?

  • 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

Fixes #136

@alemarmed alemarmed changed the title Am/allow multiple zones feat: Allow multiple domains in a single certificate Mar 6, 2024
@alemarmed
Merge remote-tracking branch 'terraform-aws-modules/master' into am/a…
858dfac 
…llow_multiple_zones

# Conflicts:
#	wrappers/main.tf
@alemarmed
Copy link
Contributor Author

Hi @antonbabenko, is there a chance of resuming the review that you left on #137 here?
Thanks.

@amontalban
Copy link
Contributor

Thank you @alemarmed for doing this, I totally dropped the ball on the #137 but glad you created this because now I'm needing this again 🤣.

@riptidewave93
Copy link

Would also love to see this feature merged! :)

@phuhl phuhl mentioned this pull request Apr 5, 2024
Closed
@amontalban
Copy link
Contributor

@bryantbiggs any chance you can check this out? Thanks!

@github-actions 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 May 16, 2024
@nicolajv
Copy link

This would be amazing to have, commenting to keep it alive

@github-actions github-actions bot removed the stale label May 17, 2024
@github-actions 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 16, 2024
@y3ti
Copy link

y3ti commented Jun 18, 2024

It would be great to have this feature!

@github-actions github-actions bot removed the stale label Jun 19, 2024
@Samseppiol
Copy link

Waiting for this. It has been open since March, can a maintainer please review?

@amontalban
Copy link
Contributor

@bryantbiggs @antonbabenko any chance you can review/merge this 🙏? Thanks!

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 4, 2024

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 Aug 4, 2024
@riptidewave93
Copy link

Another bump to prevent this PR from being stale. Really wish the maintainer would provide feedback here :(

@github-actions github-actions bot removed the stale label Aug 5, 2024
antonbabenko
antonbabenko approved these changes Aug 13, 2024
View reviewed changes
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.

Yes, it works! Thanks to everyone involved.

@antonbabenko antonbabenko merged commit 54e5422 into terraform-aws-modules:master Aug 13, 2024
9 checks passed
antonbabenko pushed a commit that referenced this pull request Aug 13, 2024
@semantic-release-bot
chore(release): version 5.1.0 [skip ci]
0ca52d1 
## [5.1.0](v5.0.1...v5.1.0) (2024-08-13)

### Features

* Allow multiple domains in a single certificate ([#149](#149)) ([54e5422](54e5422))
@antonbabenko
Copy link
Member

This PR is included in version 5.1.0 🎉

@github-actions 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 Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@antonbabenko antonbabenko antonbabenko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to support multiple hostnames and DNS zones
7 participants
@alemarmed @amontalban @riptidewave93 @nicolajv @y3ti @Samseppiol @antonbabenko