BIND 10 #3034: DHCP6 Configuration Parameter additions for DDNS

[BIND 10 Development]

#3034: DHCP6 Configuration Parameter additions for DDNS
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp-ddns     |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  32            |              Defect Severity:  N/A
         Total Hours:  10            |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tomek):

 * hours:  8 => 2
 * owner:  tomek => tmark
 * totalhours:  8 => 10


Comment:

 The comments will mostly reflect similar comments made earlier to similar
 work in DHCPv4:

 '''src/bin/dhcp6/dhcp6.spec'''
 Do we ned ncr-format? For the foreseable future, we'll only support JSON.
 The same is true for ncr-protocol. Only UDP for now.

 '''src/bin/dhcp6/dhcp6_srv.cc'''
 Please turn this comment in assignIA_NA into a @todo:
 {{{
 // For now, we assert that if we are doing forward we are also
 // doing reverse.
 }}}

 '''src/bin/dhcp6/tests/fqdn_unittest.cc'''
 Why are there bitmasks like ALWAYS_INCLUDE_FQDN defined here?
 Shouldn't they be specified in a header somewhere? If there is a good
 explanation,
 why they are defined in cc, not in a header, please add a comment with
 that reason.
 If not, please move it to a header.

 Several tests use generated DHCID. Is there an explanatory comment
 somewhere
 that explains how to generate those values by hand (or whatever technique
 you
 used to get it)? If not, please add one.

 ------------------

 A question about ChangeLog. It claims that "The parameters
 parse, store and retrieve but do not yet govern behavior."

 Is this really true? There are snippets in the code that seem
 to use those parameters, e.g.

 {{{
     if (!CfgMgr::instance().ddnsEnabled()) {
 }}}

 -------------------
 I assume that the documentation (Guide) will be added in a separate
 ticket, right?

 ------------
 The expected changes after this review are sufficiently small, so I don't
 need to see
 this ticket again if you address them.

-- 
Ticket URL: <https://bind10.isc.org/ticket/3034#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development