diff --git a/doc/sphinx/arm/hooks.rst b/doc/sphinx/arm/hooks.rst index 70af6b5d0a..5f4ec8cadf 100644 --- a/doc/sphinx/arm/hooks.rst +++ b/doc/sphinx/arm/hooks.rst @@ -1856,7 +1856,7 @@ expression. ] } -If (and only if) the query includes a ``host-name`` option (code 12), +If (and only if) the **query** includes a ``host-name`` option (code 12), a ``boot-file-name`` option (code 67) is added to the response with the host name followed by ``.boot`` for content. @@ -1877,6 +1877,9 @@ Since Kea 2.1.4, the ``client-class`` parameter specifies a guard: it takes a client class name, when not empty the entry is skipped if the query does not belong to the class. +Since Kea 2.1.4, it is allowed to have multiple entries for the same option, +but each entry must have exactly one action. + .. _host-cmds: ``host_cmds``: Host Commands diff --git a/src/hooks/dhcp/flex_option/flex_option.cc b/src/hooks/dhcp/flex_option/flex_option.cc index 067a8a6dcb..accad60291 100644 --- a/src/hooks/dhcp/flex_option/flex_option.cc +++ b/src/hooks/dhcp/flex_option/flex_option.cc @@ -204,9 +204,6 @@ FlexOptionImpl::parseOptionConfig(ConstElementPtr option) { } code = def->getCode(); } - if (option_config_map_.count(code)) { - isc_throw(BadValue, "option " << code << " was already specified"); - } bool csv_format = false; if (csv_format_elem) { @@ -250,7 +247,10 @@ FlexOptionImpl::parseOptionConfig(ConstElementPtr option) { isc_throw(BadValue, "no action: " << option->str()); } - option_config_map_[code] = opt_cfg; + // The [] operator creates the item if it does not exist before + // returning a reference to it. + OptionConfigList& opt_lst = option_config_map_[code]; + opt_lst.push_back(opt_cfg); } void diff --git a/src/hooks/dhcp/flex_option/flex_option.h b/src/hooks/dhcp/flex_option/flex_option.h index 7a9a8da53a..a651ca32f6 100644 --- a/src/hooks/dhcp/flex_option/flex_option.h +++ b/src/hooks/dhcp/flex_option/flex_option.h @@ -157,8 +157,11 @@ public: /// @brief The type of shared pointers to option config. typedef boost::shared_ptr OptionConfigPtr; + /// @brief The type of lists of shared pointers to option config. + typedef std::list OptionConfigList; + /// @brief The type of the option config map. - typedef std::map OptionConfigMap; + typedef std::map OptionConfigMap; /// @brief Constructor. FlexOptionImpl(); @@ -189,90 +192,94 @@ public: void process(isc::dhcp::Option::Universe universe, PktType query, PktType response) { for (auto pair : getOptionConfigMap()) { - const OptionConfigPtr& opt_cfg = pair.second; - const isc::dhcp::ClientClass& client_class = opt_cfg->getClass(); - if (!client_class.empty()) { - if (!query->inClass(client_class)) { - logClass(client_class, opt_cfg->getCode()); - continue; + for (const OptionConfigPtr& opt_cfg : pair.second) { + const isc::dhcp::ClientClass& client_class = + opt_cfg->getClass(); + if (!client_class.empty()) { + if (!query->inClass(client_class)) { + logClass(client_class, opt_cfg->getCode()); + continue; + } } - } - std::string value; - isc::dhcp::OptionBuffer buffer; - isc::dhcp::OptionPtr opt = response->getOption(opt_cfg->getCode()); - isc::dhcp::OptionDefinitionPtr def = opt_cfg->getOptionDef(); - switch (opt_cfg->getAction()) { - case NONE: - break; - case ADD: - // Don't add if option is already there. - if (opt) { + std::string value; + isc::dhcp::OptionBuffer buffer; + isc::dhcp::OptionPtr opt = + response->getOption(opt_cfg->getCode()); + isc::dhcp::OptionDefinitionPtr def = opt_cfg->getOptionDef(); + switch (opt_cfg->getAction()) { + case NONE: break; - } - value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), *query); - // Do nothing is the expression evaluates to empty. - if (value.empty()) { - break; - } - // Set the value. - if (def) { - std::vector split_vec = + case ADD: + // Don't add if option is already there. + if (opt) { + break; + } + value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), *query); + // Do nothing is the expression evaluates to empty. + if (value.empty()) { + break; + } + // Set the value. + if (def) { + std::vector split_vec = isc::util::str::tokens(value, ",", true); - opt = def->optionFactory(universe, opt_cfg->getCode(), - split_vec); - } else { - buffer.assign(value.begin(), value.end()); - opt.reset(new isc::dhcp::Option(universe, - opt_cfg->getCode(), - buffer)); - } - // Add the option. - response->addOption(opt); - logAction(ADD, opt_cfg->getCode(), value); - break; - case SUPERSEDE: - // Do nothing is the expression evaluates to empty. - value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), *query); - if (value.empty()) { + opt = def->optionFactory(universe, opt_cfg->getCode(), + split_vec); + } else { + buffer.assign(value.begin(), value.end()); + opt.reset(new isc::dhcp::Option(universe, + opt_cfg->getCode(), + buffer)); + } + // Add the option. + response->addOption(opt); + logAction(ADD, opt_cfg->getCode(), value); break; - } - // Remove the option if already there. - while (opt) { - response->delOption(opt_cfg->getCode()); - opt = response->getOption(opt_cfg->getCode()); - } - // Set the value. - if (def) { - std::vector split_vec = + case SUPERSEDE: + // Do nothing is the expression evaluates to empty. + value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), + *query); + if (value.empty()) { + break; + } + // Remove the option if already there. + while (opt) { + response->delOption(opt_cfg->getCode()); + opt = response->getOption(opt_cfg->getCode()); + } + // Set the value. + if (def) { + std::vector split_vec = isc::util::str::tokens(value, ",", true); - opt = def->optionFactory(universe, opt_cfg->getCode(), - split_vec); - } else { - buffer.assign(value.begin(), value.end()); - opt.reset(new isc::dhcp::Option(universe, - opt_cfg->getCode(), - buffer)); - } - // Add the option. - response->addOption(opt); - logAction(SUPERSEDE, opt_cfg->getCode(), value); - break; - case REMOVE: - // Nothing to remove if option is not present. - if (!opt) { + opt = def->optionFactory(universe, opt_cfg->getCode(), + split_vec); + } else { + buffer.assign(value.begin(), value.end()); + opt.reset(new isc::dhcp::Option(universe, + opt_cfg->getCode(), + buffer)); + } + // Add the option. + response->addOption(opt); + logAction(SUPERSEDE, opt_cfg->getCode(), value); + break; + case REMOVE: + // Nothing to remove if option is not present. + if (!opt) { + break; + } + // Do nothing is the expression evaluates to false. + if (!isc::dhcp::evaluateBool(*opt_cfg->getExpr(), *query)) { + break; + } + // Remove the option. + while (opt) { + response->delOption(opt_cfg->getCode()); + opt = response->getOption(opt_cfg->getCode()); + } + logAction(REMOVE, opt_cfg->getCode(), ""); break; } - // Do nothing is the expression evaluates to false. - if (!isc::dhcp::evaluateBool(*opt_cfg->getExpr(), *query)) { - break; - } - // Remove the option. - while (opt) { - response->delOption(opt_cfg->getCode()); - opt = response->getOption(opt_cfg->getCode()); - } - logAction(REMOVE, opt_cfg->getCode(), ""); - break; } } } diff --git a/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc b/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc index 237688f136..35504ce756 100644 --- a/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc +++ b/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc @@ -281,6 +281,8 @@ TEST_F(FlexOptionTest, optionConfigUnknownCodeNoCSVFormat) { auto map = impl_->getOptionConfigMap(); EXPECT_EQ(1, map.count(109)); + auto opt_lst = map[109]; + EXPECT_EQ(1, opt_lst.size()); } // Verify that the definition is not required when csv-format is false. @@ -299,6 +301,8 @@ TEST_F(FlexOptionTest, optionConfigUnknownCodeDisableCSVFormat) { auto map = impl_->getOptionConfigMap(); EXPECT_EQ(1, map.count(109)); + auto opt_lst = map[109]; + EXPECT_EQ(1, opt_lst.size()); } // Verify that the code must be a known option when csv-format is true. @@ -330,6 +334,8 @@ TEST_F(FlexOptionTest, optionConfigStandardName) { auto map = impl_->getOptionConfigMap(); EXPECT_EQ(1, map.count(DHO_HOST_NAME)); + auto opt_lst = map[DHO_HOST_NAME]; + EXPECT_EQ(1, opt_lst.size()); } // Verify that the name can be an user defined option. @@ -352,6 +358,8 @@ TEST_F(FlexOptionTest, optionConfigDefinedName) { auto map = impl_->getOptionConfigMap(); EXPECT_EQ(1, map.count(222)); + auto opt_lst = map[222]; + EXPECT_EQ(1, opt_lst.size()); } // Last resort is only option 43... @@ -386,7 +394,7 @@ TEST_F(FlexOptionTest, optionConfigBadCSVFormat) { EXPECT_EQ("'csv-format' must be a boolean: 123", impl_->getErrMsg()); } -// Verify that an option can be configured only once. +// Verify that an option can be configured more than once. TEST_F(FlexOptionTest, optionConfigTwice) { ElementPtr options = Element::createList(); ElementPtr option = Element::createMap(); @@ -396,9 +404,15 @@ TEST_F(FlexOptionTest, optionConfigTwice) { ElementPtr code = Element::create(DHO_HOST_NAME); option->set("code", code); + // Add it a second time. options->add(option); - EXPECT_THROW(impl_->testConfigure(options), BadValue); - EXPECT_EQ("option 12 was already specified", impl_->getErrMsg()); + EXPECT_NO_THROW(impl_->testConfigure(options)); + EXPECT_TRUE(impl_->getErrMsg().empty()); + + auto map = impl_->getOptionConfigMap(); + EXPECT_EQ(1, map.count(DHO_HOST_NAME)); + auto opt_lst = map[DHO_HOST_NAME]; + EXPECT_EQ(2, opt_lst.size()); } // Verify that the add value must be a string. @@ -456,8 +470,13 @@ TEST_F(FlexOptionTest, optionConfigAdd4) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(DHO_HOST_NAME)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(DHO_HOST_NAME)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(DHO_HOST_NAME, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::ADD, opt_cfg->getAction()); @@ -488,8 +507,13 @@ TEST_F(FlexOptionTest, optionConfigAdd6) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(D6O_BOOTFILE_URL)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(D6O_BOOTFILE_URL)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(D6O_BOOTFILE_URL, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::ADD, opt_cfg->getAction()); @@ -560,8 +584,13 @@ TEST_F(FlexOptionTest, optionConfigSupersede4) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(DHO_HOST_NAME)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(DHO_HOST_NAME)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(DHO_HOST_NAME, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::SUPERSEDE, opt_cfg->getAction()); @@ -592,8 +621,13 @@ TEST_F(FlexOptionTest, optionConfigSupersede6) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(D6O_BOOTFILE_URL)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(D6O_BOOTFILE_URL)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(D6O_BOOTFILE_URL, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::SUPERSEDE, opt_cfg->getAction()); @@ -664,8 +698,13 @@ TEST_F(FlexOptionTest, optionConfigRemove4) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(DHO_HOST_NAME)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(DHO_HOST_NAME)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(DHO_HOST_NAME, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::REMOVE, opt_cfg->getAction()); @@ -701,8 +740,13 @@ TEST_F(FlexOptionTest, optionConfigRemove6) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(D6O_BOOTFILE_URL)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(D6O_BOOTFILE_URL)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(D6O_BOOTFILE_URL, opt_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::REMOVE, opt_cfg->getAction()); @@ -783,15 +827,25 @@ TEST_F(FlexOptionTest, optionConfigList) { auto map = impl_->getOptionConfigMap(); EXPECT_EQ(2, map.size()); + FlexOptionImpl::OptionConfigList opt1_lst; + ASSERT_NO_THROW(opt1_lst = map.at(DHO_HOST_NAME)); + ASSERT_FALSE(opt1_lst.empty()); + EXPECT_EQ(1, opt1_lst.size()); FlexOptionImpl::OptionConfigPtr opt1_cfg; - ASSERT_NO_THROW(opt1_cfg = map.at(DHO_HOST_NAME)); + ASSERT_NO_THROW(opt1_cfg = opt1_lst.front()); + ASSERT_TRUE(opt1_cfg); EXPECT_EQ(DHO_HOST_NAME, opt1_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::ADD, opt1_cfg->getAction()); EXPECT_EQ("'abc'", opt1_cfg->getText()); + FlexOptionImpl::OptionConfigList opt2_lst; + ASSERT_NO_THROW(opt2_lst = map.at(DHO_ROOT_PATH)); + ASSERT_FALSE(opt2_lst.empty()); + EXPECT_EQ(1, opt2_lst.size()); FlexOptionImpl::OptionConfigPtr opt2_cfg; - ASSERT_NO_THROW(opt2_cfg = map.at(DHO_ROOT_PATH)); + ASSERT_NO_THROW(opt2_cfg = opt2_lst.front()); + ASSERT_TRUE(opt2_cfg); EXPECT_EQ(DHO_ROOT_PATH, opt2_cfg->getCode()); EXPECT_EQ(FlexOptionImpl::SUPERSEDE, opt2_cfg->getAction()); @@ -818,7 +872,8 @@ TEST_F(FlexOptionTest, processNone) { opt_cfg(new FlexOptionImpl::OptionConfig(D6O_BOOTFILE_URL, def)); EXPECT_EQ(FlexOptionImpl::NONE, opt_cfg->getAction()); auto map = impl_->getMutableOptionConfigMap(); - map[DHO_HOST_NAME] = opt_cfg; + auto& opt_lst = map[DHO_HOST_NAME]; + opt_lst.push_back(opt_cfg); Pkt6Ptr query(new Pkt6(DHCPV6_SOLICIT, 12345)); Pkt6Ptr response(new Pkt6(DHCPV6_ADVERTISE, 12345)); @@ -1163,6 +1218,77 @@ TEST_F(FlexOptionTest, processSupersedeEmpty) { EXPECT_EQ(0, memcmp(&buffer[0], "abc", 3)); } +// Verify that SUPERSEDE if exists + ADD adds a not yet existing option. +TEST_F(FlexOptionTest, processSupersedeAddNotExisting) { + CfgMgr::instance().setFamily(AF_INET6); + + ElementPtr options = Element::createList(); + ElementPtr option1 = Element::createMap(); + options->add(option1); + ElementPtr code = Element::create(D6O_BOOTFILE_URL); + option1->set("code", code); + string action = "ifelse(option[bootfile-url].exists,'supersede','')"; + ElementPtr supersede = Element::create(action); + option1->set("supersede", supersede); + ElementPtr option2 = Element::createMap(); + options->add(option2); + option2->set("code", code); + ElementPtr add = Element::create(string("'add'")); + option2->set("add", add); + + EXPECT_NO_THROW(impl_->testConfigure(options)); + EXPECT_TRUE(impl_->getErrMsg().empty()); + + Pkt6Ptr query(new Pkt6(DHCPV6_SOLICIT, 12345)); + Pkt6Ptr response(new Pkt6(DHCPV6_ADVERTISE, 12345)); + EXPECT_FALSE(response->getOption(D6O_BOOTFILE_URL)); + + EXPECT_NO_THROW(impl_->process(Option::V6, query, response)); + + OptionPtr opt = response->getOption(D6O_BOOTFILE_URL); + ASSERT_TRUE(opt); + EXPECT_EQ(D6O_BOOTFILE_URL, opt->getType()); + const OptionBuffer& buffer = opt->getData(); + ASSERT_EQ(3, buffer.size()); + EXPECT_EQ(0, memcmp(&buffer[0], "add", 3)); +} + +// Verify that SUPERSEDE if exists + ADD supersedes an existing option. +TEST_F(FlexOptionTest, processSupersedeAddExisting) { + ElementPtr options = Element::createList(); + ElementPtr option1 = Element::createMap(); + options->add(option1); + ElementPtr code = Element::create(DHO_HOST_NAME); + option1->set("code", code); + string action = "ifelse(option[host-name].exists,'supersede','')"; + ElementPtr supersede = Element::create(action); + option1->set("supersede", supersede); + ElementPtr option2 = Element::createMap(); + options->add(option2); + option2->set("code", code); + ElementPtr add = Element::create(string("'add'")); + option2->set("add", add); + + EXPECT_NO_THROW(impl_->testConfigure(options)); + EXPECT_TRUE(impl_->getErrMsg().empty()); + + Pkt4Ptr query(new Pkt4(DHCPDISCOVER, 12345)); + Pkt4Ptr response(new Pkt4(DHCPOFFER, 12345)); + OptionStringPtr str(new OptionString(Option::V4, DHO_HOST_NAME, "foobar")); + // Be careful here: the expression is related to the query. + query->addOption(str); + response->addOption(str); + + EXPECT_NO_THROW(impl_->process(Option::V4, query, response)); + + OptionPtr opt = response->getOption(DHO_HOST_NAME); + ASSERT_TRUE(opt); + EXPECT_EQ(DHO_HOST_NAME, opt->getType()); + const OptionBuffer& buffer = opt->getData(); + ASSERT_EQ(9, buffer.size()); + EXPECT_EQ(0, memcmp(&buffer[0], "supersede", 9)); +} + // Verify that REMOVE action removes an already existing option. TEST_F(FlexOptionTest, processRemove) { CfgMgr::instance().setFamily(AF_INET6); @@ -1364,8 +1490,13 @@ TEST_F(FlexOptionTest, optionConfigGuardValid) { EXPECT_TRUE(impl_->getErrMsg().empty()); auto map = impl_->getOptionConfigMap(); + FlexOptionImpl::OptionConfigList opt_lst; + ASSERT_NO_THROW(opt_lst = map.at(DHO_HOST_NAME)); + ASSERT_FALSE(opt_lst.empty()); + EXPECT_EQ(1, opt_lst.size()); FlexOptionImpl::OptionConfigPtr opt_cfg; - ASSERT_NO_THROW(opt_cfg = map.at(DHO_HOST_NAME)); + ASSERT_NO_THROW(opt_cfg = opt_lst.front()); + ASSERT_TRUE(opt_cfg); EXPECT_EQ(DHO_HOST_NAME, opt_cfg->getCode()); EXPECT_EQ("foobar", opt_cfg->getClass());