From: Marcin Siodelski Date: Tue, 16 Sep 2014 15:54:43 +0000 (+0200) Subject: [3588] Implemented CfgOptionDef class to represent user-defined option defs X-Git-Tag: rt3470_base^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=31a9ba394107c03767371c5ac045e484ed5404ac;p=thirdparty%2Fkea.git [3588] Implemented CfgOptionDef class to represent user-defined option defs --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 78ac0a7198..29f7dda0ab 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1806,8 +1806,8 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf, // Get the list of stdandard option definitions. option_defs = LibDHCP::getOptionDefs(Option::V4); } else if (!option_space.empty()) { - OptionDefContainerPtr option_defs_ptr = - CfgMgr::instance().getOptionDefs(option_space); + OptionDefContainerPtr option_defs_ptr = CfgMgr::instance() + .getCurrentCfg()->getCfgOptionDef().getAll(option_space); if (option_defs_ptr != NULL) { option_defs = *option_defs_ptr; } diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index ba63a5a8c9..73a38a7e98 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1231,7 +1231,8 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg() + ->getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1241,7 +1242,7 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition data is valid. @@ -1272,7 +1273,8 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1282,7 +1284,7 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1330,8 +1332,10 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definitions do not exist yet. - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100)); - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 101)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 101)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1340,7 +1344,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { checkResult(status, 0); // Check the first definition we have created. - OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def1 = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def1); // Check the option data. @@ -1351,7 +1356,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) { EXPECT_TRUE(def1->getEncapsulatedSpace().empty()); // Check the second option definition we have created. - OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101); + OptionDefinitionPtr def2 = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 101); ASSERT_TRUE(def2); // Check the option data. @@ -1392,7 +1398,8 @@ TEST_F(Dhcp4ParserTest, optionDefDuplicate) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definition does not exist yet. - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1422,7 +1429,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1432,7 +1440,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1463,7 +1472,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1473,7 +1483,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1670,7 +1681,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { "}"; ElementPtr json = Element::fromJSON(config); - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp4", 109); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp4", 109); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1680,7 +1692,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("dhcp4", 109); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp4", 109); ASSERT_TRUE(def); // Check the option data. @@ -1736,7 +1749,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { // Expecting success. checkResult(status, 0); - def = CfgMgr::instance().getOptionDef("dhcp4", 65); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp4", 65); ASSERT_TRUE(def); // Check the option data. diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index c5bb884768..4879560a66 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -1315,10 +1315,12 @@ TEST_F(Dhcpv4SrvTest, unpackOptions) { // Add option definitions to the Configuration Manager. Each goes under // different option space. - CfgMgr& cfgmgr = CfgMgr::instance(); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def, "space-foobar")); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def2, "space-foo")); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def3, "space-bar")); + CfgOptionDef cfg_option_def; + ASSERT_NO_THROW(cfg_option_def.add(opt_def, "space-foobar")); + ASSERT_NO_THROW(cfg_option_def.add(opt_def2, "space-foo")); + ASSERT_NO_THROW(cfg_option_def.add(opt_def3, "space-bar")); + CfgMgr::instance().getStagingCfg()->setCfgOptionDef(cfg_option_def); + CfgMgr::instance().commit(); // Create the buffer holding the structure of options. const char raw_data[] = { diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index ac7401d98a..637e4015ab 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -2444,7 +2444,8 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf, option_defs = LibDHCP::getOptionDefs(Option::V6); } else if (!option_space.empty()) { OptionDefContainerPtr option_defs_ptr = - CfgMgr::instance().getOptionDefs(option_space); + CfgMgr::instance().getCurrentCfg()->getCfgOptionDef() + .getAll(option_space); if (option_defs_ptr != NULL) { option_defs = *option_defs_ptr; } diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 21e53602d1..e9281a65f7 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -1466,7 +1466,8 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1475,7 +1476,7 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) { ASSERT_TRUE(status); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition data is valid. @@ -1505,7 +1506,8 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1515,7 +1517,7 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1562,8 +1564,10 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definitions do not exist yet. - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100)); - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 101)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 101)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1572,7 +1576,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { checkResult(status, 0); // Check the first definition we have created. - OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def1 = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def1); // Check the option data. @@ -1582,7 +1587,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) { EXPECT_FALSE(def1->getArrayType()); // Check the second option definition we have created. - OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101); + OptionDefinitionPtr def2 = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 101); ASSERT_TRUE(def2); // Check the option data. @@ -1622,7 +1628,8 @@ TEST_F(Dhcp6ParserTest, optionDefDuplicate) { ElementPtr json = Element::fromJSON(config); // Make sure that the option definition does not exist yet. - ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100)); + ASSERT_FALSE(CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100)); // Use the configuration string to create new option definitions. ConstElementPtr status; @@ -1652,7 +1659,8 @@ TEST_F(Dhcp6ParserTest, optionDefArray) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1662,7 +1670,7 @@ TEST_F(Dhcp6ParserTest, optionDefArray) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1692,7 +1700,8 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) { ElementPtr json = Element::fromJSON(config); // Make sure that the particular option definition does not exist. - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("isc", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1702,7 +1711,7 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("isc", 100); + def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Check the option data. @@ -1900,7 +1909,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { "}"; ElementPtr json = Element::fromJSON(config); - OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp6", 100); + OptionDefinitionPtr def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp6", 100); ASSERT_FALSE(def); // Use the configuration string to create new option definition. @@ -1910,7 +1920,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { checkResult(status, 0); // The option definition should now be available in the CfgMgr. - def = CfgMgr::instance().getOptionDef("dhcp6", 100); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp6", 100); ASSERT_TRUE(def); // Check the option data. @@ -1966,7 +1977,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { // Expecting success. checkResult(status, 0); - def = CfgMgr::instance().getOptionDef("dhcp6", 59); + def = CfgMgr::instance().getCurrentCfg()-> + getCfgOptionDef().get("dhcp6", 59); ASSERT_TRUE(def); // Check the option data. diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index fc56d8bb0c..bacca63618 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -1713,10 +1713,12 @@ TEST_F(Dhcpv6SrvTest, unpackOptions) { // Add option definitions to the Configuration Manager. Each goes under // different option space. - CfgMgr& cfgmgr = CfgMgr::instance(); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def, "space-foobar")); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def2, "space-foo")); - ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def3, "space-bar")); + CfgOptionDef cfg_option_def; + ASSERT_NO_THROW(cfg_option_def.add(opt_def, "space-foobar")); + ASSERT_NO_THROW(cfg_option_def.add(opt_def2, "space-foo")); + ASSERT_NO_THROW(cfg_option_def.add(opt_def3, "space-bar")); + CfgMgr::instance().getStagingCfg()->setCfgOptionDef(cfg_option_def); + CfgMgr::instance().commit(); // Create the buffer holding the structure of options. const char raw_data[] = { diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc index f4a205c85c..ba9644acfa 100644 --- a/src/lib/dhcp/option_definition.cc +++ b/src/lib/dhcp/option_definition.cc @@ -92,6 +92,25 @@ OptionDefinition::OptionDefinition(const std::string& name, encapsulated_space_(encapsulated_space) { } +OptionDefinition::OptionDefinition(const OptionDefinition& def) + : name_(def.getName()), + code_(def.getCode()), + type_(def.getType()), + array_type_(def.getArrayType()), + encapsulated_space_(def.getEncapsulatedSpace()), + record_fields_(def.getRecordFields()) { +} + +bool +OptionDefinition::equals(const OptionDefinition& other) const { + return (name_ == other.name_ && + code_ == other.code_ && + type_ == other.type_ && + array_type_ == other.array_type_ && + encapsulated_space_ == other.encapsulated_space_ && + record_fields_ == other.record_fields_); +} + void OptionDefinition::addRecordField(const std::string& data_type_name) { OptionDataType data_type = OptionDataTypeUtil::getDataType(data_type_name); diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h index 3664d9dbc4..a8d2f768bc 100644 --- a/src/lib/dhcp/option_definition.h +++ b/src/lib/dhcp/option_definition.h @@ -183,6 +183,40 @@ public: const std::string& type, const char* encapsulated_space); + /// @brief Copy constructor. + /// + /// @param def Option definition to be used to create a new instance. + explicit OptionDefinition(const OptionDefinition& def); + + /// @name Comparison functions and operators. + /// + //@{ + /// @brief Check if option definition is equal to other. + /// + /// @param other Option definition to compare to. + /// + /// @return true if two option definitions are equal, false otherwise. + bool equals(const OptionDefinition& other) const; + + /// @brief Equality operator. + /// + /// @param other Option definition to compare to. + /// + /// @return true if two option definitions are equal, false otherwise. + bool operator==(const OptionDefinition& other) const { + return (equals(other)); + } + + /// @brief Inequality operator. + /// + /// @param other Option definition to compare to. + /// + /// @return true if option definitions are not equal, false otherwise. + bool operator!=(const OptionDefinition& other) const { + return (!equals(other)); + } + //@} + /// @brief Constructor. /// /// This constructor sets the name of the option space that is @@ -247,7 +281,9 @@ public: /// @brief Return list of record fields. /// /// @return list of record fields. - const RecordFieldsCollection& getRecordFields() const { return (record_fields_); } + const RecordFieldsCollection& getRecordFields() const { + return (record_fields_); + } /// @brief Return option data type. /// diff --git a/src/lib/dhcp/option_space.h b/src/lib/dhcp/option_space.h index 9eebd76df9..ad5e1b4223 100644 --- a/src/lib/dhcp/option_space.h +++ b/src/lib/dhcp/option_space.h @@ -21,6 +21,9 @@ #include #include +#define DHCP4_OPTION_SPACE "dhcp4" +#define DHCP6_OPTION_SPACE "dhcp6" + namespace isc { namespace dhcp { diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc index 1ba68a8a7b..6c7a418e1b 100644 --- a/src/lib/dhcp/tests/option_definition_unittest.cc +++ b/src/lib/dhcp/tests/option_definition_unittest.cc @@ -47,6 +47,7 @@ class OptionDefinitionTest : public ::testing::Test { public: // @brief Constructor. OptionDefinitionTest() { } + }; // The purpose of this test is to verify that OptionDefinition @@ -115,6 +116,86 @@ TEST_F(OptionDefinitionTest, constructor) { ); } +// This test checks that the copy constructor works properly. +TEST_F(OptionDefinitionTest, copyConstructor) { + OptionDefinition opt_def("option-foo", 27, "record", true); + ASSERT_NO_THROW(opt_def.addRecordField("uint16")); + ASSERT_NO_THROW(opt_def.addRecordField("string")); + + OptionDefinition opt_def_copy(opt_def); + EXPECT_EQ("option-foo", opt_def_copy.getName()); + EXPECT_EQ(27, opt_def_copy.getCode()); + EXPECT_TRUE(opt_def_copy.getArrayType()); + EXPECT_TRUE(opt_def_copy.getEncapsulatedSpace().empty()); + const OptionDefinition::RecordFieldsCollection fields = + opt_def_copy.getRecordFields(); + ASSERT_EQ(2, fields.size()); + EXPECT_EQ(OPT_UINT16_TYPE, fields[0]); + EXPECT_EQ(OPT_STRING_TYPE, fields[1]); +} + +// This test checks that two option definitions may be compared fot equality. +TEST_F(OptionDefinitionTest, equality) { + // Equal definitions. + EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false) + == OptionDefinition("option-foo", 5, "uint16", false)); + EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false) + != OptionDefinition("option-foo", 5, "uint16", false)); + + // Differ by name. + EXPECT_FALSE(OptionDefinition("option-bar", 5, "uint16", false) + == OptionDefinition("option-foo", 5, "uint16", false)); + EXPECT_TRUE(OptionDefinition("option-bar", 5, "uint16", false) + != OptionDefinition("option-foo", 5, "uint16", false)); + + // Differ by option code. + EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false) + == OptionDefinition("option-foo", 6, "uint16", false)); + EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false) + != OptionDefinition("option-foo", 6, "uint16", false)); + + // Differ by type of the data. + EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false) + == OptionDefinition("option-foo", 5, "uint32", false)); + EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false) + != OptionDefinition("option-foo", 5, "uint32", false)); + + // Differ by array-type property. + EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false) + == OptionDefinition("option-foo", 5, "uint16", true)); + EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false) + != OptionDefinition("option-foo", 5, "uint16", true)); + + // Differ by record fields. + OptionDefinition def1("option-foo", 5, "record"); + OptionDefinition def2("option-foo", 5, "record"); + + // There are no record fields specified yet, so initially they have + // to be equal. + ASSERT_TRUE(def1 == def2); + ASSERT_FALSE(def1 != def2); + + // Add some record fields. + ASSERT_NO_THROW(def1.addRecordField("uint16")); + ASSERT_NO_THROW(def2.addRecordField("uint16")); + + // Definitions should still remain equal. + ASSERT_TRUE(def1 == def2); + ASSERT_FALSE(def1 != def2); + + // Add additional record field to one of the definitions but not the + // other. They should now be unequal. + ASSERT_NO_THROW(def1.addRecordField("string")); + ASSERT_FALSE(def1 == def2); + ASSERT_TRUE(def1 != def2); + + // Add the same record field to the other definition. They should now + // be equal again. + ASSERT_NO_THROW(def2.addRecordField("string")); + EXPECT_TRUE(def1 == def2); + EXPECT_FALSE(def1 != def2); +} + // The purpose of this test is to verify that various data fields // can be specified for an option definition when this definition // is marked as 'record' and that fields can't be added if option diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index 2a5e819213..e035bee2d2 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -46,6 +46,7 @@ libkea_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h libkea_dhcpsrv_la_SOURCES += alloc_engine.cc alloc_engine.h libkea_dhcpsrv_la_SOURCES += callout_handle_store.h libkea_dhcpsrv_la_SOURCES += cfg_iface.cc cfg_iface.h +libkea_dhcpsrv_la_SOURCES += cfg_option_def.cc cfg_option_def.h libkea_dhcpsrv_la_SOURCES += cfgmgr.cc cfgmgr.h libkea_dhcpsrv_la_SOURCES += csv_lease_file4.cc csv_lease_file4.h libkea_dhcpsrv_la_SOURCES += csv_lease_file6.cc csv_lease_file6.h diff --git a/src/lib/dhcpsrv/cfg_option_def.cc b/src/lib/dhcpsrv/cfg_option_def.cc new file mode 100644 index 0000000000..d51d7403cf --- /dev/null +++ b/src/lib/dhcpsrv/cfg_option_def.cc @@ -0,0 +1,141 @@ +// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include +#include +#include + +namespace isc { +namespace dhcp { + +void +CfgOptionDef::copy(CfgOptionDef& new_config) const { + CfgOptionDef cfg; + std::list names = option_definitions_.getOptionSpaceNames(); + for (std::list::const_iterator name = names.begin(); + name != names.end(); ++name) { + OptionDefContainerPtr defs = getAll(*name); + for (OptionDefContainer::const_iterator def = defs->begin(); + def != defs->end(); ++def) { + OptionDefinitionPtr new_def = + OptionDefinitionPtr(new OptionDefinition(**def)); + cfg.add(new_def, *name); + } + } + new_config = cfg; +} + +bool +CfgOptionDef::equals(const CfgOptionDef& other) const { + // Get our option space names. + std::list names = option_definitions_.getOptionSpaceNames(); + // Get option space names held by the other object. + std::list + other_names = other.option_definitions_.getOptionSpaceNames(); + // Compareg that sizes are the same. If they hold different number of + // option space names the objects are not equal. + if (names.size() != other_names.size()) { + return (false); + } + // Iterate over all option space names and get the definitions for each + // of them. + for (std::list::const_iterator name = names.begin(); + name != names.end(); ++name) { + // Get all definitions. + OptionDefContainerPtr defs = getAll(*name); + OptionDefContainerPtr other_defs = other.getAll(*name); + // Compare sizes. If they hold different number of definitions, + // they are unequal. + if (defs->size() != defs->size()) { + return (false); + } + // For each option definition, try to find one in the other object. + for (OptionDefContainer::const_iterator def = defs->begin(); + def != defs->end(); ++def) { + OptionDefinitionPtr + other_def = other.get(*name, (*def)->getCode()); + // Actually compare them. + if (!other_def || (*other_def != **def)) { + return (false); + } + } + } + + // All checks passed. + return (true); +} + +void +CfgOptionDef::add(const OptionDefinitionPtr& def, + const std::string& option_space) { + if (!OptionSpace::validateName(option_space)) { + isc_throw(BadValue, "invalid option space name '" + << option_space << "'"); + + // Option definition being added must be a valid pointer. + } else if (!def) { + isc_throw(MalformedOptionDefinition, "option definition must not be NULL"); + // Must not duplicate an option definition. + } else if (get(option_space, def->getCode())) { + isc_throw(DuplicateOptionDefinition, "option definition with code '" + << def->getCode() << "' already exists"); + + // Must not override standard option definition. + } else if (((option_space == DHCP4_OPTION_SPACE) && + LibDHCP::isStandardOption(Option::V4, def->getCode()) && + LibDHCP::getOptionDef(Option::V4, def->getCode())) || + ((option_space == DHCP6_OPTION_SPACE) && + LibDHCP::isStandardOption(Option::V6, def->getCode()) && + LibDHCP::getOptionDef(Option::V6, def->getCode()))) { + isc_throw(BadValue, "unable to override definition of option '" + << def->getCode() << "' in standard option space '" + << option_space << "'"); + } + // Add the definition. + option_definitions_.addItem(def, option_space); +} + +OptionDefContainerPtr +CfgOptionDef::getAll(const std::string& option_space) const { + /// @todo Does option space require any validation here? + return (option_definitions_.getItems(option_space)); +} + +OptionDefinitionPtr +CfgOptionDef::get(const std::string& option_space, + const uint16_t option_code) const { + // Get the pointer to collection of the option definitions that belong + // to the particular option space. + OptionDefContainerPtr defs = getAll(option_space); + // If there are any option definitions for this option space, get the + // one that has the specified option code. + if (defs && !defs->empty()) { + const OptionDefContainerTypeIndex& idx = defs->get<1>(); + const OptionDefContainerTypeRange& range = idx.equal_range(option_code); + // If there is more than one definition matching the option code, + // return the first one. In fact, it shouldn't happen that we have + // more than one because we check for duplicates when we add them. + if (std::distance(range.first, range.second) > 0) { + return (*range.first); + } + } + // Nothing found. Return NULL pointer. + return (OptionDefinitionPtr()); +} + + + +} // end of namespace isc::dhcp +} // end of namespace isc diff --git a/src/lib/dhcpsrv/cfg_option_def.h b/src/lib/dhcpsrv/cfg_option_def.h new file mode 100644 index 0000000000..5edc6a455e --- /dev/null +++ b/src/lib/dhcpsrv/cfg_option_def.h @@ -0,0 +1,127 @@ +// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef CFG_OPTION_DEF_H +#define CFG_OPTION_DEF_H + +#include +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Represents option definitions used by the DHCP server. +/// +/// This class provides methods to add and retrieve option definitions +/// specified by the administrator for the DHCP server. Option definitions +/// specify formats of the options. This class doesn't hold information +/// about the data being carried by the options. +/// +/// Option definitions are grouped by option spaces. The option space is +/// identified by the unique name which is specified as a string. The +/// following names: "dhcp4" and "dhcp6" are reserved, though. They are +/// names of option spaces used for standard top-level DHCPv4 and DHCPv6 +/// options respectively. +class CfgOptionDef { +public: + + /// @brief Copies this configuration to a new configuration. + /// + /// This method copies the option definitions stores in the configuration + /// to an object passed as parameter. There are no shared objects or + /// pointers between the original object and a copy. + /// + /// @param [out] new_config An object to which the configuration will be + /// copied. + void copy(CfgOptionDef& new_config) const; + + /// @name Methods and operators used for comparing objects. + /// + //@{ + /// @brief Check if configuration is equal to other configuration. + /// + /// @param other An object holding configuration to compare to. + /// + /// @return true if configurations are equal, false otherwise. + bool equals(const CfgOptionDef& other) const; + + /// @brief Equality operator. + /// + /// @param other An object holding configuration to compare to. + /// + /// @return true if configurations are equal, false otherwise. + bool operator==(const CfgOptionDef& other) const { + return (equals(other)); + } + + /// @brief Inequality operator. + /// + /// @param other An object holding configuration to compare to. + /// + /// @return true if configurations are unequal, false otherwise. + bool operator!=(const CfgOptionDef& other) const { + return (!equals(other)); + } + + //@} + + /// @brief Add new option definition. + /// + /// @param def option definition to be added. + /// @param option_space name of the option space to add definition to. + /// + /// @throw isc::dhcp::DuplicateOptionDefinition when the particular + /// option definition already exists. + /// @throw isc::dhcp::MalformedOptionDefinition when the pointer to + /// an option definition is NULL. + /// @throw isc::BadValue when the option space name is empty or + /// when trying to override the standard option (in dhcp4 or dhcp6 + /// option space). + void add(const OptionDefinitionPtr& def, const std::string& option_space); + + /// @brief Return option definitions for particular option space. + /// + /// @param option_space option space. + /// + /// @return Pointer to the collection of option definitions for + /// the particular option space. The option collection is empty + /// if no option exists for the option space specified. + OptionDefContainerPtr getAll(const std::string& option_space) const; + + /// @brief Return option definition for a particular option space and code. + /// + /// @param option_space option space. + /// @param option_code option code. + /// + /// @return An option definition or NULL pointer if option definition + /// has not been found. + OptionDefinitionPtr get(const std::string& option_space, + const uint16_t option_code) const; + +private: + + /// @brief A collection of option definitions. + /// + /// The option definitions stored in this container can be accessed + /// using the option space name they belong to. + OptionSpaceContainer option_definitions_; + +}; + +} +} + +#endif // CFG_OPTION_DEF_H diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 47b8881e47..9f8199f7ea 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -61,72 +61,6 @@ CfgMgr::addOptionSpace6(const OptionSpacePtr& space) { spaces6_.insert(make_pair(space->getName(), space)); } -void -CfgMgr::addOptionDef(const OptionDefinitionPtr& def, - const std::string& option_space) { - // @todo we need better validation of the provided option space name here. - // This will be implemented when #2313 is merged. - if (option_space.empty()) { - isc_throw(BadValue, "option space name must not be empty"); - } else if (!def) { - // Option definition must point to a valid object. - isc_throw(MalformedOptionDefinition, "option definition must not be NULL"); - - } else if (getOptionDef(option_space, def->getCode())) { - // Option definition must not be overriden. - isc_throw(DuplicateOptionDefinition, "option definition already added" - << " to option space " << option_space); - - // We must not override standard (assigned) option for which there is a - // definition in libdhcp++. The standard options belong to dhcp4 or dhcp6 - // option space. - } else if ((option_space == "dhcp4" && - LibDHCP::isStandardOption(Option::V4, def->getCode()) && - LibDHCP::getOptionDef(Option::V4, def->getCode())) || - (option_space == "dhcp6" && - LibDHCP::isStandardOption(Option::V6, def->getCode()) && - LibDHCP::getOptionDef(Option::V6, def->getCode()))) { - isc_throw(BadValue, "unable to override definition of option '" - << def->getCode() << "' in standard option space '" - << option_space << "'."); - - } - // Actually add a new item. - option_def_spaces_.addItem(def, option_space); -} - -OptionDefContainerPtr -CfgMgr::getOptionDefs(const std::string& option_space) const { - // @todo Validate the option space once the #2313 is implemented. - - return (option_def_spaces_.getItems(option_space)); -} - -OptionDefinitionPtr -CfgMgr::getOptionDef(const std::string& option_space, - const uint16_t option_code) const { - // @todo Validate the option space once the #2313 is implemented. - - // Get a reference to option definitions for a particular option space. - OptionDefContainerPtr defs = getOptionDefs(option_space); - // If there are no matching option definitions then return the empty pointer. - if (!defs || defs->empty()) { - return (OptionDefinitionPtr()); - } - // If there are some option definitions for a particular option space - // use an option code to get the one we want. - const OptionDefContainerTypeIndex& idx = defs->get<1>(); - const OptionDefContainerTypeRange& range = idx.equal_range(option_code); - // If there is no definition that matches option code, return empty pointer. - if (std::distance(range.first, range.second) == 0) { - return (OptionDefinitionPtr()); - } - // If there is more than one definition matching an option code, return - // the first one. This should not happen because we check for duplicates - // when addOptionDef is called. - return (*range.first); -} - Subnet6Ptr CfgMgr::getSubnet6(const std::string& iface, const isc::dhcp::ClientClasses& classes) { @@ -297,10 +231,6 @@ void CfgMgr::addSubnet4(const Subnet4Ptr& subnet) { subnets4_.push_back(subnet); } -void CfgMgr::deleteOptionDefs() { - option_def_spaces_.clearItems(); -} - void CfgMgr::deleteSubnets4() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_DELETE_SUBNET4); subnets4_.clear(); diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index ee1cd7d3fd..95eb2d70a1 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -17,11 +17,9 @@ #include #include -#include #include #include #include -#include #include #include #include @@ -106,41 +104,6 @@ public: /// accessing it. static CfgMgr& instance(); - /// @brief Add new option definition. - /// - /// @param def option definition to be added. - /// @param option_space name of the option space to add definition to. - /// - /// @throw isc::dhcp::DuplicateOptionDefinition when the particular - /// option definition already exists. - /// @throw isc::dhcp::MalformedOptionDefinition when the pointer to - /// an option definition is NULL. - /// @throw isc::BadValue when the option space name is empty or - /// when trying to override the standard option (in dhcp4 or dhcp6 - /// option space). - void addOptionDef(const OptionDefinitionPtr& def, - const std::string& option_space); - - /// @brief Return option definitions for particular option space. - /// - /// @param option_space option space. - /// - /// @return pointer to the collection of option definitions for - /// the particular option space. The option collection is empty - /// if no option exists for the option space specified. - OptionDefContainerPtr - getOptionDefs(const std::string& option_space) const; - - /// @brief Return option definition for a particular option space and code. - /// - /// @param option_space option space. - /// @param option_code option code. - /// - /// @return an option definition or NULL pointer if option definition - /// has not been found. - OptionDefinitionPtr getOptionDef(const std::string& option_space, - const uint16_t option_code) const; - /// @brief Adds new DHCPv4 option space to the collection. /// /// @param space option space to be added. @@ -235,9 +198,6 @@ public: /// @param subnet new subnet to be added. void addSubnet6(const Subnet6Ptr& subnet); - /// @brief Delete all option definitions. - void deleteOptionDefs(); - /// @todo: Add subnet6 removal routines. Currently it is not possible /// to remove subnets. The only case where subnet6 removal would be /// needed is a dynamic server reconfiguration - a use case that is not @@ -555,13 +515,6 @@ private: /// @return true if the duplicate subnet exists. bool isDuplicate(const Subnet6& subnet) const; - /// @brief A collection of option definitions. - /// - /// A collection of option definitions that can be accessed - /// using option space name they belong to. - OptionSpaceContainer option_def_spaces_; - /// @brief Container for defined DHCPv6 option spaces. OptionSpaceCollection spaces6_; @@ -581,8 +534,6 @@ private: /// /// This is a structure that will hold all configuration. /// @todo: migrate all other parameters to that structure. - /// @todo: maybe this should be a vector, so we could keep - /// previous configurations and do a rollback if needed? SrvConfigPtr configuration_; /// @name Configuration List. diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index 9a96c12852..aa48994479 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -43,7 +43,6 @@ ParserContext::ParserContext(Option::Universe universe): uint32_values_(new Uint32Storage()), string_values_(new StringStorage()), options_(new OptionStorage()), - option_defs_(new OptionDefStorage()), hooks_libraries_(), universe_(universe) { @@ -54,7 +53,6 @@ ParserContext::ParserContext(const ParserContext& rhs): uint32_values_(), string_values_(), options_(), - option_defs_(), hooks_libraries_(), universe_(rhs.universe_) { @@ -79,7 +77,6 @@ ParserContext::copyContext(const ParserContext& ctx) { copyContextPointer(ctx.uint32_values_, uint32_values_); copyContextPointer(ctx.string_values_, string_values_); copyContextPointer(ctx.options_, options_); - copyContextPointer(ctx.option_defs_, option_defs_); copyContextPointer(ctx.hooks_libraries_, hooks_libraries_); // Copy universe. universe_ = ctx.universe_; @@ -446,7 +443,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { // options. They are expected to be in the global storage // already. OptionDefContainerPtr defs = - global_context_->option_defs_->getItems(space); + CfgMgr::instance().getStagingCfg()->getCfgOptionDef().getAll(space); // The getItems() should never return the NULL pointer. If there are // no option definitions for the particular option space a pointer @@ -612,17 +609,11 @@ OptionDataListParser::commit() { // ******************************** OptionDefParser **************************** OptionDefParser::OptionDefParser(const std::string&, - OptionDefStoragePtr storage, ParserContextPtr global_context) - : storage_(storage), - boolean_values_(new BooleanStorage()), + : boolean_values_(new BooleanStorage()), string_values_(new StringStorage()), uint32_values_(new Uint32Storage()), global_context_(global_context) { - if (!storage_) { - isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: " - << "options storage may not be NULL"); - } } void @@ -655,31 +646,21 @@ OptionDefParser::build(ConstElementPtr option_def) { // Create an instance of option definition. createOptionDef(option_def); - // Get all items we collected so far for the particular option space. - OptionDefContainerPtr defs = storage_->getItems(option_space_name_); - - // Check if there are any items with option code the same as the - // one specified for the definition we are now creating. - const OptionDefContainerTypeIndex& idx = defs->get<1>(); - const OptionDefContainerTypeRange& range = - idx.equal_range(option_definition_->getCode()); + CfgOptionDef cfg = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); + try { + cfg.add(option_definition_, option_space_name_); - // If there are any items with this option code already we need - // to issue an error because we don't allow duplicates for - // option definitions within an option space. - if (std::distance(range.first, range.second) > 0) { - isc_throw(DhcpConfigError, "duplicated option definition for" - << " code '" << option_definition_->getCode() << "' (" + } catch (const std::exception& ex) { + // Append position + isc_throw(DhcpConfigError, ex.what() << " (" << option_def->getPosition() << ")"); } + CfgMgr::instance().getStagingCfg()->setCfgOptionDef(cfg); } void OptionDefParser::commit() { - if (storage_ && option_definition_ && - OptionSpace::validateName(option_space_name_)) { - storage_->addItem(option_definition_, option_space_name_); - } + // Do nothing. } void @@ -775,20 +756,11 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) { // ******************************** OptionDefListParser ************************ OptionDefListParser::OptionDefListParser(const std::string&, ParserContextPtr global_context) - : storage_(global_context->option_defs_), - global_context_(global_context) { - if (!storage_) { - isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: " - << "storage may not be NULL"); - } + : global_context_(global_context) { } void OptionDefListParser::build(ConstElementPtr option_def_list) { - // Clear existing items in the storage. - // We are going to replace all of them. - storage_->clearItems(); - if (!option_def_list) { isc_throw(DhcpConfigError, "parser error: a pointer to a list of" << " option definitions is NULL (" @@ -797,36 +769,8 @@ OptionDefListParser::build(ConstElementPtr option_def_list) { BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) { boost::shared_ptr - parser(new OptionDefParser("single-option-def", storage_, - global_context_)); + parser(new OptionDefParser("single-option-def", global_context_)); parser->build(option_def); - parser->commit(); - } - - CfgMgr& cfg_mgr = CfgMgr::instance(); - cfg_mgr.deleteOptionDefs(); - - // We need to move option definitions from the temporary - // storage to the storage. - std::list space_names = storage_->getOptionSpaceNames(); - - BOOST_FOREACH(std::string space_name, space_names) { - BOOST_FOREACH(OptionDefinitionPtr def, - *(storage_->getItems(space_name))) { - // All option definitions should be initialized to non-NULL - // values. The validation is expected to be made by the - // OptionDefParser when creating an option definition. - assert(def); - // The Config Manager may thrown an exception if the duplicated - // definition is being added. Catch the exceptions here to and - // append the position in the config. - try { - cfg_mgr.addOptionDef(def, space_name); - } catch (const std::exception& ex) { - isc_throw(DhcpConfigError, ex.what() << " (" - << option_def_list->getPosition() << ")"); - } - } } } @@ -1099,8 +1043,8 @@ SubnetConfigParser::appendSubOptions(const std::string& option_space, return; } } else { - const OptionDefContainerPtr defs = - global_context_->option_defs_->getItems(option_space); + OptionDefContainerPtr defs = CfgMgr::instance().getStagingCfg() + ->getCfgOptionDef().getAll(option_space); const OptionDefContainerTypeIndex& idx = defs->get<1>(); const OptionDefContainerTypeRange& range = diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index d1de4ac9fe..08d30acd3a 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -216,9 +216,6 @@ public: /// @brief Storage for options. OptionStoragePtr options_; - /// @brief Storage for option definitions. - OptionDefStoragePtr option_defs_; - /// @brief Hooks libraries pointer. /// /// The hooks libraries information is a vector of strings, each containing @@ -683,13 +680,9 @@ public: /// /// @param dummy first argument is ignored, all Parser constructors /// accept string as first argument. - /// @param storage is the definition storage in which to store the parsed - /// definition upon "commit". /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. - /// @throw isc::dhcp::DhcpConfigError if storage is null. - OptionDefParser(const std::string& dummy, OptionDefStoragePtr storage, - ParserContextPtr global_context); + OptionDefParser(const std::string& dummy, ParserContextPtr global_context); /// @brief Parses an entry that describes single option definition. /// @@ -714,10 +707,6 @@ private: /// Name of the space the option definition belongs to. std::string option_space_name_; - /// Pointer to a storage where the option definition will be - /// added when \ref commit is called. - OptionDefStoragePtr storage_; - /// Storage for boolean values. BooleanStoragePtr boolean_values_; @@ -746,7 +735,6 @@ public: /// accept string as first argument. /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. - /// @throw isc::dhcp::DhcpConfigError if storage is null. OptionDefListParser(const std::string& dummy, ParserContextPtr global_context); @@ -767,9 +755,6 @@ public: /// added to the Configuration Manager in the @c build method. void commit(); - /// @brief storage for option definitions. - OptionDefStoragePtr storage_; - /// Parsing context which contains global values, options and option /// definitions. ParserContextPtr global_context_; diff --git a/src/lib/dhcpsrv/logging.h b/src/lib/dhcpsrv/logging.h index eb22979491..1cf9137dac 100644 --- a/src/lib/dhcpsrv/logging.h +++ b/src/lib/dhcpsrv/logging.h @@ -45,7 +45,7 @@ namespace dhcp { /// The data structures don't have to originate from JSON. JSON is just a /// convenient presentation syntax. /// -/// This class uses Configuration structure to store logging configuration. +/// This class uses @c SrvConfig object to store logging configuration. class LogConfigParser { public: diff --git a/src/lib/dhcpsrv/option_space_container.h b/src/lib/dhcpsrv/option_space_container.h index 7d258d6065..abab8cae30 100644 --- a/src/lib/dhcpsrv/option_space_container.h +++ b/src/lib/dhcpsrv/option_space_container.h @@ -74,7 +74,7 @@ public: /// @todo This function is likely to be removed once /// we create a structore of OptionSpaces defined /// through the configuration manager. - std::list getOptionSpaceNames() { + std::list getOptionSpaceNames() const { std::list names; for (typename OptionSpaceMap::const_iterator space = option_space_map_.begin(); diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 4214f8527d..42b6a48a66 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -88,6 +88,8 @@ SrvConfig::copy(SrvConfig& new_config) const { } // Replace interface configuration. new_config.setCfgIface(cfg_iface_); + // Replace option definitions. + new_config.setCfgOptionDef(cfg_option_def_); } void @@ -131,7 +133,8 @@ SrvConfig::equals(const SrvConfig& other) const { } } // Logging information is equal between objects, so check other values. - return (cfg_iface_ == other.cfg_iface_); + return ((cfg_iface_ == other.cfg_iface_) && + (cfg_option_def_ == other.cfg_option_def_)); } } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 313ac53fc3..e156647328 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -16,6 +16,7 @@ #define DHCPSRV_CONFIG_H #include +#include #include #include #include @@ -104,6 +105,13 @@ public: /// @return true if sequence numbers are equal. bool sequenceEquals(const SrvConfig& other); + /// @name Modifiers and accesors for the configuration objects. + /// + /// @warning References to the objects returned by accessors are only + /// valid during the lifetime of the @c SrvConfig object which + /// returned them. + /// + //@{ /// @brief Returns logging specific configuration. const LoggingInfoStorage& getLoggingInfo() const { return (logging_info_); @@ -133,6 +141,25 @@ public: cfg_iface_ = cfg_iface; } + /// @brief Returns object which represents user-defined option definitions. + /// + /// This function returns a reference to the object which represents the + /// user defined option definitions grouped by option space name. + /// + /// @return Reference to an object holding option definitions. + const CfgOptionDef& getCfgOptionDef() const { + return (cfg_option_def_); + } + + /// @brief Sets the object representing user-defined option definitions. + /// + /// @param cfg_option_def New object representing option definitions. + void setCfgOptionDef(const CfgOptionDef& cfg_option_def) { + cfg_option_def_ = cfg_option_def; + } + + //@} + /// @brief Copies the currnet configuration to a new configuration. /// /// This method copies the parameters stored in the configuration to @@ -212,6 +239,12 @@ private: /// queries. CfgIface cfg_iface_; + /// @brief Option definitions configuration. + /// + /// This object holds the user-defined option definitions grouped + /// by option space name. + CfgOptionDef cfg_option_def_; + }; /// @name Pointers to the @c SrvConfig object. diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 5582d28e1b..0f93ddd4ff 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -55,6 +55,8 @@ libdhcpsrv_unittests_SOURCES = run_unittests.cc libdhcpsrv_unittests_SOURCES += addr_utilities_unittest.cc libdhcpsrv_unittests_SOURCES += alloc_engine_unittest.cc libdhcpsrv_unittests_SOURCES += callout_handle_store_unittest.cc +libdhcpsrv_unittests_SOURCES += cfg_iface_unittest.cc +libdhcpsrv_unittests_SOURCES += cfg_option_def_unittest.cc libdhcpsrv_unittests_SOURCES += cfgmgr_unittest.cc libdhcpsrv_unittests_SOURCES += csv_lease_file4_unittest.cc libdhcpsrv_unittests_SOURCES += csv_lease_file6_unittest.cc @@ -62,7 +64,6 @@ libdhcpsrv_unittests_SOURCES += d2_client_unittest.cc libdhcpsrv_unittests_SOURCES += d2_udp_unittest.cc libdhcpsrv_unittests_SOURCES += daemon_unittest.cc libdhcpsrv_unittests_SOURCES += dbaccess_parser_unittest.cc -libdhcpsrv_unittests_SOURCES += cfg_iface_unittest.cc libdhcpsrv_unittests_SOURCES += lease_file_io.cc lease_file_io.h libdhcpsrv_unittests_SOURCES += lease_unittest.cc libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc diff --git a/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc new file mode 100644 index 0000000000..c9dc9f7ee3 --- /dev/null +++ b/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc @@ -0,0 +1,247 @@ +// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include +#include +#include +#include +#include + +using namespace isc; +using namespace isc::dhcp; + +namespace { + +// This test checks that two option definition configurations can be +// compared for equality. +TEST(CfgOptionDefTest, equal) { + CfgOptionDef cfg1; + CfgOptionDef cfg2; + + // Default objects must be equal. + ASSERT_TRUE(cfg1 == cfg2); + ASSERT_FALSE(cfg1 != cfg2); + + // Let's add the same option but to two different option spaces. + cfg1.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 5, "uint16")), "isc"); + cfg2.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 5, "uint16")), "dns"); + // Configurations must be unequal. + ASSERT_FALSE(cfg1 == cfg2); + ASSERT_TRUE(cfg1 != cfg2); + + // Now, let's add them again but to original option spaces. Both objects + // should now contain the same options under two option spaces. The + // order should not matter so configurations should be equal. + cfg1.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 5, "uint16")), "dns"); + cfg2.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 5, "uint16")), "isc"); + + EXPECT_TRUE(cfg1 == cfg2); + EXPECT_FALSE(cfg1 != cfg2); + +} + +// This test verifies that multiple option definitions can be added +// under different option spaces. +TEST(CfgOptionDefTest, getAll) { + CfgOptionDef cfg; + + // Create a set of option definitions with codes between 100 and 109. + for (uint16_t code = 100; code < 110; ++code) { + std::ostringstream option_name; + // Option name is unique, e.g. option-100, option-101 etc. + option_name << "option-" << code; + OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, + "uint16")); + // Add option definition to "isc" option space. + // Option codes are not duplicated so expect no error + // when adding them. + ASSERT_NO_THROW(cfg.add(def, "isc")); + } + + // Create a set of option definitions with codes between 105 and 114 and + // add them to the different option space. + for (uint16_t code = 105; code < 115; ++code) { + std::ostringstream option_name; + option_name << "option-" << code; + OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, + "uint16")); + ASSERT_NO_THROW(cfg.add(def, "abcde")); + } + + // Sanity check that all 10 option definitions are there. + OptionDefContainerPtr option_defs1 = cfg.getAll("isc"); + ASSERT_TRUE(option_defs1); + ASSERT_EQ(10, option_defs1->size()); + + // Iterate over all option definitions and check that they have + // valid codes. Also, their order should be the same as they + // were added (codes 100-109). + uint16_t code = 100; + for (OptionDefContainer::const_iterator it = option_defs1->begin(); + it != option_defs1->end(); ++it, ++code) { + OptionDefinitionPtr def(*it); + ASSERT_TRUE(def); + EXPECT_EQ(code, def->getCode()); + } + + // Sanity check that all 10 option definitions are there. + OptionDefContainerPtr option_defs2 = cfg.getAll("abcde"); + ASSERT_TRUE(option_defs2); + ASSERT_EQ(10, option_defs2->size()); + + // Check that the option codes are valid. + code = 105; + for (OptionDefContainer::const_iterator it = option_defs2->begin(); + it != option_defs2->end(); ++it, ++code) { + OptionDefinitionPtr def(*it); + ASSERT_TRUE(def); + EXPECT_EQ(code, def->getCode()); + } + + // Let's make one more check that the empty set is returned when + // invalid option space is used. + OptionDefContainerPtr option_defs3 = cfg.getAll("non-existing"); + ASSERT_TRUE(option_defs3); + EXPECT_TRUE(option_defs3->empty()); +} + +// This test verifies that single option definition is correctly +// returned with getOptionDef function. +TEST(CfgOptionDefTest, get) { + CfgOptionDef cfg; + // Create a set of option definitions with codes between 100 and 109. + for (uint16_t code = 100; code < 110; ++code) { + std::ostringstream option_name; + // Option name is unique, e.g. option-100, option-101 etc. + option_name << "option-" << code; + OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, + "uint16")); + // Add option definition to "isc" option space. + // Option codes are not duplicated so expect no error + // when adding them. + ASSERT_NO_THROW(cfg.add(def, "isc")); + } + + // Create a set of option definitions with codes between 105 and 114 and + // add them to the different option space. + for (uint16_t code = 105; code < 115; ++code) { + std::ostringstream option_name; + option_name << "option-other-" << code; + OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, + "uint16")); + ASSERT_NO_THROW(cfg.add(def, "abcde")); + } + + // Try to get option definitions one by one using all codes + // that we expect to be there. + for (uint16_t code = 100; code < 110; ++code) { + OptionDefinitionPtr def = cfg.get("isc", code); + ASSERT_TRUE(def); + // Check that the option name is in the format of 'option-[code]'. + // That way we make sure that the options that have the same codes + // within different option spaces are different. + std::ostringstream option_name; + option_name << "option-" << code; + EXPECT_EQ(option_name.str(), def->getName()); + EXPECT_EQ(code, def->getCode()); + } + + // Check that the option codes are valid. + for (uint16_t code = 105; code < 115; ++code) { + OptionDefinitionPtr def = cfg.get("abcde", code); + ASSERT_TRUE(def); + // Check that the option name is in the format of 'option-other-[code]'. + // That way we make sure that the options that have the same codes + // within different option spaces are different. + std::ostringstream option_name; + option_name << "option-other-" << code; + EXPECT_EQ(option_name.str(), def->getName()); + + EXPECT_EQ(code, def->getCode()); + } + + // Check that an option definition can be added to the standard + // (dhcp4 and dhcp6) option spaces when the option code is not + // reserved by the standard option. + OptionDefinitionPtr def6(new OptionDefinition("option-foo", 79, "uint16")); + EXPECT_NO_THROW(cfg.add(def6, DHCP6_OPTION_SPACE)); + + OptionDefinitionPtr def4(new OptionDefinition("option-foo", 222, "uint16")); + EXPECT_NO_THROW(cfg.add(def4, DHCP4_OPTION_SPACE)); + + // Try to query the option definition from an non-existing + // option space and expect NULL pointer. + OptionDefinitionPtr def = cfg.get("non-existing", 56); + EXPECT_FALSE(def); + + // Try to get the non-existing option definition from an + // existing option space. + EXPECT_FALSE(cfg.get("isc", 56)); +} + +// This test verifies that it is not allowed to override a definition of the +// standard option which has its definition defined in libdhcp++, but it is +// allowed to create a definition for the standard option which doesn't have +// its definition in libdhcp++. +TEST(CfgOptionDefTest, overrideStdOptionDef) { + CfgOptionDef cfg; + OptionDefinitionPtr def; + // There is a definition for routers option in libdhcp++, so an attempt + // to add (override) another definition for this option should fail. + def.reset(new OptionDefinition("routers", DHO_ROUTERS, "uint32")); + EXPECT_THROW(cfg.add(def, DHCP4_OPTION_SPACE), isc::BadValue); + + /// @todo There is no definition for the NIS Server Addr option in + /// libdhcp++. Once it is implemented it should be not allowed to + /// add a custom definition for it. At the moment, it should be ok + /// to add a definition for this option (using configuration mechanism) + /// because we haven't implemented the one in libdhcp++. + def.reset(new OptionDefinition("nis-server-addr", 65, "uint16")); + EXPECT_NO_THROW(cfg.add(def, DHCP4_OPTION_SPACE)); + + // It is not allowed to override the definition of the option which + // has its definition in the libdhcp++. + def.reset(new OptionDefinition("sntp-servers", D6O_SNTP_SERVERS, + "ipv4-address")); + EXPECT_THROW(cfg.add(def, DHCP6_OPTION_SPACE), isc::BadValue); + // There is no definition for option 59 in libdhcp++ yet, so it should + // be possible provide a custom definition. + def.reset(new OptionDefinition("bootfile-url", 59, "uint32")); + EXPECT_NO_THROW(cfg.add(def, DHCP6_OPTION_SPACE)); +} + +// This test verifies that the function that adds new option definition +// throws exceptions when arguments are invalid. +TEST(CfgOptionDefTest, addNegative) { + CfgOptionDef cfg; + + OptionDefinitionPtr def(new OptionDefinition("option-foo", 1000, "uint16")); + + // Try empty option space name. + ASSERT_THROW(cfg.add(def, ""), isc::BadValue); + // Try NULL option definition. + ASSERT_THROW(cfg.add(OptionDefinitionPtr(), "isc"), + isc::dhcp::MalformedOptionDefinition); + // Try adding option definition twice and make sure that it + // fails on the second attempt. + ASSERT_NO_THROW(cfg.add(def, "isc")); + EXPECT_THROW(cfg.add(def, "isc"), DuplicateOptionDefinition); +} + +} diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 500bd75867..5b21b11807 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -284,7 +284,6 @@ public: CfgMgr::instance().setVerbose(false); CfgMgr::instance().deleteSubnets4(); CfgMgr::instance().deleteSubnets6(); - CfgMgr::instance().deleteOptionDefs(); CfgMgr::instance().clear(); } @@ -305,196 +304,6 @@ TEST_F(CfgMgrTest, configuration) { EXPECT_TRUE(configuration->getLoggingInfo().empty()); } -// This test verifies that multiple option definitions can be added -// under different option spaces. -TEST_F(CfgMgrTest, getOptionDefs) { - CfgMgr& cfg_mgr = CfgMgr::instance(); - // Create a set of option definitions with codes between 100 and 109. - for (uint16_t code = 100; code < 110; ++code) { - std::ostringstream option_name; - // Option name is unique, e.g. option-100, option-101 etc. - option_name << "option-" << code; - OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, - "uint16")); - // Add option definition to "isc" option space. - // Option codes are not duplicated so expect no error - // when adding them. - ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc")); - } - - // Create a set of option definitions with codes between 105 and 114 and - // add them to the different option space. - for (uint16_t code = 105; code < 115; ++code) { - std::ostringstream option_name; - option_name << "option-" << code; - OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, - "uint16")); - ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "abcde")); - } - - // Sanity check that all 10 option definitions are there. - OptionDefContainerPtr option_defs1 = cfg_mgr.getOptionDefs("isc"); - ASSERT_TRUE(option_defs1); - ASSERT_EQ(10, option_defs1->size()); - - // Iterate over all option definitions and check that they have - // valid codes. Also, their order should be the same as they - // were added (codes 100-109). - uint16_t code = 100; - for (OptionDefContainer::const_iterator it = option_defs1->begin(); - it != option_defs1->end(); ++it, ++code) { - OptionDefinitionPtr def(*it); - ASSERT_TRUE(def); - EXPECT_EQ(code, def->getCode()); - } - - // Sanity check that all 10 option definitions are there. - OptionDefContainerPtr option_defs2 = cfg_mgr.getOptionDefs("abcde"); - ASSERT_TRUE(option_defs2); - ASSERT_EQ(10, option_defs2->size()); - - // Check that the option codes are valid. - code = 105; - for (OptionDefContainer::const_iterator it = option_defs2->begin(); - it != option_defs2->end(); ++it, ++code) { - OptionDefinitionPtr def(*it); - ASSERT_TRUE(def); - EXPECT_EQ(code, def->getCode()); - } - - // Let's make one more check that the empty set is returned when - // invalid option space is used. - OptionDefContainerPtr option_defs3 = cfg_mgr.getOptionDefs("non-existing"); - ASSERT_TRUE(option_defs3); - EXPECT_TRUE(option_defs3->empty()); -} - -// This test verifies that single option definition is correctly -// returned with getOptionDef function. -TEST_F(CfgMgrTest, getOptionDef) { - CfgMgr& cfg_mgr = CfgMgr::instance(); - // Create a set of option definitions with codes between 100 and 109. - for (uint16_t code = 100; code < 110; ++code) { - std::ostringstream option_name; - // Option name is unique, e.g. option-100, option-101 etc. - option_name << "option-" << code; - OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, - "uint16")); - // Add option definition to "isc" option space. - // Option codes are not duplicated so expect no error - // when adding them. - ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc")); - } - - // Create a set of option definitions with codes between 105 and 114 and - // add them to the different option space. - for (uint16_t code = 105; code < 115; ++code) { - std::ostringstream option_name; - option_name << "option-other-" << code; - OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code, - "uint16")); - ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "abcde")); - } - - // Try to get option definitions one by one using all codes - // that we expect to be there. - for (uint16_t code = 100; code < 110; ++code) { - OptionDefinitionPtr def = cfg_mgr.getOptionDef("isc", code); - ASSERT_TRUE(def); - // Check that the option name is in the format of 'option-[code]'. - // That way we make sure that the options that have the same codes - // within different option spaces are different. - std::ostringstream option_name; - option_name << "option-" << code; - EXPECT_EQ(option_name.str(), def->getName()); - EXPECT_EQ(code, def->getCode()); - } - - // Check that the option codes are valid. - for (uint16_t code = 105; code < 115; ++code) { - OptionDefinitionPtr def = cfg_mgr.getOptionDef("abcde", code); - ASSERT_TRUE(def); - // Check that the option name is in the format of 'option-other-[code]'. - // That way we make sure that the options that have the same codes - // within different option spaces are different. - std::ostringstream option_name; - option_name << "option-other-" << code; - EXPECT_EQ(option_name.str(), def->getName()); - - EXPECT_EQ(code, def->getCode()); - } - - // Check that an option definition can be added to the standard - // (dhcp4 and dhcp6) option spaces when the option code is not - // reserved by the standard option. - OptionDefinitionPtr def6(new OptionDefinition("option-foo", 79, "uint16")); - EXPECT_NO_THROW(cfg_mgr.addOptionDef(def6, "dhcp6")); - - OptionDefinitionPtr def4(new OptionDefinition("option-foo", 222, "uint16")); - EXPECT_NO_THROW(cfg_mgr.addOptionDef(def4, "dhcp4")); - - // Try to query the option definition from an non-existing - // option space and expect NULL pointer. - OptionDefinitionPtr def = cfg_mgr.getOptionDef("non-existing", 56); - EXPECT_FALSE(def); - - // Try to get the non-existing option definition from an - // existing option space. - EXPECT_FALSE(cfg_mgr.getOptionDef("isc", 56)); - -} - -// This test verifies that it is not allowed to override a definition of the -// standard option which has its definition defined in libdhcp++, but it is -// allowed to create a definition for the standard option which doesn't have -// its definition in libdhcp++. -TEST_F(CfgMgrTest, overrideStdOptionDef) { - CfgMgr& cfg_mgr = CfgMgr::instance(); - - OptionDefinitionPtr def; - // There is a definition for routers option in libdhcp++, so an attempt - // to add (override) another definition for this option should fail. - def.reset(new OptionDefinition("routers", DHO_ROUTERS, "uint32")); - EXPECT_THROW(cfg_mgr.addOptionDef(def, "dhcp4"), isc::BadValue); - - /// @todo There is no definition for the NIS Server Addr option in - /// libdhcp++. Once it is implemented it should be not allowed to - /// add a custom definition for it. At the moment, it should be ok - /// to add a definition for this option (using configuration mechanism) - /// because we haven't implemented the one in libdhcp++. - def.reset(new OptionDefinition("nis-server-addr", 65, "uint16")); - EXPECT_NO_THROW(cfg_mgr.addOptionDef(def, "dhcp4")); - - // It is not allowed to override the definition of the option which - // has its definition in the libdhcp++. - def.reset(new OptionDefinition("sntp-servers", D6O_SNTP_SERVERS, - "ipv4-address")); - EXPECT_THROW(cfg_mgr.addOptionDef(def, "dhcp6"), isc::BadValue); - // There is no definition for option 59 in libdhcp++ yet, so it should - // be possible provide a custom definition. - def.reset(new OptionDefinition("bootfile-url", 59, "uint32")); - EXPECT_NO_THROW(cfg_mgr.addOptionDef(def, "dhcp6")); - -} - -// This test verifies that the function that adds new option definition -// throws exceptions when arguments are invalid. -TEST_F(CfgMgrTest, addOptionDefNegative) { - CfgMgr& cfg_mgr = CfgMgr::instance(); - - OptionDefinitionPtr def(new OptionDefinition("option-foo", 1000, "uint16")); - - // Try empty option space name. - ASSERT_THROW(cfg_mgr.addOptionDef(def, ""), isc::BadValue); - // Try NULL option definition. - ASSERT_THROW(cfg_mgr.addOptionDef(OptionDefinitionPtr(), "isc"), - isc::dhcp::MalformedOptionDefinition); - // Try adding option definition twice and make sure that it - // fails on the second attempt. - ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc")); - EXPECT_THROW(cfg_mgr.addOptionDef(def, "isc"), DuplicateOptionDefinition); -} - // This test verifies if the configuration manager is able to hold and return // valid subnets. TEST_F(CfgMgrTest, subnet4) { diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index a40332d1ef..d0687166ae 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -312,10 +312,12 @@ public: /// @brief Constructor ParseConfigTest() { reset_context(); + CfgMgr::instance().clear(); } ~ParseConfigTest() { reset_context(); + CfgMgr::instance().clear(); } /// @brief Parses a configuration. @@ -447,33 +449,6 @@ public: return (rcode_); } - /// @brief Find an option definition for a given space and code within - /// the parser context. - /// @param space is the space name of the desired option. - /// @param code is the numeric "type" of the desired option. - /// @return returns an OptionDefinitionPtr which points to the found - /// definition or is empty. - /// ASSERT_ tests don't work inside functions that return values - OptionDefinitionPtr getOptionDef(std::string space, uint32_t code) - { - OptionDefinitionPtr def; - OptionDefContainerPtr defs = - parser_context_->option_defs_->getItems(space); - // Should always be able to get definitions list even if it is empty. - EXPECT_TRUE(defs); - if (defs) { - // Attempt to find desired definiton. - const OptionDefContainerTypeIndex& idx = defs->get<1>(); - const OptionDefContainerTypeRange& range = idx.equal_range(code); - int cnt = std::distance(range.first, range.second); - EXPECT_EQ(1, cnt); - if (cnt == 1) { - def = *(idx.begin()); - } - } - return (def); - } - /// @brief Find an option for a given space and code within the parser /// context. /// @param space is the space name of the desired option. @@ -511,7 +486,6 @@ public: // Note set context universe to V6 as it has to be something. CfgMgr::instance().deleteSubnets4(); CfgMgr::instance().deleteSubnets6(); - CfgMgr::instance().deleteOptionDefs(); parser_context_.reset(new ParserContext(Option::V6)); // Ensure no hooks libraries are loaded. @@ -561,7 +535,8 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { // Verify that the option definition can be retrieved. - OptionDefinitionPtr def = getOptionDef("isc", 100); + OptionDefinitionPtr def = + CfgMgr::instance().getStagingCfg()->getCfgOptionDef().get("isc", 100); ASSERT_TRUE(def); // Verify that the option definition is correct. @@ -1141,21 +1116,6 @@ public: values->getPosition(name).file_)); } - /// @brief Check that option definition storage in the context holds - /// one option definition of the specified type. - /// - /// @param ctx A pointer to a context. - /// @param opt_type Expected option type. - void checkOptionDefinitionType(const ParserContext& ctx, - const uint16_t opt_type) { - OptionDefContainerPtr opt_defs = - ctx.option_defs_->getItems("option-space"); - ASSERT_TRUE(opt_defs); - OptionDefContainerTypeIndex& idx = opt_defs->get<1>(); - OptionDefContainerTypeRange range = idx.equal_range(opt_type); - EXPECT_EQ(1, std::distance(range.first, range.second)); - } - /// @brief Check that option storage in the context holds one option /// of the specified type. /// @@ -1182,7 +1142,6 @@ public: ctx.uint32_values_.reset(); ctx.string_values_.reset(); ctx.options_.reset(); - ctx.option_defs_.reset(); ctx.hooks_libraries_.reset(); // Even if the fields of the context are NULL, it should get @@ -1199,7 +1158,6 @@ public: EXPECT_FALSE(ctx_new->uint32_values_); EXPECT_FALSE(ctx_new->string_values_); EXPECT_FALSE(ctx_new->options_); - EXPECT_FALSE(ctx_new->option_defs_); EXPECT_FALSE(ctx_new->hooks_libraries_); } @@ -1264,11 +1222,6 @@ public: ASSERT_TRUE(desc1.option); ctx.options_->addItem(desc1, option_space); - // Add new option definition, with option code 123. - OptionDefinitionPtr def1(new OptionDefinition("option-foo", 123, - "string")); - ctx.option_defs_->addItem(def1, option_space); - // Allocate container for hooks libraries and add one library name. ctx.hooks_libraries_.reset(new std::vector()); ctx.hooks_libraries_->push_back("library1"); @@ -1356,14 +1309,6 @@ public: checkOptionType(*ctx_new, 10); } - // New context has the same option definition. - ASSERT_TRUE(ctx_new->option_defs_); - { - SCOPED_TRACE("check that the option definition is equal in both" - " contexts"); - checkOptionDefinitionType(*ctx_new, 123); - } - // New context has the same hooks library. ASSERT_TRUE(ctx_new->hooks_libraries_); { @@ -1494,20 +1439,6 @@ public: checkOptionType(*ctx_new, 10); } - // Change the option definition. This should not affect the option - // definition in the new context. - { - SCOPED_TRACE("Check that option definition remains the same in" - " the new context when the option definition in the" - " original context is changed"); - ctx.option_defs_->clearItems(); - OptionDefinitionPtr def(new OptionDefinition("option-foo", 234, - "string")); - - ctx.option_defs_->addItem(def, option_space); - checkOptionDefinitionType(*ctx_new, 123); - } - // Change the list of libraries. this should not affect the list in the // new context. ctx.hooks_libraries_->clear(); diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc index fc2beb739c..fa59f9354e 100644 --- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc +++ b/src/lib/dhcpsrv/tests/srv_config_unittest.cc @@ -279,8 +279,14 @@ TEST_F(SrvConfigTest, copy) { CfgIface cfg_iface; cfg_iface.use(AF_INET, "eth0"); + CfgOptionDef cfg_option_def; + cfg_option_def.add(OptionDefinitionPtr(new OptionDefinition("option-foo", 5, + "string")), + "isc"); + conf1.addLoggingInfo(info); conf1.setCfgIface(cfg_iface); + conf1.setCfgOptionDef(cfg_option_def); // Make sure both configurations are different. ASSERT_TRUE(conf1 != conf2); @@ -338,6 +344,28 @@ TEST_F(SrvConfigTest, equality) { EXPECT_TRUE(conf1 == conf2); EXPECT_FALSE(conf1 != conf2); + + // Differ by option definitions. + CfgOptionDef cfg_option_def1; + CfgOptionDef cfg_option_def2; + + cfg_option_def1.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 123, + "uint16_t")), + "isc"); + conf1.setCfgOptionDef(cfg_option_def1); + + EXPECT_FALSE(conf1 == conf2); + EXPECT_TRUE(conf1 != conf2); + + cfg_option_def2.add(OptionDefinitionPtr(new OptionDefinition("option-foo", + 123, + "uint16_t")), + "isc"); + conf2.setCfgOptionDef(cfg_option_def2); + + EXPECT_TRUE(conf1 == conf2); + EXPECT_FALSE(conf1 != conf2); } } // end of anonymous namespace