From: Tomek Mrugalski Date: Mon, 4 Sep 2017 11:57:23 +0000 (+0200) Subject: [5350] User contexts implemented for subnets. X-Git-Tag: trac5073a_base~16^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28558468b27c46bb8160848fe446476bc63a82a4;p=thirdparty%2Fkea.git [5350] User contexts implemented for subnets. --- diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index 1045238a93..a464044685 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -525,6 +525,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} \"user-context\" { switch(driver.ctx_) { + case isc::dhcp::Parser4Context::SUBNET4: case isc::dhcp::Parser4Context::POOLS: return isc::dhcp::Dhcp4Parser::make_USER_CONTEXT(driver.loc_); default: diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index c5af1f06c1..13cddf571c 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -874,6 +874,7 @@ subnet4_param: valid_lifetime | subnet_4o6_interface | subnet_4o6_interface_id | subnet_4o6_subnet + | user_context | unknown_map_entry ; diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 9cf63cd84e..04113409c2 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -119,7 +119,22 @@ const char* CONFIGS[] = { " \"always-send\": true" " }" " ]" - "}" }; + "}", + + // Configuration 3: + // - one subnet, with one pool + // - user-contexts defined in both subnet and pool + "{" + " \"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"10.254.226.0/25\"," + " \"user-context\": { \"value\": 42 } } ]," + " \"subnet\": \"10.254.226.0/24\", " + " \"user-context\": {" + " \"secure\": false" + " }" + " } ]," + "\"valid-lifetime\": 4000 }", +}; // This test verifies that the destination address of the response // message is set to giaddr, when giaddr is set to non-zero address @@ -2795,6 +2810,37 @@ TEST_F(Dhcpv4SrvTest, tooLongClientId) { EXPECT_NO_THROW(client.doDORA()); } +// Checks if user-contexts are parsed properly. +TEST_F(Dhcpv4SrvTest, userContext) { + + IfaceMgrTestConfig test_config(true); + + NakedDhcpv4Srv srv(0); + + // This config has one subnet with user-context with one + // pool (also with context). Make sure the configuration could be accepted. + cout << CONFIGS[3] << endl; + EXPECT_NO_THROW(configure(CONFIGS[3])); + + // Now make sure the data was not lost. + ConstSrvConfigPtr cfg = CfgMgr::instance().getCurrentCfg(); + const Subnet4Collection* subnets = cfg->getCfgSubnets4()->getAll(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Let's get the subnet and check its context. + Subnet4Ptr subnet1 = (*subnets)[0]; + ASSERT_TRUE(subnet1); + ASSERT_TRUE(subnet1->getContext()); + EXPECT_EQ("{ \"secure\": false }", subnet1->getContext()->str()); + + // Ok, not get the sole pool in it and check its context, too. + PoolCollection pools = subnet1->getPools(Lease::TYPE_V4); + ASSERT_EQ(1, pools.size()); + ASSERT_TRUE(pools[0]); + ASSERT_TRUE(pools[0]->getContext()); + EXPECT_EQ("{ \"value\": 42 }", pools[0]->getContext()->str()); +} /// @todo: Implement proper tests for MySQL lease/host database, /// see ticket #4214. diff --git a/src/bin/dhcp6/dhcp6_lexer.ll b/src/bin/dhcp6/dhcp6_lexer.ll index 4c7a418a9a..04e82e112f 100644 --- a/src/bin/dhcp6/dhcp6_lexer.ll +++ b/src/bin/dhcp6/dhcp6_lexer.ll @@ -784,6 +784,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} switch(driver.ctx_) { case isc::dhcp::Parser6Context::POOLS: case isc::dhcp::Parser6Context::PD_POOLS: + case isc::dhcp::Parser6Context::SUBNET6: return isc::dhcp::Dhcp6Parser::make_USER_CONTEXT(driver.loc_); default: return isc::dhcp::Dhcp6Parser::make_STRING("user-context", driver.loc_); diff --git a/src/bin/dhcp6/dhcp6_parser.yy b/src/bin/dhcp6/dhcp6_parser.yy index 6bebc2f920..a6ece57212 100644 --- a/src/bin/dhcp6/dhcp6_parser.yy +++ b/src/bin/dhcp6/dhcp6_parser.yy @@ -885,6 +885,7 @@ subnet6_param: preferred_lifetime | reservations | reservation_mode | relay + | user_context | unknown_map_entry ; diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 334bbae6b0..cb408c4b34 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -125,6 +125,26 @@ const char* CONFIGS[] = { " \"always-send\": true" " }" " ]" + "}", + + // Configuration 3: + // - one subnet with one address pool and one prefix pool + // - user-contexts defined in subnet and each pool + "{" + " \"subnet6\": [ {" + " \"pools\": [ {" + " \"pool\": \"2001:db8:1::/64\"," + " \"user-context\": { \"value\": 42 }" + " } ], " + " \"pd-pools\": [ {" + " \"prefix\": \"2001:db8:abcd::\"," + " \"prefix-len\": 48," + " \"delegated-len\": 64," + " \"user-context\": { \"type\": \"prefixes\" }" + " } ]," + " \"subnet\": \"2001:db8:1::/48\"," + " \"user-context\": { \"secure\": false }" + " } ] " "}" }; @@ -2424,6 +2444,44 @@ TEST_F(Dhcpv6SrvTest, tooLongServerId) { EXPECT_NO_THROW(client.doSARR()); } +// Checks if user-contexts are parsed properly. +TEST_F(Dhcpv6SrvTest, userContext) { + + IfaceMgrTestConfig test_config(true); + + NakedDhcpv6Srv srv(0); + + // This config has one subnet with user-context with one + // pool (also with context). Make sure the configuration could be accepted. + EXPECT_NO_THROW(configure(CONFIGS[3])); + + // Now make sure the data was not lost. + ConstSrvConfigPtr cfg = CfgMgr::instance().getCurrentCfg(); + const Subnet6Collection* subnets = cfg->getCfgSubnets6()->getAll(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Let's get the subnet and check its context. + Subnet6Ptr subnet1 = (*subnets)[0]; + ASSERT_TRUE(subnet1); + ASSERT_TRUE(subnet1->getContext()); + EXPECT_EQ("{ \"secure\": false }", subnet1->getContext()->str()); + + // Ok, not get the address pool in it and check its context, too. + PoolCollection pools = subnet1->getPools(Lease::TYPE_NA); + ASSERT_EQ(1, pools.size()); + ASSERT_TRUE(pools[0]); + ASSERT_TRUE(pools[0]->getContext()); + EXPECT_EQ("{ \"value\": 42 }", pools[0]->getContext()->str()); + + // Ok, not get the prefix pool in it and check its context, too. + pools = subnet1->getPools(Lease::TYPE_PD); + ASSERT_EQ(1, pools.size()); + ASSERT_TRUE(pools[0]); + ASSERT_TRUE(pools[0]->getContext()); + EXPECT_EQ("{ \"type\": \"prefixes\" }", pools[0]->getContext()->str()); +} + /// @todo: Add more negative tests for processX(), e.g. extend sanityCheck() test /// to call processX() methods. diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 30995de0d0..5cfb34cab0 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -445,7 +445,7 @@ PoolParser::parse(PoolStoragePtr pools, isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map (" << user_context->getPosition() << ")"); } - pool->setUserContext(user_context); + pool->setContext(user_context); } // Parser pool specific options. @@ -620,6 +620,16 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) { subnet_->allowClientClass(client_class); } + // If there's user-context specified, store it. + ConstElementPtr user_context = params->get("user-context"); + if (user_context) { + if (user_context->getType() != Element::map) { + isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map (" + << user_context->getPosition() << ")"); + } + subnet_->setContext(user_context); + } + // Here globally defined options were merged to the subnet specific // options but this is no longer the case (they have a different // and not consecutive priority). @@ -890,7 +900,7 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) { } if (user_context_) { - pool_->setUserContext(user_context_); + pool_->setContext(user_context_); } // Add the local pool to the external storage ptr. diff --git a/src/lib/dhcpsrv/pool.h b/src/lib/dhcpsrv/pool.h index 9f66370f91..56d769891d 100644 --- a/src/lib/dhcpsrv/pool.h +++ b/src/lib/dhcpsrv/pool.h @@ -102,7 +102,7 @@ public: /// @brief Sets user context. /// @param ctx user context to be stored. - void setUserContext(const data::ConstElementPtr& ctx) { + void setContext(const data::ConstElementPtr& ctx) { user_context_ = ctx; } diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index a28c0a9cbf..b848559178 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/src/lib/dhcpsrv/subnet.cc @@ -542,6 +542,12 @@ Subnet::toElement() const { ConstCfgOptionPtr opts = getCfgOption(); map->set("option-data", opts->toElement()); + // Add user-context, but only if defined. Omit if it was not. + ConstElementPtr ctx = getContext(); + if (ctx) { + map->set("user-context", ctx); + } + return (map); } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 3d91ae8bb9..0647db833f 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -349,6 +349,17 @@ public: host_reservation_mode_ = mode; } + /// @brief Sets user context. + /// @param ctx user context to be stored. + void setContext(const data::ConstElementPtr& ctx) { + user_context_ = ctx; + } + + /// @brief Returns const pointer to the user context. + data::ConstElementPtr getContext() const { + return (user_context_); + } + protected: /// @brief Returns all pools (non-const variant) /// @@ -518,6 +529,9 @@ protected: /// /// See @ref HRMode type for details. HRMode host_reservation_mode_; + + /// @brief Pointer to the user context (may be NULL) + data::ConstElementPtr user_context_; private: /// @brief Pointer to the option data configuration for this subnet. diff --git a/src/lib/dhcpsrv/tests/pool_unittest.cc b/src/lib/dhcpsrv/tests/pool_unittest.cc index 5447e99156..b78cf1fbcd 100644 --- a/src/lib/dhcpsrv/tests/pool_unittest.cc +++ b/src/lib/dhcpsrv/tests/pool_unittest.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -21,6 +22,7 @@ using boost::scoped_ptr; using namespace isc; using namespace isc::dhcp; +using namespace isc::data; using namespace isc::asiolink; namespace { @@ -172,6 +174,22 @@ TEST(Pool4Test, addOptions) { EXPECT_TRUE(options->empty()); } +// This test checks that handling for user-context is valid. +TEST(Pool4Test, userContext) { + // Create a pool to add options to it. + Pool4Ptr pool(new Pool4(IOAddress("192.0.2.0"), + IOAddress("192.0.2.255"))); + + // Context should be empty until explicitly set. + EXPECT_FALSE(pool->getContext()); + + // When set, should be returned properly. + ElementPtr ctx = Element::create("{ \"comment\": \"foo\" }"); + EXPECT_NO_THROW(pool->setContext(ctx)); + ASSERT_TRUE(pool->getContext()); + EXPECT_EQ(ctx->str(), pool->getContext()->str()); +} + TEST(Pool6Test, constructor_first_last) { // let's construct 2001:db8:1:: - 2001:db8:1::ffff:ffff:ffff:ffff pool @@ -460,4 +478,20 @@ TEST(Pool6Test, addOptions) { EXPECT_TRUE(options->empty()); } +// This test checks that handling for user-context is valid. +TEST(Pool6Test, userContext) { + // Create a pool to add options to it. + Pool6 pool(Lease::TYPE_NA, IOAddress("2001:db8::1"), + IOAddress("2001:db8::2")); + + // Context should be empty until explicitly set. + EXPECT_FALSE(pool.getContext()); + + // When set, should be returned properly. + ElementPtr ctx = Element::create("{ \"comment\": \"foo\" }"); + EXPECT_NO_THROW(pool.setContext(ctx)); + ASSERT_TRUE(pool.getContext()); + EXPECT_EQ(ctx->str(), pool.getContext()->str()); +} + }; // end of anonymous namespace