BIND 10 #1535: notify_out shouldn't look for NS addresses for out of zone NS names

[BIND 10 Development]

#1535: notify_out shouldn't look for NS addresses for out of zone NS names
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120403
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  xfrout                             |  Estimated Difficulty:  3
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''database.cc'''
 - you don't have to explicitly call toText() for getOrigin():
 {{{#!c++
         isc_throw(OutOfZoneFind, name.toText() << " not in " <<
                                  getOrigin().toText());
 }}}
   same for other similar cases.

 '''memory_datasrc.cc'''

 - I'd catch this in findNode().  If I understand it correctly, if at
   the end of this method it's not either EXACTMATCH or PARTIALMATCH
   it's out-of-zone.  I prefer this because we can integrate the
   out-of-zone check in the normal lookup.  Since it's generally better
   to avoid any overhead in the lookups path for the in-memory zone for
   performance, and since the out-of-zone case should be exceptional,
   I'd like to minimize the overhead.
 - regarding the performance, I'd also like to do benchmark tests to
   measure the overhead of this check.

 '''database_unittest.cc'''

 - ZoneFinder::FIND_DEFAULT can be omitted for ZoneFinder::find().
   same for memory_datasrc_unittest.cc, and (I believe) datasrc_test.py.
 - this comment should be revised:
 {{{#!c++
         // Note: find() rejects out-of-zone query name with NXDOMAIN
         // regardless of whether adding the RR succeeded, so this check
         // actually doesn't confirm it.
 }}}
   (there may be other similar cases.  maybe you want to check them by
   searching for "NXDOMAIN" throughout the code).

 '''zone.h'''
 - `OutOfZoneFind` maybe renamed to `OutOfZone`, merged with in-memory
   specific exception of that name, and for an error about out-of-zone
   data in general?  Not a strong opinion, just an idea.

 '''notify_out.py'''

 Code looks okay, but I think we could (should) do some tests:

 - add a test zone that contains out-of-zone NS names (from a quick
   look there's no such case right now, right?).  Confirm that it
   results in exception for the current version of notify_out.py, and
   that it doesn't happen for the revised version.
 - also add a test zone that contains out-of-zone NS names whose zone
   is however also an authoritative zone of the server.  check that
   notify_out now correctly include these addresses too.

 '''Others'''

 - I made a couple of editorial changes.
 - I think we need a changelog for this task.  Since it involves API
   change, we'd need '*'.

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