From: Francis Dupont Date: Wed, 29 Nov 2017 01:14:21 +0000 (+0100) Subject: [5351] Checkpoint: found a shared-network bug X-Git-Tag: trac5444_base~24 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=353dcb74d0cef960febe452cc9b028c05e875481;p=thirdparty%2Fkea.git [5351] Checkpoint: found a shared-network bug --- diff --git a/doc/examples/kea4/advanced.json b/doc/examples/kea4/advanced.json index 03ff4dc98c..7138533730 100644 --- a/doc/examples/kea4/advanced.json +++ b/doc/examples/kea4/advanced.json @@ -120,9 +120,9 @@ "subnet": "192.0.2.0/24", "user-context": { "comment": "Our first subnet!" - } + }, // Equivalent using smart parser - // "comment": "Our first subnet!" + "comment": "Our first subnet!" }, { // This particular subnet has match-client-id value changed. diff --git a/doc/examples/kea6/advanced.json b/doc/examples/kea6/advanced.json index 8e0fb0faae..fc9ab80278 100644 --- a/doc/examples/kea6/advanced.json +++ b/doc/examples/kea6/advanced.json @@ -107,7 +107,7 @@ // Here's the user-context for the whole subnet. "user-context": { "comment": "Floor one, west wing" }, // Equivalent using smart parser - // "comment": "Floor one, west wing", + "comment": "Floor one, west wing", // This defines PD (prefix delegation) pools. In this case // we have only one pool. That consists of /64 prefixes diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 8c00d545be..1a8cb35ee4 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -5607,4 +5607,74 @@ TEST_F(Dhcp4ParserTest, sharedNetworksDeriveClientClass) { EXPECT_TRUE(classes.empty()); } +// This test checks comments. Please keep it last. +TEST_F(Dhcp4ParserTest, comments) { + + string config = "{\n" + "\"shared-networks\": [ {\n" + " \"name\": \"foo\"\n," + " \"comment\": \"A shared-network\"\n," + " \"subnet4\": [\n" + " { \n" + " \"subnet\": \"192.0.1.0/24\",\n" + " \"comment\": \"A subnet\"\n," + " \"pools\": [\n" + " {\n" + " \"pool\": \"192.0.1.1-192.0.1.10\",\n" + " \"comment\": \"A pool\"\n" + " }\n" + " ]\n" + " }\n" + " ]\n" + " } ]\n" + "} \n"; + + extractConfig(config); + configure(config, CONTROL_RESULT_SUCCESS, ""); + + // Now verify that the shared network was indeed configured. + CfgSharedNetworks4Ptr cfg_net = CfgMgr::instance().getStagingCfg() + ->getCfgSharedNetworks4(); + ASSERT_TRUE(cfg_net); + const SharedNetwork4Collection* nets = cfg_net->getAll(); + ASSERT_TRUE(nets); + ASSERT_EQ(1, nets->size()); + SharedNetwork4Ptr net = nets->at(0); + ASSERT_TRUE(net); + + // Check shared network user context + ConstElementPtr ctx_net = net->getContext(); + ASSERT_TRUE(ctx_net); + ASSERT_EQ(1, ctx_net->size()); + ASSERT_TRUE(ctx_net->get("comment")); + EXPECT_EQ("\"A shared-network\"", ctx_net->get("comment")->str()); + + // The shared network has a subnet. + const Subnet4Collection * subs = net->getAllSubnets(); + ASSERT_TRUE(subs); + ASSERT_EQ(1, subs->size()); + Subnet4Ptr sub = subs->at(0); + ASSERT_TRUE(sub); + + // Check subnet user context + ConstElementPtr ctx_sub = sub->getContext(); + ASSERT_TRUE(ctx_sub); + ASSERT_EQ(1, ctx_sub->size()); + ASSERT_TRUE(ctx_sub->get("comment")); + EXPECT_EQ("\"A subnet\"", ctx_sub->get("comment")->str()); + + // The subnet has a pool + const PoolCollection& pools = sub->getPools(Lease::TYPE_V4); + ASSERT_EQ(1, pools.size()); + PoolPtr pool = pools.at(0); + ASSERT_TRUE(pool); + + // Check pool user context + ConstElementPtr ctx_pool = pool->getContext(); + ASSERT_TRUE(ctx_pool); + ASSERT_EQ(1, ctx_pool->size()); + ASSERT_TRUE(ctx_pool->get("comment")); + EXPECT_EQ("\"A pool\"", ctx_pool->get("comment")->str()); +} + } diff --git a/src/bin/dhcp4/tests/parser_unittest.cc b/src/bin/dhcp4/tests/parser_unittest.cc index adadbe9676..d35faef2f7 100644 --- a/src/bin/dhcp4/tests/parser_unittest.cc +++ b/src/bin/dhcp4/tests/parser_unittest.cc @@ -8,6 +8,7 @@ #include #include #include +#include using namespace isc::data; using namespace isc::test; @@ -209,6 +210,7 @@ TEST(ParserTest, multilineComments) { /// /// @param fname name of the file to be loaded void testFile(const std::string& fname) { + ElementPtr json; ElementPtr reference_json; ConstElementPtr test_json; @@ -216,7 +218,8 @@ void testFile(const std::string& fname) { cout << "Parsing file " << fname << " (" << decommented << ")" << endl; - EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true)); + EXPECT_NO_THROW(json = Element::fromJSONFile(decommented, true)); + reference_json = moveComments(json); // remove the temporary file EXPECT_NO_THROW(::remove(decommented.c_str())); diff --git a/src/bin/dhcp6/tests/parser_unittest.cc b/src/bin/dhcp6/tests/parser_unittest.cc index fdc9c85f38..bbc969e9e0 100644 --- a/src/bin/dhcp6/tests/parser_unittest.cc +++ b/src/bin/dhcp6/tests/parser_unittest.cc @@ -8,6 +8,7 @@ #include #include #include +#include using namespace isc::data; using namespace std; @@ -212,6 +213,7 @@ TEST(ParserTest, multilineComments) { /// /// @param fname name of the file to be loaded void testFile(const std::string& fname) { + ElementPtr json; ElementPtr reference_json; ConstElementPtr test_json; @@ -219,7 +221,8 @@ void testFile(const std::string& fname) { cout << "Parsing file " << fname << "(" << decommented << ")" << endl; - EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true)); + EXPECT_NO_THROW(json = Element::fromJSONFile(decommented, true)); + reference_json = moveComments(json); // remove the temporary file EXPECT_NO_THROW(::remove(decommented.c_str())); diff --git a/src/lib/dhcpsrv/pool.h b/src/lib/dhcpsrv/pool.h index 2660b90f6b..55d4235fe3 100644 --- a/src/lib/dhcpsrv/pool.h +++ b/src/lib/dhcpsrv/pool.h @@ -135,11 +135,6 @@ protected: /// @brief The last address in a pool isc::asiolink::IOAddress last_; - /// @brief Comments field - /// - /// @todo: This field is currently not used. - std::string comments_; - /// @brief defines a lease type that will be served from this pool Lease::Type type_; diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index cfd0c8d058..fc7665ec22 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -10,7 +10,8 @@ noinst_LTLIBRARIES = libkea-testutils.la libkea_testutils_la_SOURCES = io_utils.cc io_utils.h libkea_testutils_la_SOURCES += log_utils.cc log_utils.h libkea_testutils_la_SOURCES += test_to_element.cc test_to_element.h -libkea_testutils_la_SOURCES += unix_control_client.h unix_control_client.cc +libkea_testutils_la_SOURCES += unix_control_client.cc unix_control_client.h +libkea_testutils_la_SOURCES += user_context_utils.cc user_context_utils.h libkea_testutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) libkea_testutils_la_LIBADD = $(top_builddir)/src/lib/asiolink/libkea-asiolink.la libkea_testutils_la_LIBADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/lib/testutils/user_context_utils.cc b/src/lib/testutils/user_context_utils.cc new file mode 100644 index 0000000000..511ae73161 --- /dev/null +++ b/src/lib/testutils/user_context_utils.cc @@ -0,0 +1,125 @@ +// Copyright (C) 2017 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 +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include + +#include + +using namespace isc::data; + +namespace { + +/// @brief Exception to handle a unmodified value +/// @tparam EP ElementPtr or ConstElementPtr (compiler can't infer which one) +template +class UnModified { +public: + /// @brief Constructor + /// @param value the unmodified value + UnModified(EP value) : value_(value) { } + + /// @brief Get the value + /// @return the value + EP get() const { return (value_); } + +protected: + /// @brief the unmodified value + EP value_; +}; + +/// @brief Recursive helper +/// +/// Instead of returning a unmodified copy a @ref UnModified exception +/// is raised with the unmodified value +/// +/// @tparam EP ElementPtr or ConstElementPtr (compiler will infer which one) +/// @param element the element to traverse +/// @return a modified copy where comment entries were moved to user-context +/// @throw UnModified with the unmodified value +template +EP moveComments1(EP element) { + bool modified = false; + + // On lists recurse on items + if (element->getType() == Element::list) { + ElementPtr result = ElementPtr(new ListElement()); + typedef std::vector ListType; + const ListType& list = element->listValue(); + for (ListType::const_iterator it = list.cbegin(); + it != list.cend(); ++it) { + try { + result->add(moveComments1(*it)); + modified = true; + } + catch (const UnModified& ex) { + result->add(ex.get()); + } + } + if (!modified) { + throw UnModified(element); + } + return (result); + } else if (element->getType() != Element::map) { + throw UnModified(element); + } + + // Process maps: recurse on items + ElementPtr result = ElementPtr(new MapElement()); + bool has_comment = false; + typedef std::map map_type; + const map_type& map = element->mapValue(); + for (map_type::const_iterator it = map.cbegin(); it != map.cend(); ++it) { + if (it->first == "comment") { + // Note there is a comment entry to move + has_comment = true; + } else if (it->first == "user-context") { + // Do not traverse user-context entries + result->set("user-context", it->second); + } else { + // Not comment or user-context + try { + result->set(it->first, moveComments1(it->second)); + modified = true; + } + catch (const UnModified& ex) { + result->set(it->first, ex.get()); + } + } + } + // Check if the value should be not modified + if (!has_comment && !modified) { + throw UnModified(element); + } + + if (has_comment) { + // Move the comment entry + ConstElementPtr comment = element->get("comment"); + ElementPtr moved = Element::createMap(); + moved->set("comment", comment); + result->combine_set("user-context", moved); + } + + return (result); +} + +} // anonymous namespace + +namespace isc { +namespace test { + +ElementPtr moveComments(ElementPtr element) { + ElementPtr result; + try { + result = moveComments1(element); + } + catch (const UnModified& ex) { + result = ex.get(); + } + return result; +} + +}; // end of isc::test namespace +}; // end of isc namespace diff --git a/src/lib/testutils/user_context_utils.h b/src/lib/testutils/user_context_utils.h new file mode 100644 index 0000000000..417fdc6290 --- /dev/null +++ b/src/lib/testutils/user_context_utils.h @@ -0,0 +1,24 @@ +// Copyright (C) 2017 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 +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef USER_CONTEXT_UTILS_H +#define USER_CONTEXT_UTILS_H + +#include + +namespace isc { +namespace test { + +/// @brief Move comment entries to user-context +/// +/// @param element +/// @return a processed copy of element or unmodified element +isc::data::ElementPtr moveComments(isc::data::ElementPtr element); + +}; // end of isc::test namespace +}; // end of isc namespace + +#endif // USER_CONTEXT_UTILS_H