BIND 10 #3151: PD: Configuration parsers much be able to parse PD configuration

[BIND 10 Development]

#3151: PD: Configuration parsers much be able to parse PD configuration
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpconf      |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130918
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tomek):

 * owner:  tomek => tmark


Comment:

 '''src/bin/dhcp6/tests/test_data_files_config.h.in'''
 Unless there's something you'd like to share about your side project that
 involved breakthrough in time travel, I think you should update the header
 to the current year, which is 2013. ;)

 '''src/bin/dhcp6/config_parser.cc'''
 !PdPoolParser class should have a class comment. It would be very useful,
 if that comment could have a piece of configuration that specific class is
 handling e.g.
 {{{
 {
   "prefix": "2001:db8:1:02::",
   "prefix-len": 72,
   "delegated-len": 88
 },
 }}}

 PdPoolListParser::build() checks if the list of pool definitions is empty
 and throws if it is. It is perfectly valid configuration to have only
 address pools specified, without any PD pools. I'm not sure whether the
 code or the comment should be updated.

 So what happens if there is address-only configuration? The parser is not
 created at all? It is created, but an empty list passed?

 '''src/bin/dhcp6/tests/config_parser_unittest.cc'''

 Dhcp6ParserTest.pdPoolBasics: the test should confirm that the last
 address in a pool is is ok (in words checking if 'prefix-len' was
 processed correctly). You can calculate it manually or use
 lastAddrInPrefix(prefix, prefix_len).

 I'd like to see four additional tests. First, checking that it is possible
 to delegate the whole prefix (i.e. prefix-len == delegated-len). The
 second one would be to have adress only configuration (some addresses, no
 prefixes). The third one would be the opposite (no address, some
 prefixes). The fourth would be to have the whole subnet delegated as
 prefixes.

 It is likely that the second test is already there, so just a comment
 added that address-only test doubles as a test that verifies that a prefix
 can remain not-configured.

 '''src/lib/dhcpsrv/tests/test_libraries.h'''
 You modified its content and replaced /home/thomson/... with
 /Users/tmark/... I'm not sure if I like that change :)
 In fact, why they test_libraries.h file is in the repo at all?

 The changes look good. The code builds and unit-tests pass on Ubuntu.

 One important thing is missing: BIND10 User's Guide is not updated.
 Although there is a separate ticket for doing a full User's Guide update
 with all the explanations, for now adding a boilerplate would be
 sufficient. Please add a section to doc/guide/bind10-guide.xml, with a
 simple PD config and a @todo tag.

 Please make sure that you tell Włodek when this ticket is merged, so he
 can start updating Forge tests to cover configuration.

-- 
Ticket URL: <http://bind10.isc.org/ticket/3151#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development