Add $INCLUDE parsing and RFC 2308 $TTL compliance

[xdrr]

This pull request contains both the $INCLUDE parsing functionality and RFC 2308 $TTL complaince changes into the dns-zoneparse-perl repo (both of which are explained below).

$INCLUDE parsing

$INCLUDE directives are read and parsed before the main record parsing loop to enable the included records to be parsed in the same manner as all other records.

The domain and comment parameters are supported by this new functionality however the latter is not used in the code. The domain parameter sets the relative origin of the records that are included. If excluded, the origin of the including zone is used.

The behavior of the $INCLUDE directive is to include the records of the zone file specified at the position of the directive, meaning that if the first record in the included zone has a blank name it will inherit the name of the last named record, as expected in any zone.

Added new test 'dns-zoneparse-include.t' to test the inclusion of records from another zone but without impacting the primary test. The new test is mostly a clone of dns-zoneparse.t however testing of the parsing of previously parsed and outputted records is not performed as this returns the included records being returned in an order different than when parsed directly from file or variable.

The addition of the test also saw the addition of 2 new zones, 'include-test-zone.db' which contains the $INCLUDE directives and 'included-test-zone.db' which is included by the former zone and has slight alternations in the record name and or values to distinguish the records in the test.

RFC2308 $TTL compliance

The RFC above introduces the $TTL directive that when configured causes all RRs to have their TTL set to the $TTL value if not explicitly set. This overrides the behavior of records inheriting the TTL from the last record that had it explicitly set.

The $dns_dollar_ttl variable has been introduced to store the TTL value when set by the directive and used to set the TTL of records when one is not already defined by the record.

When the $TTL directive is not used, records will use the original behavior of inheriting the last explicitly set TTL value or using the SOA record's minimum TTL.

An additional test 'dns-zoneparse-dollar-ttl.t' has been added to prove that the original behavior works when the $TTL directive is not set. The master test 'dns-zoneparse.t' has been updated to reflect the parsed result when the $TTL directive is set.

[mschilli]

Thanks for the patch, I ran the test suite and noticed it fails on my Ubuntu system:

t/dns-zoneparse-dollar-ttl.t ....... ok     
t/dns-zoneparse-exact-soa.t ........ 1/11 
#   Failed test 'NS records parsed OK'
#   at t/dns-zoneparse-exact-soa.t line 55.
#     Structures begin differing at:
#          $got->[1]{ttl} = '1H'
#     $expected->[1]{ttl} = '43200'

#   Failed test 'NS records parsed OK'
#   at t/dns-zoneparse-exact-soa.t line 55.
#     Structures begin differing at:
#          $got->[1]{ttl} = '1H'
#     $expected->[1]{ttl} = '43200'
# Looks like you failed 2 tests of 11.
t/dns-zoneparse-exact-soa.t ........ Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/11 subtests 
t/dns-zoneparse-implied-soa.t ...... 1/11 
#   Failed test 'NS records parsed OK'
#   at t/dns-zoneparse-implied-soa.t line 55.
#     Structures begin differing at:
#          $got->[1]{ttl} = '1H'
     $expected->[1]{ttl} = '43200'

[mschilli]

Also, what is the reason for these replacements?

-                'ttl'    => '43200',
+                'ttl'    => '1H',

[mschilli]

This test file has 2634 lines, most of which is just test data, is there a more compact way to test the functionality?

$ ls -l dns-zoneparse-include.t
89177 Oct 21 21:03 dns-zoneparse-include.t
$ wc -l dns-zoneparse-include.t
2634 dns-zoneparse-include.t

[xdrr]

I was able to replicate the failing test and it looks like its due to the way in which the test includes the lib directory. Running:

/usr/bin/perl t/dns-zoneparse.t

Will cause the test to use the library ../lib as well as @inc and then attempt to include the module. ../lib from the relative path of the git repo turns into dns-zoneparse-perl/../lib which doesn't exist so the next best path is the installed version of the module.

I was able to have the test fail by removing my installed version of DNS::ZoneParse from /usr/local/share/perl (or wherever it might be for you).

I cloned your repo and noted that it has the same problem meaning the module would already have to be installed before the test will work successfully.

We can fix this by changing all of the tests to:

use lib 'lib/';

and run them from the root of the repo or we can run all the tests from within the t directory.

Please let me know what you think.

P.S. I'm working on reducing the size of the tests for the $INCLUDE functionality so it's more compact.

[xdrr]

I've since updated the tests to use much smaller zone files, test only basic records and focus more closely on the $INCLUDE functionality e.g. proving that the new records are added in place of the directive and comply with the expected origin, name and TTL behavior.

I've left the use lib paths the same on all tests as I'm not sure whether you'd prefer to run tests from within the t directory or from the root of the repo.

I've updated some of the tests that existed at the time of forking as they needed to reflect the RFC2308 $TTL behavior differences (mainly that when $TTL is present the a TTL isn't specified, the TTL of a record is taken from the $TTL value and not inherited from the last explicitly mentioned TTL).

The total pull request is now down to 585 additions so please let me know what you think.

[mschilli]

Looks like the test suite is still failing:

$ perl Makefile.PL
$ make
$ make test
t/dns-zoneparse-dollar-ttl.t ....... ok
t/dns-zoneparse-exact-soa.t ........ ok
t/dns-zoneparse-implied-soa.t ...... ok
t/dns-zoneparse-include.t .......... 1/19 DNS::ZoneParse $INCLUDE references missing file: "included-test-zone.db" at t/dns-zoneparse-include.t line 28.
# Looks like you planned 19 tests but ran 1.
# Looks like your test exited with 2 just after 1.
t/dns-zoneparse-include.t .......... Dubious, test returned 2 (wstat 512, 0x200)
Failed 18/19 subtests
t/dns-zoneparse-minimum-for-ttl.t .. ok

I've left the use lib paths the same on all tests as I'm not sure whether you'd prefer to run tests from > within the t directory or from the root of the repo.

No worries, we won't require DNS::ZoneParse to be installed or the individual test case files dealing with the @inc path, we just need to make sure the test suite passes when run like shown above.

[xdrr]

I've updated the 'including' zone file to use the relative path of the repo directory so that when we run

make test

the include-test-zone.db file will include the zone file with the correct path for executng the build test.

Typically, bind and other DNS service deployments aren't designed to be portable thus a full path is used in $INCLUDE directives but the way in which we use the relative path is sufficient for testing the functionality correctly.

[mschilli]

Okay, test suite succeeds now, what I'm still wondering is why you replaced so many results to make

$got->[1]{ttl} = '1H'

$expected->[1]{ttl} = '43200'

go away. This can't possibly be backward compatible, right? I mean, if someone out there relies on the result being "43200", won't their application break now that the module returns "1H"?

[xdrr]

That's true. This implementation wouldn't be backward compatible if someone was relying on a particular parsing method.

Making this parsing method an option which could be activated by a subroutine or argument during construction with new() would certainly be more compatible and allow those using the module's original functionality to continue doing so.

Parsing SOA's as per RFC2308 allows this module to read zone files as most modern name servers do but as you've mentioned, the result affects the parsing of the entire zone and replaces existing functionality.

I'll add a commit to this pull request which demonstrates the functionality as an option that can be activated by subroutine call prior to parsing a zone.