Merge #2436

Let isc::datasrc::ZoneLoader validate the data before committing by
isc::dns::checkZone. Also, remove the in-house validation from b10-loadzone,
since it's only a subset of the checkZone, which is now performed.

Import a merge fixup from mukund.

src/bin/loadzone/b10-loadzone.xml

@@ -224,21 +224,7 @@
   <refsect1>
     <title>BUGS</title>
     <para>
-      As of the initial implementation, the underlying library that
-      this tool uses does not fully validate the loaded zone; for
-      example, loading will succeed even if it doesn't have the SOA or
-      NS record at its origin name.  Such checks will be implemented
-      in a near future version, but until then, the
-      <command>b10-loadzone</command> performs the existence of the
-      SOA and NS records by itself.  However, <command>b10-loadzone</command>
-      only warns about it, and does not cancel the load itself.
-      If this warning message is produced, it's the user's
-      responsibility to fix the errors and reload it.  When the
-      library is updated with the post load checks, it will be more
-      sophisticated and the such zone won't be successfully loaded.
-    </para>
-    <para>
-      There are some other issues noted in the DESCRIPTION section.
+      There are some issues noted in the DESCRIPTION section.
     </para>
   </refsect1>
 </refentry><!--

src/bin/loadzone/loadzone.py.in

@@ -259,28 +259,6 @@ class LoadZoneRunner:
                              self._zone_class)
             raise LoadFailure(str(ex))
 
-    def _post_load_checks(self):
-        '''Perform minimal validity checks on the loaded zone.
-
-        We do this ourselves because the underlying library currently
-        doesn't do any checks.  Once the library support post-load validation
-        this check should be removed.
-
-        '''
-        datasrc_client = DataSourceClient(self._datasrc_type,
-                                          self._datasrc_config)
-        _, finder = datasrc_client.find_zone(self._zone_name) # should succeed
-        result = finder.find(self._zone_name, RRType.SOA())[0]
-        if result is not finder.SUCCESS:
-            self._post_load_warning('zone has no SOA')
-        result = finder.find(self._zone_name, RRType.NS())[0]
-        if result is not finder.SUCCESS:
-            self._post_load_warning('zone has no NS')
-
-    def _post_load_warning(self, msg):
-        logger.warn(LOADZONE_POSTLOAD_ISSUE, self._zone_name,
-                    self._zone_class, msg)
-
     def _set_signal_handlers(self):
         signal.signal(signal.SIGINT, self._interrupt_handler)
         signal.signal(signal.SIGTERM, self._interrupt_handler)
@@ -298,7 +276,6 @@ class LoadZoneRunner:
             total_elapsed_txt = "%.2f" % (time.time() - self.__start_time)
             logger.info(LOADZONE_DONE, self.__loaded_rrs, self._zone_name,
                         self._zone_class, total_elapsed_txt)
-            self._post_load_checks()
             return 0
         except BadArgument as ex:
             logger.error(LOADZONE_ARGUMENT_ERROR, ex)

src/bin/loadzone/loadzone_messages.mes

@@ -46,14 +46,6 @@ in the zone file.  When this happens, the RRs loaded so far are
 effectively deleted from the zone, and the old version (if exists)
 will still remain valid for operations.
 
-% LOADZONE_POSTLOAD_ISSUE New version of zone %1/%2 has an issue: %3
-b10-loadzone detected a problem after a successful load of zone:
-either or both of SOA and NS records are missing at the zone origin.
-In the current implementation the load will not be canceled for such
-problems.  The operator will need to fix the issues and reload the
-zone; otherwise applications (such as b10-auth) that use this data
-source will not work as expected.
-
 % LOADZONE_SQLITE3_USING_DEFAULT_CONFIG Using default configuration with SQLite3 DB file %1
 The SQLite3 data source is specified as the data source type without a
 data source configuration.  b10-loadzone uses the default

src/bin/loadzone/tests/loadzone_test.py

@@ -237,7 +237,7 @@ class TestLoadZoneRunner(unittest.TestCase):
         self.__check_zone_soa(None, zone_name=Name('example.com'))
 
     def __common_post_load_setup(self, zone_file):
-        '''Common setup procedure for post load tests.'''
+        '''Common setup procedure for post load tests which should fail.'''
         # replace the LoadZoneRunner's original _post_load_warning() for
         # inspection
         self.__warnings = []
@@ -248,26 +248,18 @@ class TestLoadZoneRunner(unittest.TestCase):
         self.__common_load_setup()
         self.__runner._zone_file = zone_file
         self.__check_zone_soa(ORIG_SOA_TXT)
-        self.__runner._do_load()
-        self.__runner._post_load_checks()
+        # It fails because there's problem with the zone data
+        self.assertRaises(LoadFailure, self.__runner._do_load)
 
     def test_load_post_check_fail_soa(self):
         '''Load succeeds but warns about missing SOA, should cause warn'''
-        self.__common_load_setup()
         self.__common_post_load_setup(LOCAL_TESTDATA_PATH +
                                       '/example-nosoa.org.zone')
-        self.__check_zone_soa(False)
-        self.assertEqual(1, len(self.__warnings))
-        self.assertEqual('zone has no SOA', self.__warnings[0])
 
     def test_load_post_check_fail_ns(self):
         '''Load succeeds but warns about missing NS, should cause warn'''
-        self.__common_load_setup()
         self.__common_post_load_setup(LOCAL_TESTDATA_PATH +
                                       '/example-nons.org.zone')
-        self.__check_zone_soa(NEW_SOA_TXT)
-        self.assertEqual(1, len(self.__warnings))
-        self.assertEqual('zone has no NS', self.__warnings[0])
 
     def __interrupt_progress(self, loaded_rrs):
         '''A helper emulating a signal in the middle of loading.

src/lib/datasrc/datasrc_messages.mes

@@ -70,6 +70,19 @@ The maximum allowed number of items of the hotspot cache is set to the given
 number. If there are too many, some of them will be dropped. The size of 0
 means no limit.
 
+% DATASRC_CHECK_ERROR post-load check of zone %1/%2 failed: %3
+The zone was loaded into the data source successfully, but the content fails
+basic sanity checks. See the message if you want to know what exactly is wrong
+with the data. The data can not be used and previous version, if any, will be
+preserved.
+
+% DATASRC_CHECK_WARNING %1/%2: %3
+The zone was loaded into the data source successfully, but there's some problem
+with the content. The problem does not stop the new version from being used
+(though there may be other problems that do, see DATASRC_CHECK_ERROR),
+but it should still be checked and fixed. See the message to know what exactly
+is wrong with the data.
+
 % DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED %1 doesn't support DNSSEC when asked for NSEC data covering %2
 The datasource tried to provide an NSEC proof that the named domain does not
 exist, but the database backend doesn't support DNSSEC. No proof is included

src/lib/datasrc/tests/Makefile.am

@@ -118,3 +118,5 @@ EXTRA_DIST += testdata/new_minor_schema.sqlite3
 EXTRA_DIST += testdata/newschema.sqlite3
 EXTRA_DIST += testdata/oldschema.sqlite3
 EXTRA_DIST += testdata/static.zone
+EXTRA_DIST += testdata/novalidate.zone
+EXTRA_DIST += testdata/checkwarn.zone

src/lib/datasrc/tests/testdata/checkwarn.zone

@@ -0,0 +1,4 @@
+example.org.			86400	IN	SOA	a.root-servers.net. nstld.verisign-grs.com. 2010030802 1800 900 604800 86400
+example.org.            86400   IN  NS  ns.example.org.
+; Missing the address for the nameserver. This should generate a warning, but not error.
+www.example.org.        3600    IN  A   192.0.2.1

src/lib/datasrc/tests/testdata/novalidate.zone

@@ -0,0 +1,3 @@
+example.org.			86400	IN	SOA	a.root-servers.net. nstld.verisign-grs.com. 2010030802 1800 900 604800 86400
+; Missing the NS here, will generate an error in post-load check of the zone.
+www.example.org.        3600    IN  A   192.0.2.1

src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -27,6 +27,8 @@
 #include <gtest/gtest.h>
 
 #include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
+#include <boost/foreach.hpp>
 #include <string>
 #include <vector>
 
@@ -34,6 +36,7 @@ using isc::dns::RRClass;
 using isc::dns::Name;
 using isc::dns::RRType;
 using isc::dns::ConstRRsetPtr;
+using isc::dns::RRsetPtr;
 using std::string;
 using std::vector;
 using boost::shared_ptr;
@@ -64,10 +67,9 @@ public:
     // since many client methods are const, but we still want to know they
     // were called.
     mutable vector<Name> provided_updaters_;
-    // We store string representations of the RRsets. This is simpler than
-    // copying them and we can't really put them into shared pointers, because
-    // we get them as references.
-    vector<string> rrsets_;
+    vector<RRsetPtr> rrsets_;
+    // List of rrsets as texts, for easier manipulation
+    vector<string> rrset_texts_;
     bool commit_called_;
     // If set to true, getUpdater returns NULL
     bool missing_zone_;
@@ -75,6 +77,26 @@ public:
     RRClass rrclass_;
 };
 
+// Test implementation of RRsetCollectionBase.
+class TestRRsetCollection : public isc::datasrc::RRsetCollectionBase {
+public:
+    TestRRsetCollection(ZoneUpdater& updater,
+                        const isc::dns::RRClass& rrclass) :
+        isc::datasrc::RRsetCollectionBase(updater, rrclass)
+    {}
+
+    virtual ~TestRRsetCollection() {}
+
+protected:
+    virtual RRsetCollectionBase::IterPtr getBeginning() {
+        isc_throw(isc::NotImplemented, "This method is not implemented.");
+    }
+
+    virtual RRsetCollectionBase::IterPtr getEnd() {
+        isc_throw(isc::NotImplemented, "This method is not implemented.");
+    }
+};
+
 // The updater isn't really correct according to the API. For example,
 // the whole client can be committed only once in its lifetime. The
 // updaters would influence each other if there were more. But we
@@ -82,21 +104,36 @@ public:
 // and this way, it is much simpler.
 class Updater : public ZoneUpdater {
 public:
-    Updater(MockClient* client) :
+    Updater(MockClient* client, const Name& name) :
         client_(client),
-        finder_(client_->rrclass_)
+        finder_(client_->rrclass_, name, client_->rrsets_)
     {}
     virtual ZoneFinder& getFinder() {
         return (finder_);
     }
     virtual isc::datasrc::RRsetCollectionBase& getRRsetCollection() {
-        isc_throw(isc::NotImplemented, "Method not used in tests");
+        if (!rrset_collection_) {
+            rrset_collection_.reset(new TestRRsetCollection(*this,
+                                                            client_->rrclass_));
+        }
+        return (*rrset_collection_);
     }
     virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
         if (client_->commit_called_) {
             isc_throw(DataSourceError, "Add after commit");
         }
-        client_->rrsets_.push_back(rrset.toText());
+        // We need to copy the RRset. We don't do it properly (we omit the
+        // signature, for example), because we don't need to.
+        RRsetPtr new_rrset(new isc::dns::BasicRRset(rrset.getName(),
+                                                    rrset.getClass(),
+                                                    rrset.getType(),
+                                                    rrset.getTTL()));
+        for (isc::dns::RdataIteratorPtr i(rrset.getRdataIterator());
+             !i->isLast(); i->next()) {
+            new_rrset->addRdata(i->getCurrent());
+        }
+        client_->rrsets_.push_back(new_rrset);
+        client_->rrset_texts_.push_back(rrset.toText());
     }
     virtual void deleteRRset(const isc::dns::AbstractRRset&) {
         isc_throw(isc::NotImplemented, "Method not used in tests");
@@ -106,21 +143,37 @@ public:
     }
 private:
     MockClient* client_;
+    boost::scoped_ptr<TestRRsetCollection> rrset_collection_;
     class Finder : public ZoneFinder {
     public:
-        Finder(const RRClass& rrclass) :
-            class_(rrclass)
+        Finder(const RRClass& rrclass, const Name& name,
+               const vector<RRsetPtr>& rrsets) :
+            class_(rrclass),
+            name_(name),
+            rrsets_(rrsets)
         {}
         virtual RRClass getClass() const {
             return (class_);
         }
         virtual Name getOrigin() const {
-            isc_throw(isc::NotImplemented, "Method not used in tests");
+            return (name_);
         }
-        virtual shared_ptr<Context> find(const Name&, const RRType&,
-                                         const FindOptions)
+        virtual shared_ptr<Context> find(const Name& name, const RRType& type,
+                                         const FindOptions options)
         {
-            isc_throw(isc::NotImplemented, "Method not used in tests");
+            // The method is not completely correct. It ignores many special
+            // cases and also the options except for the result. But this is
+            // enough for the tests.  We care only about exact match here.
+            BOOST_FOREACH(const RRsetPtr& rrset, rrsets_) {
+                if (rrset->getName() == name && rrset->getType() == type) {
+                    return (shared_ptr<Context>(
+                        new GenericContext(*this, options,
+                                           ResultContext(SUCCESS, rrset))));
+                }
+            }
+            return (shared_ptr<Context>(
+                new GenericContext(*this, options,
+                                   ResultContext(NXRRSET, ConstRRsetPtr()))));
         }
         virtual shared_ptr<Context> findAll(const Name&,
                                             vector<ConstRRsetPtr>&,
@@ -133,6 +186,8 @@ private:
         }
     private:
         const RRClass class_;
+        const Name name_;
+        const vector<RRsetPtr>& rrsets_;
     } finder_;
 };
 
@@ -147,7 +202,7 @@ MockClient::getUpdater(const Name& name, bool replace, bool journaling) const {
     // const_cast is bad. But the const on getUpdater seems wrong in the first
     // place, since updater will be modifying the data there. And the updater
     // wants to store data into the client so we can examine it later.
-    return (ZoneUpdaterPtr(new Updater(const_cast<MockClient*>(this))));
+    return (ZoneUpdaterPtr(new Updater(const_cast<MockClient*>(this), name)));
 }
 
 class ZoneLoaderTest : public ::testing::Test {
@@ -160,10 +215,12 @@ protected:
     {}
     void prepareSource(const Name& zone, const char* filename) {
         // TODO:
-        // Currently, load uses an urelated implementation. In the long term,
-        // the method will probably be deprecated. At that time, we should
-        // probably prepare the data in some other way (using sqlite3 or
-        // something). This is simpler for now.
+        // Currently, source_client_ is of InMemoryClient and its load()
+        // uses a different code than the ZoneLoader (so we can cross-check
+        // the implementations). Currently, the load() doesn't perform any
+        // post-load checks. It will change in #2499, at which point the
+        // loading may start failing depending on details of the test data. We
+        // should prepare the data by some different method then.
         source_client_.load(zone, string(TEST_DATA_DIR) + "/" + filename);
     }
 private:
@@ -198,12 +255,12 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
     // Ensure known order.
-    std::sort(destination_client_.rrsets_.begin(),
-              destination_client_.rrsets_.end());
+    std::sort(destination_client_.rrset_texts_.begin(),
+              destination_client_.rrset_texts_.end());
     EXPECT_EQ(". 518400 IN NS a.root-servers.net.\n",
-              destination_client_.rrsets_.front());
+              destination_client_.rrset_texts_.front());
     EXPECT_EQ("m.root-servers.net. 3600000 IN AAAA 2001:dc3::35\n",
-              destination_client_.rrsets_.back());
+              destination_client_.rrset_texts_.back());
 
     // It isn't possible to try again now
     EXPECT_THROW(loader.load(), isc::InvalidOperation);
@@ -252,18 +309,18 @@ TEST_F(ZoneLoaderTest, copySigned) {
     EXPECT_EQ(14, destination_client_.rrsets_.size());
     EXPECT_TRUE(destination_client_.commit_called_);
     // Same trick with sorting to know where they are
-    std::sort(destination_client_.rrsets_.begin(),
-              destination_client_.rrsets_.end());
+    std::sort(destination_client_.rrset_texts_.begin(),
+              destination_client_.rrset_texts_.end());
     // Due to the R at the beginning, this one should be last
     EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN NSEC3 "
               "1 0 10 AABBCCDD RKOF8QMFRB5F2V9EJHFBVB2JPVSA0DJD A RRSIG\n",
-              destination_client_.rrsets_[0]);
+              destination_client_.rrset_texts_[0]);
     EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN RRSIG "
               "NSEC3 7 3 1200 20120301040838 20120131040838 19562 example.org."
               " EdwMeepLf//lV+KpCAN+213Scv1rrZyj4i2OwoCP4XxxS3CWGSuvYuKOyfZc8w"
               "KRcrD/4YG6nZVXE0s5O8NahjBJmDIyVt4WkfZ6QthxGg8ggLVvcD3dFksPyiKHf"
               "/WrTOZPSsxvN5m/i1Ey6+YWS01Gf3WDCMWDauC7Nmh3CTM=\n",
-              destination_client_.rrsets_[1]);
+              destination_client_.rrset_texts_[1]);
 }
 
 // If the destination zone does not exist, it throws
@@ -304,12 +361,12 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
     // Ensure known order.
-    std::sort(destination_client_.rrsets_.begin(),
-              destination_client_.rrsets_.end());
+    std::sort(destination_client_.rrset_texts_.begin(),
+              destination_client_.rrset_texts_.end());
     EXPECT_EQ(". 518400 IN NS a.root-servers.net.\n",
-              destination_client_.rrsets_.front());
+              destination_client_.rrset_texts_.front());
     EXPECT_EQ("m.root-servers.net. 3600000 IN AAAA 2001:dc3::35\n",
-              destination_client_.rrsets_.back());
+              destination_client_.rrset_texts_.back());
 
     // It isn't possible to try again now
     EXPECT_THROW(loader.load(), isc::InvalidOperation);
@@ -362,18 +419,18 @@ TEST_F(ZoneLoaderTest, loadSigned) {
     EXPECT_EQ(14, destination_client_.rrsets_.size());
     EXPECT_TRUE(destination_client_.commit_called_);
     // Same trick with sorting to know where they are
-    std::sort(destination_client_.rrsets_.begin(),
-              destination_client_.rrsets_.end());
+    std::sort(destination_client_.rrset_texts_.begin(),
+              destination_client_.rrset_texts_.end());
     // Due to the R at the beginning, this one should be last
     EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN NSEC3 "
               "1 0 10 AABBCCDD RKOF8QMFRB5F2V9EJHFBVB2JPVSA0DJD A RRSIG\n",
-              destination_client_.rrsets_[0]);
+              destination_client_.rrset_texts_[0]);
     EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN RRSIG "
               "NSEC3 7 3 1200 20120301040838 20120131040838 19562 example.org."
               " EdwMeepLf//lV+KpCAN+213Scv1rrZyj4i2OwoCP4XxxS3CWGSuvYuKOyfZc8w"
               "KRcrD/4YG6nZVXE0s5O8NahjBJmDIyVt4WkfZ6QthxGg8ggLVvcD3dFksPyiKHf"
               "/WrTOZPSsxvN5m/i1Ey6+YWS01Gf3WDCMWDauC7Nmh3CTM=\n",
-              destination_client_.rrsets_[1]);
+              destination_client_.rrset_texts_[1]);
 }
 
 // Test it throws when there's no such file
@@ -395,4 +452,47 @@ TEST_F(ZoneLoaderTest, loadSyntaxError) {
     EXPECT_FALSE(destination_client_.commit_called_);
 }
 
+// Test there's validation of the data in the zone loader.
+TEST_F(ZoneLoaderTest, loadCheck) {
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      TEST_DATA_DIR "/novalidate.zone");
+    EXPECT_THROW(loader.loadIncremental(10), ZoneContentError);
+    // The messages go to the log. We don't have an easy way to examine them.
+    EXPECT_FALSE(destination_client_.commit_called_);
+}
+
+// The same test, but for copying from other data source
+TEST_F(ZoneLoaderTest, copyCheck) {
+    prepareSource(Name("example.org"), "novalidate.zone");
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      source_client_);
+
+    EXPECT_THROW(loader.loadIncremental(10), ZoneContentError);
+    // The messages go to the log. We don't have an easy way to examine them.
+    EXPECT_FALSE(destination_client_.commit_called_);
+}
+
+// Check a warning doesn't disrupt the loading of the zone
+TEST_F(ZoneLoaderTest, loadCheckWarn) {
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      TEST_DATA_DIR "/checkwarn.zone");
+    EXPECT_TRUE(loader.loadIncremental(10));
+    // The messages go to the log. We don't have an easy way to examine them.
+    // But the zone was committed and contains all 3 RRs
+    EXPECT_TRUE(destination_client_.commit_called_);
+    EXPECT_EQ(3, destination_client_.rrsets_.size());
+}
+
+TEST_F(ZoneLoaderTest, copyCheckWarn) {
+    prepareSource(Name("example.org"), "checkwarn.zone");
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      source_client_);
+    EXPECT_TRUE(loader.loadIncremental(10));
+    // The messages go to the log. We don't have an easy way to examine them.
+    // But the zone was committed and contains all 3 RRs
+    EXPECT_TRUE(destination_client_.commit_called_);
+    EXPECT_EQ(3, destination_client_.rrsets_.size());
+
+}
+
 }

src/lib/datasrc/zone_loader.cc

@@ -19,8 +19,17 @@
 #include <datasrc/data_source.h>
 #include <datasrc/iterator.h>
 #include <datasrc/zone.h>
+#include <datasrc/logger.h>
+#include <datasrc/rrset_collection_base.h>
 
 #include <dns/rrset.h>
+#include <dns/zone_checker.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <boost/bind.hpp>
+
+#include <string>
 
 using isc::dns::Name;
 using isc::dns::ConstRRsetPtr;
@@ -99,6 +108,22 @@ copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
     return (false); // Not yet, there may be more
 }
 
+void
+logWarning(const dns::Name* zone_name, const dns::RRClass* rrclass,
+           const std::string& reason)
+{
+    LOG_WARN(logger, DATASRC_CHECK_WARNING).arg(*zone_name).arg(*rrclass).
+        arg(reason);
+}
+
+void
+logError(const dns::Name* zone_name, const dns::RRClass* rrclass,
+         const std::string& reason)
+{
+    LOG_ERROR(logger, DATASRC_CHECK_ERROR).arg(*zone_name).arg(*rrclass).
+        arg(reason);
+}
+
 } // end unnamed namespace
 
 bool
@@ -123,6 +148,19 @@ ZoneLoader::loadIncremental(size_t limit) {
     }
 
     if (complete_) {
+        // Everything is loaded. Perform some basic sanity checks on the zone.
+        RRsetCollectionBase& collection = updater_->getRRsetCollection();
+        const dns::Name& zone_name(updater_->getFinder().getOrigin());
+        const dns::RRClass& zone_class(updater_->getFinder().getClass());
+        const dns::ZoneCheckerCallbacks
+            callbacks(boost::bind(&logError, &zone_name, &zone_class, _1),
+                      boost::bind(&logWarning, &zone_name, &zone_class, _1));
+        if (!dns::checkZone(zone_name, zone_class, collection, callbacks)) {
+            // The post-load check failed.
+            loaded_ok_ = false;
+            isc_throw(ZoneContentError, "Errors found when validating zone " <<
+                      zone_name << "/" << zone_class);
+        }
         updater_->commit();
     }
     return (complete_);

src/lib/datasrc/zone_loader.h

@@ -48,6 +48,17 @@ public:
     {}
 };
 
+/// \brief Exception thrown when the zone doesn't pass post-load check.
+///
+/// This is thrown by the ZoneLoader when the zone is loaded, but it
+/// doesn't pass basic sanity checks.
+class ZoneContentError : public DataSourceError {
+public:
+    ZoneContentError(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what)
+    {}
+};
+
 /// \brief Class to load data into a data source client.
 ///
 /// This is a small wrapper class that is able to load data into a data source.
@@ -107,6 +118,7 @@ public:
     /// \throw DataSourceError in case some error (possibly low-level) happens.
     /// \throw MasterFileError when the master_file is badly formatted or some
     ///     similar problem is found when loading the master file.
+    /// \throw ZoneContentError when the zone doesn't pass sanity check.
     void load() {
         while (!loadIncremental(1000)) { // 1000 is arbitrary largish number
             // Body intentionally left blank.
@@ -123,6 +135,12 @@ public:
     /// pauses in the loading for some purposes (for example reporting
     /// progress).
     ///
+    /// After the last RR is loaded, a sanity check of the zone is performed by
+    /// isc::dns::validateZone. It reports errors and warnings by logging them
+    /// directly. If there are any errors, a ZoneContentError exception is
+    /// thrown and the load is aborted (preserving the old version of zone, if
+    /// any).
+    ///
     /// \param limit The maximum allowed number of RRs to be loaded during this
     ///     call.
     /// \return True in case the loading is completed, false if there's more
@@ -133,6 +151,7 @@ public:
     /// \throw DataSourceError in case some error (possibly low-level) happens.
     /// \throw MasterFileError when the master_file is badly formatted or some
     ///     similar problem is found when loading the master file.
+    /// \throw ZoneContentError when the zone doesn't pass sanity check.
     /// \note If the limit is exactly the number of RRs available to be loaded,
     ///     the method still returns false and true'll be returned on the next
     ///     call (which will load 0 RRs). This is because the end of iterator or