Fixes #17003 - DNS Plugins now support rewriting of PTR records

[arogge]

nsupdate, nsupdate_gss and dnscmd now support a new option called
dns_ptr_rewritemap.
You can provide a hash of regex => replacement that will be used as
a map to rewrite your PTR just before it is sent to the backend.

[theforeman-bot]

There were the following issues with the commit message:

• commit message for 90c8290 is not wrapped at 72nd column
• commit message for 90c8290 is not wrapped at 72nd column

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available in Coding Standards or on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[dmitri-d]

Thanks for the PR!

I feel that the logic of remapping of ptr records would be better implemented in foreman:

• Ptr mapping feature completely changes api behavior -- instead of creating or deleting a record in one zone (implicitly defined) it creates a similar record in a completely different zone.
• While I don't expect such a mapping to change often (if at all), I think of it as a part of network design/definition, something Foreman is responsible for.

Therefore I think it would be best to split this feature into smart-proxy and foreman parts. Smart proxy dns api can be extended to accept zone during PTR record create and delete calls. Foreman part would manage mapping of ptr records to rfc2317 zones and pass this information on to smart-proxy in dns create/delete record calls.

@domcleal, @dLobatog, @GregSutcliffe (and anyone else): any thoughts?

[arogge]

Thank you for your comment!
I thought about this for quite some time, so let me get my arguments straight:

• PTR rewriting is ugly, but so is RFC2317. Of course it does create a record in a different zone - that is the whole point of it
• While you see it as a part of network design/definition, I think of it as an implementation detail that can and should be handled by the proxy

You can move the implementation into foreman. The current proxy API is sufficient for this, because you don't need any additional information (i.e. the zone name is not required as it is easily derived from the PTR and nsupate doesn't need it at all).
The problem with that approach is the implementation in Smart Proxy:

• the (unrewritten) PTR is used to call ptr_to_ip() to obtain the ip address for ptr_record_conflicts()
• dns_find() cuts and pastes parts of the PTR to obtain the IP address itself

Thus, if you try to work with RFC2317 PTRs through the current API things fail horribly (I already tried to do it that way).
As there is no sane way to programmatically convert the PTR back to the IP, because 33.2.0.192.in-addr.arpa could also be delegated as ip-33.reverse.your-domain.tld, we'd end up with an API that takes the IP as an extra argument unless Smart Proxy can work without it.
I don't think it is a good idea to change the API just to support something that is probably a corner-case.

[dmitri-d]

The problem with that approach is the implementation in Smart Proxy...

This is quite different from a "normal" ptr record creation, I would think that it warrants dedicated, backend-specific calls, something like

Proxy::Dns::Record#create_rfc2317_ptr_record(fqdn, value)

which can perform their own validations. We could use RFC2317 as the record type to denote that RFC2317-type record needs to be created/deleted.

Please note that #465 deprecates Proxy::Dns::Record#dns_find call in favour of more specific get_name/get_address calls.

[arogge]

I just came up with another approach that is probably much saner.
All RFC2317 delegations utilize CNAMEs - the original PTR (e.g. 33.2.0.192.in-addr.arpa) is a CNAME pointing into another zone (e.g. ip-33.reverse.your-domain.tld).
What would be quite simple to implement is the following (warning, pseudo-code ahead):
if target_ptr = dns_lookup_cname(ptr) {
create_record(target_ptr, fqdn)
} else {
create_record(ptr, fqdn)
}

[dmitri-d]

I would prefer it if the code didn't try to guess caller's intention; a possible approach I suggested in the comment above would be to use a special record type together with a dedicated code path for creation/removal of rfc2317 records.

[arogge]

I had a look at #465 and suggested another change. If that change is going to happen we would have everything in smart proxy that is required to implement rfc2317. All other changes required could then be done in foreman itself.

[dmitri-d]

@arogge: thanks for the suggestion of using Reolver::DNS#getresource(s). I opened a PR that implements the change #469:

[mmoll]

@arogge this needs a rebase.

[dmitri-d]

@arogge: are you planning to continue working on this PR?

[arogge]

@witlessbird not really. As you already stated it is probably better to implement the feature in foreman itself. #469 "fixes" the proxy code, so this is finally possible.
If I find the time to do it, I'll implement it on the foreman-side. So this PR can be closed.