Commit 0e5e017d authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[5535] RelayInfo parsing handles both ip-address and ip-addresses

src/lib/dhcpsrv/parsers/dhcp_parsers.*
    RelayInfoParser::parse() - reworked to support either
    ip-address or ip-addresses

    RelayInfoParser::addAddress() - new parser helper method

src/lib/dhcpsrv/parsers/shared_network_parser.cc
    SharedNetwork4Parser::parse()
    SharedNetwork6Parser::parse()
    - both now parse "relay" element (was missing)

src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
    Modified to support testing "relay" element parsing
    Added new tests:
        TEST_F(SharedNetwork4ParserTest, relayInfoTests)
        TEST_F(SharedNetwork6ParserTest, relayInfoTests)
parent 9e362be0
......@@ -233,24 +233,79 @@ RelayInfoParser::RelayInfoParser(const Option::Universe& family)
};
void
RelayInfoParser::parse(const isc::dhcp::Network::RelayInfoPtr& cfg,
ConstElementPtr relay_info) {
// There is only one parameter which is mandatory
IOAddress ip = getAddress(relay_info, "ip-address");
RelayInfoParser::parse(const isc::dhcp::Network::RelayInfoPtr& relay_info,
ConstElementPtr relay_elem) {
if (relay_elem->getType() != Element::map) {
isc_throw(DhcpConfigError, "relay must be a map");
}
ConstElementPtr address = relay_elem->get("ip-address");
ConstElementPtr addresses = relay_elem->get("ip-addresses");
if (address && addresses) {
isc_throw(DhcpConfigError,
"specify either ip-address or ip-addresses, not both");
}
if (!address && !addresses) {
isc_throw(DhcpConfigError, "ip-addresses is required");
}
// Create our resultant RelayInfo structure
*relay_info = isc::dhcp::Network::RelayInfo();
if (address) {
// log a deprec debug message ?
addAddress("ip-address", getString(relay_elem, "ip-address"),
relay_elem, relay_info);
return;
}
if (addresses->getType() != Element::list) {
isc_throw(DhcpConfigError, "ip-addresses must be a list "
<< " (" << getPosition("ip-addresses", relay_elem) << ")");
}
BOOST_FOREACH(ConstElementPtr address_element, addresses->listValue()) {
addAddress("ip-addresses", address_element->stringValue(),
relay_elem, relay_info);
}
}
void
RelayInfoParser::addAddress(const std::string& name,
const std::string& address_str,
ConstElementPtr relay_elem,
const isc::dhcp::Network::RelayInfoPtr& relay_info) {
boost::scoped_ptr<isc::asiolink::IOAddress> ip;
try {
ip.reset(new isc::asiolink::IOAddress(address_str));
} catch (const std::exception& ex) {
isc_throw(DhcpConfigError, "address " << address_str
<< " is not a valid: "
<< (family_ == Option::V4 ? "IPv4" : "IPv6")
<< "address"
<< " (" << getPosition(name, relay_elem) << ")");
}
// Check if the address family matches.
if ((ip.isV4() && family_ != Option::V4) ||
(ip.isV6() && family_ != Option::V6) ) {
isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
<< " does not have IP address of expected family type: "
if ((ip->isV4() && family_ != Option::V4) ||
(ip->isV6() && family_ != Option::V6) ) {
isc_throw(DhcpConfigError, "address " << address_str
<< " is not a: "
<< (family_ == Option::V4 ? "IPv4" : "IPv6")
<< " (" << getPosition("ip-address", relay_info) << ")");
<< "address"
<< " (" << getPosition(name, relay_elem) << ")");
}
// Ok, we're done with parsing. Let's store the result in the structure
// we were given as configuration storage.
*cfg = isc::dhcp::Network::RelayInfo();
cfg->addAddress(ip);
try {
relay_info->addAddress(*ip);
} catch (const std::exception& ex) {
isc_throw(DhcpConfigError, "cannot add address: " << address_str
<< "to relay info: " << ex.what()
<< " (" << getPosition(name, relay_elem) << ")");
}
}
//****************************** PoolParser ********************************
......@@ -698,7 +753,7 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params,
try {
std::string hr_mode = getString(params, "reservation-mode");
subnet4->setHostReservationMode(hrModeFromText(hr_mode));
} catch (const BadValue& ex) {
} catch (const BadValue& ex) {
isc_throw(DhcpConfigError, "Failed to process specified value "
" of reservation-mode parameter: " << ex.what()
<< "(" << getPosition("reservation-mode", params) << ")");
......@@ -879,7 +934,7 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) {
OptionDataListParser opts_parser(AF_INET6);
opts_parser.parse(options_, option_data);
}
ConstElementPtr user_context = pd_pool_->get("user-context");
if (user_context) {
user_context_ = user_context;
......@@ -921,7 +976,7 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) {
pool_->allowClientClass(cclass);
}
}
if (class_list) {
const std::vector<data::ElementPtr>& classes = class_list->listValue();
for (auto cclass = classes.cbegin();
......@@ -1096,7 +1151,7 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params,
try {
std::string hr_mode = getString(params, "reservation-mode");
subnet6->setHostReservationMode(hrModeFromText(hr_mode));
} catch (const BadValue& ex) {
} catch (const BadValue& ex) {
isc_throw(DhcpConfigError, "Failed to process specified value "
" of reservation-mode parameter: " << ex.what()
<< "(" << getPosition("reservation-mode", params) << ")");
......@@ -1213,9 +1268,9 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
std::string sender_ip_str = getString(client_config, "sender-ip");
uint32_t sender_port = getUint32(client_config, "sender-port");
uint32_t sender_port = getUint32(client_config, "sender-port");
uint32_t max_queue_size = getUint32(client_config, "max-queue-size");
uint32_t max_queue_size = getUint32(client_config, "max-queue-size");
dhcp_ddns::NameChangeProtocol ncr_protocol =
getProtocol(client_config, "ncr-protocol");
......@@ -1244,7 +1299,7 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
if (client_config->contains("qualifying-suffix")) {
qualifying_suffix = getString(client_config, "qualifying-suffix");
found_qualifying_suffix = true;
}
}
IOAddress sender_ip(0);
if (sender_ip_str.empty()) {
......
......@@ -397,12 +397,32 @@ public:
///
/// The elements currently supported are:
/// -# ip-address
///
/// @param cfg configuration will be stored here
/// @param relay_info JSON structure holding relay parameters to parse
void parse(const isc::dhcp::Network::RelayInfoPtr& cfg,
isc::data::ConstElementPtr relay_info);
/// -# ip-addresses
///
/// Note that ip-address and ip-addresses are mutually exclusive, with
/// former being deprecated. The use of ip-address will cause an debug
/// log to be emitted, reminded users to switch.
///
/// @param relay_info configuration will be stored here
/// @param relay_elem Element tree containing the relay and its members
/// @throw isc::dhcp::DhcpConfigError if both or neither of ip-address
/// and ip-addresses are specified.
void parse(const isc::dhcp::Network::RelayInfoPtr& relay_info,
isc::data::ConstElementPtr relay_elem);
/// @brief Attempts to add an IP address to list of relay addresses
///
/// @param name name of the element supplying the address string, (either
/// "ip-address" or "ip-addresses")
/// @param address string form of the IP address to add
/// @param relay_elem parent relay element (needed for position info)
/// @param relay_info RelayInfo to which the address should be added
/// @throw isc::dhcp::DhcpConfigError if the address string is not a valid
/// IP address, is an address of the wrong family, or is already in the
/// relay address list
void addAddress(const std::string& name, const std::string& address_str,
isc::data::ConstElementPtr relay_elem,
const isc::dhcp::Network::RelayInfoPtr& relay_info);
private:
/// Protocol family (IPv4 or IPv6)
......
......@@ -89,6 +89,15 @@ SharedNetwork4Parser::parse(const data::ConstElementPtr& shared_network_data) {
}
}
if (shared_network_data->contains("relay")) {
auto relay_parms = shared_network_data->get("relay");
if (relay_parms) {
RelayInfoParser parser(Option::V4);
Network::RelayInfoPtr relay_info(new Network::RelayInfo());
parser.parse(relay_info, relay_parms);
shared_network->setRelayInfo(*relay_info);
}
}
} catch (const DhcpConfigError&) {
// Position was already added
throw;
......@@ -164,6 +173,15 @@ SharedNetwork6Parser::parse(const data::ConstElementPtr& shared_network_data) {
}
}
if (shared_network_data->contains("relay")) {
auto relay_parms = shared_network_data->get("relay");
if (relay_parms) {
RelayInfoParser parser(Option::V6);
Network::RelayInfoPtr relay_info(new Network::RelayInfo());
parser.parse(relay_info, relay_parms);
shared_network->setRelayInfo(*relay_info);
}
}
} catch (const std::exception& ex) {
isc_throw(DhcpConfigError, ex.what() << " ("
<< shared_network_data->getPosition() << ")");
......
......@@ -21,15 +21,97 @@ using namespace isc::data;
using namespace isc::dhcp;
namespace {
class SharedNetworkParserTest : public ::testing::Test {
public:
/// @brief Structure for describing a single relay test scenario
struct RelayTest {
/// @brief Description of the test scenario, used for logging
std::string description_;
/// @brief JSON configuration body of the "relay" element
std::string json_content_;
/// @brief indicates if parsing should pass or fail
bool should_parse_;
/// @brief list of addresses expected after parsing
IOAddressList addresses_;
};
/// @brief virtual destructor
virtual ~SharedNetworkParserTest(){};
/// @brief Fetch valid shared network configuration JSON text
virtual std::string getWorkingConfig() const = 0;
ElementPtr makeTestConfig(const std::string& name, const std::string& json_content) {
// Create working config element tree
ElementPtr config = Element::fromJSON(getWorkingConfig());
// Create test element contents
ElementPtr content = Element::fromJSON(json_content);
// Add the test element to working config
config->set(name, content);
return (config);
}
/// @brief Executes a single "relay" parsing scenario
///
/// Each test pass consists of the following steps:
/// -# Attempt to parse the given JSON text
/// -# If parsing is expected to fail and it does return otherwise fatal fail
/// -# If parsing is expected to succeed, fatal fail if it does not
/// -# Verify the network's relay address list matches the expected list
/// in size and content.
///
/// @param test RelayTest which describes the test to conduct
void relayTest(const RelayTest& test) {
ElementPtr test_config;
ASSERT_NO_THROW(test_config =
makeTestConfig("relay", test.json_content_));
// Init our ref to a place holder
Network4 dummy;
Network& network = dummy;
// If parsing should fail, call parse expecting a throw.
if (!test.should_parse_) {
ASSERT_THROW(network = parseIntoNetwork(test_config), DhcpConfigError);
// No throw so test outcome is correct, nothing else to do.
return;
}
// Should parse without error, let's see if it does.
ASSERT_NO_THROW(network = parseIntoNetwork(test_config));
// It parsed, are the number of entries correct?
ASSERT_EQ(test.addresses_.size(), network.getRelayAddresses().size());
// Are the expected addresses in the list?
for (auto exp_address = test.addresses_.begin(); exp_address != test.addresses_.end();
++exp_address) {
EXPECT_TRUE(network.hasRelayAddress(*exp_address))
<< " expected address: " << (*exp_address).toText() << " not found" ;
}
}
/// @brief Attempts to parse the given configuration into a shared network
///
/// Virtual function used by relayTest() to parse a test configuration.
/// Implementation should not catch parsing exceptions.
///
/// @param test_config JSON configuration text to parse
/// @return A reference to the Network created if parsing is successful
virtual Network& parseIntoNetwork(ConstElementPtr test_config) = 0;
};
/// @brief Test fixture class for SharedNetwork4Parser class.
class SharedNetwork4ParserTest : public ::testing::Test {
class SharedNetwork4ParserTest : public SharedNetworkParserTest {
public:
/// @brief Creates valid shared network configuration.
///
/// @return Valid shared network configuration.
std::string getWorkingConfig() const {
virtual std::string getWorkingConfig() const {
std::string config = "{"
" \"user-context\": { \"comment\": \"example\" },"
" \"name\": \"bird\","
......@@ -88,6 +170,16 @@ public:
return (config);
}
virtual Network& parseIntoNetwork(ConstElementPtr test_config) {
// Parse configuration.
SharedNetwork4Parser parser;
network_ = parser.parse(test_config);
return (*network_);
}
private:
SharedNetwork4Ptr network_;
};
// This test verifies that shared network parser for IPv4 works properly
......@@ -152,8 +244,6 @@ TEST_F(SharedNetwork4ParserTest, missingName) {
ASSERT_THROW(network = parser.parse(config_element), DhcpConfigError);
}
// This test verifies that it's possible to specify client-class
// and match-client-id on shared-network level.
TEST_F(SharedNetwork4ParserTest, clientClassMatchClientId) {
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
......@@ -172,14 +262,96 @@ TEST_F(SharedNetwork4ParserTest, clientClassMatchClientId) {
EXPECT_FALSE(network->getMatchClientId());
}
// This test verifies that parsing of the "relay" element.
// It checks both valid and invalid scenarios.
TEST_F(SharedNetwork4ParserTest, relayInfoTests) {
// Create the vector of test scenarios.
std::vector<RelayTest> tests = {
{
"valid ip-address #1",
"{ \"ip-address\": \"192.168.2.1\" }",
true,
{ asiolink::IOAddress("192.168.2.1") }
},
{
"invalid ip-address #1",
"{ \"ip-address\": \"not an address\" }",
false,
{ }
},
{
"invalid ip-address #2",
"{ \"ip-address\": \"2001:db8::1\" }",
false,
{ }
},
{
"valid ip-addresses #1",
"{ \"ip-addresses\": [ ] }",
true,
{}
},
{
"valid ip-addresses #2",
"{ \"ip-addresses\": [ \"192.168.2.1\" ] }",
true,
{ asiolink::IOAddress("192.168.2.1") }
},
{
"valid ip-addresses #3",
"{ \"ip-addresses\": [ \"192.168.2.1\", \"192.168.2.2\" ] }",
true,
{ asiolink::IOAddress("192.168.2.1"), asiolink::IOAddress("192.168.2.2") }
},
{
"invalid ip-addresses #1",
"{ \"ip-addresses\": [ \"not an address\" ] }",
false,
{ }
},
{
"invalid ip-addresses #2",
"{ \"ip-addresses\": [ \"2001:db8::1\" ] }",
false,
{ }
},
{
"invalid both ip-address and ip-addresses",
"{"
" \"ip-address\": \"192.168.2.1\", "
" \"ip-addresses\": [ \"192.168.2.1\", \"192.168.2.2\" ]"
" }",
false,
{ }
},
{
"invalid neither ip-address nor ip-addresses",
"{}",
false,
{ }
}
};
// Iterate over the test scenarios, verifying each prescribed
// outcome.
for (auto test = tests.begin(); test != tests.end(); ++test) {
{
SCOPED_TRACE((*test).description_);
relayTest(*test);
}
}
}
/// @brief Test fixture class for SharedNetwork6Parser class.
class SharedNetwork6ParserTest : public ::testing::Test {
class SharedNetwork6ParserTest : public SharedNetworkParserTest {
public:
/// @brief Creates valid shared network configuration.
///
/// @return Valid shared network configuration.
std::string getWorkingConfig() const {
virtual std::string getWorkingConfig() const {
std::string config = "{"
" \"name\": \"bird\","
" \"interface\": \"eth1\","
......@@ -228,6 +400,16 @@ public:
return (config);
}
virtual Network& parseIntoNetwork(ConstElementPtr test_config) {
// Parse configuration.
SharedNetwork6Parser parser;
network_ = parser.parse(test_config);
return (*network_);
}
private:
SharedNetwork6Ptr network_;
};
// This test verifies that shared network parser for IPv4 works properly
......@@ -346,4 +528,85 @@ TEST_F(SharedNetwork6ParserTest, badEvalClientClasses) {
EXPECT_THROW(network = parser.parse(config_element), DhcpConfigError);
}
// This test verifies that v6 parsing of the "relay" element.
// It checks both valid and invalid scenarios.
TEST_F(SharedNetwork6ParserTest, relayInfoTests) {
// Create the vector of test scenarios.
std::vector<RelayTest> tests = {
{
"valid ip-address #1",
"{ \"ip-address\": \"2001:db8::1\" }",
true,
{ asiolink::IOAddress("2001:db8::1") }
},
{
"invalid ip-address #1",
"{ \"ip-address\": \"not an address\" }",
false,
{ }
},
{
"invalid ip-address #2",
"{ \"ip-address\": \"192.168.2.1\" }",
false,
{ }
},
{
"valid ip-addresses #1",
"{ \"ip-addresses\": [ ] }",
true,
{}
},
{
"valid ip-addresses #2",
"{ \"ip-addresses\": [ \"2001:db8::1\" ] }",
true,
{ asiolink::IOAddress("2001:db8::1") }
},
{
"valid ip-addresses #3",
"{ \"ip-addresses\": [ \"2001:db8::1\", \"2001:db8::2\" ] }",
true,
{ asiolink::IOAddress("2001:db8::1"), asiolink::IOAddress("2001:db8::2") }
},
{
"invalid ip-addresses #1",
"{ \"ip-addresses\": [ \"not an address\" ] }",
false,
{ }
},
{
"invalid ip-addresses #2",
"{ \"ip-addresses\": [ \"192.168.1.1\" ] }",
false,
{ }
},
{
"invalid both ip-address and ip-addresses",
"{"
" \"ip-address\": \"2001:db8::1\", "
" \"ip-addresses\": [ \"2001:db8::1\", \"2001:db8::2\" ]"
" }",
false,
{ }
},
{
"invalid neither ip-address nor ip-addresses",
"{}",
false,
{ }
}
};
// Iterate over the test scenarios, verifying each prescribed
// outcome.
for (auto test = tests.begin(); test != tests.end(); ++test) {
{
SCOPED_TRACE((*test).description_);
relayTest(*test);
}
}
}
} // end of anonymous namespace
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment