BIND 10 master, updated. c84dafee22cf9c39e1dde4d15c62246ec25fc09a Changelog for #2436
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 16 12:09:57 UTC 2013
The branch, master has been updated
via c84dafee22cf9c39e1dde4d15c62246ec25fc09a (commit)
via 48d999f1cb59f308f9f30ba2639521d2a5a85baa (commit)
via f8fb3f6f2f6f42888be0cd4e8c9df9502c944081 (commit)
via 29e27da67a1558281a52a5a1c233db3bf84efcb3 (commit)
via 2dbb441195792e760669119ea99e945f66a71a69 (commit)
via 4209615da0b85838cc481a7ad602d7783bfdd78a (commit)
via aff266bfb0c5ea0d8553fb249aacd91f4f414476 (commit)
via 30bb4f426cc06433dbfdbad4d6080ff9ab80ca10 (commit)
via f8dfc94a4a5941def5a4624f7755c0478b8d6375 (commit)
via 5bfa13805e9a9ee2585426ddd8807175eac1a6a3 (commit)
via ba7d30083a692af134c9b4c52b92b7856b34096a (commit)
via e18d03ada70eab16d6f5855b1bf95e78167c4c82 (commit)
via 7174cca8bf02759d3947f2bdc5437e11c362d2a7 (commit)
via 88a70cf0dcc476dd911e812eaca03e61274f81ac (commit)
via 421343439bb3852a4aea4d9f9ff945a4b8b6ac9a (commit)
via 92db29d982892679b7ef097f55975357c5abc759 (commit)
via 8dc51055585e92db23cfb97fd280a10eecf3ff7a (commit)
via 18adb5bce11c191f14aa26e58fca838aeb9d80d0 (commit)
via f4e6ab2a98e80e8832b201a6ed2db716bd1d4ee1 (commit)
via b25df37f16da91f53b2e068fdd5aeb78e349311b (commit)
via dac912f20c6f315f951979906e79c9a47ea172fe (commit)
via 1dc55b22817baca2b9750082dbf408209affbf02 (commit)
from 61a7a86532aac5a0b84cadee0330f0eb20762ba6 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c84dafee22cf9c39e1dde4d15c62246ec25fc09a
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Jan 16 13:09:39 2013 +0100
Changelog for #2436
commit 48d999f1cb59f308f9f30ba2639521d2a5a85baa
Merge: 61a7a86 f8fb3f6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Jan 16 12:34:41 2013 +0100
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.
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 5 +
src/bin/loadzone/b10-loadzone.xml | 16 +--
src/bin/loadzone/loadzone.py.in | 23 ----
src/bin/loadzone/loadzone_messages.mes | 8 --
src/bin/loadzone/tests/loadzone_test.py | 14 +-
src/lib/datasrc/datasrc_messages.mes | 13 ++
src/lib/datasrc/tests/Makefile.am | 2 +
src/lib/datasrc/tests/testdata/checkwarn.zone | 4 +
src/lib/datasrc/tests/testdata/novalidate.zone | 3 +
src/lib/datasrc/tests/zone_loader_unittest.cc | 170 +++++++++++++++++++-----
src/lib/datasrc/zone_loader.cc | 38 ++++++
src/lib/datasrc/zone_loader.h | 19 +++
12 files changed, 223 insertions(+), 92 deletions(-)
create mode 100644 src/lib/datasrc/tests/testdata/checkwarn.zone
create mode 100644 src/lib/datasrc/tests/testdata/novalidate.zone
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index e9e2bd3..58d4e4a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+547. [func]* vorner
+ The b10-loadzone now performs more thorough sanity check on the
+ loaded data. Some of the checks are now fatal and zone failing
+ them will be rejected.
+
546. [func] marcin
DHCP option definitions can be now created using the
Configuration Manager. The option definition specifies
diff --git a/src/bin/loadzone/b10-loadzone.xml b/src/bin/loadzone/b10-loadzone.xml
index 8c55f9b..aa14053 100644
--- a/src/bin/loadzone/b10-loadzone.xml
+++ b/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><!--
diff --git a/src/bin/loadzone/loadzone.py.in b/src/bin/loadzone/loadzone.py.in
index 0cd68db..3ed3b7d 100755
--- a/src/bin/loadzone/loadzone.py.in
+++ b/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)
diff --git a/src/bin/loadzone/loadzone_messages.mes b/src/bin/loadzone/loadzone_messages.mes
index db79269..ca241b3 100644
--- a/src/bin/loadzone/loadzone_messages.mes
+++ b/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
diff --git a/src/bin/loadzone/tests/loadzone_test.py b/src/bin/loadzone/tests/loadzone_test.py
index 4f13c5d..d1ee131 100755
--- a/src/bin/loadzone/tests/loadzone_test.py
+++ b/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.
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 6983459..6ac9db0 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/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
diff --git a/src/lib/datasrc/tests/Makefile.am b/src/lib/datasrc/tests/Makefile.am
index 61858bd..7c61826 100644
--- a/src/lib/datasrc/tests/Makefile.am
+++ b/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
diff --git a/src/lib/datasrc/tests/testdata/checkwarn.zone b/src/lib/datasrc/tests/testdata/checkwarn.zone
new file mode 100644
index 0000000..e985085
--- /dev/null
+++ b/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
diff --git a/src/lib/datasrc/tests/testdata/novalidate.zone b/src/lib/datasrc/tests/testdata/novalidate.zone
new file mode 100644
index 0000000..4bbf12a
--- /dev/null
+++ b/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
diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc
index 943ea2f..f0ad814 100644
--- a/src/lib/datasrc/tests/zone_loader_unittest.cc
+++ b/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());
+
+}
+
}
diff --git a/src/lib/datasrc/zone_loader.cc b/src/lib/datasrc/zone_loader.cc
index 01f216e..098b47a 100644
--- a/src/lib/datasrc/zone_loader.cc
+++ b/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_);
diff --git a/src/lib/datasrc/zone_loader.h b/src/lib/datasrc/zone_loader.h
index 2946116..c7e5ccd 100644
--- a/src/lib/datasrc/zone_loader.h
+++ b/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
More information about the bind10-changes
mailing list