Commit 3b30023d authored by Francis Dupont's avatar Francis Dupont

[850-unable-to-set-not-present-interface-for-a-subnet-via-remote] Revamped...

[850-unable-to-set-not-present-interface-for-a-subnet-via-remote] Revamped interface/network syntax existence checks
parent 011f0be3
......@@ -9231,7 +9231,6 @@ const char* UNPARSED_CONFIGS[] = {
" \"comment\": \"A shared network\",\n"
" \"authoritative\": false,\n"
" \"calculate-tee-times\": false,\n"
" \"interface\": \"\",\n"
" \"match-client-id\": true,\n"
" \"name\": \"foo\",\n"
" \"next-server\": \"0.0.0.0\",\n"
......@@ -9537,7 +9536,6 @@ const char* UNPARSED_CONFIGS[] = {
" {\n"
" \"authoritative\": false,\n"
" \"calculate-tee-times\": true,\n"
" \"interface\": \"\",\n"
" \"match-client-id\": true,\n"
" \"name\": \"foo\",\n"
" \"next-server\": \"0.0.0.0\",\n"
......
......@@ -8179,7 +8179,6 @@ const char* UNPARSED_CONFIGS[] = {
" {\n"
" \"comment\": \"A shared network\",\n"
" \"calculate-tee-times\": true,\n"
" \"interface\": \"\",\n"
" \"name\": \"foo\",\n"
" \"option-data\": [ ],\n"
" \"preferred-lifetime\": 3600,\n"
......
......@@ -735,11 +735,9 @@ void
Subnet4ConfigParser::initSubnet(data::ConstElementPtr params,
asiolink::IOAddress addr, uint8_t len) {
// Subnet ID is optional. If it is not supplied the value of 0 is used,
// which means autogenerate.
SubnetID subnet_id = 0;
if (params->contains("id")) {
subnet_id = static_cast<SubnetID>(getInteger(params, "id"));
}
// which means autogenerate. The value was inserted earlier by calling
// SimpleParser6::setAllDefaults.
SubnetID subnet_id = static_cast<SubnetID>(getInteger(params, "id"));
Subnet4Ptr subnet4(new Subnet4(addr, len, Triplet<uint32_t>(),
Triplet<uint32_t>(), Triplet<uint32_t>(),
......
......@@ -47,8 +47,19 @@ SharedNetwork4Parser::parse(const data::ConstElementPtr& shared_network_data,
// interface is an optional parameter
if (shared_network_data->contains("interface")) {
// check_iface applies only on subnets?
shared_network->setIface(getString(shared_network_data, "interface"));
std::string iface = getString(shared_network_data, "interface");
if (!iface.empty()) {
if (check_iface && !IfaceMgr::instance().getIface(iface)) {
ConstElementPtr error =
shared_network_data->get("interface");
isc_throw(DhcpConfigError,
"Specified network interface name " << iface
<< " for shared network " << name
<< " is not present in the system ("
<< error->getPosition() << ")");
}
shared_network->setIface(iface);
}
}
if (shared_network_data->contains("option-data")) {
......@@ -225,6 +236,7 @@ SharedNetwork6Parser::parse(const data::ConstElementPtr& shared_network_data,
ifaceid = getString(shared_network_data, "interface-id");
}
// Interface is an optional parameter
Optional<std::string> iface;
if (shared_network_data->contains("interface")) {
iface = getString(shared_network_data, "interface");
......@@ -251,18 +263,20 @@ SharedNetwork6Parser::parse(const data::ConstElementPtr& shared_network_data,
shared_network->setInterfaceId(opt);
}
// Get interface name. If it is defined, then the subnet is available
// Set interface name. If it is defined, then subnets are available
// directly over specified network interface.
// check_iface applies only on subnets?
if (!iface.unspecified() && !iface.empty()) {
if (check_iface && !IfaceMgr::instance().getIface(iface)) {
ConstElementPtr error = shared_network_data->get("interface");
isc_throw(DhcpConfigError,
"Specified network interface name " << iface
<< " for shared network " << name
<< " is not present in the system ("
<< error->getPosition() << ")");
}
shared_network->setIface(iface);
}
// Interface is an optional parameter
if (shared_network_data->contains("interface")) {
shared_network->setIface(getString(shared_network_data, "interface"));
}
if (shared_network_data->contains("rapid-commit")) {
shared_network->setRapidCommit(getBoolean(shared_network_data,
"rapid-commit"));
......
// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2017-2019 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
......@@ -31,18 +31,21 @@ public:
/// @brief Parses a list of shared networks.
///
/// @tparam Type of the configuration structure into which the result
/// will be stored, i.e. @ref CfgSharedNetworks4 or @ref CfgSharedNetworks6.
/// @param [out] cfg Shared networks configuration structure into which
/// the data should be parsed.
/// @param shared_networks_list_data List element holding a list of
/// shared networks.
/// @tparam Type of the configuration structure into which the result
/// will be stored, i.e. @ref CfgSharedNetworks4 or @ref CfgSharedNetworks6.
/// @param check_iface Check if the specified interface exists in
/// the system.
///
/// @throw DhcpConfigError when error has occurred, e.g. when networks
/// with duplicated names have been specified.
template<typename CfgSharedNetworksTypePtr>
void parse(CfgSharedNetworksTypePtr& cfg,
const data::ConstElementPtr& shared_networks_list_data) {
const data::ConstElementPtr& shared_networks_list_data,
bool check_iface = true) {
try {
// Get the C++ vector holding networks.
const std::vector<data::ElementPtr>& networks_list =
......@@ -51,7 +54,7 @@ public:
for (auto network_element = networks_list.cbegin();
network_element != networks_list.cend(); ++network_element) {
SharedNetworkParserType parser;
auto network = parser.parse(*network_element);
auto network = parser.parse(*network_element, check_iface);
cfg->add(network);
}
} catch (const DhcpConfigError&) {
......
......@@ -1616,4 +1616,28 @@ TEST(CfgSubnets4Test, hostnameSanitizierValidation) {
}
}
// This test verifies that an interface which is not in the system is rejected.
TEST(CfgSubnets4Test, iface) {
// Create a configuration.
std::string json =
" {"
" \"id\": 1,\n"
" \"subnet\": \"10.1.2.0/24\", \n"
" \"interface\": \"eth1\"\n"
" }";
data::ElementPtr elems;
ASSERT_NO_THROW(elems = data::Element::fromJSON(json))
<< "invalid JSON:" << json << "\n test is broken";
Subnet4ConfigParser parser;
EXPECT_NO_THROW(parser.parse(elems, false));
EXPECT_THROW(parser.parse(elems), DhcpConfigError);
// Configure default test interfaces.
IfaceMgrTestConfig config(true);
EXPECT_NO_THROW(parser.parse(elems, false));
EXPECT_NO_THROW(parser.parse(elems));
}
} // end of anonymous namespace
......@@ -9,6 +9,7 @@
#include <dhcp/dhcp6.h>
#include <dhcp/option.h>
#include <dhcp/option_string.h>
#include <dhcp/tests/iface_mgr_test_config.h>
#include <dhcpsrv/cfg_shared_networks.h>
#include <dhcpsrv/cfg_subnets6.h>
#include <dhcpsrv/parsers/dhcp_parsers.h>
......@@ -25,6 +26,7 @@
using namespace isc;
using namespace isc::asiolink;
using namespace isc::dhcp;
using namespace isc::dhcp::test;
using namespace isc::test;
namespace {
......@@ -1397,4 +1399,28 @@ TEST(CfgSubnets6Test, hostnameSanitizierValidation) {
}
}
// This test verifies that an interface which is not in the system is rejected.
TEST(CfgSubnets6Test, iface) {
// Create a configuration.
std::string json =
" {"
" \"id\": 1,\n"
" \"subnet\": \"2001:db8:1::/64\", \n"
" \"interface\": \"eth1\"\n"
" }";
data::ElementPtr elems;
ASSERT_NO_THROW(elems = data::Element::fromJSON(json))
<< "invalid JSON:" << json << "\n test is broken";
Subnet6ConfigParser parser;
EXPECT_NO_THROW(parser.parse(elems, false));
EXPECT_THROW(parser.parse(elems), DhcpConfigError);
// Configure default test interfaces.
IfaceMgrTestConfig config(true);
EXPECT_NO_THROW(parser.parse(elems, false));
EXPECT_NO_THROW(parser.parse(elems));
}
} // end of anonymous namespace
......@@ -12,6 +12,7 @@
#include <dhcp/option.h>
#include <dhcp/option4_addrlst.h>
#include <dhcp/option6_addrlst.h>
#include <dhcp/tests/iface_mgr_test_config.h>
#include <dhcpsrv/cfg_option.h>
#include <dhcpsrv/parsers/shared_network_parser.h>
#include <gtest/gtest.h>
......@@ -21,6 +22,7 @@ using namespace isc;
using namespace isc::asiolink;
using namespace isc::data;
using namespace isc::dhcp;
using namespace isc::dhcp::test;
namespace {
class SharedNetworkParserTest : public ::testing::Test {
......@@ -219,6 +221,8 @@ private:
// This test verifies that shared network parser for IPv4 works properly
// in a positive test scenario.
TEST_F(SharedNetwork4ParserTest, parse) {
IfaceMgrTestConfig ifmgr(true);
// Basic configuration for shared network. A bunch of parameters
// have to be specified for subnets because subnet parsers expect
// that default and global values are set.
......@@ -229,12 +233,6 @@ TEST_F(SharedNetwork4ParserTest, parse) {
SharedNetwork4Parser parser;
SharedNetwork4Ptr network;
try {
network = parser.parse(config_element);
} catch (const std::exception& ex) {
std::cout << "kabook: " << ex.what() << std::endl;
}
ASSERT_NO_THROW(network = parser.parse(config_element));
ASSERT_TRUE(network);
......@@ -330,6 +328,8 @@ TEST_F(SharedNetwork4ParserTest, missingName) {
// This test verifies that it's possible to specify client-class,
// match-client-id, and authoritative on shared-network level.
TEST_F(SharedNetwork4ParserTest, clientClassMatchClientIdAuthoritative) {
IfaceMgrTestConfig ifmgr(true);
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
......@@ -353,6 +353,7 @@ TEST_F(SharedNetwork4ParserTest, clientClassMatchClientIdAuthoritative) {
// This test verifies that parsing of the "relay" element.
// It checks both valid and invalid scenarios.
TEST_F(SharedNetwork4ParserTest, relayInfoTests) {
IfaceMgrTestConfig ifmgr(true);
// Create the vector of test scenarios.
std::vector<RelayTest> tests = {
......@@ -431,6 +432,24 @@ TEST_F(SharedNetwork4ParserTest, relayInfoTests) {
}
}
// This test verifies that an interface which is not in the system is rejected.
TEST_F(SharedNetwork4ParserTest, iface) {
// Basic configuration for shared network.
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
// Parse configuration specified above.
SharedNetwork4Parser parser;
EXPECT_NO_THROW(parser.parse(config_element, false));
EXPECT_THROW(parser.parse(config_element), DhcpConfigError);
// Configure default test interfaces.
IfaceMgrTestConfig ifmgr(true);
EXPECT_NO_THROW(parser.parse(config_element, false));
EXPECT_NO_THROW(parser.parse(config_element));
}
/// @brief Test fixture class for SharedNetwork6Parser class.
class SharedNetwork6ParserTest : public SharedNetworkParserTest {
......@@ -539,6 +558,8 @@ public:
// This test verifies that shared network parser for IPv4 works properly
// in a positive test scenario.
TEST_F(SharedNetwork6ParserTest, parse) {
IfaceMgrTestConfig ifmgr(true);
// Basic configuration for shared network. A bunch of parameters
// have to be specified for subnets because subnet parsers expect
// that default and global values are set.
......@@ -634,6 +655,8 @@ TEST_F(SharedNetwork6ParserTest, parse) {
// This test verifies that shared network parser for IPv4 works properly
// in a positive test scenario.
TEST_F(SharedNetwork6ParserTest, parseWithInterfaceId) {
IfaceMgrTestConfig ifmgr(true);
// Use the configuration with interface-id instead of interface parameter.
use_iface_id_ = true;
std::string config = getWorkingConfig();
......@@ -653,6 +676,8 @@ TEST_F(SharedNetwork6ParserTest, parseWithInterfaceId) {
// This test verifies that error is returned when trying to configure a
// shared network with both interface and interface id.
TEST_F(SharedNetwork6ParserTest, mutuallyExclusiveInterfaceId) {
IfaceMgrTestConfig ifmgr(true);
// Use the configuration with interface-id instead of interface parameter.
use_iface_id_ = true;
std::string config = getWorkingConfig();
......@@ -669,6 +694,8 @@ TEST_F(SharedNetwork6ParserTest, mutuallyExclusiveInterfaceId) {
// This test verifies that it's possible to specify client-class
// on shared-network level.
TEST_F(SharedNetwork6ParserTest, clientClass) {
IfaceMgrTestConfig ifmgr(true);
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
......@@ -686,6 +713,8 @@ TEST_F(SharedNetwork6ParserTest, clientClass) {
// This test verifies that it's possible to specify require-client-classes
// on shared-network level.
TEST_F(SharedNetwork6ParserTest, evalClientClasses) {
IfaceMgrTestConfig ifmgr(true);
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
......@@ -708,6 +737,8 @@ TEST_F(SharedNetwork6ParserTest, evalClientClasses) {
// This test verifies that bad require-client-classes configs raise
// expected errors.
TEST_F(SharedNetwork6ParserTest, badEvalClientClasses) {
IfaceMgrTestConfig ifmgr(true);
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
......@@ -737,6 +768,8 @@ TEST_F(SharedNetwork6ParserTest, badEvalClientClasses) {
// This test verifies that v6 parsing of the "relay" element.
// It checks both valid and invalid scenarios.
TEST_F(SharedNetwork6ParserTest, relayInfoTests) {
IfaceMgrTestConfig ifmgr(true);
// Create the vector of test scenarios.
std::vector<RelayTest> tests = {
......@@ -815,4 +848,22 @@ TEST_F(SharedNetwork6ParserTest, relayInfoTests) {
}
}
// This test verifies that an interface which is not in the system is rejected.
TEST_F(SharedNetwork6ParserTest, iface) {
// Basic configuration for shared network.
std::string config = getWorkingConfig();
ElementPtr config_element = Element::fromJSON(config);
// Parse configuration specified above.
SharedNetwork6Parser parser;
EXPECT_NO_THROW(parser.parse(config_element, false));
EXPECT_THROW(parser.parse(config_element), DhcpConfigError);
// Configure default test interfaces.
IfaceMgrTestConfig ifmgr(true);
EXPECT_NO_THROW(parser.parse(config_element, false));
EXPECT_NO_THROW(parser.parse(config_element));
}
} // end of anonymous namespace
......@@ -6,6 +6,7 @@
#include <config.h>
#include <cc/data.h>
#include <dhcp/tests/iface_mgr_test_config.h>
#include <dhcpsrv/cfg_shared_networks.h>
#include <dhcpsrv/shared_network.h>
#include <dhcpsrv/parsers/shared_networks_list_parser.h>
......@@ -15,12 +16,15 @@
using namespace isc;
using namespace isc::data;
using namespace isc::dhcp;
using namespace isc::dhcp::test;
namespace {
// This is a basic test verifying that all shared networks are correctly
// parsed.
TEST(SharedNetworkListParserTest, parse) {
IfaceMgrTestConfig ifmgr(true);
// Basic configuration with array of shared networks.
std::string config = "["
" {"
......@@ -37,7 +41,7 @@ TEST(SharedNetworkListParserTest, parse) {
ElementPtr config_element = Element::fromJSON(config);
SharedNetworks4ListParser parser;
CfgSharedNetworks4Ptr cfg(new CfgSharedNetworks4());;
CfgSharedNetworks4Ptr cfg(new CfgSharedNetworks4());
ASSERT_NO_THROW(parser.parse(cfg, config_element));
SharedNetwork4Ptr network1 = cfg->getByName("bird");
......@@ -58,6 +62,8 @@ TEST(SharedNetworkListParserTest, parse) {
// This test verifies that specifying two networks with the same name
// yields an error.
TEST(SharedNetworkListParserTest, duplicatedName) {
IfaceMgrTestConfig ifmgr(true);
// Basic configuration with two networks having the same name.
std::string config = "["
" {"
......@@ -73,8 +79,33 @@ TEST(SharedNetworkListParserTest, duplicatedName) {
ElementPtr config_element = Element::fromJSON(config);
SharedNetworks4ListParser parser;
CfgSharedNetworks4Ptr cfg(new CfgSharedNetworks4());;
CfgSharedNetworks4Ptr cfg(new CfgSharedNetworks4());
EXPECT_THROW(parser.parse(cfg, config_element), DhcpConfigError);
}
// This test verifies that an interface which is not in the system is rejected.
TEST(SharedNetworkListParserTest, iface) {
// Basic configuration with a shared network.
std::string config = "["
" {"
" \"name\": \"bird\","
" \"interface\": \"eth0\""
" }"
"]";
ElementPtr config_element = Element::fromJSON(config);
SharedNetworks6ListParser parser;
CfgSharedNetworks6Ptr cfg(new CfgSharedNetworks6());
EXPECT_NO_THROW(parser.parse(cfg, config_element, false));
cfg.reset(new CfgSharedNetworks6());
EXPECT_THROW(parser.parse(cfg, config_element), DhcpConfigError);
// Configure default test interfaces.
IfaceMgrTestConfig ifmgr(true);
cfg.reset(new CfgSharedNetworks6());
EXPECT_NO_THROW(parser.parse(cfg, config_element, false));
cfg.reset(new CfgSharedNetworks6());
EXPECT_NO_THROW(parser.parse(cfg, config_element));
}
} // 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