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

IPv6 Address Parsing #1

Closed
wants to merge 5 commits into from
Closed

Conversation

dlove24
Copy link

@dlove24 dlove24 commented Sep 18, 2012

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
Copy link
Member

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
Copy link
Author

dlove24 commented Sep 23, 2012

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 (#1 (comment)).

@mattpascoe
Copy link
Member

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 added a commit that referenced this pull request Oct 3, 2012
@mattpascoe
Copy link
Member

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.

@mattpascoe mattpascoe closed this Oct 3, 2012
@dlove24
Copy link
Author

dlove24 commented Oct 4, 2012

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 (#1 (comment)).

@dlove24
Copy link
Author

dlove24 commented Oct 4, 2012

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

  • $dnsrecord['type'] = (strpos($int['ip_addr_text'],':') ? 'AAAA' : 'A');
  • $dnsrecord['type'] = (strpos($interface['ip_addr_text'],':') ? 'AAAA' : 'A');

$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

  • $arpatype = (strpos($int['ip_addr_text'],':') ? 'ip6' : 'in-addr');
  • $arpatype = (strpos($interface['ip_addr_text'],':') ? 'ip6' : 'in-addr');

// 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 (#1 (comment)).

@mattpascoe
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants