BIND 10 #2911: don't fallback from ixfr to axfr due to data source error

[BIND 10 Development]

#2911: don't fallback from ixfr to axfr due to data source error
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  xfrin         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130611
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:16 jinmei]:
 > As a general matter, I agree.  For this particular ticket, one
 > possible issue to discuss is whether we ignore the obsolete use_ixfr
 > item possibly with a warning log message or we reject it.  The current
 > branch takes the latter approach (as I thought we agreed on jabber or
 > ML), but without providing offline config update mechanism it means
 > the user will need to edit the json file by hand.  On the other hand,
 > it's more likely that users won't notice the migration if we simply
 > ignore the deprecated item (and we know users don't read log
 > messages).

 I think rejecting it is fine for now. Most people probably didn't set the
 flag and I doubt there are many real-world instances either.

 I'll bring it up on the mailing list to discuss the plans.

 > I don't have a strong opinion, but this sounds like a reasonable
 > suggestion.

 OK. But, thinking about it again, we probably want it as part of the
 configuration overwrite.

 > I didn't find a tab in xfrin.spec introduced in the branch.  There are
 > some tabs already existing in the file, but for completeness I removed
 > them too.

 Sorry, that one is probably OK. I just copy pasted one more file as I
 didn't notice it was different one already.

 > And I don't understand why this part is cited: is it a tab issue or
 > something else?
 >
 > > {{{#!python
 > >     def test_config_handler_use_ixfr(self):
 > >         # use_ixfr was deprecated and explicitly rejected for now.
 > >         config = { 'zones': [
 > >                    { 'name': 'test.example.',
 > >                     'master_addr': '192.0.2.1',
 > >                     'master_port': 53,
 > >                      'use_ixfr': True
 > >                    }
 > >                  ]}
 > >         self.assertEqual(self.xfr.config_handler(config)['result'][0],
 1)
 > > }}}
 >
 > There's not a tab, but the indentation is certainly incorrect.  I
 > guess it's a simple mistyping.  Fixed.

 It may have not been a tab, tabs just show in the diff as differently
 indented (because of the `+` at the beginning). In such case it was just
 bad indentation.

 However, I don't see any fix for that in the branch and it seems still
 broken in my copy.

 > > I see no threads mentioned in
 `fb3b94fa5eec711ff84ef281370eb2bfa95a0b5a` (except the commit message).
 Did you mean methods?
 >
 > Ah, right.  Should I update the commit using rebase?  (Since the
 > branch is already shared I've not touched the history at the moment).

 Yes, unless it's a lot of work, as it's only a minor detail.

 > > Does the current way of creating zones still have the problem of
 leaving an empty zone upon failed attempt?
 >
 > I don't understand the question...is this about config_handler()
 > replacing `_zones`?

 No. I mean the fact that we create a new zone if there wasn't one before
 in the database. And it's not inside the transaction, so if the transfer
 fails for some reason, the data is not committed, but the new zone is
 still left in the database, empty.

 > Regarding where, maybe you're referring to an intermediate point of
 > the branch?  In the latest version the check is the following part of
 > `_get_zone_soa`:
 > {{{#!python
 >     if soa_rrset.get_rdata_count() != 1:
 >         logger.warn(XFRIN_ZONE_MULTIPLE_SOA,
 >                     format_zone_str(zone_name, zone_class),
 >                     soa_rrset.get_rdata_count())
 >         return None
 > }}}

 Possibly, yes, I did read it by separate commits.

 > As for what we should do in this case may be debatable.  But I don't
 > think it a bad idea to continue the transfer using AXFR in such a case
 > (which is what the current branch should do).

 Hmm. What will happen if the master copy is broken this way? Will it
 reject this on xfr-in, effectively making the transfer fail and retry soon
 (and infinitely so)?

 To be clear, I don't have a proposal what to do. But that situation with
 multiple SOA seems more broken than it looks at the first sigh.

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