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:
                Type:  defect        |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp-ddns     |  reviewing
            Keywords:                |                    Milestone:  DHCP-
           Sensitive:  0             |  Kea0.9
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  12            |                 CVSS Scoring:
         Total Hours:  8             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => marcin


Comment:

 Replying to [comment:4 marcin]:
 > 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));
 >
 > }}}

 Oops, got it.

 >
 >
 > '''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?
 >
 You found that did ya? Got it.

 > 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?
 >
 >

 You are right, this is a better approach and even though I don't really
 have time to do it, I did it anyway.  Next time don't set bad examples ;)



 > '''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
 > }}}

 Dang it!  Fixed it.

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