opennetadmin / build_bind

OpenNetAdmin plugin to manage and build BIND DNS server configurations
13 stars 13 forks source link

IPv6 Address Parsing #1

Closed dlove24 closed 11 years ago

dlove24 commented 11 years ago

You probably have a better idea of how to do this, but I would like to add IPv6 support to the Bind record generator. As far as I can tell ONA does not flag IPv6 addresses (using the IPv6 branch of the ona master on GitHub). So I have added a regex to do an address check, before adding either 'A' or 'AAAA' to the zone record. This should also be safe for IPv4 only zones, and doesn't require an further changes to the ona database.

mattpascoe commented 11 years ago

Hey, I guess I didnt have a watch set up properly on this repo.. Now I'll get notified!

Overall it looks good.. I think it would be better to do the test similarly to this: https://github.com/opennetadmin/build_tinydns/blob/master/build_tinydns.inc.php#L290

I have updated my tinydns code for the IPv6 support and that is how I did it there. The ona_get_interface_record function that gathers the ip_addr_text data already has the smarts to validate/verify that the IP is legit so all I care about is if it has a : in it or not.

If you want to update your code to work similarly then I'll merge it in.

I've also not looked through the bind code yet thoroughly for other ipv6 related changes but hey, we can catch those as we go. Thanks for the code updates!

dlove24 commented 11 years ago

Sorry for the late reply: we are just gearing up for the start of term and finding time to do useful things is a bit hard.

Thanks for the suggestion to look at the tinydns export: I had forgotten about the IPv6 patches to that code base. That approach is certainly simpler, and ':' should never be present in a dotted quad. I would be happy to remove the regex to make the intent clearer, and keeping the two DNS export scripts reasonably similar is certainly better.

Do you want me to modify the current fork? Or re-fork to keep the history neater?

Thanks

David.

On Thursday, 20 September 2012 at 23:05, mattpascoe wrote:

Hey, I guess I didnt have a watch set up properly on this repo.. Now I'll get notified! Overall it looks good.. I think it would be better to do the test similarly to this: https://github.com/opennetadmin/build_tinydns/blob/master/build_tinydns.inc.php#L290 I have updated my tinydns code for the IPv6 support and that is how I did it there. The ona_get_interface_record function that gathers the ip_addr_text data already has the smarts to validate/verify that the IP is legit so all I care about is if it has a : in it or not.
If you want to update your code to work similarly then I'll merge it in. I've also not looked through the bind code yet thoroughly for other ipv6 related changes but hey, we can catch those as we go. Thanks for the code updates!

— Reply to this email directly or view it on GitHub (https://github.com/opennetadmin/build_bind/pull/1#issuecomment-8747369).

mattpascoe commented 11 years ago

Not a problem I know the pull of other things that use up time.

I'm fine with either way, do whatever is easiest for you.

Thanks again!

mattpascoe commented 11 years ago

As you can see, I have created an ipv6 branch and put changes in that implement what your pull request does here. Give it a try and see if it works for you.. I'll close this pull request for now.

dlove24 commented 11 years ago

Excellent, thanks. I'll give that a try later and report any problems.

David.

On Wednesday, 3 October 2012 at 23:38, mattpascoe wrote:

As you can see, I have created an ipv6 branch and put changes in that implement what your pull request does here. Give it a try and see if it works for you.. I'll close this pull request for now.

— Reply to this email directly or view it on GitHub (https://github.com/opennetadmin/build_bind/pull/1#issuecomment-9124970).

dlove24 commented 11 years ago

Hi,

I am having problems with the latest updates to the ipv6 branch, which doesn't seem to use the right record for string parsing. The code seems to be looking for "$int['ip_addr_text']", which I think should be "$interface['ip_addr_text']". Changing the code builds both the forward and reverse zones correctly on my servers at any rate.

Thanks,

David.

diff --git a/build_bind.inc.php b/build_bind.inc.php index d2a36fc..232e7e2 100644 --- a/build_bind.inc.php +++ b/build_bind.inc.php @@ -419,7 +419,8 @@ EOF }

// Determine A record type if it is IPv6

$fqdn = $dnsrecord['name'].$domain['fqdn']; $text .= sprintf("%-50s %-8s IN %-8s %-30s %s\n" ,$fqdn.'.',$dnsrecord['ttl'],$dnsrecord['type'],$interface['ip_addr_text'],$dnsrecord['notes']); @@ -438,7 +439,7 @@ EOF list($status, $rows, $ptr) = ona_get_dns_record(array('id' => $dnsrecord['dns_id']), '');

// set the ptr zone type for IPv6 records

// If this is a delegation domain, find the subnet cidr if ($ptrdelegation) {

On Thursday, 4 October 2012 at 09:33, David Love wrote:

Excellent, thanks. I'll give that a try later and report any problems.

David.

On Wednesday, 3 October 2012 at 23:38, mattpascoe wrote:

As you can see, I have created an ipv6 branch and put changes in that implement what your pull request does here. Give it a try and see if it works for you.. I'll close this pull request for now.

— Reply to this email directly or view it on GitHub (https://github.com/opennetadmin/build_bind/pull/1#issuecomment-9124970).

mattpascoe commented 11 years ago

Ugh.. thats what I get for copying and pasting without actually testing the code!.. good thing for branches.. Thanks for testing and catching that.. I'll get it updated.