BIND 10 #1414: Zonemgr/secondary_zones[]/class required

[BIND 10 Development]

#1414: Zonemgr/secondary_zones[]/class required
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20111220
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  configuration                      |           Sub-Project:  Core
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  4
Feature Depending on Ticket:  none   |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:10 jelte]:

 > >   xfrout does this, btw.  See _get_transfer_acl().  It can be deferred
 > >   to a separate ticket, though.  I mentioned it simply because I
 > >   noticed it while I reading the patch.
 > >
 >
 > Might as well fix it now, added to-and-from conversion, with exception
 catch to provide a slightly better error (and to convert to one
 exception). Added a few tests as well (but not extensively for every
 possible bad dname)

 Looks okay, but to be even more perfect, I guess we need to do:
 - perform syntax check on the class, too (reject "BADCLASS", etc)
 - confirm whether the comparison is case-insensitive ('example.COM/in'
   should be treated as 'example.com./IN'.)

 BTW, I believe checking only one bad name is okay at this level.  I
 also think it's okay to use the catch-all Exception (ideally we'd like
 to introduce module level exception and/or all isc-exceptions, but
 currently we don't have them, so Exception would be the best
 compromise)

 > I applied your patch as-is. It did require one extra addition; it
 introduces SPECFILE_LOCATION, which is fine, but then we must also set
 B10_FROM_BUILD in the Makefile test call, so that it calculates the
 correct path for it. Added that too.

 Ah, right, good catch (about B10_FROM_BUILD).

 > I have no idea whether it was intentional, but what it did was add to
 one-letter names with one-letter classes, and then making sure it didn't
 set the original one. Yeah that's weird, and most probably wrong.
 >
 > Again, might as well fix it now; I've changed the test; what it now does
 is set the 'normal' name example.net, but class CH (and use the weirdly
 named but conveniently defined ZONE_NAME_CLASS1_CH for that), then checks
 whether the zonemgr did not make IN of it.
 >
 > Whatever the test was supposed to do, it probably changed :p
 >
 > Removed the duplicate test after 'skip soa'. No idea what that was for.

 Okay.

 > > - I believe we need a changelog entry for this.
 >
 > [bug]   jelte
 > Fixed a bug where adding Zonemgr/secondary_zones without explicitely
 setting the class value of the added zone resulted in a cryptic error in
 bindctl ("Error: class"). It will now correctly default to IN if not set.
 This also adds better checks on the name value, and a better error when it
 is bad.

 This looks fine.

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