feat: Added extension for alternative zone_id's for SAN's #100
Conversation
ZeePal
changed the title
ZeePal
changed the title
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Hi @antonbabenko , don't wana pressure/rush anything. just commenting to keep the bot from closing :) |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Would be great to get this across. @ZeePal it seems there are some conflicts I guess meanwhile you can try and solve them? Please Mr Bot don't close this PR |
|
@FabioAntunes srry for the delay, git conflict resolved. be kind for a little longer Mr Bot. |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Mr bot, don't |
| @@ -36,7 +36,7 @@ variable "domain_name" { | |||
|
|
|||
| variable "subject_alternative_names" { | |||
| description = "A list of domains that should be SANs in the issued certificate" | |||
| type = list(string) | |||
| type = any # list(string | map(string)) | |||
There was a problem hiding this comment.
I have a belief that this change won't work with terragrunt because of type any. There is a similar issue in Route53 module.
Could you change the type from list(string) to list(map(string)) to prevent the issue?
PS: I have not executed this code yet.
There was a problem hiding this comment.
if we change from any to list(map(string)) it will no-longer be a backwards compatible change for pure-terraform as they currently provide a list(string).
if we keep the any and follow the terragrunt workaround it will be backwards compatible for pure-terraform but not for terragrunt.
if backwards compatibility is required for both pure-terraform & terragrunt the vars interface will need to be changed (something like a 2nd optional list of zone_ids but this isnt clean/reliable) or we bump major version to use list(map(string)).
if we going to bump the major version to use list(map(string)), do we want to consider any other changes at the same time?
current user input:
subject_alternative_names = [
"subdomain.mydomain.com",
"www.subdomain.mydomain.com",
]user input if list(map(string)) is required but for the same result as above:
subject_alternative_names = [
{name = "subdomain.mydomain.com"},
{name = "www.subdomain.mydomain.com"},
]There was a problem hiding this comment.
hi @antonbabenko , have you had a chance to go over my previous reply & which direction we want to head?
There was a problem hiding this comment.
Sorry that it takes more time to process these PRs than one would expect.
I looked into #108 a few days ago, and I think the feature that this PR (#100) adds should be considered and implemented after it.
The main reason to order PRs like this is that users may want to create records using different instances of the providers (zone_id + provider). It will be easier to implement that way and update examples to show a variety of combinations.
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Mr bot, stop trying to close this. |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
This PR was automatically closed because of stale in 10 days |
|
Can this be reopened? |
|
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. |
Description
This is a proposed change to keep backwards compatibility while providing functionality to allow subject alternative names to optionally include a different Route53 Zone ID (if not provided assume
var.zone_id)Motivation and Context
Resolves: #21
This is an alternative approach to #91 but to keep extra data objects outside of the module.
This implementation doesnt use data route53 calls and keeps provider & terraform version backwards compatibility while using explicit configuration for the SAN you wish to handle differently.
Breaking Changes
Should be fully backwards compatible.
The
try()calls allow both old (strings) and alternative (map) representations of each SAN element.How Has This Been Tested?
examples/*projectsTested with DNS example and created a DNS example for multiple Route53 zones which was also tested with the below combinations: