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
feat: Allow users to specify multiple Route53 zones as well as cross account zones with external-dns
#838
Conversation
…account zones with `external-dns`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, minor questions/comments
@@ -1,4 +1,3 @@ | |||
provider: aws | |||
zoneIdFilters: ${zone_filter_ids} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we know what was the original reason for having zoneIdFilters
set? can we safely remove it? checking https://github.com/bitnami/charts/blob/master/bitnami/external-dns/values.yaml#L609-L611 I am not sure, does external DNS know automatically which zone id to use? iirc domain filter is what used mainly but I might be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I looked this up as well and it all looks to be optional. so to provide better flexibility, I think defaulting to the minimum required values and letting users opt in to additional values will scale better (they now can choose to set zone ID filters, zone name filters, domain name filters, etc. or not)
source = "./external-dns" | ||
|
||
count = var.enable_external_dns ? 1 : 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit question: does it matter if source comes before count (and do we need extra line in-between)? looking at other modules it's usually source->count
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it matter - no. its just my OCD in that its typically source directly under the module declaration. the other changes are just whitespace to break out to sections:
module "foo" {
source = "<URL/PATH>
count or for_each
parameters
tags
depends_on
}
…account zones with `external-dns` (aws-ia#838)
What does this PR do?
external-dns
set_values
throughhelm_config
variableexternal-dns
helm chart version to latestMotivation
external-dns
addon with one or more Route53 zones in the current account, external account(s), or both while not requiring them to re-provide the values set in the defaultvalues.yaml
file provided in the addonMore
pre-commit run -a
with this PRNote: Not all the PRs required examples and docs except a new pattern or add-on added.
For Moderators
Additional Notes