From f3e4bc82282abdf4b091af11c3c8f3e370f8ef1e Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 15 Mar 2016 11:01:09 +0100 Subject: [PATCH 1/5] [4319] Extend host reservation parser to parse options. --- src/lib/dhcpsrv/cfg_hosts.cc | 7 +- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 5 +- .../parsers/host_reservation_parser.cc | 50 +++- .../tests/host_reservation_parser_unittest.cc | 244 +++++++++++++++++- 4 files changed, 288 insertions(+), 18 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index 877c182f60..cfb2bedbcb 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/src/lib/dhcpsrv/cfg_hosts.cc @@ -454,7 +454,9 @@ CfgHosts::add4(const HostPtr& host) { // address, IPv6 address or prefix. if (host->getHostname().empty() && (host->getIPv4Reservation().isV4Zero()) && - (!host->hasIPv6Reservation())) { + (!host->hasIPv6Reservation()) && + host->getCfgOption4()->empty() && + host->getCfgOption6()->empty()) { std::ostringstream s; if (hwaddr) { s << "for DUID: " << hwaddr->toText(); @@ -463,7 +465,8 @@ CfgHosts::add4(const HostPtr& host) { } isc_throw(BadValue, "specified reservation " << s.str() << " must include at least one resource, i.e. " - "hostname, IPv4 address or IPv6 address/prefix"); + "hostname, IPv4 address, IPv6 address/prefix, " + "options"); } // Check for duplicates for the specified IPv4 subnet. diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index c34c578141..0b1fced8b4 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -608,8 +608,9 @@ OptionDataParser::createOption(ConstElementPtr option_data) { } else if (name_param.isSpecified() && !code_param.isSpecified()) { isc_throw(DhcpConfigError, "definition for the option '" << space_param << "." << name_param - << " does not exist (" - << string_values_->getPosition("name", option_data)); + << "' does not exist (" + << string_values_->getPosition("name", option_data) + << ")"); } } diff --git a/src/lib/dhcpsrv/parsers/host_reservation_parser.cc b/src/lib/dhcpsrv/parsers/host_reservation_parser.cc index 93a51bb88e..32d2e3fa29 100644 --- a/src/lib/dhcpsrv/parsers/host_reservation_parser.cc +++ b/src/lib/dhcpsrv/parsers/host_reservation_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,9 +7,11 @@ #include #include #include +#include #include #include #include +#include #include using namespace isc::asiolink; @@ -25,7 +27,8 @@ const std::set& getSupportedParams4() { static std::set params_set; if (params_set.empty()) { const char* params[] = { - "duid", "hw-address", "hostname", "ip-address", NULL + "duid", "hw-address", "hostname", "ip-address", + "option-data", NULL }; for (int i = 0; params[i] != NULL; ++i) { params_set.insert(std::string(params[i])); @@ -42,7 +45,8 @@ const std::set& getSupportedParams6() { static std::set params_set; if (params_set.empty()) { const char* params[] = { - "duid", "hw-address", "hostname", "ip-addresses", "prefixes", NULL + "duid", "hw-address", "hostname", "ip-addresses", "prefixes", + "option-data", NULL }; for (int i = 0; params[i] != NULL; ++i) { params_set.insert(std::string(params[i])); @@ -136,17 +140,28 @@ HostReservationParser4::build(isc::data::ConstElementPtr reservation_data) { host_->setIPv4SubnetID(subnet_id_); BOOST_FOREACH(ConfigPair element, reservation_data->mapValue()) { - try { - if (element.first == "ip-address") { - host_->setIPv4Reservation(IOAddress(element.second-> - stringValue())); + // For 'option-data' element we will use another parser which + // already returns errors with position appended, so don't + // surround it with try-catch. + if (element.first == "option-data") { + CfgOptionPtr cfg_option = host_->getCfgOption4(); + OptionDataListParser parser(element.first, cfg_option, AF_INET); + parser.build(element.second); + + // Everything else should be surrounded with try-catch to append + // position. + } else { + try { + if (element.first == "ip-address") { + host_->setIPv4Reservation(IOAddress(element.second-> + stringValue())); + } + } catch (const std::exception& ex) { + // Append line number where the error occurred. + isc_throw(DhcpConfigError, ex.what() << " (" + << reservation_data->getPosition() << ")"); } } - catch (const std::exception& ex) { - // Append line number where the error occurred. - isc_throw(DhcpConfigError, ex.what() << " (" - << reservation_data->getPosition() << ")"); - } } addHost(reservation_data); @@ -168,7 +183,16 @@ HostReservationParser6::build(isc::data::ConstElementPtr reservation_data) { host_->setIPv6SubnetID(subnet_id_); BOOST_FOREACH(ConfigPair element, reservation_data->mapValue()) { - if (element.first == "ip-addresses" || element.first == "prefixes") { + // Parse option values. Note that the configuration option parser + // returns errors with position information appended, so there is no + // need to surround it with try-clause (and rethrow with position + // appended). + if (element.first == "option-data") { + CfgOptionPtr cfg_option = host_->getCfgOption6(); + OptionDataListParser parser(element.first, cfg_option, AF_INET6); + parser.build(element.second); + + } else if (element.first == "ip-addresses" || element.first == "prefixes") { BOOST_FOREACH(ConstElementPtr prefix_element, element.second->listValue()) { try { diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc index 533e7ec493..ac50416a00 100644 --- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -10,10 +10,14 @@ #include #include #include +#include +#include +#include #include #include #include #include +#include #include #include #include @@ -54,10 +58,86 @@ protected: return (false); } + /// @brief Retrieves DHCP option from a host. + /// + /// @param host Reference to a host object for which an option should be + /// retrieved. + /// @param option_space Option space name. + /// @param option_code Code of an option to be retrieved. + /// + /// @return Pointer to the option retrieved or NULL if option hasn't been + /// found. + OptionPtr + retrieveOption(const Host& host, const std::string& option_space, + const uint16_t option_code) const { + if ((option_space != "dhcp6") && (option_space != "dhcp4")) { + return (OptionPtr()); + } + + // Retrieve a pointer to the appropriate container depending if we're + // interested in DHCPv4 or DHCPv6 options. + ConstCfgOptionPtr cfg_option = (option_space == "dhcp4" ? + host.getCfgOption4() : host.getCfgOption6()); + + // Retrieve options. + OptionContainerPtr options = cfg_option->getAll(option_space); + if (options) { + const OptionContainerTypeIndex& idx = options->get<1>(); + OptionContainerTypeIndex::const_iterator it = idx.find(option_code); + if (it != idx.end()) { + return (it->option_); + } + } + + return (OptionPtr()); + } + void expectFailure(const HostReservationParser& parser, const std::string& config) const; + /// @brief This test verifies that it is possible to specify an empty list + /// of options for a host. + /// + /// @tparam ParserType Type of the parser to be tested. + template + void testEmptyOptionList() const { + // Create configuration with empty option list. Note that we have to + // add reservation for at least one resource because host declarations + // without any reservations are rejected. Thus, we have added hostname. + std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + "\"hostname\": \"foo.isc.org\"," + "\"option-data\": [ ]" + "}"; + + ElementPtr config_element = Element::fromJSON(config); + + ParserType parser(SubnetID(10)); + ASSERT_NO_THROW(parser.build(config_element)); + + // Retrieve a host. + HostCollection hosts; + CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts(); + ASSERT_NO_THROW(hosts = cfg_hosts->getAll(HWAddrPtr(), duid_)); + ASSERT_EQ(1, hosts.size()); + + // There should be no options assigned to a host. + EXPECT_TRUE(hosts[0]->getCfgOption4()->empty()); + EXPECT_TRUE(hosts[0]->getCfgOption6()->empty()); + } + + /// @brief This test verfies that the parser returns an error when + /// configuration is invalid. + /// + /// @param config JSON configuration to be tested. + /// @tparam ParserType Type of the parser class to use. + template + void testInvalidConfig(const std::string& config) const { + ElementPtr config_element = Element::fromJSON(config); + ParserType parser(SubnetID(10)); + EXPECT_THROW(parser.build(config_element), DhcpConfigError); + } + /// @brief HW Address object used by tests. HWAddrPtr hwaddr_; @@ -508,4 +588,166 @@ TEST_F(HostReservationParserTest, dhcp6invalidParameterName) { EXPECT_THROW(parser.build(config_element), DhcpConfigError); } +// This test verifies that it is possible to specify DHCPv4 options for +// a host. +TEST_F(HostReservationParserTest, options4) { + // Create configuration with three options for a host. + std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\"," + "\"option-data\": [" + "{" + "\"name\": \"name-servers\"," + "\"data\": \"172.16.15.10, 172.16.15.20\"" + "}," + "{" + "\"name\": \"log-servers\"," + "\"code\": 7," + "\"csv-format\": true," + "\"space\": \"dhcp4\"," + "\"data\": \"172.16.15.23\"" + "}," + "{" + "\"name\": \"default-ip-ttl\"," + "\"data\": \"64\"" + "} ]" + "}"; + + ElementPtr config_element = Element::fromJSON(config); + + HostReservationParser4 parser(SubnetID(10)); + ASSERT_NO_THROW(parser.build(config_element)); + + CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts(); + HostCollection hosts; + ASSERT_NO_THROW(hosts = cfg_hosts->getAll(hwaddr_)); + ASSERT_EQ(1, hosts.size()); + + // Retrieve and sanity check name servers. + Option4AddrLstPtr opt_dns = boost::dynamic_pointer_cast< + Option4AddrLst>(retrieveOption(*hosts[0], "dhcp4", DHO_NAME_SERVERS)); + ASSERT_TRUE(opt_dns); + Option4AddrLst::AddressContainer dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(2, dns_addrs.size()); + EXPECT_EQ("172.16.15.10", dns_addrs[0].toText()); + EXPECT_EQ("172.16.15.20", dns_addrs[1].toText()); + + // Retrieve and sanity check log servers. + Option4AddrLstPtr opt_log = boost::dynamic_pointer_cast< + Option4AddrLst>(retrieveOption(*hosts[0], "dhcp4", DHO_LOG_SERVERS)); + ASSERT_TRUE(opt_log); + Option4AddrLst::AddressContainer log_addrs = opt_log->getAddresses(); + ASSERT_EQ(1, log_addrs.size()); + EXPECT_EQ("172.16.15.23", log_addrs[0].toText()); + + // Retrieve and sanity check default IP TTL. + OptionUint8Ptr opt_ttl = boost::dynamic_pointer_cast< + OptionUint8>(retrieveOption(*hosts[0], "dhcp4", DHO_DEFAULT_IP_TTL)); + ASSERT_TRUE(opt_ttl); + EXPECT_EQ(64, opt_ttl->getValue()); +} + +// This test verifies that it is possible to specify DHCPv6 options for +// a host. +TEST_F(HostReservationParserTest, options6) { + // Create configuration with three options for a host. + std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + "\"option-data\": [" + "{" + "\"name\": \"dns-servers\"," + "\"data\": \"2001:db8:1::1, 2001:db8:1::2\"" + "}," + "{" + "\"name\": \"nis-servers\"," + "\"code\": 27," + "\"csv-format\": true," + "\"space\": \"dhcp6\"," + "\"data\": \"2001:db8:1::1204\"" + "}," + "{" + "\"name\": \"preference\"," + "\"data\": \"11\"" + "} ]" + "}"; + + ElementPtr config_element = Element::fromJSON(config); + + HostReservationParser6 parser(SubnetID(10)); + ASSERT_NO_THROW(parser.build(config_element)); + + // One host should have been added to the configuration. + CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts(); + HostCollection hosts; + ASSERT_NO_THROW(hosts = cfg_hosts->getAll(HWAddrPtr(), duid_)); + ASSERT_EQ(1, hosts.size()); + + // Retrieve and sanity check DNS servers option. + Option6AddrLstPtr opt_dns = boost::dynamic_pointer_cast< + Option6AddrLst>(retrieveOption(*hosts[0], "dhcp6", D6O_NAME_SERVERS)); + ASSERT_TRUE(opt_dns); + Option6AddrLst::AddressContainer dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(2, dns_addrs.size()); + EXPECT_EQ("2001:db8:1::1", dns_addrs[0].toText()); + EXPECT_EQ("2001:db8:1::2", dns_addrs[1].toText()); + + // Retrieve and sanity check NIS servers option. + Option6AddrLstPtr opt_nis = boost::dynamic_pointer_cast< + Option6AddrLst>(retrieveOption(*hosts[0], "dhcp6", D6O_NIS_SERVERS)); + ASSERT_TRUE(opt_nis); + Option6AddrLst::AddressContainer nis_addrs = opt_nis->getAddresses(); + ASSERT_EQ(1, nis_addrs.size()); + EXPECT_EQ("2001:db8:1::1204", nis_addrs[0].toText()); + + // Retrieve and sanity check preference option. + OptionUint8Ptr opt_prf = boost::dynamic_pointer_cast< + OptionUint8>(retrieveOption(*hosts[0], "dhcp6", D6O_PREFERENCE)); + ASSERT_TRUE(opt_prf); + EXPECT_EQ(11, opt_prf->getValue()); +} + +// This test verifies that it is possible to specify an empty list of +// options for a host declaration. +TEST_F(HostReservationParserTest, options4Empty) { + testEmptyOptionList(); +} + +// This test verifies that it is possible to specify an empty list of +// options for a host declaration. +TEST_F(HostReservationParserTest, options6Empty) { + testEmptyOptionList(); +} + +// This test checks that specifying DHCPv6 options for the DHCPv4 host +// reservation parser is not allowed. +TEST_F(HostReservationParserTest, options4InvalidOptionSpace) { + // Create configuration specifying DHCPv6 option for a DHCPv4 host + // reservation parser. + std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + "\"option-data\": [" + "{" + "\"name\": \"dns-servers\"," + "\"space\": \"dhcp6\"," + "\"data\": \"2001:db8:1::1, 2001:db8:1::2\"" + "} ]" + "}"; + + testInvalidConfig(config); +} + +// This test checks that specifying DHCPv4 options for the DHCPv6 host +// reservation parser is not allowed. +TEST_F(HostReservationParserTest, options6InvalidOptionSpace) { + // Create configuration specifying DHCPv4 option for a DHCPv6 host + // reservation parser. + std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + "\"option-data\": [" + "{" + "\"name\": \"name-servers\"," + "\"space\": \"dhcp4\"," + "\"data\": \"172.16.15.10, 172.16.15.20\"" + "} ]" + "}"; + + testInvalidConfig(config); +} + + } // end of anonymous namespace From 99e0bdcc17997ebb0a0dfda27a7651ab93425ed7 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 15 Mar 2016 12:56:15 +0100 Subject: [PATCH 2/5] [4319] Added new config parser unit test for DHCPv4 and DHCPv6. These tests check that it is possible to specify options for hosts. --- src/bin/dhcp4/tests/config_parser_unittest.cc | 79 ++++++++++++++++++- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 6e517dd2f9..e14dfc244a 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -463,6 +463,27 @@ public: CfgMgr::instance().clear(); } + /// @brief Retrieve an option associated with a host. + /// + /// @param host Reference to a host for which an option should be retrieved. + /// @param option_code Option code. + /// @tparam ReturnType Type of the pointer object returned. + /// + /// @return Pointer to an option or NULL if not found. + template + ReturnType + retrieveOption(const Host& host, const uint16_t option_code) const { + ConstCfgOptionPtr cfg_option = host.getCfgOption4(); + if (cfg_option) { + OptionDescriptor opt_desc = cfg_option->get(std::string("dhcp4"), + option_code); + if (opt_desc.option_) { + return (boost::dynamic_pointer_cast< + typename ReturnType::element_type>(opt_desc.option_)); + } + } + return (ReturnType()); + } boost::scoped_ptr srv_; ///< DHCP4 server under test int rcode_; ///< Return code from element parsing @@ -3364,12 +3385,32 @@ TEST_F(Dhcp4ParserTest, reservations) { " {" " \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," " \"ip-address\": \"192.0.3.112\"," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"name-servers\"," + " \"data\": \"192.0.3.15\"" + " }," + " {" + " \"name\": \"default-ip-ttl\"," + " \"data\": \"32\"" + " }" + " ]" " }," " {" " \"hw-address\": \"01:02:03:04:05:06\"," " \"ip-address\": \"192.0.3.120\"," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"name-servers\"," + " \"data\": \"192.0.3.95\"" + " }," + " {" + " \"name\": \"default-ip-ttl\"," + " \"data\": \"11\"" + " }" + " ]" " }" " ]," " \"pools\": [ { \"pool\": \"192.0.3.101 - 192.0.3.150\" } ]," @@ -3384,7 +3425,17 @@ TEST_F(Dhcp4ParserTest, reservations) { " {" " \"duid\": \"0A:09:08:07:06:05:04:03:02:01\"," " \"ip-address\": \"192.0.4.101\"," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"name-servers\"," + " \"data\": \"192.0.4.11\"" + " }," + " {" + " \"name\": \"default-ip-ttl\"," + " \"data\": \"95\"" + " }" + " ]" " }," " {" " \"hw-address\": \"06:05:04:03:02:01\"," @@ -3428,6 +3479,17 @@ TEST_F(Dhcp4ParserTest, reservations) { // and not to other two. EXPECT_FALSE(hosts_cfg->get4(123, hwaddr)); EXPECT_FALSE(hosts_cfg->get4(542, hwaddr)); + // Check that options are assigned correctly. + Option4AddrLstPtr opt_dns = + retrieveOption(*host, DHO_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + Option4AddrLst::AddressContainer dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("192.0.3.95", dns_addrs[0].toText()); + OptionUint8Ptr opt_ttl = + retrieveOption(*host, DHO_DEFAULT_IP_TTL); + ASSERT_TRUE(opt_ttl); + EXPECT_EQ(11, static_cast(opt_ttl->getValue())); // Do the same test for the DUID based reservation. std::vector duid_vec; @@ -3440,6 +3502,15 @@ TEST_F(Dhcp4ParserTest, reservations) { EXPECT_EQ("192.0.3.112", host->getIPv4Reservation().toText()); EXPECT_FALSE(hosts_cfg->get4(123, HWAddrPtr(), duid)); EXPECT_FALSE(hosts_cfg->get4(542, HWAddrPtr(), duid)); + // Check that options are assigned correctly. + opt_dns = retrieveOption(*host, DHO_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("192.0.3.15", dns_addrs[0].toText()); + opt_ttl = retrieveOption(*host, DHO_DEFAULT_IP_TTL); + ASSERT_TRUE(opt_ttl); + EXPECT_EQ(32, static_cast(opt_ttl->getValue())); // The HW address used for one of the reservations in the subnet 542 // consists of numbers from 6 to 1. So, let's just reverse the order From 72f841e8a102e8674c984122597b00d7a4b67f97 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 15 Mar 2016 15:41:32 +0100 Subject: [PATCH 3/5] [4319] Added additional configuration parser tests. New test check that it is possible to specify a non-standard option within a host declaration. --- src/bin/dhcp4/tests/config_parser_unittest.cc | 126 ++++++++++- src/bin/dhcp6/tests/config_parser_unittest.cc | 208 +++++++++++++++++- 2 files changed, 328 insertions(+), 6 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index e14dfc244a..04de12ffdf 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -465,6 +465,8 @@ public: /// @brief Retrieve an option associated with a host. /// + /// The option is retrieved from the "dhcp4" option space. + /// /// @param host Reference to a host for which an option should be retrieved. /// @param option_code Option code. /// @tparam ReturnType Type of the pointer object returned. @@ -473,10 +475,24 @@ public: template ReturnType retrieveOption(const Host& host, const uint16_t option_code) const { + return (retrieveOption(host, "dhcp4", option_code)); + } + + /// @brief Retrieve an option associated with a host. + /// + /// @param host Reference to a host for which an option should be retrieved. + /// @param space Option space from which option should be retrieved. + /// @param option_code Option code. + /// @tparam ReturnType Type of the pointer object returned. + /// + /// @return Pointer to an option or NULL if not found. + template + ReturnType + retrieveOption(const Host& host, const std::string& space, + const uint16_t option_code) const { ConstCfgOptionPtr cfg_option = host.getCfgOption4(); if (cfg_option) { - OptionDescriptor opt_desc = cfg_option->get(std::string("dhcp4"), - option_code); + OptionDescriptor opt_desc = cfg_option->get(space, option_code); if (opt_desc.option_) { return (boost::dynamic_pointer_cast< typename ReturnType::element_type>(opt_desc.option_)); @@ -3531,7 +3547,82 @@ TEST_F(Dhcp4ParserTest, reservations) { EXPECT_EQ("192.0.4.101", host->getIPv4Reservation().toText()); EXPECT_FALSE(hosts_cfg->get4(123, HWAddrPtr(), duid)); EXPECT_FALSE(hosts_cfg->get4(234, HWAddrPtr(), duid)); + // Check that options are assigned correctly. + opt_dns = retrieveOption(*host, DHO_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("192.0.4.11", dns_addrs[0].toText()); + opt_ttl = retrieveOption(*host, DHO_DEFAULT_IP_TTL); + ASSERT_TRUE(opt_ttl); + EXPECT_EQ(95, static_cast(opt_ttl->getValue())); +} +// This test checks that it is possible to configure option data for a +// host using a user defined option format. +TEST_F(Dhcp4ParserTest, reservationWithOptionDefinition) { + ConstElementPtr x; + // The following configuration contains host declaration in which + // a non-standard option is used. This option has option definition + // specified in the configuration. + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 100," + " \"type\": \"uint32\"," + " \"space\": \"isc\"" + "} ]," + "\"subnet4\": [ " + " {" + " \"reservations\": [" + " {" + " \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + " \"ip-address\": \"192.0.3.112\"," + " \"option-data\": [" + " {" + " \"name\": \"foo\"," + " \"data\": \"123\"," + " \"space\": \"isc\"" + " }" + " ]" + " }," + " ]," + " \"pools\": [ { \"pool\": \"192.0.3.101 - 192.0.3.150\" } ]," + " \"subnet\": \"192.0.3.0/24\", " + " \"id\": 234" + " } ]," + "\"valid-lifetime\": 4000" + "}"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); + checkResult(x, 0); + + // Hosts configuration must be available. + CfgHostsPtr hosts_cfg = CfgMgr::instance().getStagingCfg()->getCfgHosts(); + ASSERT_TRUE(hosts_cfg); + + // Let's create an object holding DUID of the host. For simplicity the + // address is a collection of numbers from 1 to A. + std::vector duid_vec; + for (int i = 1; i < 0xB; ++i) { + duid_vec.push_back(static_cast(i)); + } + DuidPtr duid(new DUID(duid_vec)); + // Retrieve the reservation and sanity check the address reserved. + ConstHostPtr host = hosts_cfg->get4(234, HWAddrPtr(), duid); + ASSERT_TRUE(host); + EXPECT_EQ("192.0.3.112", host->getIPv4Reservation().toText()); + + // Check if the option has been parsed. + OptionUint32Ptr opt_foo = retrieveOption(*host, "isc", + 100); + ASSERT_TRUE(opt_foo); + EXPECT_EQ(100, opt_foo->getType()); + EXPECT_EQ(123, opt_foo->getValue()); } // This test verifies that the bogus host reservation would trigger a @@ -3616,6 +3707,37 @@ TEST_F(Dhcp4ParserTest, reservationBogus) { EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); checkResult(x, 1); + + // Case 4: Broken specification of option data. + config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ " + " { " + " \"pools\": [ { \"pool\": \"192.0.4.101 - 192.0.4.150\" } ]," + " \"subnet\": \"192.0.4.0/24\"," + " \"id\": 542," + " \"reservations\": [" + " {" + " \"hw-address\": \"06:05:04:03:02:01\"," + " \"option-data\": [" + " {" + " \"name\": \"name-servers\"," + " \"data\": \"bogus-ip-address\"" + " }" + " ]" + " }" + " ]" + " } ]," + "\"valid-lifetime\": 4000 }"; + + json = Element::fromJSON(config); + + // Remove existing configuration, if any. + CfgMgr::instance().clear(); + + EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); + checkResult(x, 1); } /// The goal of this test is to verify that Host Reservation modes can be diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index b81342784e..61b4e4c6db 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -370,6 +371,45 @@ public: globalContext()->copyContext(ParserContext(Option::V6)); } + /// @brief Retrieve an option associated with a host. + /// + /// The option is retrieved from the "dhcp6" option space. + /// + /// @param host Reference to a host for which an option should be retrieved. + /// @param option_code Option code. + /// @tparam ReturnType Type of the pointer object returned. + /// + /// @return Pointer to an option or NULL if not found. + template + ReturnType + retrieveOption(const Host& host, const uint16_t option_code) const { + return (retrieveOption(host, "dhcp6", option_code)); + } + + /// @brief Retrieve an option associated with a host. + /// + /// @param host Reference to a host for which an option should be retrieved. + /// @param space Option space from which option should be retrieved. + /// @param option_code Option code. + /// @tparam ReturnType Type of the pointer object returned. + /// + /// @return Pointer to an option or NULL if not found. + template + ReturnType + retrieveOption(const Host& host, const std::string& space, + const uint16_t option_code) const { + ConstCfgOptionPtr cfg_option = host.getCfgOption6(); + if (cfg_option) { + OptionDescriptor opt_desc = cfg_option->get(space, option_code); + if (opt_desc.option_) { + return (boost::dynamic_pointer_cast< + typename ReturnType::element_type>(opt_desc.option_)); + } + } + return (ReturnType()); + } + + /// @brief Test invalid option parameter value. /// /// This test function constructs the simple configuration @@ -3491,12 +3531,32 @@ TEST_F(Dhcp6ParserTest, reservations) { " {" " \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," " \"ip-addresses\": [ \"2001:db8:2::1234\" ]," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"dns-servers\"," + " \"data\": \"2001:db8:2::1111\"" + " }," + " {" + " \"name\": \"preference\"," + " \"data\": \"11\"" + " }" + " ]" " }," " {" " \"hw-address\": \"01:02:03:04:05:06\"," " \"ip-addresses\": [ \"2001:db8:2::abcd\" ]," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"dns-servers\"," + " \"data\": \"2001:db8:2::abbc\"" + " }," + " {" + " \"name\": \"preference\"," + " \"data\": \"25\"" + " }" + " ]" " }" " ]," " \"pools\": [ ]," @@ -3511,7 +3571,17 @@ TEST_F(Dhcp6ParserTest, reservations) { " {" " \"duid\": \"0A:09:08:07:06:05:04:03:02:01\"," " \"prefixes\": [ \"2001:db8:3:2::/96\" ]," - " \"hostname\": \"\"" + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"dns-servers\"," + " \"data\": \"2001:db8:3::3333\"" + " }," + " {" + " \"name\": \"preference\"," + " \"data\": \"33\"" + " }" + " ]" " }," " {" " \"hw-address\": \"06:05:04:03:02:01\"," @@ -3561,6 +3631,17 @@ TEST_F(Dhcp6ParserTest, reservations) { // and not to other two. EXPECT_FALSE(hosts_cfg->get6(123, DuidPtr(), hwaddr)); EXPECT_FALSE(hosts_cfg->get6(542, DuidPtr(), hwaddr)); + // Check that options are assigned correctly. + Option6AddrLstPtr opt_dns = + retrieveOption(*host, D6O_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + Option6AddrLst::AddressContainer dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("2001:db8:2::abbc", dns_addrs[0].toText()); + OptionUint8Ptr opt_prf = + retrieveOption(*host, D6O_PREFERENCE); + ASSERT_TRUE(opt_prf); + EXPECT_EQ(25, static_cast(opt_prf->getValue())); // Do the same test for the DUID based reservation. std::vector duid_vec; @@ -3577,6 +3658,16 @@ TEST_F(Dhcp6ParserTest, reservations) { resrv)); EXPECT_FALSE(hosts_cfg->get6(123, duid)); EXPECT_FALSE(hosts_cfg->get6(542, duid)); + // Check that options are assigned correctly. + opt_dns = retrieveOption(*host, D6O_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("2001:db8:2::1111", dns_addrs[0].toText()); + opt_prf = retrieveOption(*host, D6O_PREFERENCE); + ASSERT_TRUE(opt_prf); + EXPECT_EQ(11, static_cast(opt_prf->getValue())); + // The HW address used for one of the reservations in the subnet 542 // consists of numbers from 6 to 1. So, let's just reverse the order @@ -3607,6 +3698,83 @@ TEST_F(Dhcp6ParserTest, reservations) { EXPECT_FALSE(hosts_cfg->get6(123, duid)); EXPECT_FALSE(hosts_cfg->get6(234, duid)); + // Check that options are assigned correctly. + opt_dns = retrieveOption(*host, D6O_NAME_SERVERS); + ASSERT_TRUE(opt_dns); + dns_addrs = opt_dns->getAddresses(); + ASSERT_EQ(1, dns_addrs.size()); + EXPECT_EQ("2001:db8:3::3333", dns_addrs[0].toText()); + opt_prf = retrieveOption(*host, D6O_PREFERENCE); + ASSERT_TRUE(opt_prf); + EXPECT_EQ(33, static_cast(opt_prf->getValue())); +} + +// This test checks that it is possible to configure option data for a +// host using a user defined option format. +TEST_F(Dhcp6ParserTest, reservationWithOptionDefinition) { + ConstElementPtr x; + // The following configuration contains host declaration in which + // a non-standard option is used. This option has option definition + // specified in the configuration. + string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 100," + " \"type\": \"uint32\"," + " \"space\": \"isc\"" + "} ]," + "\"subnet6\": [ " + " {" + " \"reservations\": [" + " {" + " \"duid\": \"01:02:03:04:05:06:07:08:09:0A\"," + " \"ip-addresses\": [ \"2001:db8:2::1234\" ]," + " \"hostname\": \"\"," + " \"option-data\": [" + " {" + " \"name\": \"foo\"," + " \"data\": \"11\"," + " \"space\": \"isc\"" + " }" + " ]" + " }" + " ]," + " \"pools\": [ ]," + " \"subnet\": \"2001:db8:2::/64\", " + " \"id\": 234" + " }" + "]," + "\"preferred-lifetime\": 3000," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); + checkResult(x, 0); + + // Hosts configuration must be available. + CfgHostsPtr hosts_cfg = CfgMgr::instance().getStagingCfg()->getCfgHosts(); + ASSERT_TRUE(hosts_cfg); + + // Let's create an object holding DUID of the host. For simplicity the + // address is a collection of numbers from 1 to A. + std::vector duid_vec; + for (int i = 1; i < 0xB; ++i) { + duid_vec.push_back(static_cast(i)); + } + DuidPtr duid(new DUID(duid_vec)); + // Retrieve the reservation and sanity check the address reserved. + ConstHostPtr host = hosts_cfg->get6(234, duid); + ASSERT_TRUE(host); + + // Check if the option has been parsed. + OptionUint32Ptr opt_foo = retrieveOption(*host, "isc", + 100); + ASSERT_TRUE(opt_foo); + EXPECT_EQ(100, opt_foo->getType()); + EXPECT_EQ(11, opt_foo->getValue()); } // This test verifies that the bogus host reservation would trigger a @@ -3696,6 +3864,38 @@ TEST_F(Dhcp6ParserTest, reservationBogus) { EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); checkResult(x, 1); + // Case 4: Broken specification of option data. + config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet6\": [ " + " { " + " \"pools\": [ ]," + " \"subnet\": \"2001:db8:3::/64\", " + " \"id\": 542," + " \"reservations\": [" + " {" + " \"duid\": \"0A:09:08:07:06:05:04:03:02:01\"," + " \"option-data\": [" + " {" + " \"name\": \"dns-servers\"," + " \"data\": \"invalid-ip-address\"" + " }" + " ]" + " }" + " ]" + " } " + "], " + "\"preferred-lifetime\": 3000," + "\"valid-lifetime\": 4000 }"; + + json = Element::fromJSON(config); + + // Remove existing configuration, if any. + CfgMgr::instance().clear(); + + EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); + checkResult(x, 1); } /// The goal of this test is to verify that configuration can include From 8576546a1b2a1a0e6d082328c62bf67407ecc8df Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 17 Mar 2016 19:02:44 +0100 Subject: [PATCH 4/5] [4319] Addressed some review comments. - use unsigned int in for loops - NULL pointer rather than NULL in function descriptions --- src/bin/dhcp4/tests/config_parser_unittest.cc | 12 ++++++------ src/bin/dhcp6/tests/config_parser_unittest.cc | 16 ++++++++-------- .../tests/host_reservation_parser_unittest.cc | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 04de12ffdf..fd12ce6a46 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -471,7 +471,7 @@ public: /// @param option_code Option code. /// @tparam ReturnType Type of the pointer object returned. /// - /// @return Pointer to an option or NULL if not found. + /// @return Pointer to an option or NULL pointer if not found. template ReturnType retrieveOption(const Host& host, const uint16_t option_code) const { @@ -485,7 +485,7 @@ public: /// @param option_code Option code. /// @tparam ReturnType Type of the pointer object returned. /// - /// @return Pointer to an option or NULL if not found. + /// @return Pointer to an option or NULL pointer if not found. template ReturnType retrieveOption(const Host& host, const std::string& space, @@ -2911,7 +2911,7 @@ buildHooksLibrariesConfig(const std::vector& libraries) { "\"hooks-libraries\": ["; // Append the libraries (separated by commas if needed) - for (int i = 0; i < libraries.size(); ++i) { + for (unsigned int i = 0; i < libraries.size(); ++i) { if (i > 0) { config += string(", "); } @@ -3483,7 +3483,7 @@ TEST_F(Dhcp4ParserTest, reservations) { // a reservation in the subnet having id of 234. For simplicity the // address is a collection of numbers from 1 to 6. std::vector hwaddr_vec; - for (int i = 1; i < 7; ++i) { + for (unsigned int i = 1; i < 7; ++i) { hwaddr_vec.push_back(static_cast(i)); } HWAddrPtr hwaddr(new HWAddr(hwaddr_vec, HTYPE_ETHER)); @@ -3509,7 +3509,7 @@ TEST_F(Dhcp4ParserTest, reservations) { // Do the same test for the DUID based reservation. std::vector duid_vec; - for (int i = 1; i < 0xb; ++i) { + for (unsigned int i = 1; i < 0xb; ++i) { duid_vec.push_back(static_cast(i)); } DuidPtr duid(new DUID(duid_vec)); @@ -3608,7 +3608,7 @@ TEST_F(Dhcp4ParserTest, reservationWithOptionDefinition) { // Let's create an object holding DUID of the host. For simplicity the // address is a collection of numbers from 1 to A. std::vector duid_vec; - for (int i = 1; i < 0xB; ++i) { + for (unsigned int i = 1; i < 0xB; ++i) { duid_vec.push_back(static_cast(i)); } DuidPtr duid(new DUID(duid_vec)); diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 61b4e4c6db..7876713d09 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -379,7 +379,7 @@ public: /// @param option_code Option code. /// @tparam ReturnType Type of the pointer object returned. /// - /// @return Pointer to an option or NULL if not found. + /// @return Pointer to an option or NULL pointer if not found. template ReturnType retrieveOption(const Host& host, const uint16_t option_code) const { @@ -393,7 +393,7 @@ public: /// @param option_code Option code. /// @tparam ReturnType Type of the pointer object returned. /// - /// @return Pointer to an option or NULL if not found. + /// @return Pointer to an option or NULL pointer if not found. template ReturnType retrieveOption(const Host& host, const std::string& space, @@ -1444,7 +1444,7 @@ TEST_F(Dhcp6ParserTest, pdPoolList) { EXPECT_EQ(3, pc.size()); // Loop through the pools and verify their contents. - for (int i = 0; i < 3; i++) { + for (unsigned int i = 0; i < 3; i++) { Pool6Ptr p6; ASSERT_NO_THROW(p6 = boost::dynamic_pointer_cast(pc[i])); ASSERT_TRUE(p6); @@ -1577,7 +1577,7 @@ TEST_F(Dhcp6ParserTest, invalidPdPools) { ElementPtr json; int num_msgs = sizeof(config)/sizeof(char*); - for (int i = 0; i < num_msgs; i++) { + for (unsigned int i = 0; i < num_msgs; i++) { // Convert JSON string to Elements. ASSERT_NO_THROW(json = Element::fromJSON(config[i])); @@ -3065,7 +3065,7 @@ buildHooksLibrariesConfig(const std::vector& libraries) { "\"hooks-libraries\": ["; // Append the libraries (separated by commas if needed) - for (int i = 0; i < libraries.size(); ++i) { + for (unsigned int i = 0; i < libraries.size(); ++i) { if (i > 0) { config += string(", "); } @@ -3615,7 +3615,7 @@ TEST_F(Dhcp6ParserTest, reservations) { // a reservation in the subnet having id of 234. For simplicity the // address is a collection of numbers from 1 to 6. std::vector hwaddr_vec; - for (int i = 1; i < 7; ++i) { + for (unsigned int i = 1; i < 7; ++i) { hwaddr_vec.push_back(static_cast(i)); } HWAddrPtr hwaddr(new HWAddr(hwaddr_vec, HTYPE_ETHER)); @@ -3645,7 +3645,7 @@ TEST_F(Dhcp6ParserTest, reservations) { // Do the same test for the DUID based reservation. std::vector duid_vec; - for (int i = 1; i < 0xb; ++i) { + for (unsigned int i = 1; i < 0xb; ++i) { duid_vec.push_back(static_cast(i)); } DuidPtr duid(new DUID(duid_vec)); @@ -3761,7 +3761,7 @@ TEST_F(Dhcp6ParserTest, reservationWithOptionDefinition) { // Let's create an object holding DUID of the host. For simplicity the // address is a collection of numbers from 1 to A. std::vector duid_vec; - for (int i = 1; i < 0xB; ++i) { + for (unsigned int i = 1; i < 0xB; ++i) { duid_vec.push_back(static_cast(i)); } DuidPtr duid(new DUID(duid_vec)); diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc index ac50416a00..ce6c5defdf 100644 --- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc @@ -65,8 +65,8 @@ protected: /// @param option_space Option space name. /// @param option_code Code of an option to be retrieved. /// - /// @return Pointer to the option retrieved or NULL if option hasn't been - /// found. + /// @return Pointer to the option retrieved or NULL pointer if option + /// hasn't been found. OptionPtr retrieveOption(const Host& host, const std::string& option_space, const uint16_t option_code) const { From b2b3cb0b93411ae5e600fa3448a951b2e3db2d12 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 18 Mar 2016 12:30:48 +0100 Subject: [PATCH 5/5] [4319] Updated comments for empty options in host tests. --- src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc index ce6c5defdf..32bd6dddc2 100644 --- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc @@ -704,13 +704,13 @@ TEST_F(HostReservationParserTest, options6) { } // This test verifies that it is possible to specify an empty list of -// options for a host declaration. +// DHCPv4 options for a host declaration. TEST_F(HostReservationParserTest, options4Empty) { testEmptyOptionList(); } // This test verifies that it is possible to specify an empty list of -// options for a host declaration. +// DHCPv6 options for a host declaration. TEST_F(HostReservationParserTest, options6Empty) { testEmptyOptionList(); }