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

Allow providing AWS zone ID as well as domain name filter #322

Closed
wants to merge 3 commits into from

Conversation

dileepfrog
Copy link

The Tectonic installer, for example, will always create a private zone with the same DNS name as the public one (a shortcoming of Tectonic to be sure), which breaks external-dns. Explicitly being able to specify the public zone would address this issue.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 19, 2017
@jrnt30
Copy link
Contributor

jrnt30 commented Aug 19, 2017

We would find value in this contribution as well. There was some discussion around something similar in #287 but you raise a good point, which is that in this case the permissions will most likely still permit the publishing of records to both of the hosted zones anyways.

I would say though, that if the flag is multi-purposed like this that it deserves an explicit call out in the documentation.

@sean-krail
Copy link

Another way of fixing this issue is by implementing #327 with the added bonus of not needing to hard code the zoneId.

@jrnt30
Copy link
Contributor

jrnt30 commented Sep 1, 2017

@sean-krail One thing that #327 doesn't afford is if you have multiple private hosted zones with the same name, peered to VPCs differently.

While I agree that a simple flag for some cases is warranted, it does still make sense to have the ability to very explicitly declare this attribute as the filter

@redbaron
Copy link
Contributor

Tag filtering.Better filter zones by tag

@linki
Copy link
Member

linki commented Oct 11, 2017

As this has a lot of upvotes and enables ExternalDNS to be used on Tectonic ❤️ I would like get this in.

I'm not in favour of this implementation howver as it's a bit too magical and enforces users to use the trailing dot notation if they indeed want to keep matching by dns name.

How about we add another flag called --aws-zone-id similar to the other filters for hosted zones that "filters" by ID. It would ever only return zero or one result.

What do you think @dileepfrog @jrnt30 @sean-krail @redbaron ?

@linki linki added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Oct 11, 2017
@redbaron
Copy link
Contributor

If you can specify that flag multiple times to allow multiple zones to be managed, I am fine. Still prefer tagging approach, but since I am not supporting my preference with concrete code in a form of PR, it is not relevant :)

@chancez
Copy link

chancez commented Oct 12, 2017

I also think it would be nice to be able to specify a list of zoneIDs, but most commonly would use one.

FWIW; the reason Tectonic does this is to enable components within the cluster to use the private network when communicating with the control plane, so theres two hosted zones created so that within the cluster. It's effectively split horizon DNS. Everything from inside the network can use the private hosted zone which uses the private IPs for everything. This is only for if your exposing your cluster publicly though. If it's private, then theres only the private hosted zone.

Also if this is blocking you, I believe the tectonic installer supports not creating a private hosted zone if you don't want one.

@linki
Copy link
Member

linki commented Nov 13, 2017

@jose5918 Fixed the conflicts in jose5918#1

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 14, 2017
@linki
Copy link
Member

linki commented Jan 2, 2018

@dileepfrog I believe this PR is now obsolete since we added a separate flag for filtering by zoneID in #422. @dileepfrog Can you please double check if that's true.

@linki linki self-assigned this Jan 2, 2018
@linki linki added this to the cw01 milestone Jan 2, 2018
@linki
Copy link
Member

linki commented Jan 5, 2018

Closing this because of #322 (comment)

@dileepfrog feel free to reopen this PR if I'm wrong.

@linki linki closed this Jan 5, 2018
lou-lan pushed a commit to lou-lan/external-dns that referenced this pull request May 11, 2022
* lint: integrate impi

* install impi in a tmp folder

* put install into func and install master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants