BIND 10 #3352: If client requests delegation, (S=0, N=0), Kea should set NCR to do only reverse updates

[BIND 10 Development]

#3352: If client requests delegation, (S=0,N=0), Kea should set NCR to do only
reverse updates
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:  tmark
                Type:  defect        |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp-ddns     |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  12            |              Defect Severity:  N/A
         Total Hours:  8             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * hours:  6 => 2
 * owner:  marcin => tmark
 * totalhours:  6 => 8


Comment:

 Reviewed commit 4fc2c2cb250f93809d3287bdf9d50f5cbbc0fd17:

 '''src/bin/dhcp4/tests/fqdn_unittest.cc'''
 Commented line should probably be removed from the code below
 (respectClientDelegation):
 {{{
     flagVsConfigScenario(Option4ClientFqdn::FLAG_E,
                          Option4ClientFqdn::FLAG_E);
                          // Option4ClientFqdn::FLAG_N));

 }}}


 '''src/bin/dhcp6/tests/fqdn_unittest.cc'''
 There is new but unusued variable declared in generateMessage function:
 {{{
         OptionPtr tmp = pkt->getOption(D6O_CLIENTID);
 }}}

 Is it a leftover after debugging?

 testFqdn (code following the line 345):
 Wasn't it possible to use verifyNameChangeRequest function to test the NCR
 created?

 Now something to consider.... you don't have to do anything about it if
 you don't want...
 When looking just at last three parameters in this call:
 {{{
     testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S,
              "myhost.example.com",
              Option6ClientFqdn::FULL, Option6ClientFqdn::FLAG_S,
              "myhost.example.com.", true, true, true);
 }}}

 it is completelly unclear what they mean. This is a common problem with a
 set of boolean values passed one next to another to a test function. There
 are ways out from it - like wrap the boolean values into individual types
 say:
 {{{
 struct ExpFwd {
     ExpFwd(bool exp_fwd) : value_(exp_fwd);
     bool value_;
 }
 }}}

 The three parameters would look like this:
 {{{
 "myhost.example.com.", CreateNcrCheck(true), ExpFwd(true), ExpRev(true));
 }}}

 That has another advantage, that you can't mix them up, because there is
 no implicit cast between then.

 I know that Tomek hates doing things like this and also I don't want to
 put too much work on you here. And I realize that there are other
 functions here that I wrote, which don't follow this idea. So, maybe
 instead some comment could precede the invocations?


 '''src/lib/dhcpsrv/d2_client_mgr.h'''
 getUpdateDirections: By doxygen convention the [out] should precede the
 name of the parameter:
 {{{
     /// @param [out] forward bool value will be set to true if forward
 udpates
 }}}

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