BIND 10 master, updated. 06069bf9520dc24133605c0ef19ffdeb3414d669 [master] Merge branch 'trac2463'
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Nov 13 12:36:28 UTC 2012
The branch, master has been updated
via 06069bf9520dc24133605c0ef19ffdeb3414d669 (commit)
via 436eae016dfa024f786c88b7100a9ba8a95fd8b7 (commit)
via 8fd47c03a3100c41e34181b95465aa98a77538d1 (commit)
via 43d61adcf5a876a3a5c76ba78ea4c980d7e70704 (commit)
via 248da8cb1f68c93e411d3b1b22479acaaa255c15 (commit)
via 2a9fd6c61e6b9c3d5c8bc2dba64f4196a2bf953f (commit)
via 03e57865c07485b2e8165ae353207c2c6400297c (commit)
from 42c59bc235a58ce57c93168e35a297d944ce2a33 (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 06069bf9520dc24133605c0ef19ffdeb3414d669
Merge: 42c59bc 436eae0
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Nov 13 13:02:17 2012 +0100
[master] Merge branch 'trac2463'
Conflicts:
src/bin/dhcp6/tests/config_parser_unittest.cc
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/config_parser.cc | 211 ++++++++++++++++---------
src/bin/dhcp6/tests/config_parser_unittest.cc | 49 +++++-
src/lib/dhcp/subnet.h | 6 +
3 files changed, 186 insertions(+), 80 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index e330e19..da393ac 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -41,13 +41,13 @@ using namespace isc::asiolink;
namespace isc {
namespace dhcp {
-/// @brief auxiliary type used for storing element name and its parser
+/// @brief an auxiliary type used for storing an element name and its parser
typedef pair<string, ConstElementPtr> ConfigPair;
/// @brief a factory method that will create a parser for a given element name
typedef DhcpConfigParser* ParserFactory(const std::string& config_id);
-/// @brief a collection of factories that creates parsers for specified element names
+/// @brief a collection of factories that create parsers for specified element names
typedef std::map<std::string, ParserFactory*> FactoryMap;
/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
@@ -58,12 +58,14 @@ typedef std::map<string, string> StringStorage;
/// @brief a collection of pools
///
-/// That type is used as intermediate storage, when pools are parsed, but there is
+/// This type is used as intermediate storage, when pools are parsed, but there is
/// no subnet object created yet to store them.
typedef std::vector<Pool6Ptr> PoolStorage;
-/// @brief Collection of options.
-typedef std::vector<OptionPtr> OptionStorage;
+/// @brief Collection of option descriptors. This container allows searching for
+/// options using the option code or persistency flag. This is useful when merging
+/// existing options with newly configured options.
+typedef Subnet::OptionContainer OptionStorage;
/// @brief Global uint32 parameters that will be used as defaults.
Uint32Storage uint32_defaults;
@@ -76,9 +78,9 @@ OptionStorage option_defaults;
/// @brief a dummy configuration parser
///
-/// It is a debugging parser. It does not configure anything,
+/// This is a debugging parser. It does not configure anything,
/// will accept any configuration and will just print it out
-/// on commit. Useful for debugging existing configurations and
+/// on commit. Useful for debugging existing configurations and
/// adding new ones.
class DebugParser : public DhcpConfigParser {
public:
@@ -105,7 +107,7 @@ public:
/// @brief pretends to apply the configuration
///
- /// This is a method required by base class. It pretends to apply the
+ /// This is a method required by the base class. It pretends to apply the
/// configuration, but in fact it only prints the parameter out.
///
/// See \ref DhcpConfigParser class for details.
@@ -169,7 +171,7 @@ public:
// Parsing the value as a int64 value allows to
// check if the provided value is within the range
// of uint32_t (is not negative or greater than
- // maximal uint32_t value.
+ // maximal uint32_t value).
int64value = boost::lexical_cast<int64_t>(value->str());
} catch (const boost::bad_lexical_cast&) {
parse_error = true;
@@ -193,19 +195,20 @@ public:
<< " as unsigned 32-bit integer.");
}
- storage_->insert(pair<string, uint32_t>(param_name_, value_));
+ // If a given parameter already exists in the storage we override
+ // its value. If it doesn't we insert a new element.
+ (*storage_)[param_name_] = value_;
}
/// @brief does nothing
///
- /// This method is required for all parser. The value itself
+ /// This method is required for all parsers. The value itself
/// is not commited anywhere. Higher level parsers are expected to
/// use values stored in the storage, e.g. renew-timer for a given
/// subnet is stored in subnet-specific storage. It is not commited
/// here, but is rather used by \ref Subnet6Parser when constructing
/// the subnet.
- virtual void commit() {
- }
+ virtual void commit() { }
/// @brief factory that constructs Uint32Parser objects
///
@@ -237,9 +240,9 @@ protected:
/// @brief Configuration parser for string parameters
///
/// This class is a generic parser that is able to handle any string
-/// parameter. By default it stores the value in external global container
+/// parameter. By default it stores the value in an external global container
/// (string_defaults). If used in smaller scopes (e.g. to parse parameters
-/// in subnet config), it can be pointed to a different storage, using
+/// in subnet config), it can be pointed to a different storage, using the
/// setStorage() method. This class follows the parser interface, laid out
/// in its base class, \ref DhcpConfigParser.
///
@@ -256,26 +259,27 @@ public:
/// @brief parses parameter value
///
- /// Parses configuration entry and stored it in storage. See
+ /// Parses configuration entry and stores it in storage. See
/// \ref setStorage() for details.
///
/// @param value pointer to the content of parsed values
virtual void build(ConstElementPtr value) {
value_ = value->str();
boost::erase_all(value_, "\"");
- storage_->insert(pair<string, string>(param_name_, value_));
+ // If a given parameter already exists in the storage we override
+ // its value. If it doesn't we insert a new element.
+ (*storage_)[param_name_] = value_;
}
/// @brief does nothing
///
- /// This method is required for all parser. The value itself
+ /// This method is required for all parsers. The value itself
/// is not commited anywhere. Higher level parsers are expected to
/// use values stored in the storage, e.g. renew-timer for a given
/// subnet is stored in subnet-specific storage. It is not commited
/// here, but is rather used by its parent parser when constructing
/// an object, e.g. the subnet.
- virtual void commit() {
- }
+ virtual void commit() { }
/// @brief factory that constructs StringParser objects
///
@@ -366,7 +370,7 @@ protected:
/// and stored in chosen PoolStorage container.
///
/// As there are no default values for pool, setStorage() must be called
-/// before build(). Otherwise exception will be thrown.
+/// before build(). Otherwise an exception will be thrown.
///
/// It is useful for parsing Dhcp6/subnet6[X]/pool parameters.
class PoolParser : public DhcpConfigParser {
@@ -393,7 +397,7 @@ public:
BOOST_FOREACH(ConstElementPtr text_pool, pools_list->listValue()) {
// That should be a single pool representation. It should contain
- // text is form prefix/len or first - last. Note that spaces
+ // text in the form prefix/len or first - last. Note that spaces
// are allowed
string txt = text_pool->stringValue();
@@ -412,7 +416,7 @@ public:
// start with the first character after /
string prefix_len = txt.substr(pos + 1);
- // It is lexical cast to int and then downcast to uint8_t.
+ // It is lexically cast to int and then downcast to uint8_t.
// Direct cast to uint8_t (which is really an unsigned char)
// will result in interpreting the first digit as output
// value and throwing exception if length is written on two
@@ -461,7 +465,7 @@ public:
/// @brief does nothing.
///
- /// This method is required for all parser. The value itself
+ /// This method is required for all parsers. The value itself
/// is not commited anywhere. Higher level parsers (for subnet) are expected
/// to use values stored in the storage.
virtual void commit() {}
@@ -476,7 +480,7 @@ public:
protected:
/// @brief pointer to the actual Pools storage
///
- /// That is typically a storage somewhere in Subnet parser
+ /// This is typically a storage somewhere in Subnet parser
/// (an upper level parser).
PoolStorage* pools_;
};
@@ -485,11 +489,11 @@ protected:
///
/// This parser parses configuration entries that specify value of
/// a single option. These entries include option name, option code
-/// and data carried by the option. If parsing is successful than
+/// and data carried by the option. If parsing is successful than an
/// instance of an option is created and added to the storage provided
/// by the calling class.
///
-/// @todo This class parses and validates option name. However it is
+/// @todo This class parses and validates the option name. However it is
/// not used anywhere util support for option spaces is implemented
/// (see tickets #2319, #2314). When option spaces are implemented
/// there will be a way to reference the particular option using
@@ -501,7 +505,9 @@ public:
///
/// Class constructor.
OptionDataParser(const std::string&)
- : options_(NULL) { }
+ : options_(NULL),
+ // initialize option to NULL ptr
+ option_descriptor_(false) { }
/// @brief Parses the single option data.
///
@@ -559,11 +565,37 @@ public:
createOption();
}
- /// @brief Does nothing.
+ /// @brief Commits option value.
///
- /// This function does noting because option data is committed
- /// by a higher level parser.
- virtual void commit() { }
+ /// This function adds a new option to the storage or replaces an existing option
+ /// with the same code.
+ ///
+ /// @throw isc::InvalidOperation if failed to set pointer to storage or failed
+ /// to call build() prior to commit. If that happens data in the storage
+ /// remain un-modified.
+ virtual void commit() {
+ if (options_ == NULL) {
+ isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before "
+ "commiting option data.");
+ } else if (!option_descriptor_.option) {
+ // Before we can commit the new option should be configured. If it is not
+ // than somebody must have called commit() before build().
+ isc_throw(isc::InvalidOperation, "Parser logic error: no option has been configured and"
+ " thus there is nothing to commit. Has build() been called?");
+ }
+ uint16_t opt_type = option_descriptor_.option->getType();
+ Subnet::OptionContainerTypeIndex& idx = options_->get<1>();
+ // Try to find options with the particular option code in the main
+ // storage. If found, remove these options because they will be
+ // replaced with new one.
+ Subnet::OptionContainerTypeRange range =
+ idx.equal_range(opt_type);
+ if (std::distance(range.first, range.second) > 0) {
+ idx.erase(range.first, range.second);
+ }
+ // Append new option to the main storage.
+ options_->push_back(option_descriptor_);
+ }
/// @brief Set storage for the parser.
///
@@ -584,11 +616,11 @@ private:
///
/// Creates an instance of an option and adds it to the provided
/// options storage. If the option data parsed by \ref build function
- /// are invalid or insufficient it emits exception.
+ /// are invalid or insufficient this function emits an exception.
///
/// @warning this function does not check if options_ storage pointer
- /// is intitialized but this is not needed here because it is checked in
- /// \ref build function.
+ /// is intitialized but this check is not needed here because it is done
+ /// in the \ref build function.
///
/// @throw Dhcp6ConfigError if parameters provided in the configuration
/// are invalid.
@@ -605,7 +637,7 @@ private:
isc_throw(Dhcp6ConfigError, "Parser error: value of 'code' must not"
<< " exceed " << std::numeric_limits<uint16_t>::max());
}
- // Check the option name has been specified, is non-empty and does not
+ // Check that the option name has been specified, is non-empty and does not
// contain spaces.
// @todo possibly some more restrictions apply here?
std::string option_name = getStringParam("name");
@@ -649,23 +681,28 @@ private:
} else if (num_defs == 0) {
// @todo We have a limited set of option definitions intiialized at the moment.
// In the future we want to initialize option definitions for all options.
- // Consequently error will be issued if option definition does not exist
+ // Consequently an error will be issued if an option definition does not exist
// for a particular option code. For now it is ok to create generic option
// if definition does not exist.
OptionPtr option(new Option(Option::V6, static_cast<uint16_t>(option_code),
binary));
- // If option is created succesfully, add it to the storage.
- options_->push_back(option);
+ // The created option is stored in option_descriptor_ class member until the
+ // commit stage when it is inserted into the main storage. If an option with the
+ // same code exists in main storage already the old option is replaced.
+ option_descriptor_.option = option;
+ option_descriptor_.persistent = false;
} else {
- // We have exactly one option definition for the particular option code.
- // use it to create option instance.
+ // We have exactly one option definition for the particular option code
+ // use it to create the option instance.
const OptionDefinitionPtr& def = *(range.first);
// getFactory should never return NULL pointer.
Option::Factory* factory = def->getFactory();
assert(factory != NULL);
try {
OptionPtr option = factory(Option::V6, option_code, binary);
- options_->push_back(option);
+ Subnet::OptionDescriptor desc(option, false);
+ option_descriptor_.option = option;
+ option_descriptor_.persistent = false;
} catch (const isc::Exception& ex) {
isc_throw(Dhcp6ConfigError, "Parser error: option data does not match"
<< " option definition (code " << option_code << "): "
@@ -707,9 +744,11 @@ private:
/// Pointer to options storage. This storage is provided by
/// the calling class and is shared by all OptionDataParser objects.
OptionStorage* options_;
+ /// Option descriptor holds newly configured option.
+ Subnet::OptionDescriptor option_descriptor_;
};
-/// @brief Parser for option data values with ina subnet.
+/// @brief Parser for option data values within a subnet.
///
/// This parser iterates over all entries that define options
/// data for a particular subnet and creates a collection of options.
@@ -721,10 +760,10 @@ public:
/// @brief Constructor.
///
/// Unless otherwise specified, parsed options will be stored in
- /// a global option containers (option_default). That storage location
+ /// a global option container (option_default). That storage location
/// is overriden on a subnet basis.
OptionDataListParser(const std::string&)
- : options_(&option_defaults) { }
+ : options_(&option_defaults), local_options_() { }
/// @brief Parses entries that define options' data for a subnet.
///
@@ -738,9 +777,11 @@ public:
boost::shared_ptr<OptionDataParser> parser(new OptionDataParser("option-data"));
// options_ member will hold instances of all options thus
// each OptionDataParser takes it as a storage.
- parser->setStorage(options_);
- // Build the instance of a singkle option.
+ parser->setStorage(&local_options_);
+ // Build the instance of a single option.
parser->build(option_value);
+ // Store a parser as it will be used to commit.
+ parsers_.push_back(parser);
}
}
@@ -752,11 +793,18 @@ public:
}
- /// @brief Does nothing.
+ /// @brief Commit all option values.
///
- /// @todo Currently this function does nothing but in the future
- /// we may need to extend it to commit at this level.
- void commit() { }
+ /// This function invokes commit for all option values.
+ void commit() {
+ BOOST_FOREACH(ParserPtr parser, parsers_) {
+ parser->commit();
+ }
+ // Parsing was successful and we have all configured
+ // options in local storage. We can now replace old values
+ // with new values.
+ std::swap(local_options_, *options_);
+ }
/// @brief Create DhcpDataListParser object
///
@@ -767,8 +815,15 @@ public:
return (new OptionDataListParser(param_name));
}
+ /// Intermediate option storage. This storage is used by
+ /// lower level parsers to add new options. Values held
+ /// in this storage are assigned to main storage (options_)
+ /// if overall parsing was successful.
+ OptionStorage local_options_;
/// Pointer to options instances storage.
OptionStorage* options_;
+ /// Collection of parsers;
+ ParserCollection parsers_;
};
/// @brief this class parses a single subnet
@@ -780,8 +835,8 @@ public:
/// @brief constructor
Subnet6ConfigParser(const std::string& ) {
- // The parameter should always be "subnet", but we don't check here
- // against it in case some wants to reuse this parser somewhere.
+ // The parameter should always be "subnet", but we don't check
+ // against that here in case some wants to reuse this parser somewhere.
}
/// @brief parses parameter value
@@ -792,8 +847,8 @@ public:
BOOST_FOREACH(ConfigPair param, subnet->mapValue()) {
ParserPtr parser(createSubnet6ConfigParser(param.first));
// The actual type of the parser is unknown here. We have to discover
- // parser type here to invoke corresponding setStorage function on it.
- // We discover parser type by trying to cast the parser to various
+ // the parser type here to invoke the corresponding setStorage function
+ // on it. We discover parser type by trying to cast the parser to various
// parser types and checking which one was successful. For this one
// a setStorage and build methods are invoked.
@@ -830,6 +885,10 @@ public:
/// created in other parsers are used here and added to newly created Subnet6
/// objects. Subnet6 are then added to DHCP CfgMgr.
void commit() {
+ // Invoke commit on all sub-data parsers.
+ BOOST_FOREACH(ParserPtr parser, parsers_) {
+ parser->commit();
+ }
StringStorage::const_iterator it = string_values_.find("subnet");
if (it == string_values_.end()) {
@@ -871,33 +930,33 @@ public:
const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
// Add subnet specific options.
- BOOST_FOREACH(OptionPtr option, options_) {
- Subnet::OptionContainerTypeRange range = idx.equal_range(option->getType());
+ BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
+ Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
if (std::distance(range.first, range.second) > 0) {
LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE)
- .arg(option->getType()).arg(addr.toText());
+ .arg(desc.option->getType()).arg(addr.toText());
}
- subnet->addOption(option);
+ subnet->addOption(desc.option);
}
// Check all global options and add them to the subnet object if
// they have been configured in the global scope. If they have been
// configured in the subnet scope we don't add global option because
- // the one configured in the subnet scope always takes precedense.
- BOOST_FOREACH(OptionPtr option, option_defaults) {
+ // the one configured in the subnet scope always takes precedence.
+ BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
// Get all options specified locally in the subnet and having
// code equal to global option's code.
- Subnet::OptionContainerTypeRange range = idx.equal_range(option->getType());
+ Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
// @todo: In the future we will be searching for options using either
- // option code or namespace. Currently we have only the option
+ // an option code or namespace. Currently we have only the option
// code available so if there is at least one option found with the
- // specific code we don't add globally configured option.
+ // specific code we don't add the globally configured option.
// @todo with this code the first globally configured option
// with the given code will be added to a subnet. We may
- // want to issue warning about dropping configuration of
- // global option if one already exsist.
+ // want to issue a warning about dropping the configuration of
+ // a global option if one already exsists.
if (std::distance(range.first, range.second) == 0) {
- subnet->addOption(option);
+ subnet->addOption(desc.option);
}
}
@@ -908,9 +967,9 @@ private:
/// @brief Set storage for a parser and invoke build.
///
- /// This helper method casts the provided parser pointer to specified
- /// type. If cast is successful it sets the corresponding storage for
- /// this parser, invokes build on it and save the parser.
+ /// This helper method casts the provided parser pointer to the specified
+ /// type. If the cast is successful it sets the corresponding storage for
+ /// this parser, invokes build on it and saves the parser.
///
/// @tparam T parser type to which parser argument should be cast.
/// @tparam Y storage type for the specified parser type.
@@ -982,7 +1041,7 @@ private:
/// @brief returns value for a given parameter (after using inheritance)
///
- /// This method implements inheritance. For a given parameter name, it first
+ /// This method implements inheritance. For a given parameter name, it first
/// checks if there is a global value for it and overwrites it with specific
/// value if such value was defined in subnet.
///
@@ -1028,7 +1087,7 @@ private:
ParserCollection parsers_;
};
-/// @brief this class parses list of subnets
+/// @brief this class parses a list of subnets
///
/// This is a wrapper parser that handles the whole list of Subnet6
/// definitions. It iterates over all entries and creates Subnet6ConfigParser
@@ -1044,7 +1103,7 @@ public:
/// @brief parses contents of the list
///
- /// Iterates over all entries on the list and creates Subnet6ConfigParser
+ /// Iterates over all entries on the list and creates a Subnet6ConfigParser
/// for each entry.
///
/// @param subnets_list pointer to a list of IPv6 subnets
@@ -1153,12 +1212,6 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) {
"Null pointer is passed to configuration parser");
}
- /// Reset global storage. Containers being reset below may contain
- /// data from the previous configuration attempts.
- option_defaults.clear();
- uint32_defaults.clear();
- string_defaults.clear();
-
/// @todo: append most essential info here (like "2 new subnets configured")
string config_details;
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 780b5db..95c7897 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -53,6 +53,8 @@ public:
}
~Dhcp6ParserTest() {
+ // Reset configuration database after each test.
+ resetConfiguration();
};
/// @brief Create the simple configuration with single option.
@@ -116,6 +118,51 @@ public:
return (stream.str());
}
+ /// @brief Reset configuration database.
+ ///
+ /// This function resets configuration data base by
+ /// removing all subnets and option-data. Reset must
+ /// be performed after each test to make sure that
+ /// contents of the database do not affect result of
+ /// subsequent tests.
+ void resetConfiguration() {
+ ConstElementPtr status;
+
+ string config = "{ \"interface\": [ \"all\" ],"
+ "\"preferred-lifetime\": 3000,"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"valid-lifetime\": 4000, "
+ "\"subnet6\": [ ], "
+ "\"option-data\": [ ] }";
+
+ try {
+ ElementPtr json = Element::fromJSON(config);
+ status = configureDhcp6Server(srv_, json);
+ } catch (const std::exception& ex) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. The following configuration was used"
+ << " to reset database: " << std::endl
+ << config << std::endl
+ << " and the following error message was returned:"
+ << ex.what() << std::endl;
+ }
+
+
+ // returned value should be 0 (configuration success)
+ if (!status) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. Configuration function returned"
+ << " NULL pointer" << std::endl;
+ }
+ comment_ = parseAnswer(rcode_, status);
+ if (rcode_ != 0) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. Configuration function returned"
+ << " error code " << rcode_ << std::endl;
+ }
+ }
+
/// @brief Test invalid option parameter value.
///
/// This test function constructs the simple configuration
@@ -691,7 +738,7 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
// Option code 3 means OPTION_IA_NA.
params["code"] = "3";
params["data"] = "ABCDEF01 02030405 06070809";
-
+
std::string config = createConfigWithOption(params);
ElementPtr json = Element::fromJSON(config);
diff --git a/src/lib/dhcp/subnet.h b/src/lib/dhcp/subnet.h
index 00b3375..fcedda6 100644
--- a/src/lib/dhcp/subnet.h
+++ b/src/lib/dhcp/subnet.h
@@ -69,6 +69,12 @@ public:
/// @param persist if true option is always sent.
OptionDescriptor(OptionPtr& opt, bool persist)
: option(opt), persistent(persist) {};
+
+ /// @brief Constructor
+ ///
+ /// @param persist if true option is always sent.
+ OptionDescriptor(bool persist)
+ : option(OptionPtr()), persistent(persist) {};
};
/// @brief Extractor class to extract key with another key.
More information about the bind10-changes
mailing list