Foreman DNS: Converting IP's to PTR-Records

Hello dev team,

currently, when creating/deleting PTR's Foreman converts IP's to PTR
records [1] (hopefully the right place) it seems before sending them off
as attributes to smart-proxy. Turns out, all the tests in smart-proxy
are not honoring this fact. [2]

Can somebody explain to me the reasoning behind this?

I am asking of course because I question it as it leads to issues
downstream with smart-proxy, as I am currently experiencing them [3]:

  • I think the provider / smart-proxy should be checking for duplicates
    as foreman might use other / unreachable name resolution. Nevertheless,
    the implemented providers do so already
  • Reverse DNS find, which we use to search for duplicates, only works
    with IP addresses AFAIK
  • There is no reliable way I know of to to convert a PTR record back to
    an IP address. While this might be done with some questionable
    .split('.').reverse with IPv4, this gets much more complicated with IPv6

We should remove complexion from Foreman, always send the IP to
smart-proxy and let the provider figure out what method would be best to
set it.
This is a breaking API change; though I think PTRs are currently noop
anyway the way smart-proxy is implementing them.

Thanks!

[1]


[2]


[3] https://github.com/theforeman/smart-proxy/pull/379

··· -- Daniel Helgenberger (helge000) daniel@helgenberger.net

> Hello dev team,
>
> currently, when creating/deleting PTR's Foreman converts IP's to PTR
> records [1] (hopefully the right place) it seems before sending them off
> as attributes to smart-proxy. Turns out, all the tests in smart-proxy
> are not honoring this fact. [2]
>
> Can somebody explain to me the reasoning behind this?
>
> I am asking of course because I question it as it leads to issues
> downstream with smart-proxy, as I am currently experiencing them [3]:
>
> - I think the provider / smart-proxy should be checking for duplicates
> as foreman might use other / unreachable name resolution. Nevertheless,
> the implemented providers do so already

For MS Dns this makes sense for create PTR operations only, deletes
will always succeed, whether record exists or not.

> - Reverse DNS find, which we use to search for duplicates, only works
> with IP addresses AFAIK

Please see https://github.com/theforeman/smart-proxy/pull/379#issuecomment-187140738,
where I suggest a solution that will work.

> - There is no reliable way I know of to to convert a PTR record back to
> an IP address. While this might be done with some questionable
> .split('.').reverse with IPv4, this gets much more complicated with IPv6

See above.

>
>
> We should remove complexion from Foreman, always send the IP to
> smart-proxy and let the provider figure out what method would be best to
> set it.
> This is a breaking API change; though I think PTRs are currently noop
> anyway the way smart-proxy is implementing them.

I think it’s highly unlikely that a breaking change such as the one
you are suggesting will be implemented, esp. considering there’s a way
to make provider work without changing the api. That aside, versioning
of api has to happen before such a change can be safely implemented.

Cheers,
-d

··· On Mon, Feb 22, 2016 at 11:50 AM, Daniel Helgenberger wrote:

Thanks!

[1]
https://github.com/theforeman/foreman/blob/8bbfa2c36cb07d6c9d48631384b051c2532c4e67/lib/net/dns/ptr_record.rb#L41
[2]
https://github.com/theforeman/smart-proxy/blob/develop/test/dns_dnscmd/dnscmd_test.rb#L53
https://github.com/theforeman/smart-proxy/blob/develop/test/dns_nsupdate/dns_nsupdate_test.rb#L35
[3] https://github.com/theforeman/smart-proxy/pull/379

Daniel Helgenberger (helge000)
daniel@helgenberger.net


You received this message because you are subscribed to the Google Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

> Hello dev team,
>
> currently, when creating/deleting PTR's Foreman converts IP's to PTR
> records [1] (hopefully the right place) it seems before sending them off
> as attributes to smart-proxy. Turns out, all the tests in smart-proxy
> are not honoring this fact. [2]
>
> Can somebody explain to me the reasoning behind this?

It is a bug in our tests. I have a commit[4] in PR 371[5] that fixes
this. I'll create a separate PR for this.

> I am asking of course because I question it as it leads to issues
> downstream with smart-proxy, as I am currently experiencing them [3]:
>
> - I think the provider / smart-proxy should be checking for duplicates
> as foreman might use other / unreachable name resolution. Nevertheless,
> the implemented providers do so already
> - Reverse DNS find, which we use to search for duplicates, only works
> with IP addresses AFAIK
> - There is no reliable way I know of to to convert a PTR record back to
> an IP address. While this might be done with some questionable
> .split('.').reverse with IPv4, this gets much more complicated with IPv6

It's trivial to convert a PTR record to an IP, both for IPv4 and IPv6.

> We should remove complexion from Foreman, always send the IP to
> smart-proxy and let the provider figure out what method would be best to
> set it.
> This is a breaking API change; though I think PTRs are currently noop
> anyway the way smart-proxy is implementing them.

I'm pretty sure it currently works. You can use my integration test[6]
to verify it. You only need to modify it to point to the correct
nameserver, smart-proxy and remove the purge_cache (or adapt it to your
backend if it needs it).

I can expand on this if you like or maybe poke me on IRC (ewoud) to
debug together.

> [1]
> https://github.com/theforeman/foreman/blob/8bbfa2c36cb07d6c9d48631384b051c2532c4e67/lib/net/dns/ptr_record.rb#L41
> [2]
> https://github.com/theforeman/smart-proxy/blob/develop/test/dns_dnscmd/dnscmd_test.rb#L53
> https://github.com/theforeman/smart-proxy/blob/develop/test/dns_nsupdate/dns_nsupdate_test.rb#L35
> [3] https://github.com/theforeman/smart-proxy/pull/379

[4] https://github.com/ekohl/smart-proxy/commit/7f6d5bc7d59afa09854ade3ce07e0786157e5f90
[5] https://github.com/theforeman/smart-proxy/pull/371
[6] https://github.com/theforeman/smart_proxy_dns_powerdns/blob/master/test/integration/integration_test.rb

··· On Mon, Feb 22, 2016 at 12:50:41PM +0100, Daniel Helgenberger wrote:

I don't think we need to change the Foreman <-> Proxy API for this.
Besides that, we created a consistent internal API with create_record
and remove
_record. The only inconsistent thing there is
remove_ptr_record is passed a parameter named ip while it's the reverse
IP notation.

I submitted https://github.com/theforeman/smart-proxy/pull/384 to make
the unit tests consistent with the real API.

··· On Mon, Feb 22, 2016 at 12:03:08PM +0000, Dmitri Dolguikh wrote: > On Mon, Feb 22, 2016 at 11:50 AM, Daniel Helgenberger > wrote: > > We should remove complexion from Foreman, always send the IP to > > smart-proxy and let the provider figure out what method would be best to > > set it. > > This is a breaking API change; though I think PTRs are currently noop > > anyway the way smart-proxy is implementing them. > > I think it’s highly unlikely that a breaking change such as the one > you are suggesting will be implemented, esp. considering there’s a way > to make provider work without changing the api. That aside, versioning > of api has to happen before such a change can be safely implemented.