From: Razvan Becheriu Date: Mon, 12 Sep 2022 06:08:47 +0000 (+0300) Subject: [#248] added template classes X-Git-Tag: Kea-2.3.2~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=033cc7bd7c2152da5a5d0de31d0b8526553c262e;p=thirdparty%2Fkea.git [#248] added template classes --- diff --git a/doc/examples/kea4/all-keys.json b/doc/examples/kea4/all-keys.json index 3ec9849a6f..2d0ee979fe 100644 --- a/doc/examples/kea4/all-keys.json +++ b/doc/examples/kea4/all-keys.json @@ -96,7 +96,10 @@ "only-if-required": true, // Class selection expression. - "test": "member('ALL')" + "test": "member('ALL')", + + // Template class flag. + "template-class": false } ], diff --git a/doc/examples/kea6/all-keys.json b/doc/examples/kea6/all-keys.json index 5baaaa71c2..0156c3918e 100644 --- a/doc/examples/kea6/all-keys.json +++ b/doc/examples/kea6/all-keys.json @@ -65,7 +65,10 @@ "only-if-required": true, // Class selection expression. - "test": "member('ALL')" + "test": "member('ALL')", + + // Template class flag. + "template-class": false } ], diff --git a/src/bin/d2/parser_context.cc b/src/bin/d2/parser_context.cc index f27a628c6a..703460a6de 100644 --- a/src/bin/d2/parser_context.cc +++ b/src/bin/d2/parser_context.cc @@ -169,25 +169,25 @@ D2ParserContext::contextName() case TSIG_KEYS: return ("tsig-keys"); case ALGORITHM: - return("algorithm"); + return ("algorithm"); case DIGEST_BITS: - return("digest-bits"); + return ("digest-bits"); case SECRET: - return("secret"); + return ("secret"); case FORWARD_DDNS: - return("forward-ddns"); + return ("forward-ddns"); case REVERSE_DDNS: - return("reverse-ddns"); + return ("reverse-ddns"); case DDNS_DOMAIN: - return("ddns-domain"); + return ("ddns-domain"); case DDNS_DOMAINS: - return("ddns-domains"); + return ("ddns-domains"); case DNS_SERVER: - return("dns-server"); + return ("dns-server"); case DNS_SERVERS: - return("dns-servers"); + return ("dns-servers"); case CONTROL_SOCKET: - return("control-socket"); + return ("control-socket"); case LOGGERS: return ("loggers"); case OUTPUT_OPTIONS: diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index 1114f673eb..1aebaf1551 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -1279,6 +1279,15 @@ ControlCharacterFill [^"\\]|\\["\\/bfnrtu] } } +\"template-class\" { + switch(driver.ctx_) { + case isc::dhcp::Parser4Context::CLIENT_CLASSES: + return isc::dhcp::Dhcp4Parser::make_TEMPLATE_CLASS(driver.loc_); + default: + return isc::dhcp::Dhcp4Parser::make_STRING("template-class", driver.loc_); + } +} + \"only-if-required\" { switch(driver.ctx_) { case isc::dhcp::Parser4Context::CLIENT_CLASSES: diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc index c789ab4f56..e93a9b0022 100644 --- a/src/bin/dhcp4/dhcp4_messages.cc +++ b/src/bin/dhcp4/dhcp4_messages.cc @@ -55,12 +55,17 @@ extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_MISSING = "DHCP4_DEFERRED extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_UNPACK_FAIL = "DHCP4_DEFERRED_OPTION_UNPACK_FAIL"; extern const isc::log::MessageID DHCP4_DEVELOPMENT_VERSION = "DHCP4_DEVELOPMENT_VERSION"; extern const isc::log::MessageID DHCP4_DHCP4O6_BAD_PACKET = "DHCP4_DHCP4O6_BAD_PACKET"; +extern const isc::log::MessageID DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP = "DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP"; +extern const isc::log::MessageID DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP = "DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP"; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_RECEIVED = "DHCP4_DHCP4O6_PACKET_RECEIVED"; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND = "DHCP4_DHCP4O6_PACKET_SEND"; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND_FAIL = "DHCP4_DHCP4O6_PACKET_SEND_FAIL"; extern const isc::log::MessageID DHCP4_DHCP4O6_RECEIVE_FAIL = "DHCP4_DHCP4O6_RECEIVE_FAIL"; extern const isc::log::MessageID DHCP4_DHCP4O6_RECEIVING = "DHCP4_DHCP4O6_RECEIVING"; extern const isc::log::MessageID DHCP4_DHCP4O6_RESPONSE_DATA = "DHCP4_DHCP4O6_RESPONSE_DATA"; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_DATA = "DHCP4_DHCP4O6_SUBNET_DATA"; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_SELECTED = "DHCP4_DHCP4O6_SUBNET_SELECTED"; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED = "DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED"; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION = "DHCP4_DYNAMIC_RECONFIGURATION"; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION_FAIL = "DHCP4_DYNAMIC_RECONFIGURATION_FAIL"; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION_SUCCESS = "DHCP4_DYNAMIC_RECONFIGURATION_SUCCESS"; @@ -217,12 +222,17 @@ const char* values[] = { "DHCP4_DEFERRED_OPTION_UNPACK_FAIL", "An error unpacking the deferred option %1: %2", "DHCP4_DEVELOPMENT_VERSION", "This software is a development branch of Kea. It is not recommended for production use.", "DHCP4_DHCP4O6_BAD_PACKET", "received malformed DHCPv4o6 packet: %1", + "DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP", "%1: packet was dropped, because a callout set the next step to 'drop'", + "DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP", "%1: no subnet was selected, because a callout set the next skip flag", "DHCP4_DHCP4O6_PACKET_RECEIVED", "received DHCPv4o6 packet from DHCPv4 server (type %1) for %2 on interface %3", "DHCP4_DHCP4O6_PACKET_SEND", "%1: trying to send packet %2 (type %3) to %4 port %5 on interface %6 encapsulating %7: %8 (type %9)", "DHCP4_DHCP4O6_PACKET_SEND_FAIL", "%1: failed to send DHCPv4o6 packet: %2", "DHCP4_DHCP4O6_RECEIVE_FAIL", "failed to receive DHCPv4o6: %1", "DHCP4_DHCP4O6_RECEIVING", "receiving DHCPv4o6 packet from DHCPv6 server", "DHCP4_DHCP4O6_RESPONSE_DATA", "%1: responding with packet %2 (type %3), packet details: %4", + "DHCP4_DHCP4O6_SUBNET_DATA", "%1: the selected subnet details: %2", + "DHCP4_DHCP4O6_SUBNET_SELECTED", "%1: the subnet with ID %2 was selected for client assignments", + "DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED", "%1: failed to select subnet for the client", "DHCP4_DYNAMIC_RECONFIGURATION", "initiate server reconfiguration using file: %1, after receiving SIGHUP signal or config-reload command", "DHCP4_DYNAMIC_RECONFIGURATION_FAIL", "dynamic server reconfiguration failed with file: %1", "DHCP4_DYNAMIC_RECONFIGURATION_SUCCESS", "dynamic server reconfiguration succeeded with file: %1", diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h index c69c877b20..1ff14ac5e0 100644 --- a/src/bin/dhcp4/dhcp4_messages.h +++ b/src/bin/dhcp4/dhcp4_messages.h @@ -56,12 +56,17 @@ extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_MISSING; extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_UNPACK_FAIL; extern const isc::log::MessageID DHCP4_DEVELOPMENT_VERSION; extern const isc::log::MessageID DHCP4_DHCP4O6_BAD_PACKET; +extern const isc::log::MessageID DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP; +extern const isc::log::MessageID DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_RECEIVED; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND; extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND_FAIL; extern const isc::log::MessageID DHCP4_DHCP4O6_RECEIVE_FAIL; extern const isc::log::MessageID DHCP4_DHCP4O6_RECEIVING; extern const isc::log::MessageID DHCP4_DHCP4O6_RESPONSE_DATA; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_DATA; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_SELECTED; +extern const isc::log::MessageID DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION_FAIL; extern const isc::log::MessageID DHCP4_DYNAMIC_RECONFIGURATION_SUCCESS; diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 1356ed2a69..a5afe6e568 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -445,6 +445,22 @@ will be only able to offer global options - no addresses will be assigned. The argument specifies the client and transaction identification information. +% DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP %1: packet was dropped, because a callout set the next step to 'drop' +This debug message is printed when a callout installed on the +subnet4_select hook point sets the next step to 'drop' value. For this particular hook +point, the setting to that value instructs the server to drop the received +packet. The argument specifies the client and transaction identification +information. + +% DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP %1: no subnet was selected, because a callout set the next skip flag +This debug message is printed when a callout installed on the +subnet4_select hook point sets the next step to SKIP value. For this particular hook +point, the setting of the flag instructs the server not to choose a +subnet, an action that severely limits further processing; the server +will be only able to offer global options - no addresses will be assigned. +The argument specifies the client and transaction identification +information. + % DHCP4_INFORM_DIRECT_REPLY %1: DHCPACK in reply to the DHCPINFORM will be sent directly to %2 over %3 This debug message is issued when the DHCPACK will be sent directly to the client, rather than via a relay. The first argument contains the client @@ -878,7 +894,7 @@ upon restart. The reason for the failure is given within the message. % DHCP4_SRV_DHCP4O6_ERROR error stopping IO with DHCPv4o6 during shutdown: %1 This error message indicates that during shutdown, an error occurred while -stopping IO between the DHCPv4 server and the DHCPv6o6 server. This is +stopping IO between the DHCPv4 server and the DHCPv4o6 server. This is probably due to a programmatic error is not likely to impact either server upon restart. The reason for the failure is given within the message. @@ -910,6 +926,12 @@ the client. The first argument includes the client and the transaction identification information. The second arguments includes the subnet details. +% DHCP4_DHCP4O6_SUBNET_DATA %1: the selected subnet details: %2 +This debug message includes the details of the subnet selected for +the client. The first argument includes the client and the +transaction identification information. The second arguments +includes the subnet details. + % DHCP4_SUBNET_DYNAMICALLY_CHANGED %1: changed selected subnet %2 to subnet %3 from shared network %4 for client assignments This debug message indicates that the server is using another subnet than initially selected for client assignments. This newly selected @@ -934,6 +956,22 @@ and will send DHCPNAK if the received message was DHCPREQUEST. The argument includes the client and the transaction identification information. +% DHCP4_DHCP4O6_SUBNET_SELECTED %1: the subnet with ID %2 was selected for client assignments +This is a debug message noting the selection of a subnet to be used for +address and option assignment. Subnet selection is one of the early +steps in the processing of incoming client message. The first +argument includes the client and the transaction identification +information. The second argument holds the selected subnet id. + +% DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED %1: failed to select subnet for the client +This debug message indicates that the server failed to select the +subnet for the client which has sent a message to the server. +The server will not be able to offer any lease to the client and +will drop its message if the received message was DHCPDISCOVER, +and will send DHCPNAK if the received message was DHCPREQUEST. +The argument includes the client and the transaction identification +information. + % DHCP4_TESTING_MODE_SEND_TO_SOURCE_ENABLED All packets will be send to source address of an incoming packet - use only for testing This message is printed then KEA_TEST_SEND_RESPONSES_TO_SOURCE environment variable is set. It's causing Kea to send packets to diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index 0ad1f5ee1e..9783cc9d18 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -170,6 +170,7 @@ using namespace std; CLIENT_CLASSES "client-classes" REQUIRE_CLIENT_CLASSES "require-client-classes" TEST "test" + TEMPLATE_CLASS "template-class" ONLY_IF_REQUIRED "only-if-required" CLIENT_CLASS "client-class" @@ -2335,6 +2336,7 @@ not_empty_client_class_params: client_class_param client_class_param: client_class_name | client_class_test + | client_class_template_class | only_if_required | option_def_list | option_data_list @@ -2360,6 +2362,12 @@ client_class_test: TEST { ctx.leave(); }; +client_class_template_class: TEMPLATE_CLASS COLON BOOLEAN { + ctx.unique("template-class", ctx.loc2pos(@1)); + ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); + ctx.stack_.back()->set("template-class", b); +}; + only_if_required: ONLY_IF_REQUIRED COLON BOOLEAN { ctx.unique("only-if-required", ctx.loc2pos(@1)); ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 263cb8726c..67cc5a333a 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -250,7 +250,7 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, // Perform second pass of classification. evaluateClasses(query, true); - const ClientClasses& classes = query_->getClasses(); + const ClientClasses& classes = query_->getClassesAndSubClasses(); if (!classes.empty()) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_ASSIGNED) .arg(query_->getLabel()) @@ -595,30 +595,7 @@ void Dhcpv4Exchange::evaluateClasses(const Pkt4Ptr& pkt, bool depend_on_known) { if ((*it)->getDependOnKnown() != depend_on_known) { continue; } - // Evaluate the expression which can return false (no match), - // true (match) or raise an exception (error) - try { - bool status = evaluateBool(*expr_ptr, *pkt); - if (status) { - LOG_INFO(options4_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg(status); - // Matching: add the class - pkt->addClass((*it)->getName()); - } else { - LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, EVAL_RESULT) - .arg((*it)->getName()) - .arg(status); - } - } catch (const Exception& ex) { - LOG_ERROR(options4_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg(ex.what()); - } catch (...) { - LOG_ERROR(options4_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg("get exception?"); - } + (*it)->test(pkt, expr_ptr); } } @@ -820,7 +797,6 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query, bool& drop, isc::dhcp::Subnet4Ptr Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop, bool sanity_only) const { - Subnet4Ptr subnet; SubnetSelector selector; @@ -896,7 +872,7 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop, // be severely limited (i.e. only global options will be assigned) if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) { LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, - DHCP4_HOOK_SUBNET4_SELECT_SKIP) + DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_SKIP) .arg(query->getLabel()); return (Subnet4Ptr()); } @@ -905,7 +881,7 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop, // skip case so no subnet will be selected. if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) { LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, - DHCP4_HOOK_SUBNET4_SELECT_DROP) + DHCP4_DHCP4O6_HOOK_SUBNET4_SELECT_DROP) .arg(query->getLabel()); drop = true; return (Subnet4Ptr()); @@ -917,18 +893,20 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop, if (subnet) { // Log at higher debug level that subnet has been found. - LOG_DEBUG(packet4_logger, DBG_DHCP4_BASIC_DATA, DHCP4_SUBNET_SELECTED) + LOG_DEBUG(packet4_logger, DBG_DHCP4_BASIC_DATA, + DHCP4_DHCP4O6_SUBNET_SELECTED) .arg(query->getLabel()) .arg(subnet->getID()); // Log detailed information about the selected subnet at the // lower debug level. - LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_DATA) + LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL_DATA, + DHCP4_DHCP4O6_SUBNET_DATA) .arg(query->getLabel()) .arg(subnet->toText()); } else { LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL, - DHCP4_SUBNET_SELECTION_FAILED) + DHCP4_DHCP4O6_SUBNET_SELECTION_FAILED) .arg(query->getLabel()); } @@ -1794,7 +1772,7 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) { } // Each class in the incoming packet - const ClientClasses& classes = ex.getQuery()->getClasses(); + const ClientClasses& classes = ex.getQuery()->getClassesAndTemplates(); for (ClientClasses::const_iterator cclass = classes.cbegin(); cclass != classes.cend(); ++cclass) { // Find the client class definition for this class @@ -2511,6 +2489,8 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { Lease4Ptr lease; + auto const& classes = query->getClassesAndSubClasses(); + // We used to issue a separate query (two actually: one for client-id // and another one for hw-addr for) each subnet in the shared network. // That was horribly inefficient if the client didn't have any lease @@ -2540,7 +2520,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { break; } else { - s = s->getNextSubnet(original_subnet, query->getClasses()); + s = s->getNextSubnet(original_subnet, classes); } } } @@ -2568,7 +2548,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { break; } else { - s = s->getNextSubnet(original_subnet, query->getClasses()); + s = s->getNextSubnet(original_subnet, classes); } } } @@ -3141,7 +3121,7 @@ Dhcpv4Srv::setFixedFields(Dhcpv4Exchange& ex) { // Step 2: Try to set the values based on classes. // Any values defined in classes will override those from subnet level. - const ClientClasses classes = query->getClasses(); + const ClientClasses classes = query->getClassesAndTemplates(); if (!classes.empty()) { // Let's get class definitions @@ -3926,7 +3906,7 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& query) const { } // Check if the server identifier is configured at client class level. - const ClientClasses& classes = query->getClasses(); + const ClientClasses& classes = query->getClassesAndTemplates(); for (ClientClasses::const_iterator cclass = classes.cbegin(); cclass != classes.cend(); ++cclass) { // Find the client class definition for this class @@ -4006,7 +3986,7 @@ void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) { void Dhcpv4Srv::requiredClassify(Dhcpv4Exchange& ex) { // First collect required classes Pkt4Ptr query = ex.getQuery(); - ClientClasses classes = query->getClasses(true); + ClientClasses classes = query->getClassesAndTemplates(true); Subnet4Ptr subnet = ex.getContext()->subnet_; if (subnet) { @@ -4100,7 +4080,7 @@ Dhcpv4Srv::deferredUnpack(Pkt4Ptr& query) { BOOST_FOREACH(const uint16_t& code, query->getDeferredOptions()) { OptionDefinitionPtr def; // Iterate on client classes - const ClientClasses& classes = query->getClasses(); + const ClientClasses& classes = query->getClassesAndTemplates(); for (ClientClasses::const_iterator cclass = classes.cbegin(); cclass != classes.cend(); ++cclass) { // Get the client class definition for this class diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 01b625ffe7..a263daf940 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -3712,13 +3712,13 @@ TEST_F(Dhcp4ParserTest, stdOptionData) { // in the structure returned. OptionPtr option = range.first->option_; ASSERT_TRUE(option); - // Option object returned for here is expected to be Option6IA + // Option object returned for here is expected to be Option4AddrLst // which is derived from Option. This class is dedicated to - // represent standard option IA_NA. + // represent standard option DHO_NIS_SERVERS. boost::shared_ptr option_addrs = boost::dynamic_pointer_cast(option); // If cast is unsuccessful than option returned was of a - // different type than Option6IA. This is wrong. + // different type than Option4AddrLst. This is wrong. ASSERT_TRUE(option_addrs); // Get addresses from the option. @@ -6093,6 +6093,64 @@ TEST_F(Dhcp4ParserTest, clientClassValidLifetime) { EXPECT_TRUE(class_def->getValid().unspecified()); } +// Verifies that simple list of valid template classes parses and +// is staged for commit. +TEST_F(Dhcp4ParserTest, templateClientClassValidLifetime) { + string config = "{ " + genIfaceConfig() + "," + + "\"client-classes\" : [ \n" + " { \n" + " \"name\": \"one\", \n" + " \"min-valid-lifetime\": 1000, \n" + " \"valid-lifetime\": 2000, \n" + " \"max-valid-lifetime\": 3000, \n" + " \"template-class\": true, \n" + " }, \n" + " { \n" + " \"name\": \"two\", \n" + " \"template-class\": true \n" + " } \n" + "], \n" + "\"subnet4\": [ { \n" + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ], \n" + " \"subnet\": \"192.0.2.0/24\" \n" + " } ] \n" + "} \n"; + + ConstElementPtr json; + ASSERT_NO_THROW_LOG(json = parseDHCP4(config)); + extractConfig(config); + + ConstElementPtr status; + ASSERT_NO_THROW_LOG(status = configureDhcp4Server(*srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 0); + + // We check staging config because CfgMgr::commit hasn't been executed. + ClientClassDictionaryPtr dictionary; + dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ASSERT_TRUE(dictionary); + EXPECT_EQ(2, dictionary->getClasses()->size()); + + // Execute the commit + ASSERT_NO_THROW(CfgMgr::instance().commit()); + + // Verify that after commit, the current config has the correct dictionary + dictionary = CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); + ASSERT_TRUE(dictionary); + EXPECT_EQ(2, dictionary->getClasses()->size()); + + ClientClassDefPtr class_def = dictionary->findClass("one"); + ASSERT_TRUE(class_def); + ASSERT_TRUE(dynamic_cast(class_def.get())); + EXPECT_EQ(class_def->getValid().getMin(), 1000); + EXPECT_EQ(class_def->getValid().get(), 2000); + EXPECT_EQ(class_def->getValid().getMax(), 3000); + + class_def = dictionary->findClass("two"); + ASSERT_TRUE(class_def); + ASSERT_TRUE(dynamic_cast(class_def.get())); + EXPECT_TRUE(class_def->getValid().unspecified()); +} // Test verifies that regular configuration does not provide any user context // in the address pool. diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index 2a275c6041..2caf0ec6dd 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/src/bin/dhcp4/tests/fqdn_unittest.cc @@ -398,7 +398,7 @@ public: return (DdnsParamsPtr(new DdnsParams())); } - return(CfgMgr::instance().getCurrentCfg()->getDdnsParams(subnet_)); + return (CfgMgr::instance().getCurrentCfg()->getDdnsParams(subnet_)); } // Create a lease to be used by various tests. @@ -457,8 +457,8 @@ public: /// @return An std::string contained the generated FQDN. std::string generatedNameFromAddress(const IOAddress& addr, const bool trailing_dot = true) { - return(CfgMgr::instance().getD2ClientMgr() - .generateFqdn(addr, *getDdnsParams(), trailing_dot)); + return (CfgMgr::instance().getD2ClientMgr() + .generateFqdn(addr, *getDdnsParams(), trailing_dot)); } // Get the Client FQDN Option from the given message. diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 049a065dca..84d37f8d97 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -112,7 +112,7 @@ public: cerr << "(fixture ctor) unloadLibraries failed" << endl; } - // Allocate new DHCPv6 Server + // Allocate new DHCPv4 Server srv_ = new NakedDhcpv4Srv(0); // clear static buffers @@ -981,7 +981,7 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveValueChange) { srv_->fakeReceive(discover); // Server will now process to run its normal loop, but instead of calling - // IfaceMgr::receive6(), it will read all packets from the list set by + // IfaceMgr::receive4(), it will read all packets from the list set by // fakeReceive() // In particular, it should call registered buffer4_receive callback. srv_->run(); @@ -1025,7 +1025,7 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveSkip) { srv_->fakeReceive(discover); // Server will now process to run its normal loop, but instead of calling - // IfaceMgr::receive6(), it will read all packets from the list set by + // IfaceMgr::receive4(), it will read all packets from the list set by // fakeReceive() // In particular, it should call registered pkt4_receive callback. srv_->run(); @@ -1054,7 +1054,7 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveDrop) { srv_->fakeReceive(discover); // Server will now process to run its normal loop, but instead of calling - // IfaceMgr::receive6(), it will read all packets from the list set by + // IfaceMgr::receive4(), it will read all packets from the list set by // fakeReceive() // In particular, it should call registered pkt4_receive callback. srv_->run(); diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 86d9d099b5..2f5b581b3f 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -721,7 +721,7 @@ TEST_F(JSONFileBackendTest, configBroken) { string config_empty = ""; // This config does not have mandatory Dhcp4 element - string config_v4 = "{ \"Dhcp6\": { \"interfaces\": [ \"*\" ]," + string config_v6 = "{ \"Dhcp6\": { \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -747,7 +747,7 @@ TEST_F(JSONFileBackendTest, configBroken) { EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config that does not have Dhcp4 component. - writeFile(TEST_FILE, config_v4); + writeFile(TEST_FILE, config_v6); EXPECT_THROW(srv->init(TEST_FILE), BadValue); // Now try to load a config with Dhcp4 full of nonsense. diff --git a/src/bin/dhcp4/tests/simple_parser4_unittest.cc b/src/bin/dhcp4/tests/simple_parser4_unittest.cc index 6ac157a99c..df104f8293 100644 --- a/src/bin/dhcp4/tests/simple_parser4_unittest.cc +++ b/src/bin/dhcp4/tests/simple_parser4_unittest.cc @@ -210,6 +210,6 @@ TEST_F(SimpleParser4Test, optionDefDefaults4) { checkBoolValue(def, "array", false); } -}; -}; -}; +} +} +} diff --git a/src/bin/dhcp6/dhcp6_lexer.ll b/src/bin/dhcp6/dhcp6_lexer.ll index 4476de9709..e9577eec15 100644 --- a/src/bin/dhcp6/dhcp6_lexer.ll +++ b/src/bin/dhcp6/dhcp6_lexer.ll @@ -1611,6 +1611,15 @@ ControlCharacterFill [^"\\]|\\["\\/bfnrtu] } } +\"template-class\" { + switch(driver.ctx_) { + case isc::dhcp::Parser6Context::CLIENT_CLASSES: + return isc::dhcp::Dhcp6Parser::make_TEMPLATE_CLASS(driver.loc_); + default: + return isc::dhcp::Dhcp6Parser::make_STRING("template-class", driver.loc_); + } +} + \"only-if-required\" { switch(driver.ctx_) { case isc::dhcp::Parser6Context::CLIENT_CLASSES: diff --git a/src/bin/dhcp6/dhcp6_parser.yy b/src/bin/dhcp6/dhcp6_parser.yy index 48901241c5..23d12eb637 100644 --- a/src/bin/dhcp6/dhcp6_parser.yy +++ b/src/bin/dhcp6/dhcp6_parser.yy @@ -168,6 +168,7 @@ using namespace std; CLIENT_CLASSES "client-classes" REQUIRE_CLIENT_CLASSES "require-client-classes" TEST "test" + TEMPLATE_CLASS "template-class" ONLY_IF_REQUIRED "only-if-required" CLIENT_CLASS "client-class" @@ -2407,6 +2408,7 @@ not_empty_client_class_params: client_class_param client_class_param: client_class_name | client_class_test + | client_class_template_class | only_if_required | option_data_list | user_context @@ -2431,6 +2433,12 @@ client_class_test: TEST { ctx.leave(); }; +client_class_template_class: TEMPLATE_CLASS COLON BOOLEAN { + ctx.unique("template-class", ctx.loc2pos(@1)); + ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); + ctx.stack_.back()->set("template-class", b); +}; + only_if_required: ONLY_IF_REQUIRED COLON BOOLEAN { ctx.unique("only-if-required", ctx.loc2pos(@1)); ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index b7d3644b76..6c31b38a85 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -569,6 +569,13 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt, // Perform second pass of classification. evaluateClasses(pkt, true); + const ClientClasses& classes = pkt->getClassesAndSubClasses(); + if (!classes.empty()) { + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLASS_ASSIGNED) + .arg(pkt->getLabel()) + .arg(classes.toText()); + } + // Check the DROP special class. if (pkt->inClass("DROP")) { LOG_DEBUG(packet6_logger, DBGLVL_PKT_HANDLING, DHCP6_PACKET_DROP_DROP_CLASS2) @@ -1430,7 +1437,7 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question, } // Each class in the incoming packet - const ClientClasses& classes = question->getClasses(); + const ClientClasses& classes = question->getClassesAndTemplates(); for (ClientClasses::const_iterator cclass = classes.cbegin(); cclass != classes.cend(); ++cclass) { // Find the client class definition for this class @@ -4019,30 +4026,7 @@ void Dhcpv6Srv::evaluateClasses(const Pkt6Ptr& pkt, bool depend_on_known) { if ((*it)->getDependOnKnown() != depend_on_known) { continue; } - // Evaluate the expression which can return false (no match), - // true (match) or raise an exception (error) - try { - bool status = evaluateBool(*expr_ptr, *pkt); - if (status) { - LOG_INFO(dhcp6_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg(status); - // Matching: add the class - pkt->addClass((*it)->getName()); - } else { - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, EVAL_RESULT) - .arg((*it)->getName()) - .arg(status); - } - } catch (const Exception& ex) { - LOG_ERROR(dhcp6_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg(ex.what()); - } catch (...) { - LOG_ERROR(dhcp6_logger, EVAL_RESULT) - .arg((*it)->getName()) - .arg("get exception?"); - } + (*it)->test(pkt, expr_ptr); } } @@ -4072,7 +4056,7 @@ Dhcpv6Srv::setReservedClientClasses(const Pkt6Ptr& pkt, } } - const ClientClasses& classes = pkt->getClasses(); + const ClientClasses& classes = pkt->getClassesAndSubClasses(); if (!classes.empty()) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLASS_ASSIGNED) .arg(pkt->getLabel()) @@ -4098,7 +4082,7 @@ Dhcpv6Srv::conditionallySetReservedClientClasses(const Pkt6Ptr& pkt, void Dhcpv6Srv::requiredClassify(const Pkt6Ptr& pkt, AllocEngine::ClientContext6& ctx) { // First collect required classes - ClientClasses classes = pkt->getClasses(true); + ClientClasses classes = pkt->getClassesAndTemplates(true); Subnet6Ptr subnet = ctx.subnet_; if (subnet) { diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 07c975d4b1..70990b8467 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -266,7 +266,7 @@ public: /// in the future. /// /// @param cfg - the parsed structure - /// @param global global Dhcp4 scope + /// @param global global Dhcp6 scope /// @throw DhcpConfigError in case of issues found void sanityChecks(const SrvConfigPtr& cfg, const ConstElementPtr& global) { @@ -416,7 +416,7 @@ void configureCommandChannel() { if (sock_cfg) { // This will create a control socket and install the external // socket in IfaceMgr. That socket will be monitored when - // Dhcp4Srv::receivePacket() calls IfaceMgr::receive4() and + // Dhcp6Srv::receivePacket() calls IfaceMgr::receive6() and // callback in CommandMgr will be called, if necessary. isc::config::CommandMgr::instance().openCommandSocket(sock_cfg); } diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index e8bffc2453..cc3c6a8989 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -8156,4 +8156,70 @@ TEST_F(Dhcp6ParserTest, clientClassValidPreferredLifetime) { EXPECT_TRUE(class_def->getValid().unspecified()); } +// Verifies that client class definitions may specify +// valid and preferred lifetime triplets. +TEST_F(Dhcp6ParserTest, templateClientClassValidPreferredLifetime) { + string config = "{ " + genIfaceConfig() + "," + + "\"client-classes\" : [ \n" + " { \n" + " \"name\": \"one\", \n" + " \"min-valid-lifetime\": 1000, \n" + " \"valid-lifetime\": 2000, \n" + " \"max-valid-lifetime\": 3000, \n" + " \"min-preferred-lifetime\": 4000, \n" + " \"preferred-lifetime\": 5000, \n" + " \"max-preferred-lifetime\": 6000, \n" + " \"template-class\": true \n" + " }, \n" + " { \n" + " \"name\": \"two\", \n" + " \"template-class\": true \n" + " } \n" + "], \n" + "\"subnet6\": [ { \n" + " \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::ffff\" } ]," + " \"subnet\": \"2001:db8:1::/64\"" + " } ] \n" + "} \n"; + + ConstElementPtr json; + ASSERT_NO_THROW_LOG(json = parseDHCP6(config)); + extractConfig(config); + + ConstElementPtr status; + ASSERT_NO_THROW_LOG(status = configureDhcp6Server(srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 0); + + // We check staging config because CfgMgr::commit hasn't been executed. + ClientClassDictionaryPtr dictionary; + dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ASSERT_TRUE(dictionary); + EXPECT_EQ(2, dictionary->getClasses()->size()); + + // Execute the commit + ASSERT_NO_THROW(CfgMgr::instance().commit()); + + // Verify that after commit, the current config has the correct dictionary + dictionary = CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); + ASSERT_TRUE(dictionary); + EXPECT_EQ(2, dictionary->getClasses()->size()); + + ClientClassDefPtr class_def = dictionary->findClass("one"); + ASSERT_TRUE(class_def); + ASSERT_TRUE(dynamic_cast(class_def.get())); + EXPECT_EQ(class_def->getValid().getMin(), 1000); + EXPECT_EQ(class_def->getValid().get(), 2000); + EXPECT_EQ(class_def->getValid().getMax(), 3000); + + EXPECT_EQ(class_def->getPreferred().getMin(), 4000); + EXPECT_EQ(class_def->getPreferred().get(), 5000); + EXPECT_EQ(class_def->getPreferred().getMax(), 6000); + + class_def = dictionary->findClass("two"); + ASSERT_TRUE(class_def); + ASSERT_TRUE(dynamic_cast(class_def.get())); + EXPECT_TRUE(class_def->getValid().unspecified()); +} + } // namespace diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc index 683a71cf62..a26c0b7e8c 100644 --- a/src/bin/dhcp6/tests/fqdn_unittest.cc +++ b/src/bin/dhcp6/tests/fqdn_unittest.cc @@ -145,7 +145,7 @@ public: return (DdnsParamsPtr(new DdnsParams())); } - return(CfgMgr::instance().getCurrentCfg()->getDdnsParams(subnet_)); + return (CfgMgr::instance().getCurrentCfg()->getDdnsParams(subnet_)); } /// @brief Construct the DHCPv6 Client FQDN option using flags and diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc b/src/bin/dhcp6/tests/simple_parser6_unittest.cc index 16f3cea273..9e86813913 100644 --- a/src/bin/dhcp6/tests/simple_parser6_unittest.cc +++ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc @@ -228,7 +228,7 @@ TEST_F(SimpleParser6Test, optionDataDefaults6) { ASSERT_TRUE(option); // we should have appropriate default value set. See - // SimpleParser4::OPTION4_DEFAULTS for a list of default values. + // SimpleParser6::OPTION6_DEFAULTS for a list of default values. checkStringValue(option, "space", "dhcp6"); checkBoolValue(option, "csv-format", true); } @@ -251,12 +251,11 @@ TEST_F(SimpleParser6Test, optionDefDefaults6) { ASSERT_TRUE(def); // we should have appropriate default value set. See - // SimpleParser4::OPTION4_DEFAULTS for a list of default values. + // SimpleParser6::OPTION6_DEFAULTS for a list of default values. checkStringValue(def, "record-types", ""); checkStringValue(def, "space", "dhcp6"); checkStringValue(def, "encapsulate", ""); checkBoolValue(def, "array", false); } - -}; +} diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc index 7d431b40dd..b284aaab3f 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc @@ -34,7 +34,7 @@ extern "C" { /// 1 otherwise. int lease4_add(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.leaseAddHandler(handle)); + return (lease_cmds.leaseAddHandler(handle)); } /// @brief This is a command callout for 'lease6-add' command. @@ -45,7 +45,7 @@ int lease4_add(CalloutHandle& handle) { /// 1 otherwise. int lease6_add(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.leaseAddHandler(handle)); + return (lease_cmds.leaseAddHandler(handle)); } /// @brief This is a command callout for 'lease6-bulk-apply' command. @@ -67,7 +67,7 @@ int lease6_bulk_apply(CalloutHandle& handle) { /// 1 otherwise. int lease4_get(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.leaseGetHandler(handle)); + return (lease_cmds.leaseGetHandler(handle)); } /// @brief This is a command callout for 'lease6-get' command. @@ -78,7 +78,7 @@ int lease4_get(CalloutHandle& handle) { /// 1 otherwise. int lease6_get(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.leaseGetHandler(handle)); + return (lease_cmds.leaseGetHandler(handle)); } /// @brief This is a command callout for 'lease4-get-all' command. @@ -188,7 +188,7 @@ int lease6_get_by_hostname(CalloutHandle& handle) { /// 1 otherwise. int lease4_del(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease4DelHandler(handle)); + return (lease_cmds.lease4DelHandler(handle)); } /// @brief This is a command callout for 'lease6-del' command. @@ -199,7 +199,7 @@ int lease4_del(CalloutHandle& handle) { /// 1 otherwise. int lease6_del(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease6DelHandler(handle)); + return (lease_cmds.lease6DelHandler(handle)); } /// @brief This is a command callout for 'lease4-update' command. @@ -210,7 +210,7 @@ int lease6_del(CalloutHandle& handle) { /// 1 otherwise. int lease4_update(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease4UpdateHandler(handle)); + return (lease_cmds.lease4UpdateHandler(handle)); } /// @brief This is a command callout for 'lease6-update' command. @@ -221,7 +221,7 @@ int lease4_update(CalloutHandle& handle) { /// 1 otherwise. int lease6_update(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease6UpdateHandler(handle)); + return (lease_cmds.lease6UpdateHandler(handle)); } /// @brief This is a command callout for 'lease4-wipe' command. @@ -232,7 +232,7 @@ int lease6_update(CalloutHandle& handle) { /// 1 otherwise. int lease4_wipe(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease4WipeHandler(handle)); + return (lease_cmds.lease4WipeHandler(handle)); } /// @brief This is a command callout for 'lease6-wipe' command. @@ -243,7 +243,7 @@ int lease4_wipe(CalloutHandle& handle) { /// 1 otherwise. int lease6_wipe(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease6WipeHandler(handle)); + return (lease_cmds.lease6WipeHandler(handle)); } /// @brief This is a command callout for 'lease4-resend-ddns' command. @@ -254,7 +254,7 @@ int lease6_wipe(CalloutHandle& handle) { /// 1 otherwise. int lease4_resend_ddns(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease4ResendDdnsHandler(handle)); + return (lease_cmds.lease4ResendDdnsHandler(handle)); } /// @brief This is a command callout for 'lease6-resend-ddns' command. @@ -265,7 +265,7 @@ int lease4_resend_ddns(CalloutHandle& handle) { /// 1 otherwise. int lease6_resend_ddns(CalloutHandle& handle) { LeaseCmds lease_cmds; - return(lease_cmds.lease6ResendDdnsHandler(handle)); + return (lease_cmds.lease6ResendDdnsHandler(handle)); } /// @brief This is a command callout for 'lease4-write' command. diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc index afaceced64..d70fa258c1 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc @@ -4525,7 +4525,7 @@ PgSqlConfigBackendDHCPv4Impl::getStatement(size_t index) const { << index << ", is invalid"); } - return(tagged_statements[index]); + return (tagged_statements[index]); } PgSqlConfigBackendDHCPv4::PgSqlConfigBackendDHCPv4(const DatabaseConnection::ParameterMap& parameters) diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp6.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp6.cc index 5ad7e43188..b78b4d63e3 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp6.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp6.cc @@ -4994,7 +4994,7 @@ PgSqlConfigBackendDHCPv6Impl::getStatement(size_t index) const { << index << ", is invalid"); } - return(tagged_statements[index]); + return (tagged_statements[index]); } PgSqlConfigBackendDHCPv6::PgSqlConfigBackendDHCPv6(const DatabaseConnection::ParameterMap& parameters) diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc index 4399cb08a2..c04eb74c14 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc @@ -50,7 +50,7 @@ PgSqlConfigBackendImpl::insertQuery(size_t index, uint64_t PgSqlConfigBackendImpl::updateDeleteQuery(size_t index, const PsqlBindArray& in_bindings) { - return(conn_.updateDeleteQuery(getStatement(index), in_bindings)); + return (conn_.updateDeleteQuery(getStatement(index), in_bindings)); } PgSqlConfigBackendImpl::ScopedAuditRevision::ScopedAuditRevision( diff --git a/src/hooks/dhcp/run_script/tests/run_script_unittests.cc b/src/hooks/dhcp/run_script/tests/run_script_unittests.cc index 46d9c8d277..7ca5babee7 100644 --- a/src/hooks/dhcp/run_script/tests/run_script_unittests.cc +++ b/src/hooks/dhcp/run_script/tests/run_script_unittests.cc @@ -156,7 +156,7 @@ generateLease4() { Lease4Ptr lease4(new Lease4(IOAddress("192.168.0.1"), hwaddr, clientid, 2, 3, 4, false, false, "test.hostname")); - return(lease4); + return (lease4); } /// @brief Generate a valid Lease6. diff --git a/src/hooks/dhcp/stat_cmds/stat_cmds_callouts.cc b/src/hooks/dhcp/stat_cmds/stat_cmds_callouts.cc index 228772f981..df90c55fe1 100644 --- a/src/hooks/dhcp/stat_cmds/stat_cmds_callouts.cc +++ b/src/hooks/dhcp/stat_cmds/stat_cmds_callouts.cc @@ -32,7 +32,7 @@ extern "C" { /// 1 otherwise. int stat_lease4_get(CalloutHandle& handle) { StatCmds stat_cmds; - return(stat_cmds.statLease4GetHandler(handle)); + return (stat_cmds.statLease4GetHandler(handle)); } /// @brief This is a command callout for 'stat-lease6-get' command. @@ -43,7 +43,7 @@ int stat_lease4_get(CalloutHandle& handle) { /// 1 otherwise. int stat_lease6_get(CalloutHandle& handle) { StatCmds stat_cmds; - return(stat_cmds.statLease6GetHandler(handle)); + return (stat_cmds.statLease6GetHandler(handle)); } /// @brief This function is called when the library is loaded. diff --git a/src/hooks/dhcp/user_chk/pkt_send_co.cc b/src/hooks/dhcp/user_chk/pkt_send_co.cc index 740bc4b6c3..788d133022 100644 --- a/src/hooks/dhcp/user_chk/pkt_send_co.cc +++ b/src/hooks/dhcp/user_chk/pkt_send_co.cc @@ -402,7 +402,7 @@ void generate_output_record(const std::string& id_type_str, std::string getV6AddrStr(Pkt6Ptr response) { OptionPtr tmp = response->getOption(D6O_IA_NA); if (tmp) { - return(getAddrStrIA_NA(tmp)); + return (getAddrStrIA_NA(tmp)); } // IA_NA not there so try IA_PD @@ -411,7 +411,7 @@ std::string getV6AddrStr(Pkt6Ptr response) { isc_throw (isc::BadValue, "Response has neither IA_NA nor IA_PD"); } - return(getAddrStrIA_PD(tmp)); + return (getAddrStrIA_PD(tmp)); } /// @brief Stringify the lease address in an D6O_IA_NA option set diff --git a/src/hooks/dhcp/user_chk/user_file.cc b/src/hooks/dhcp/user_chk/user_file.cc index 96cb6a0fc4..e324bcb2b0 100644 --- a/src/hooks/dhcp/user_chk/user_file.cc +++ b/src/hooks/dhcp/user_chk/user_file.cc @@ -54,7 +54,7 @@ UserFile::readNextUser() { // We got something, try to make a user out of it. if (file_.gcount() > 0) { - return(makeUser(buf)); + return (makeUser(buf)); } } diff --git a/src/lib/database/dbaccess_parser.cc b/src/lib/database/dbaccess_parser.cc index caee236282..76ba13799e 100644 --- a/src/lib/database/dbaccess_parser.cc +++ b/src/lib/database/dbaccess_parser.cc @@ -91,6 +91,7 @@ DbAccessParser::parse(std::string& access_string, max_row_errors = param.second->intValue(); values_copy[param.first] = boost::lexical_cast(max_row_errors); + } else { // all remaining string parameters diff --git a/src/lib/dhcp/classify.h b/src/lib/dhcp/classify.h index 5cf675c81d..177a8dd3b0 100644 --- a/src/lib/dhcp/classify.h +++ b/src/lib/dhcp/classify.h @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -35,7 +36,6 @@ /// though they reside in the dhcp directory. namespace isc { - namespace dhcp { /// @brief Defines a single class name. @@ -63,6 +63,42 @@ namespace dhcp { > > ClientClassContainer; + /// @brief Defines a single subclass (template class instantiation). + struct SubClass { + /// @brief Constructor. + SubClass(const ClientClass& template_class, const ClientClass& subclass) : + template_class_(template_class), subclass_(subclass) { + } + + /// @brief The template class name. + ClientClass template_class_; + + /// @brief The subclass name. + ClientClass subclass_; + }; + + /// @brief Tag for the sequence index. + struct SubClassSequenceTag { }; + + /// @brief Tag for the name index. + struct SubClassNameTag { }; + + /// @brief the subclass multi-index. + typedef boost::multi_index_container< + SubClass, + boost::multi_index::indexed_by< + // First index is the sequence one. + boost::multi_index::sequenced< + boost::multi_index::tag + >, + // Second index is the name hash one. + boost::multi_index::ordered_unique< + boost::multi_index::tag, + boost::multi_index::member + > + > + > SubClassContainer; + /// @brief Container for storing client class names /// /// Both a list to iterate on it in insert order and unordered @@ -82,7 +118,7 @@ namespace dhcp { /// /// @param class_names A string containing a client classes separated /// with commas. The class names are trimmed before insertion to the set. - ClientClasses(const ClientClass& class_names); + ClientClasses(const std::string& class_names); /// @brief Insert an element. /// @@ -165,7 +201,6 @@ namespace dhcp { ClientClassContainer container_; }; } - } #endif /* CLASSIFY_H */ diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index 4d214c7c25..62c2232780 100644 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -76,12 +76,19 @@ Pkt::delOption(uint16_t type) { } bool -Pkt::inClass(const std::string& client_class) { - return (classes_.contains(client_class)); +Pkt::inClass(const ClientClass& client_class) { + if (classes_.contains(client_class)) { + return (true); + } + auto const& idx = subclasses_.get(); + if (idx.count(client_class)) { + return (true); + } + return (false); } void -Pkt::addClass(const std::string& client_class, bool required) { +Pkt::addClass(const ClientClass& client_class, bool required) { // Always have ALL first. if (classes_.empty()) { classes_.insert("ALL"); @@ -92,6 +99,15 @@ Pkt::addClass(const std::string& client_class, bool required) { } } +void +Pkt::addSubClass(const ClientClass& template_class, const ClientClass& subclass) { + auto const& idx = subclasses_.get(); + if (idx.count(subclass)) { + return; + } + subclasses_.push_back(SubClass(template_class, subclass)); +} + void Pkt::updateTimestamp() { timestamp_ = boost::posix_time::microsec_clock::universal_time(); diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 3436d2695b..93979a4ec5 100644 --- a/src/lib/dhcp/pkt.h +++ b/src/lib/dhcp/pkt.h @@ -171,7 +171,9 @@ public: /// @note This buffer is only valid till object that returned it exists. /// /// @return reference to output buffer - isc::util::OutputBuffer& getBuffer() { return (buffer_out_); }; + isc::util::OutputBuffer& getBuffer() { + return (buffer_out_); + } /// @brief Adds an option to this packet. /// @@ -258,12 +260,16 @@ public: /// @brief Sets transaction-id value. /// /// @param transid transaction-id to be set. - void setTransid(uint32_t transid) { transid_ = transid; } + void setTransid(uint32_t transid) { + transid_ = transid; + } /// @brief Returns value of transaction-id field. /// /// @return transaction-id - uint32_t getTransid() const { return (transid_); }; + uint32_t getTransid() const { + return (transid_); + } /// @brief Checks whether a client belongs to a given class. /// @@ -290,6 +296,26 @@ public: void addClass(const isc::dhcp::ClientClass& client_class, bool required = false); + /// @brief Adds packet to a specified subclass. + /// + /// A packet can be added to the same subclass repeatedly. Any additional + /// attempts to add to a subclass the packet already belongs to, will be + /// ignored silently. + /// + /// @note It is a matter of naming convention. Conceptually, the server + /// processes a stream of packets, with some packets belonging to given + /// subclasses. From that perspective, this method adds a packet to specified + /// subclass. Implementation wise, it looks the opposite - the subclass name + /// is added to the packet. Perhaps the most appropriate name for this + /// method would be associateWithSubClass()? But that seems overly long, + /// so I decided to stick with addSubClass(). + /// @note The template class name is used to access the class definition. + /// + /// @param template_class name of the template class to be added + /// @param subclass name of the subclass to be added + void addSubClass(const isc::dhcp::ClientClass& template_class, + const isc::dhcp::ClientClass& subclass); + /// @brief Returns the class set /// /// @note This should be used only to iterate over the class set. @@ -301,6 +327,47 @@ public: return (!required ? classes_ : required_classes_); } + /// @brief Returns the subclass set + /// + /// @note This should be used only to iterate over the subclass set. + const SubClassContainer& getSubClasses() const { + return (subclasses_); + } + + /// @brief Returns the class set including template classes associated with + /// subclasses + /// + /// @note This should be used only to iterate over the class set. + /// @note Template classes are always last. + /// @param required return classes or required to be evaluated classes. + /// @return if required is false (the default) the classes the + /// packet belongs to else the classes which are required to be + /// evaluated. + const ClientClasses getClassesAndTemplates(bool required = false) const { + ClientClasses result = getClasses(required); + for (auto const& sclass : subclasses_) { + result.insert(sclass.template_class_); + } + return (result); + } + + /// @brief Returns the class set including template classes associated with + /// subclasses + /// + /// @note This should be used only to iterate over the class set. + /// @note SubClasses are always last. + /// @param required return classes or required to be evaluated classes. + /// @return if required is false (the default) the classes the + /// packet belongs to else the classes which are required to be + /// evaluated. + const ClientClasses getClassesAndSubClasses(bool required = false) const { + ClientClasses result = getClasses(required); + for (auto const& sclass : subclasses_) { + result.insert(sclass.subclass_); + } + return (result); + } + /// @brief Unparsed data (in received packets). /// /// @warning This public member is accessed by derived @@ -482,7 +549,7 @@ public: /// @param ifindex specifies interface index. void setIndex(int ifindex) { ifindex_ = ifindex; - }; + } /// @brief Resets interface index to negative value. void resetIndex() { @@ -494,7 +561,7 @@ public: /// @return interface index int getIndex() const { return (ifindex_); - }; + } /// @brief Checks if interface index has been set. /// @@ -509,7 +576,9 @@ public: /// going to be transmitted. /// /// @return interface name - std::string getIface() const { return (iface_); }; + std::string getIface() const { + return (iface_); + } /// @brief Sets interface name. /// @@ -517,7 +586,9 @@ public: /// going to be transmitted. /// /// @param iface The interface name - void setIface(const std::string& iface) { iface_ = iface; }; + void setIface(const std::string& iface) { + iface_ = iface; + } /// @brief Sets remote hardware address. /// @@ -592,7 +663,7 @@ public: /// This field is public, so the code outside of Pkt4 or Pkt6 class can /// iterate over existing classes. Having it public also solves the problem /// of returned reference lifetime. It is preferred to use @ref inClass and - /// @ref addClass should be used to operate on this field. + /// @ref addClass to operate on this field. ClientClasses classes_; /// @brief Classes which are required to be evaluated. @@ -603,6 +674,14 @@ public: /// and if evaluation status is true added to the previous collection. ClientClasses required_classes_; + /// @brief SubClasses this packet belongs to. + /// + /// This field is public, so the code outside of Pkt4 or Pkt6 class can + /// iterate over existing classes. Having it public also solves the problem + /// of returned reference lifetime. It is preferred to use @ref inClass and + /// @ref addSubClass to operate on this field. + SubClassContainer subclasses_; + /// @brief Collection of options present in this message. /// /// @warning This public member is accessed by derived diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 98a7d3c4b5..d4dd7f9a40 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/src/lib/dhcp/tests/pkt4_unittest.cc @@ -933,12 +933,14 @@ TEST_F(Pkt4Test, clientClasses) { EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_EROUTER)); EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_MODEM)); EXPECT_TRUE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to the first class pkt.addClass(DOCSIS3_CLASS_EROUTER); EXPECT_TRUE(pkt.inClass(DOCSIS3_CLASS_EROUTER)); EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_MODEM)); ASSERT_FALSE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to a second class pkt.addClass(DOCSIS3_CLASS_MODEM); @@ -949,6 +951,7 @@ TEST_F(Pkt4Test, clientClasses) { EXPECT_NO_THROW(pkt.addClass("foo")); EXPECT_NO_THROW(pkt.addClass("foo")); EXPECT_NO_THROW(pkt.addClass("foo")); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Check that the packet belongs to 'foo' EXPECT_TRUE(pkt.inClass("foo")); @@ -961,14 +964,17 @@ TEST_F(Pkt4Test, deferredClientClasses) { // Default values (do not belong to any class) EXPECT_TRUE(pkt.getClasses(true).empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to the first class pkt.addClass(DOCSIS3_CLASS_EROUTER, true); EXPECT_EQ(1, pkt.getClasses(true).size()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to a second class pkt.addClass(DOCSIS3_CLASS_MODEM, true); EXPECT_EQ(2, pkt.getClasses(true).size()); + EXPECT_TRUE(pkt.getSubClasses().empty()); EXPECT_TRUE(pkt.getClasses(true).contains(DOCSIS3_CLASS_EROUTER)); EXPECT_TRUE(pkt.getClasses(true).contains(DOCSIS3_CLASS_MODEM)); EXPECT_FALSE(pkt.getClasses(true).contains("foo")); @@ -977,11 +983,44 @@ TEST_F(Pkt4Test, deferredClientClasses) { EXPECT_NO_THROW(pkt.addClass("foo", true)); EXPECT_NO_THROW(pkt.addClass("foo", true)); EXPECT_NO_THROW(pkt.addClass("foo", true)); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Check that the packet belongs to 'foo' EXPECT_TRUE(pkt.getClasses(true).contains("foo")); } +// Tests whether a packet can be assigned to a subclass and later +// checked if it belongs to a given subclass +TEST_F(Pkt4Test, templateClasses) { + Pkt4 pkt(DHCPOFFER, 1234); + + // Default values (do not belong to any subclass) + EXPECT_FALSE(pkt.inClass("eth0")); + EXPECT_FALSE(pkt.inClass("interface-id0")); + EXPECT_TRUE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); + + // Add to the first subclass + pkt.addSubClass("template-interface-name", "eth0"); + EXPECT_TRUE(pkt.inClass("eth0")); + EXPECT_FALSE(pkt.inClass("interface-id0")); + ASSERT_TRUE(pkt.getClasses().empty()); + ASSERT_FALSE(pkt.getSubClasses().empty()); + + // Add to a second subclass + pkt.addSubClass("template-interface-id", "interface-id0"); + EXPECT_TRUE(pkt.inClass("eth0")); + EXPECT_TRUE(pkt.inClass("interface-id0")); + + // Check that it's ok to add to the same subclass repeatedly + EXPECT_NO_THROW(pkt.addSubClass("template-foo", "bar")); + EXPECT_NO_THROW(pkt.addSubClass("template-foo", "bar")); + EXPECT_NO_THROW(pkt.addSubClass("template-bar", "bar")); + + // Check that the packet belongs to 'bar' + EXPECT_TRUE(pkt.inClass("bar")); +} + // Tests whether MAC can be obtained and that MAC sources are not // confused. TEST_F(Pkt4Test, getMAC) { diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index 0ed2b98b4c..d4e1538d91 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -1307,12 +1307,14 @@ TEST_F(Pkt6Test, clientClasses) { EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_EROUTER)); EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_MODEM)); EXPECT_TRUE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to the first class pkt.addClass(DOCSIS3_CLASS_EROUTER); EXPECT_TRUE(pkt.inClass(DOCSIS3_CLASS_EROUTER)); EXPECT_FALSE(pkt.inClass(DOCSIS3_CLASS_MODEM)); ASSERT_FALSE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to a second class pkt.addClass(DOCSIS3_CLASS_MODEM); @@ -1323,6 +1325,7 @@ TEST_F(Pkt6Test, clientClasses) { EXPECT_NO_THROW(pkt.addClass("foo")); EXPECT_NO_THROW(pkt.addClass("foo")); EXPECT_NO_THROW(pkt.addClass("foo")); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Check that the packet belongs to 'foo' EXPECT_TRUE(pkt.inClass("foo")); @@ -1335,14 +1338,17 @@ TEST_F(Pkt6Test, deferredClientClasses) { // Default values (do not belong to any class) EXPECT_TRUE(pkt.getClasses(true).empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to the first class pkt.addClass(DOCSIS3_CLASS_EROUTER, true); EXPECT_EQ(1, pkt.getClasses(true).size()); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Add to a second class pkt.addClass(DOCSIS3_CLASS_MODEM, true); EXPECT_EQ(2, pkt.getClasses(true).size()); + EXPECT_TRUE(pkt.getSubClasses().empty()); EXPECT_TRUE(pkt.getClasses(true).contains(DOCSIS3_CLASS_EROUTER)); EXPECT_TRUE(pkt.getClasses(true).contains(DOCSIS3_CLASS_MODEM)); EXPECT_FALSE(pkt.getClasses(true).contains("foo")); @@ -1351,11 +1357,44 @@ TEST_F(Pkt6Test, deferredClientClasses) { EXPECT_NO_THROW(pkt.addClass("foo", true)); EXPECT_NO_THROW(pkt.addClass("foo", true)); EXPECT_NO_THROW(pkt.addClass("foo", true)); + EXPECT_TRUE(pkt.getSubClasses().empty()); // Check that the packet belongs to 'foo' EXPECT_TRUE(pkt.getClasses(true).contains("foo")); } +// Tests whether a packet can be assigned to a subclass and later +// checked if it belongs to a given subclass +TEST_F(Pkt6Test, templateClasses) { + Pkt6 pkt(DHCPV6_ADVERTISE, 1234); + + // Default values (do not belong to any subclass) + EXPECT_FALSE(pkt.inClass("eth0")); + EXPECT_FALSE(pkt.inClass("interface-id0")); + EXPECT_TRUE(pkt.getClasses().empty()); + EXPECT_TRUE(pkt.getSubClasses().empty()); + + // Add to the first subclass + pkt.addSubClass("template-interface-name", "eth0"); + EXPECT_TRUE(pkt.inClass("eth0")); + EXPECT_FALSE(pkt.inClass("interface-id0")); + ASSERT_TRUE(pkt.getClasses().empty()); + ASSERT_FALSE(pkt.getSubClasses().empty()); + + // Add to a second subclass + pkt.addSubClass("template-interface-id", "interface-id0"); + EXPECT_TRUE(pkt.inClass("eth0")); + EXPECT_TRUE(pkt.inClass("interface-id0")); + + // Check that it's ok to add to the same subclass repeatedly + EXPECT_NO_THROW(pkt.addSubClass("template-foo", "bar")); + EXPECT_NO_THROW(pkt.addSubClass("template-foo", "bar")); + EXPECT_NO_THROW(pkt.addSubClass("template-bar", "bar")); + + // Check that the packet belongs to 'bar' + EXPECT_TRUE(pkt.inClass("bar")); +} + // Tests whether MAC can be obtained and that MAC sources are not // confused. TEST_F(Pkt6Test, getMAC) { diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index da7b615f52..b3d52dc39c 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -425,16 +425,16 @@ inAllowedPool(AllocEngine::ClientContext6& ctx, const Lease::Type& lease_type, // If the subnet belongs to a shared network we will be iterating // over the subnets that belong to this shared network. Subnet6Ptr current_subnet = ctx.subnet_; - while (current_subnet) { + auto const& classes = ctx.query_->getClassesAndSubClasses(); - if (current_subnet->clientSupported(ctx.query_->getClasses())) { + while (current_subnet) { + if (current_subnet->clientSupported(classes)) { if (check_subnet) { if (current_subnet->inPool(lease_type, address)) { return (true); } } else { - if (current_subnet->inPool(lease_type, address, - ctx.query_->getClasses())) { + if (current_subnet->inPool(lease_type, address, classes)) { return (true); } } @@ -658,13 +658,14 @@ AllocEngine::findReservation(ClientContext6& ctx) { } } + auto const& classes = ctx.query_->getClassesAndSubClasses(); + // We can only search for the reservation if a subnet has been selected. while (subnet) { // Only makes sense to get reservations if the client has access // to the class and host reservations are enabled for this subnet. - if (subnet->clientSupported(ctx.query_->getClasses()) && - subnet->getReservationsInSubnet()) { + if (subnet->clientSupported(classes) && subnet->getReservationsInSubnet()) { // Iterate over configured identifiers in the order of preference // and try to use each of them to search for the reservations. if (use_single_query) { @@ -690,7 +691,7 @@ AllocEngine::findReservation(ClientContext6& ctx) { // We need to get to the next subnet if this is a shared network. If it // is not (a plain subnet), getNextSubnet will return NULL and we're // done here. - subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses()); + subnet = subnet->getNextSubnet(ctx.subnet_, classes); } } @@ -918,9 +919,10 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { CalloutHandle::CalloutNextStep callout_status = CalloutHandle::NEXT_STEP_CONTINUE; - for (; subnet; subnet = subnet->getNextSubnet(original_subnet)) { + auto const& classes = ctx.query_->getClassesAndSubClasses(); - if (!subnet->clientSupported(ctx.query_->getClasses())) { + for (; subnet; subnet = subnet->getNextSubnet(original_subnet)) { + if (!subnet->clientSupported(classes)) { continue; } @@ -929,11 +931,10 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { // check if the hint is in pool and is available // This is equivalent of subnet->inPool(hint), but returns the pool pool = boost::dynamic_pointer_cast - (subnet->getPool(ctx.currentIA().type_, ctx.query_->getClasses(), - hint)); + (subnet->getPool(ctx.currentIA().type_, classes, hint)); // check if the pool is allowed - if (!pool || !pool->clientSupported(ctx.query_->getClasses())) { + if (!pool || !pool->clientSupported(classes)) { continue; } @@ -1066,8 +1067,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { uint64_t subnets_with_unavail_pools = 0; for (; subnet; subnet = subnet->getNextSubnet(original_subnet)) { - - if (!subnet->clientSupported(ctx.query_->getClasses())) { + if (!subnet->clientSupported(classes)) { continue; } @@ -1078,8 +1078,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { // - we find an address for which the lease has expired // - we exhaust number of tries uint64_t possible_attempts = - subnet->getPoolCapacity(ctx.currentIA().type_, - ctx.query_->getClasses()); + subnet->getPoolCapacity(ctx.currentIA().type_, classes); // If the number of tries specified in the allocation engine constructor // is set to 0 (unlimited) or the pools capacity is lower than that number, @@ -1114,7 +1113,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { ++total_attempts; IOAddress candidate = allocator->pickAddress(subnet, - ctx.query_->getClasses(), + classes, ctx.duid_, hint); // The first step is to find out prefix length. It is 128 for @@ -1123,7 +1122,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { if (ctx.currentIA().type_ == Lease::TYPE_PD) { pool = boost::dynamic_pointer_cast( subnet->getPool(ctx.currentIA().type_, - ctx.query_->getClasses(), + classes, candidate)); if (pool) { prefix_len = pool->getLength(); @@ -1273,7 +1272,6 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { static_cast(1)); } - const ClientClasses& classes = ctx.query_->getClasses(); if (!classes.empty()) { LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V6_ALLOC_FAIL_CLASSES) .arg(ctx.query_->getLabel()) @@ -1362,14 +1360,14 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx, // There is no lease for a reservation in this IA. So, let's now iterate // over reservations specified and try to allocate one of them for the IA. + auto const& classes = ctx.query_->getClassesAndSubClasses(); for (Subnet6Ptr subnet = ctx.subnet_; subnet; subnet = subnet->getNextSubnet(ctx.subnet_)) { SubnetID subnet_id = subnet->getID(); // No hosts for this subnet or the subnet not supported. - if (!subnet->clientSupported(ctx.query_->getClasses()) || - ctx.hosts_.count(subnet_id) == 0) { + if (!subnet->clientSupported(classes) || ctx.hosts_.count(subnet_id) == 0) { continue; } @@ -1966,13 +1964,13 @@ AllocEngine::getLifetimes6(ClientContext6& ctx, uint32_t& preferred, uint32_t& v // We use the first one we find for each lifetime. Triplet candidate_preferred; Triplet candidate_valid; - const ClientClasses classes = ctx.query_->getClasses(); + const ClientClasses classes = ctx.query_->getClassesAndTemplates(); if (!classes.empty()) { // Let's get class definitions const ClientClassDictionaryPtr& dict = CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); - // Iterate over the assigned class defintions. + // Iterate over the assigned class definitions. int have_both = 0; for (auto name = classes.cbegin(); name != classes.cend() && have_both < 2; ++name) { @@ -2261,7 +2259,7 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) { // is not permitted by subnet client classification, delete it. if (!(ctx.hasGlobalReservation(makeIPv6Resrv(*lease))) && (((lease->type_ != Lease::TYPE_PD) && !ctx.subnet_->inRange(lease->addr_)) || - !ctx.subnet_->clientSupported(ctx.query_->getClasses()))) { + !ctx.subnet_->clientSupported(ctx.query_->getClassesAndSubClasses()))) { // Oh dear, the lease is no longer valid. We need to get rid of it. // Remove this lease from LeaseMgr @@ -3292,7 +3290,7 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) { // No address reservation found here, so let's try another subnet // within the same shared network. - subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses()); + subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClassesAndSubClasses()); } return (false); @@ -3318,11 +3316,12 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) Subnet4Ptr original_subnet = ctx.subnet_; + auto const& classes = ctx.query_->getClassesAndSubClasses(); + // Client identifier is optional. First check if we can try to lookup // by client-id. bool try_clientid_lookup = (ctx.clientid_ && - SharedNetwork4::subnetsIncludeMatchClientId(original_subnet, - ctx.query_->getClasses())); + SharedNetwork4::subnetsIncludeMatchClientId(original_subnet, classes)); // If it is possible to use client identifier to try to find client's lease. if (try_clientid_lookup) { @@ -3334,8 +3333,7 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) // Iterate over the subnets within the shared network to see if any client's // lease belongs to them. for (Subnet4Ptr subnet = original_subnet; subnet; - subnet = subnet->getNextSubnet(original_subnet, - ctx.query_->getClasses())) { + subnet = subnet->getNextSubnet(original_subnet, classes)) { // If client identifier has been supplied and the server wasn't // explicitly configured to ignore client identifiers for this subnet @@ -3361,8 +3359,7 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) Lease4Collection leases_hw_address = lease_mgr.getLease4(*ctx.hwaddr_); for (Subnet4Ptr subnet = original_subnet; subnet; - subnet = subnet->getNextSubnet(original_subnet, - ctx.query_->getClasses())) { + subnet = subnet->getNextSubnet(original_subnet, classes)) { ClientIdPtr client_id; if (subnet->getMatchClientId()) { client_id = ctx.clientid_; @@ -3404,10 +3401,10 @@ inAllowedPool(AllocEngine::ClientContext4& ctx, const IOAddress& address) { // If the subnet belongs to a shared network we will be iterating // over the subnets that belong to this shared network. Subnet4Ptr current_subnet = ctx.subnet_; - while (current_subnet) { + auto const& classes = ctx.query_->getClassesAndSubClasses(); - if (current_subnet->inPool(Lease::TYPE_V4, address, - ctx.query_->getClasses())) { + while (current_subnet) { + if (current_subnet->inPool(Lease::TYPE_V4, address, classes)) { // We found a subnet that this address belongs to, so it // seems that this subnet is the good candidate for allocation. // Let's update the selected subnet. @@ -3415,8 +3412,7 @@ inAllowedPool(AllocEngine::ClientContext4& ctx, const IOAddress& address) { return (true); } - current_subnet = current_subnet->getNextSubnet(ctx.subnet_, - ctx.query_->getClasses()); + current_subnet = current_subnet->getNextSubnet(ctx.subnet_, classes); } return (false); @@ -3517,8 +3513,9 @@ AllocEngine::allocateLease4(ClientContext4& ctx) { // subnets allowed for this client within the shared network, we // can't allocate a lease. Subnet4Ptr subnet = ctx.subnet_; - if (subnet && !subnet->clientSupported(ctx.query_->getClasses())) { - ctx.subnet_ = subnet->getNextSubnet(subnet, ctx.query_->getClasses()); + auto const& classes = ctx.query_->getClassesAndSubClasses(); + if (subnet && !subnet->clientSupported(classes)) { + ctx.subnet_ = subnet->getNextSubnet(subnet, classes); } try { @@ -3607,13 +3604,13 @@ AllocEngine::findReservation(ClientContext4& ctx) { } } + auto const& classes = ctx.query_->getClassesAndSubClasses(); // We can only search for the reservation if a subnet has been selected. while (subnet) { // Only makes sense to get reservations if the client has access // to the class and host reservations are enabled for this subnet. - if (subnet->clientSupported(ctx.query_->getClasses()) && - subnet->getReservationsInSubnet()) { + if (subnet->clientSupported(classes) && subnet->getReservationsInSubnet()) { // Iterate over configured identifiers in the order of preference // and try to use each of them to search for the reservations. if (use_single_query) { @@ -3639,7 +3636,7 @@ AllocEngine::findReservation(ClientContext4& ctx) { // We need to get to the next subnet if this is a shared network. If it // is not (a plain subnet), getNextSubnet will return NULL and we're // done here. - subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses()); + subnet = subnet->getNextSubnet(ctx.subnet_, classes); } } @@ -3985,13 +3982,13 @@ AllocEngine::getValidLft(const ClientContext4& ctx) { // If the triplet is specified in one of our classes use it. // We use the first one we find. Triplet candidate_lft; - const ClientClasses classes = ctx.query_->getClasses(); + const ClientClasses classes = ctx.query_->getClassesAndTemplates(); if (!classes.empty()) { // Let's get class definitions const ClientClassDictionaryPtr& dict = CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); - // Iterate over the assigned class defintions. + // Iterate over the assigned class definitions. for (ClientClasses::const_iterator name = classes.cbegin(); name != classes.cend(); ++name) { ClientClassDefPtr cl = dict->findClass(*name); @@ -4414,6 +4411,8 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { // no matching pools for the client. uint64_t subnets_with_unavail_pools = 0; + auto const& classes = ctx.query_->getClassesAndSubClasses(); + while (subnet) { ClientIdPtr client_id; if (subnet->getMatchClientId()) { @@ -4421,8 +4420,7 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { } uint64_t possible_attempts = - subnet->getPoolCapacity(Lease::TYPE_V4, - ctx.query_->getClasses()); + subnet->getPoolCapacity(Lease::TYPE_V4, classes); // If the number of tries specified in the allocation engine constructor // is set to 0 (unlimited) or the pools capacity is lower than that number, @@ -4448,7 +4446,7 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { ++total_attempts; IOAddress candidate = allocator->pickAddress(subnet, - ctx.query_->getClasses(), + classes, client_id, ctx.requested_address_); // First check for reservation when it is the choice. @@ -4497,7 +4495,7 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { // This pointer may be set to NULL if hooks set SKIP status. if (subnet) { - subnet = subnet->getNextSubnet(original_subnet, ctx.query_->getClasses()); + subnet = subnet->getNextSubnet(original_subnet, classes); if (subnet) { ctx.subnet_ = subnet; @@ -4567,7 +4565,6 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { static_cast(1)); } - const ClientClasses& classes = ctx.query_->getClasses(); if (!classes.empty()) { LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_ALLOC_FAIL_CLASSES) .arg(ctx.query_->getLabel()) diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc index 70a5168cf8..53e229994d 100644 --- a/src/lib/dhcpsrv/client_class_def.cc +++ b/src/lib/dhcpsrv/client_class_def.cc @@ -7,8 +7,11 @@ #include #include +#include +#include #include #include +#include #include #include @@ -137,6 +140,64 @@ ClientClassDef::setCfgOption(const CfgOptionPtr& cfg_option) { cfg_option_ = cfg_option; } +void +ClientClassDef::test(PktPtr pkt, const ExpressionPtr& expr_ptr) { + // Evaluate the expression which can return false (no match), + // true (match) or raise an exception (error) + try { + bool status = evaluateBool(*expr_ptr, *pkt); + if (status) { + LOG_INFO(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg(status); + // Matching: add the class + pkt->addClass(getName()); + } else { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, EVAL_RESULT) + .arg(getName()) + .arg(status); + } + } catch (const Exception& ex) { + LOG_ERROR(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg(ex.what()); + } catch (...) { + LOG_ERROR(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg("get exception?"); + } +} + +TemplateClientClassDef::TemplateClientClassDef(const std::string& name, + const ExpressionPtr& match_expr, + const CfgOptionPtr& options) : + ClientClassDef(name, match_expr, options) { +} + +void +TemplateClientClassDef::test(PktPtr pkt, const ExpressionPtr& expr_ptr) { + // Evaluate the expression which can return false (no match), + // true (match) or raise an exception (error) + try { + std::string subclass = evaluateString(*expr_ptr, *pkt); + if (!subclass.empty()) { + LOG_INFO(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg(subclass); + // Matching: add the subclass + pkt->addSubClass(getName(), subclass); + } + } catch (const Exception& ex) { + LOG_ERROR(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg(ex.what()); + } catch (...) { + LOG_ERROR(dhcpsrv_logger, EVAL_RESULT) + .arg(getName()) + .arg("get exception?"); + } +} + bool ClientClassDef::dependOnClass(const std::string& name) const { return (isc::dhcp::dependOnClass(match_expr_, name)); @@ -162,7 +223,7 @@ ClientClassDef::equals(const ClientClassDef& other) const { } ElementPtr -ClientClassDef:: toElement() const { +ClientClassDef::toElement() const { uint16_t family = CfgMgr::instance().getFamily(); ElementPtr result = Element::createMap(); // Set user-context @@ -230,6 +291,13 @@ ClientClassDef:: toElement() const { return (result); } +ElementPtr +TemplateClientClassDef::toElement() const { + auto const& result = ClientClassDef::toElement(); + result->set("template-class", Element::create(true)); + return (result); +} + std::ostream& operator<<(std::ostream& os, const ClientClassDef& x) { os << "ClientClassDef:" << x.getName(); return (os); @@ -265,8 +333,14 @@ ClientClassDictionary::addClass(const std::string& name, const std::string& sname, const std::string& filename, const util::Triplet& valid, - const util::Triplet& preferred) { - ClientClassDefPtr cclass(new ClientClassDef(name, match_expr, cfg_option)); + const util::Triplet& preferred, + bool is_template) { + ClientClassDefPtr cclass; + if (is_template) { + cclass.reset(new TemplateClientClassDef(name, match_expr, cfg_option)); + } else { + cclass.reset(new ClientClassDef(name, match_expr, cfg_option)); + } cclass->setTest(test); cclass->setRequired(required); cclass->setDependOnKnown(depend_on_known); @@ -394,7 +468,12 @@ ClientClassDictionary::initMatchExpr(uint16_t family) { if (!c->getTest().empty()) { ExpressionPtr match_expr = boost::make_shared(); ExpressionParser parser; - parser.parse(match_expr, Element::create(c->getTest()), family); + EvalContext::ParserType parser_type = EvalContext::PARSER_BOOL; + if (dynamic_cast(c.get())) { + parser_type = EvalContext::PARSER_STRING; + } + parser.parse(match_expr, Element::create(c->getTest()), family, + EvalContext::acceptAll, parser_type); expressions.push(match_expr); } } diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h index 44e47c4699..8469a5cdde 100644 --- a/src/lib/dhcpsrv/client_class_def.h +++ b/src/lib/dhcpsrv/client_class_def.h @@ -57,7 +57,6 @@ public: ClientClassDef(const std::string& name, const ExpressionPtr& match_expr, const CfgOptionPtr& options = CfgOptionPtr()); - /// Copy constructor ClientClassDef(const ClientClassDef& rhs); @@ -222,6 +221,13 @@ public: preferred_ = preferred; } + /// @brief Test method which checks if the packet belongs to the class + /// + /// If the packet belongs to the class, the class is added to the packet. + /// + /// @param pkt The packet checked if it belongs to the class. + virtual void test(PktPtr pkt, const ExpressionPtr& expr_ptr); + /// @brief Unparse a configuration object /// /// @return a pointer to unparsed configuration @@ -285,6 +291,29 @@ private: util::Triplet preferred_; }; +class TemplateClientClassDef : public ClientClassDef { +public: + /// @brief Constructor + /// + /// @param name Name to assign to this class + /// @param match_expr Expression the class will use to determine membership + /// @param options Collection of options members should be given + TemplateClientClassDef(const std::string& name, const ExpressionPtr& match_expr, + const CfgOptionPtr& options = CfgOptionPtr()); + + /// @brief Test method which checks if the packet belongs to the class + /// + /// If the packet belongs to the class, the class is added to the packet. + /// + /// @param pkt The packet checked if it belongs to the class. + virtual void test(PktPtr pkt, const ExpressionPtr& expr_ptr) override; + + /// @brief Unparse a configuration object + /// + /// @return a pointer to unparsed configuration + virtual isc::data::ElementPtr toElement() const; +}; + /// @brief a pointer to an ClientClassDef typedef boost::shared_ptr ClientClassDefPtr; @@ -340,7 +369,8 @@ public: const std::string& sname = std::string(), const std::string& filename = std::string(), const util::Triplet&valid = util::Triplet(), - const util::Triplet&preferred = util::Triplet()); + const util::Triplet&preferred = util::Triplet(), + bool is_template = false); /// @brief Adds a new class to the list /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index c62fe59610..61c8ce8747 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -1882,7 +1882,7 @@ Memfile_LeaseMgr::loadLeasesFromFiles(const std::string& filename, lease_file.reset(new LeaseFileType(filename)); LeaseFileLoader::load(*lease_file, storage, max_row_errors, false); - conversion_needed = conversion_needed || lease_file->needsConversion(); + conversion_needed = conversion_needed || lease_file->needsConversion(); return (conversion_needed); } @@ -2006,7 +2006,7 @@ Memfile_LeaseMgr::startLeaseStatsQuery4() { query->start(); } - return(query); + return (query); } LeaseStatsQueryPtr @@ -2019,7 +2019,7 @@ Memfile_LeaseMgr::startSubnetLeaseStatsQuery4(const SubnetID& subnet_id) { query->start(); } - return(query); + return (query); } LeaseStatsQueryPtr @@ -2034,7 +2034,7 @@ Memfile_LeaseMgr::startSubnetRangeLeaseStatsQuery4(const SubnetID& first_subnet_ query->start(); } - return(query); + return (query); } LeaseStatsQueryPtr @@ -2047,7 +2047,7 @@ Memfile_LeaseMgr::startLeaseStatsQuery6() { query->start(); } - return(query); + return (query); } LeaseStatsQueryPtr @@ -2060,7 +2060,7 @@ Memfile_LeaseMgr::startSubnetLeaseStatsQuery6(const SubnetID& subnet_id) { query->start(); } - return(query); + return (query); } LeaseStatsQueryPtr @@ -2075,7 +2075,7 @@ Memfile_LeaseMgr::startSubnetRangeLeaseStatsQuery6(const SubnetID& first_subnet_ query->start(); } - return(query); + return (query); } size_t @@ -2163,15 +2163,15 @@ Memfile_LeaseMgr::getClassLeaseCount(const ClientClass& client_class, const Lease::Type& ltype /* = Lease::TYPE_V4*/) const { if (MultiThreadingMgr::instance().getMode()) { std::lock_guard lock(*mutex_); - return(class_lease_counter_.getClassCount(client_class, ltype)); + return (class_lease_counter_.getClassCount(client_class, ltype)); } else { - return(class_lease_counter_.getClassCount(client_class, ltype)); + return (class_lease_counter_.getClassCount(client_class, ltype)); } } void Memfile_LeaseMgr::clearClassLeaseCounts() { - return(class_lease_counter_.clear()); + return (class_lease_counter_.clear()); } std::string diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc index 59fd475db8..8c48a9afbc 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc @@ -38,7 +38,8 @@ void ExpressionParser::parse(ExpressionPtr& expression, ConstElementPtr expression_cfg, uint16_t family, - EvalContext::CheckDefined check_defined) { + EvalContext::CheckDefined check_defined, + EvalContext::ParserType parser_type) { if (expression_cfg->getType() != Element::string) { isc_throw(DhcpConfigError, "expression [" << expression_cfg->str() << "] must be a string, at (" @@ -52,15 +53,15 @@ ExpressionParser::parse(ExpressionPtr& expression, try { EvalContext eval_ctx(family == AF_INET ? Option::V4 : Option::V6, check_defined); - eval_ctx.parseString(value); + eval_ctx.parseString(value, parser_type); expression.reset(new Expression()); *expression = eval_ctx.expression; } catch (const std::exception& ex) { // Append position if there is a failure. isc_throw(DhcpConfigError, "expression: [" << value - << "] error: " << ex.what() << " at (" - << expression_cfg->getPosition() << ")"); + << "] error: " << ex.what() << " at (" + << expression_cfg->getPosition() << ")"); } } @@ -80,6 +81,17 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, << getPosition("name", class_def_cfg) << ")"); } + EvalContext::ParserType parser_type = EvalContext::PARSER_BOOL; + + // Let's try to parse the template-class flag + bool is_template = false; + if (class_def_cfg->contains("template-class")) { + is_template = getBoolean(class_def_cfg, "template-class"); + if (is_template) { + parser_type = EvalContext::PARSER_STRING; + } + } + // Parse matching expression ExpressionPtr match_expr; ConstElementPtr test_cfg = class_def_cfg->get("test"); @@ -94,7 +106,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, depend_on_known, cclass)); }; - parser.parse(match_expr, test_cfg, family, check_defined); + parser.parse(match_expr, test_cfg, family, check_defined, parser_type); test = test_cfg->stringValue(); } @@ -205,7 +217,6 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, << filename.length() << " (" << getPosition("boot-file-name", class_def_cfg) << ")"); } - } // Parse valid lifetime triplet. @@ -245,7 +256,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, class_dictionary->addClass(name, match_expr, test, required, depend_on_known, options, defs, user_context, next_server, sname, filename, - valid_lft, preferred_lft); + valid_lft, preferred_lft, is_template); } catch (const std::exception& ex) { std::ostringstream s; s << "Can't add class: " << ex.what(); @@ -273,8 +284,8 @@ ClientClassDefParser::checkParametersSupported(const ConstElementPtr& class_def_ "only-if-required", "valid-lifetime", "min-valid-lifetime", - "max-valid-lifetime" }; - + "max-valid-lifetime", + "template-class"}; // The v4 client class supports additional parameters. static std::set supported_params_v4 = { "option-def", diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.h b/src/lib/dhcpsrv/parsers/client_class_def_parser.h index 060ab10abc..e6693ed9e1 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.h +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.h @@ -69,13 +69,14 @@ public: /// @param expression_cfg the configuration entry to be parsed. /// @param family the address family of the expression. /// @param check_defined a closure to check if a client class is defined. + /// @param parser_type the expected type of the evaluated expression. /// /// @throw DhcpConfigError if parsing was unsuccessful. void parse(ExpressionPtr& expression, isc::data::ConstElementPtr expression_cfg, uint16_t family, - isc::eval::EvalContext::CheckDefined check_defined = - isc::eval::EvalContext::acceptAll); + isc::eval::EvalContext::CheckDefined check_defined = isc::eval::EvalContext::acceptAll, + isc::eval::EvalContext::ParserType parser_type = isc::eval::EvalContext::PARSER_BOOL); }; /// @brief Parser for a single client class definition. diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index d74cf2d302..a83c53794b 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -11,12 +11,14 @@ #include #include #include +#include #include #include -#include +#include #include #include #include +#include #if defined HAVE_MYSQL #include @@ -4648,6 +4650,155 @@ TEST_F(AllocEngine4Test, getValidLft4) { } } +// Verifies that AllocEngine::getValidLft(ctx4) returns the appropriate +// lifetime value based on the context content. +TEST_F(AllocEngine4Test, getTemplateClassValidLft4) { + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + // Let's make three classes, two with valid-lifetime and one without, + // and add them to the dictionary. + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ExpressionPtr match_expr; + ExpressionParser parser; + + ElementPtr test_cfg = Element::create("'valid_one_value'"); + parser.parse(match_expr, test_cfg, AF_INET, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + ClientClassDefPtr class_def(new TemplateClientClassDef("valid_one", match_expr)); + Triplet valid_one(50, 100, 150); + class_def->setValid(valid_one); + dictionary->addClass(class_def); + + test_cfg = Element::create("'valid_two_value'"); + parser.parse(match_expr, test_cfg, AF_INET, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("valid_two", match_expr)); + Tripletvalid_two(200, 250, 300); + class_def->setValid(valid_two); + dictionary->addClass(class_def); + + test_cfg = Element::create("'valid_unspec_value'"); + parser.parse(match_expr, test_cfg, AF_INET, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("valid_unspec", match_expr)); + dictionary->addClass(class_def); + + // Commit our class changes. + CfgMgr::instance().commit(); + + // Update the subnet's triplet to something more useful. + subnet_->setValid(Triplet(500, 1000, 1500)); + + // Describes a test scenario. + struct Scenario { + std::string desc_; // descriptive text for logging + std::vector classes_; // class list of assigned classes + uint32_t requested_lft_; // use as option 51 is > 0 + uint32_t exp_valid_; // expected lifetime + }; + + // Scenarios to test. + std::vector scenarios = { + { + "BOOTP", + { "BOOTP" }, + 0, + Lease::INFINITY_LFT + }, + { + "no classes, no option", + {}, + 0, + subnet_->getValid() + }, + { + "no classes, option", + {}, + subnet_->getValid().getMin() + 50, + subnet_->getValid().getMin() + 50 + }, + { + "no classes, option too small", + {}, + subnet_->getValid().getMin() - 50, + subnet_->getValid().getMin() + }, + { + "no classes, option too big", + {}, + subnet_->getValid().getMax() + 50, + subnet_->getValid().getMax() + }, + { + "class unspecified, no option", + { "valid_unspec" }, + 0, + subnet_->getValid() + }, + { + "from last class, no option", + { "valid_unspec", "valid_one" }, + 0, + valid_one.get() + }, + { + "from first class, no option", + { "valid_two", "valid_one" }, + 0, + valid_two.get() + }, + { + "class plus option", + { "valid_one" }, + valid_one.getMin() + 25, + valid_one.getMin() + 25 + }, + { + "class plus option too small", + { "valid_one" }, + valid_one.getMin() - 25, + valid_one.getMin() + }, + { + "class plus option too big", + { "valid_one" }, + valid_one.getMax() + 25, + valid_one.getMax() + } + }; + + // Iterate over the scenarios and verify the correct outcome. + for (auto scenario : scenarios) { + SCOPED_TRACE(scenario.desc_); { + // Create a context; + AllocEngine::ClientContext4 ctx(subnet_, ClientIdPtr(), hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", false); + ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + + // Add client classes (if any) + for (auto class_name : scenario.classes_) { + if (class_name == "BOOTP") { + ctx.query_->addClass(class_name); + } else { + ctx.query_->addSubClass(class_name, class_name + "_value"); + } + } + + // Add client option (if one) + if (scenario.requested_lft_) { + OptionUint32Ptr opt(new OptionUint32(Option::V4, DHO_DHCP_LEASE_TIME, + scenario.requested_lft_)); + ctx.query_->addOption(opt); + } + + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ(lease->valid_lft_, scenario.exp_valid_); + } + } +} + // This test checks that deleteRelease handles BOOTP leases. TEST_F(AllocEngine4Test, bootpDelete) { boost::scoped_ptr engine; diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index 675ca0dbd0..1c7f58ccfa 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -8,8 +8,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -5433,8 +5435,7 @@ TEST_F(AllocEngine6Test, getValidLifetime) { // Let's make three classes, two with valid-lifetime and one without, // and add them to the dictionary. - ClientClassDictionaryPtr dictionary = - CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); ClientClassDefPtr class_def(new ClientClassDef("valid_one", ExpressionPtr())); Triplet valid_one(50, 100, 150); @@ -5552,6 +5553,144 @@ TEST_F(AllocEngine6Test, getValidLifetime) { } } +// Verifies that AllocEngine::getLifetimes6() returns the appropriate +// valid lifetime value based on the context content. +TEST_F(AllocEngine6Test, getTemplateClassValidLifetime) { + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100))); + ASSERT_TRUE(engine); + + // Let's make three classes, two with valid-lifetime and one without, + // and add them to the dictionary. + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ExpressionPtr match_expr; + ExpressionParser parser; + + ElementPtr test_cfg = Element::create("'valid_one_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + ClientClassDefPtr class_def(new TemplateClientClassDef("valid_one", match_expr)); + Triplet valid_one(50, 100, 150); + class_def->setValid(valid_one); + dictionary->addClass(class_def); + + test_cfg = Element::create("'valid_two_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("valid_two", match_expr)); + Tripletvalid_two(200, 250, 300); + class_def->setValid(valid_two); + dictionary->addClass(class_def); + + test_cfg = Element::create("'valid_unspec_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("valid_unspec", match_expr)); + dictionary->addClass(class_def); + + // Commit our class changes. + CfgMgr::instance().commit(); + + // Update the subnet's triplet to something more useful. + subnet_->setValid(Triplet(500, 1000, 1500)); + + // Describes a test scenario. + struct Scenario { + std::string desc_; // descriptive text for logging + std::vector classes_; // class list of assigned classes + uint32_t requested_lft_; // use as option 51 is > 0 + uint32_t exp_valid_; // expected lifetime + }; + + // Scenarios to test. + std::vector scenarios = { + { + "no classes, no hint", + {}, + 0, + subnet_->getValid() + }, + { + "no classes, hint", + {}, + subnet_->getValid().getMin() + 50, + subnet_->getValid().getMin() + 50 + }, + { + "no classes, hint too small", + {}, + subnet_->getValid().getMin() - 50, + subnet_->getValid().getMin() + }, + { + "no classes, hint too big", + {}, + subnet_->getValid().getMax() + 50, + subnet_->getValid().getMax() + }, + { + "class unspecified, no hint", + { "valid_unspec" }, + 0, + subnet_->getValid() + }, + { + "from last class, no hint", + { "valid_unspec", "valid_one" }, + 0, + valid_one.get() + }, + { + "from first class, no hint", + { "valid_two", "valid_one" }, + 0, + valid_two.get() + }, + { + "class plus hint", + { "valid_one" }, + valid_one.getMin() + 25, + valid_one.getMin() + 25 + }, + { + "class plus hint too small", + { "valid_one" }, + valid_one.getMin() - 25, + valid_one.getMin() + }, + { + "class plus hint too big", + { "valid_one" }, + valid_one.getMax() + 25, + valid_one.getMax() + } + }; + + // Iterate over the scenarios and verify the correct outcome. + for (auto scenario : scenarios) { + SCOPED_TRACE(scenario.desc_); { + // Create a context; + AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", true, + Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234))); + // Add client classes (if any) + for (auto class_name : scenario.classes_) { + ctx.query_->addClass(class_name); + } + + // Add hint + ctx.currentIA().iaid_ = iaid_; + + // prefix,prefixlen, preferred, valid + ctx.currentIA().addHint(IOAddress("::"), 128, 0, scenario.requested_lft_); + + Lease6Ptr lease; + ASSERT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx))); + ASSERT_TRUE(lease); + EXPECT_EQ(lease->valid_lft_, scenario.exp_valid_); + } + } +} + // Verifies that AllocEngine::getLifetimes6() returns the appropriate // preferred lifetime value based on the context content. TEST_F(AllocEngine6Test, getPreferredLifetime) { @@ -5561,8 +5700,7 @@ TEST_F(AllocEngine6Test, getPreferredLifetime) { // Let's make three classes, two with preferred-lifetime and one without, // and add them to the dictionary. - ClientClassDictionaryPtr dictionary = - CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); ClientClassDefPtr class_def(new ClientClassDef("preferred_one", ExpressionPtr())); Triplet preferred_one(50, 100, 150); @@ -5588,7 +5726,7 @@ TEST_F(AllocEngine6Test, getPreferredLifetime) { std::string desc_; // descriptive text for logging std::vector classes_; // class list of assigned classes uint32_t requested_lft_; // use as option 51 is > 0 - uint32_t exp_preferred_; // expected lifetime + uint32_t exp_preferred_; // expected lifetime }; // Scenarios to test. @@ -5663,7 +5801,145 @@ TEST_F(AllocEngine6Test, getPreferredLifetime) { Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234))); // Add client classes (if any) for (auto class_name : scenario.classes_) { - ctx.query_->addClass(class_name); + ctx.query_->addSubClass(class_name, class_name + "_value"); + } + + // Add hint + ctx.currentIA().iaid_ = iaid_; + + // prefix,prefixlen, preferred, preferred + ctx.currentIA().addHint(IOAddress("::"), 128, scenario.requested_lft_, 0); + + Lease6Ptr lease; + ASSERT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx))); + ASSERT_TRUE(lease); + EXPECT_EQ(lease->preferred_lft_, scenario.exp_preferred_); + } + } +} + +// Verifies that AllocEngine::getLifetimes6() returns the appropriate +// preferred lifetime value based on the context content. +TEST_F(AllocEngine6Test, getTemplateClassPreferredLifetime) { + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100))); + ASSERT_TRUE(engine); + + // Let's make three classes, two with preferred-lifetime and one without, + // and add them to the dictionary. + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + ExpressionPtr match_expr; + ExpressionParser parser; + + ElementPtr test_cfg = Element::create("'preferred_one_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + ClientClassDefPtr class_def(new TemplateClientClassDef("preferred_one", match_expr)); + Triplet preferred_one(50, 100, 150); + class_def->setPreferred(preferred_one); + dictionary->addClass(class_def); + + test_cfg = Element::create("'preferred_two_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("preferred_two", match_expr)); + Tripletpreferred_two(200, 250, 300); + class_def->setPreferred(preferred_two); + dictionary->addClass(class_def); + + test_cfg = Element::create("'preferred_unspec_value'"); + parser.parse(match_expr, test_cfg, AF_INET6, EvalContext::acceptAll, EvalContext::PARSER_STRING); + + class_def.reset(new TemplateClientClassDef("preferred_unspec", match_expr)); + dictionary->addClass(class_def); + + // Commit our class changes. + CfgMgr::instance().commit(); + + // Update the subnet's triplet to something more useful. + subnet_->setPreferred(Triplet(500, 1000, 1500)); + + // Describes a test scenario. + struct Scenario { + std::string desc_; // descriptive text for logging + std::vector classes_; // class list of assigned classes + uint32_t requested_lft_; // use as option 51 is > 0 + uint32_t exp_preferred_; // expected lifetime + }; + + // Scenarios to test. + std::vector scenarios = { + { + "no classes, no hint", + {}, + 0, + subnet_->getPreferred() + }, + { + "no classes, hint", + {}, + subnet_->getPreferred().getMin() + 50, + subnet_->getPreferred().getMin() + 50 + }, + { + "no classes, hint too small", + {}, + subnet_->getPreferred().getMin() - 50, + subnet_->getPreferred().getMin() + }, + { + "no classes, hint too big", + {}, + subnet_->getPreferred().getMax() + 50, + subnet_->getPreferred().getMax() + }, + { + "class unspecified, no hint", + { "preferred_unspec" }, + 0, + subnet_->getPreferred() + }, + { + "from last class, no hint", + { "preferred_unspec", "preferred_one" }, + 0, + preferred_one.get() + }, + { + "from first class, no hint", + { "preferred_two", "preferred_one" }, + 0, + preferred_two.get() + }, + { + "class plus hint", + { "preferred_one" }, + preferred_one.getMin() + 25, + preferred_one.getMin() + 25 + }, + { + "class plus hint too small", + { "preferred_one" }, + preferred_one.getMin() - 25, + preferred_one.getMin() + }, + { + "class plus hint too big", + { "preferred_one" }, + preferred_one.getMax() + 25, + preferred_one.getMax() + } + }; + + // Iterate over the scenarios and verify the correct outcome. + for (auto scenario : scenarios) { + SCOPED_TRACE(scenario.desc_); { + // Create a context; + AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", true, + Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234))); + // Add client classes (if any) + for (auto class_name : scenario.classes_) { + ctx.query_->addSubClass(class_name, class_name + "_value"); } // Add hint diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc index 7751591ff1..305d024c52 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc @@ -47,11 +47,13 @@ protected: /// evaluated. /// @param option_string String data to be placed in the hostname /// option, being placed in the packet used for evaluation. + /// @param parser_type the expected type of the evaluated expression. /// @tparam Type of the packet: @c Pkt4 or @c Pkt6. template void testValidExpression(uint16_t family, const std::string& expression, - const std::string& option_string) { + const std::string& option_string, + EvalContext::ParserType parser_type = EvalContext::PARSER_BOOL) { ExpressionPtr parsed_expr; ExpressionParser parser; @@ -59,7 +61,7 @@ protected: ElementPtr config_element = Element::fromJSON(expression); // Expression should parse. - ASSERT_NO_THROW(parser.parse(parsed_expr, config_element, family)); + ASSERT_NO_THROW(parser.parse(parsed_expr, config_element, family, EvalContext::acceptAll, parser_type)); // Parsed expression should exist. ASSERT_TRUE(parsed_expr); @@ -72,7 +74,11 @@ protected: message_type = DHCPV6_SOLICIT; } boost::shared_ptr pkt(new PktType(message_type, 123)); - EXPECT_FALSE(evaluateBool(*parsed_expr, *pkt)); + if (parser_type == EvalContext::PARSER_BOOL) { + EXPECT_FALSE(evaluateBool(*parsed_expr, *pkt)); + } else { + EXPECT_TRUE(evaluateString(*parsed_expr, *pkt).empty()); + } // Now add the option so it will pass. Use a standard option carrying a // single string value, i.e. hostname for DHCPv4 and bootfile url for @@ -86,7 +92,11 @@ protected: } OptionPtr opt(new OptionString(universe, option_type, option_string)); pkt->addOption(opt); - EXPECT_TRUE(evaluateBool(*parsed_expr, *pkt)); + if (parser_type == EvalContext::PARSER_BOOL) { + EXPECT_TRUE(evaluateBool(*parsed_expr, *pkt)); + } else { + EXPECT_EQ(evaluateString(*parsed_expr, *pkt), option_string); + } } }; @@ -184,6 +194,14 @@ TEST_F(ExpressionParserTest, validExpression4) { "hundred4"); } +// Verifies that given a valid expression, the ExpressionParser +// produces an Expression which can be evaluated against a v4 packet. +TEST_F(ExpressionParserTest, templateValidExpression4) { + testValidExpression(AF_INET, + "\"option[12].text\"", + "hundred4", EvalContext::PARSER_STRING); +} + // Verifies that the option name can be used in the evaluated expression. TEST_F(ExpressionParserTest, validExpressionWithOptionName4) { testValidExpression(AF_INET, @@ -191,6 +209,13 @@ TEST_F(ExpressionParserTest, validExpressionWithOptionName4) { "hundred4"); } +// Verifies that the option name can be used in the evaluated expression. +TEST_F(ExpressionParserTest, templateValidExpressionWithOptionName4) { + testValidExpression(AF_INET, + "\"option[host-name].text\"", + "hundred4", EvalContext::PARSER_STRING); +} + // Verifies that given a valid expression using .hex operator for option, the // ExpressionParser produces an Expression which can be evaluated against // a v4 packet. @@ -200,6 +225,15 @@ TEST_F(ExpressionParserTest, validExpressionWithHex4) { "hundred4"); } +// Verifies that given a valid expression using .hex operator for option, the +// ExpressionParser produces an Expression which can be evaluated against +// a v4 packet. +TEST_F(ExpressionParserTest, templateValidExpressionWithHex4) { + testValidExpression(AF_INET, + "\"option[12].hex\"", + "hundred4", EvalContext::PARSER_STRING); +} + // Verifies that the option name can be used together with .hex operator in // the evaluated expression. TEST_F(ExpressionParserTest, validExpressionWithOptionNameAndHex4) { @@ -208,6 +242,14 @@ TEST_F(ExpressionParserTest, validExpressionWithOptionNameAndHex4) { "hundred4"); } +// Verifies that the option name can be used together with .hex operator in +// the evaluated expression. +TEST_F(ExpressionParserTest, templateValidExpressionWithOptionNameAndHex4) { + testValidExpression(AF_INET, + "\"option[host-name].text\"", + "hundred4", EvalContext::PARSER_STRING); +} + // Verifies that given a valid expression, the ExpressionParser // produces an Expression which can be evaluated against a v6 packet. TEST_F(ExpressionParserTest, validExpression6) { @@ -216,6 +258,14 @@ TEST_F(ExpressionParserTest, validExpression6) { "hundred6"); } +// Verifies that given a valid expression, the ExpressionParser +// produces an Expression which can be evaluated against a v6 packet. +TEST_F(ExpressionParserTest, templateValidExpression6) { + testValidExpression(AF_INET6, + "\"option[59].text\"", + "hundred6", EvalContext::PARSER_STRING); +} + // Verifies that the option name can be used in the evaluated expression. TEST_F(ExpressionParserTest, validExpressionWithOptionName6) { testValidExpression(AF_INET6, @@ -223,6 +273,13 @@ TEST_F(ExpressionParserTest, validExpressionWithOptionName6) { "hundred6"); } +// Verifies that the option name can be used in the evaluated expression. +TEST_F(ExpressionParserTest, templateValidExpressionWithOptionName6) { + testValidExpression(AF_INET6, + "\"option[bootfile-url].text\"", + "hundred6", EvalContext::PARSER_STRING); +} + // Verifies that given a valid expression using .hex operator for option, the // ExpressionParser produces an Expression which can be evaluated against // a v6 packet. @@ -232,6 +289,15 @@ TEST_F(ExpressionParserTest, validExpressionWithHex6) { "hundred6"); } +// Verifies that given a valid expression using .hex operator for option, the +// ExpressionParser produces an Expression which can be evaluated against +// a v6 packet. +TEST_F(ExpressionParserTest, templateValidExpressionWithHex6) { + testValidExpression(AF_INET6, + "\"option[59].hex\"", + "hundred6", EvalContext::PARSER_STRING); +} + // Verifies that the option name can be used together with .hex operator in // the evaluated expression. TEST_F(ExpressionParserTest, validExpressionWithOptionNameAndHex6) { @@ -240,6 +306,14 @@ TEST_F(ExpressionParserTest, validExpressionWithOptionNameAndHex6) { "hundred6"); } +// Verifies that the option name can be used together with .hex operator in +// the evaluated expression. +TEST_F(ExpressionParserTest, templateValidExpressionWithOptionNameAndHex6) { + testValidExpression(AF_INET6, + "\"option[bootfile-url].text\"", + "hundred6", EvalContext::PARSER_STRING); +} + // Verifies that an the ExpressionParser only accepts StringElements. TEST_F(ExpressionParserTest, invalidExpressionElement) { // This will create an integer element should fail. @@ -300,7 +374,11 @@ TEST_F(ClientClassDefParserTest, checkAllSupported4) { " \"option-data\": [ ],\n" " \"user-context\": { },\n" " \"only-if-required\": false,\n" + " \"valid-lifetime\": 1000,\n" + " \"min-valid-lifetime\": 1000,\n" + " \"max-valid-lifetime\": 1000,\n" " \"next-server\": \"192.0.2.3\",\n" + " \"template-class\": true,\n" " \"server-hostname\": \"myhost\",\n" " \"boot-file-name\": \"efi\"" "}\n"; @@ -321,7 +399,11 @@ TEST_F(ClientClassDefParserTest, checkAllSupported6) { " \"test\": \"member('ALL')\"," " \"option-data\": [ ],\n" " \"user-context\": { },\n" - " \"only-if-required\": false\n" + " \"only-if-required\": false,\n" + " \"template-class\": true,\n" + " \"preferred-lifetime\": 800,\n" + " \"min-preferred-lifetime\": 800,\n" + " \"max-preferred-lifetime\": 800\n" "}\n"; ElementPtr config_element = Element::fromJSON(cfg_text); @@ -397,6 +479,58 @@ TEST_F(ClientClassDefParserTest, checkParams4Unsupported6) { } } +// Verifies that the function checking if specified client class parameters +// are supported throws if DHCPv6 specific parameters are specified for the +// DHCPv4 client class. +TEST_F(ClientClassDefParserTest, checkParams6Unsupported4) { + std::string cfg_text; + + { + SCOPED_TRACE("preferred-lifetime"); + cfg_text = + "{\n" + " \"name\": \"foo\"," + " \"test\": \"member('ALL')\"," + " \"preferred-lifetime\": 800,\n" + " \"option-data\": [ ],\n" + " \"user-context\": { },\n" + " \"only-if-required\": false\n" + "}\n"; + + testClassParamsUnsupported(cfg_text, AF_INET); + } + + { + SCOPED_TRACE("min-preferred-lifetime"); + cfg_text = + "{\n" + " \"name\": \"foo\"," + " \"test\": \"member('ALL')\"," + " \"min-preferred-lifetime\": 800,\n" + " \"option-data\": [ ],\n" + " \"user-context\": { },\n" + " \"only-if-required\": false\n" + "}\n"; + + testClassParamsUnsupported(cfg_text, AF_INET); + } + + { + SCOPED_TRACE("max-preferred-lifetime"); + cfg_text = + "{\n" + " \"name\": \"foo\"," + " \"test\": \"member('ALL')\"," + " \"max-preferred-lifetime\": 800,\n" + " \"option-data\": [ ],\n" + " \"user-context\": { },\n" + " \"only-if-required\": false\n" + "}\n"; + + testClassParamsUnsupported(cfg_text, AF_INET); + } +} + // Verifies that the function checking if specified DHCPv4 client class // parameters are supported throws if one of the parameters is not recognized. TEST_F(ClientClassDefParserTest, checkParams4Unsupported) { @@ -453,8 +587,7 @@ TEST_F(ClientClassDefParserTest, nameOnlyValid) { // Verifies you can create a class with a name, expression, // but no options. -// @todo same with AF_INET6 -TEST_F(ClientClassDefParserTest, nameAndExpressionClass) { +TEST_F(ClientClassDefParserTest, nameAndExpressionClass4) { std::string test = "option[100].text == 'works right'"; std::string cfg_text = @@ -497,10 +630,148 @@ TEST_F(ClientClassDefParserTest, nameAndExpressionClass) { EXPECT_TRUE(evaluateBool(*match_expr, *pkt4)); } +// Verifies you can create a class with a name, expression, +// but no options. +TEST_F(ClientClassDefParserTest, templateNameAndExpressionClass4) { + + std::string test = "option[100].text"; + std::string cfg_text = + "{ \n" + " \"name\": \"class_one\", \n" + " \"template-class\": true, \n" + " \"test\": \"" + test + "\" \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET)); + ASSERT_TRUE(dynamic_cast(cclass.get())); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("class_one", cclass->getName()); + + // CfgOption should be a non-null pointer but there + // should be no options. Currently there's no good + // way to test that there no options. + CfgOptionPtr cfg_option; + cfg_option = cclass->getCfgOption(); + ASSERT_TRUE(cfg_option); + OptionContainerPtr oc; + ASSERT_TRUE(oc = cclass->getCfgOption()->getAll(DHCP4_OPTION_SPACE)); + EXPECT_EQ(0, oc->size()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt4Ptr pkt4(new Pkt4(DHCPDISCOVER, 123)); + EXPECT_TRUE(evaluateString(*match_expr, *pkt4).empty()); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V4, 100, "works right")); + pkt4->addOption(opt); + EXPECT_EQ(evaluateString(*match_expr, *pkt4), "works right"); +} + +// Verifies you can create a class with a name, expression, +// but no options. +TEST_F(ClientClassDefParserTest, nameAndExpressionClass6) { + + std::string test = "option[59].text == 'works right'"; + std::string cfg_text = + "{ \n" + " \"name\": \"class_one\", \n" + " \"test\": \"" + test + "\" \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("class_one", cclass->getName()); + + // CfgOption should be a non-null pointer but there + // should be no options. Currently there's no good + // way to test that there no options. + CfgOptionPtr cfg_option; + cfg_option = cclass->getCfgOption(); + ASSERT_TRUE(cfg_option); + OptionContainerPtr oc; + ASSERT_TRUE(oc = cclass->getCfgOption()->getAll(DHCP6_OPTION_SPACE)); + EXPECT_EQ(0, oc->size()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt6Ptr pkt6(new Pkt6(DHCPV6_SOLICIT, 123)); + EXPECT_FALSE(evaluateBool(*match_expr, *pkt6)); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V6, 59, "works right")); + pkt6->addOption(opt); + EXPECT_TRUE(evaluateBool(*match_expr, *pkt6)); +} + +// Verifies you can create a class with a name, expression, +// but no options. +TEST_F(ClientClassDefParserTest, templateNameAndExpressionClass6) { + + std::string test = "option[59].text"; + std::string cfg_text = + "{ \n" + " \"name\": \"class_one\", \n" + " \"template-class\": true, \n" + " \"test\": \"" + test + "\" \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + ASSERT_TRUE(dynamic_cast(cclass.get())); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("class_one", cclass->getName()); + + // CfgOption should be a non-null pointer but there + // should be no options. Currently there's no good + // way to test that there no options. + CfgOptionPtr cfg_option; + cfg_option = cclass->getCfgOption(); + ASSERT_TRUE(cfg_option); + OptionContainerPtr oc; + ASSERT_TRUE(oc = cclass->getCfgOption()->getAll(DHCP6_OPTION_SPACE)); + EXPECT_EQ(0, oc->size()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt6Ptr pkt6(new Pkt6(DHCPV6_SOLICIT, 123)); + EXPECT_TRUE(evaluateString(*match_expr, *pkt6).empty()); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V6, 59, "works right")); + pkt6->addOption(opt); + EXPECT_EQ(evaluateString(*match_expr, *pkt6), "works right"); +} + // Verifies you can create a class with a name and options, // but no expression. -// @todo same with AF_INET6 -TEST_F(ClientClassDefParserTest, nameAndOptionsClass) { +TEST_F(ClientClassDefParserTest, nameAndOptionsClass4) { std::string cfg_text = "{ \n" @@ -532,11 +803,43 @@ TEST_F(ClientClassDefParserTest, nameAndOptionsClass) { ASSERT_FALSE(cclass->getMatchExpr()); } +// Verifies you can create a class with a name and options, +// but no expression. +TEST_F(ClientClassDefParserTest, nameAndOptionsClass6) { + + std::string cfg_text = + "{ \n" + " \"name\": \"MICROSOFT\", \n" + " \"option-data\": [ \n" + " { \n" + " \"name\": \"sip-server-addr\", \n" + " \"code\": 22, \n" + " \"space\": \"dhcp6\", \n" + " \"csv-format\": true, \n" + " \"data\": \"2003:db8::1, 2003:db8::2\" \n" + " } \n" + " ] \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("MICROSOFT", cclass->getName()); + + // Our one option should exist. + OptionDescriptor od = cclass->getCfgOption()->get(DHCP6_OPTION_SPACE, 22); + ASSERT_TRUE(od.option_); + EXPECT_EQ(22, od.option_->getType()); + + // Verify we have no expression + ASSERT_FALSE(cclass->getMatchExpr()); +} // Verifies you can create a class with a name, expression, // and options. -// @todo same with AF_INET6 -TEST_F(ClientClassDefParserTest, basicValidClass) { +TEST_F(ClientClassDefParserTest, basicValidClass4) { std::string test = "option[100].text == 'booya'"; std::string cfg_text = @@ -583,6 +886,157 @@ TEST_F(ClientClassDefParserTest, basicValidClass) { EXPECT_TRUE(evaluateBool(*match_expr, *pkt4)); } +// Verifies you can create a class with a name, expression, +// and options. +TEST_F(ClientClassDefParserTest, templateBasicValidClass4) { + + std::string test = "option[100].text"; + std::string cfg_text = + "{ \n" + " \"name\": \"MICROSOFT\", \n" + " \"test\": \"" + test + "\", \n" + " \"template-class\": true, \n" + " \"option-data\": [ \n" + " { \n" + " \"name\": \"domain-name-servers\", \n" + " \"code\": 6, \n" + " \"space\": \"dhcp4\", \n" + " \"csv-format\": true, \n" + " \"data\": \"192.0.2.1, 192.0.2.2\" \n" + " } \n" + " ] \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET)); + ASSERT_TRUE(dynamic_cast(cclass.get())); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("MICROSOFT", cclass->getName()); + + // Our one option should exist. + OptionDescriptor od = cclass->getCfgOption()->get(DHCP4_OPTION_SPACE, 6); + ASSERT_TRUE(od.option_); + EXPECT_EQ(6, od.option_->getType()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt4Ptr pkt4(new Pkt4(DHCPDISCOVER, 123)); + EXPECT_TRUE(evaluateString(*match_expr, *pkt4).empty()); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V4, 100, "booya")); + pkt4->addOption(opt); + EXPECT_EQ(evaluateString(*match_expr, *pkt4), "booya"); +} + +// Verifies you can create a class with a name, expression, +// and options. +TEST_F(ClientClassDefParserTest, basicValidClass6) { + + std::string test = "option[59].text == 'booya'"; + std::string cfg_text = + "{ \n" + " \"name\": \"MICROSOFT\", \n" + " \"test\": \"" + test + "\", \n" + " \"option-data\": [ \n" + " { \n" + " \"name\": \"sip-server-addr\", \n" + " \"code\": 22, \n" + " \"space\": \"dhcp6\", \n" + " \"csv-format\": true, \n" + " \"data\": \"2003:db8::1, 2003:db8::2\" \n" + " } \n" + " ] \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("MICROSOFT", cclass->getName()); + + // Our one option should exist. + OptionDescriptor od = cclass->getCfgOption()->get(DHCP6_OPTION_SPACE, 22); + ASSERT_TRUE(od.option_); + EXPECT_EQ(22, od.option_->getType()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt6Ptr pkt6(new Pkt6(DHCPV6_SOLICIT, 123)); + EXPECT_FALSE(evaluateBool(*match_expr, *pkt6)); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V6, 59, "booya")); + pkt6->addOption(opt); + EXPECT_TRUE(evaluateBool(*match_expr, *pkt6)); +} + +// Verifies you can create a class with a name, expression, +// and options. +TEST_F(ClientClassDefParserTest, templateBasicValidClass6) { + + std::string test = "option[59].text"; + std::string cfg_text = + "{ \n" + " \"name\": \"MICROSOFT\", \n" + " \"test\": \"" + test + "\", \n" + " \"template-class\": true, \n" + " \"option-data\": [ \n" + " { \n" + " \"name\": \"sip-server-addr\", \n" + " \"code\": 22, \n" + " \"space\": \"dhcp6\", \n" + " \"csv-format\": true, \n" + " \"data\": \"2003:db8::1, 2003:db8::2\" \n" + " } \n" + " ] \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + ASSERT_TRUE(dynamic_cast(cclass.get())); + + // We should find our class. + ASSERT_TRUE(cclass); + EXPECT_EQ("MICROSOFT", cclass->getName()); + + // Our one option should exist. + OptionDescriptor od = cclass->getCfgOption()->get(DHCP6_OPTION_SPACE, 22); + ASSERT_TRUE(od.option_); + EXPECT_EQ(22, od.option_->getType()); + + // Verify we can retrieve the expression + ExpressionPtr match_expr = cclass->getMatchExpr(); + ASSERT_TRUE(match_expr); + + // Verify the original expression was saved. + EXPECT_EQ(test, cclass->getTest()); + + // Build a packet that will fail evaluation. + Pkt6Ptr pkt6(new Pkt6(DHCPV6_SOLICIT, 123)); + EXPECT_TRUE(evaluateString(*match_expr, *pkt6).empty()); + + // Now add the option so it will pass. + OptionPtr opt(new OptionString(Option::V6, 59, "booya")); + pkt6->addOption(opt); + EXPECT_EQ(evaluateString(*match_expr, *pkt6), "booya"); +} + // Verifies that a class with no name, fails to parse. TEST_F(ClientClassDefParserTest, noClassName) { @@ -641,6 +1095,20 @@ TEST_F(ClientClassDefParserTest, invalidExpression) { DhcpConfigError); } +// Verifies that a class with an invalid expression, fails to parse. +TEST_F(ClientClassDefParserTest, templateInvalidExpression) { + std::string cfg_text = + "{ \n" + " \"name\": \"one\", \n" + " \"template-class\": true, \n" + " \"test\": 777 \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6), + DhcpConfigError); +} + // Verifies that a class with invalid option-def, fails to parse. TEST_F(ClientClassDefParserTest, invalidOptionDef) { std::string cfg_text = @@ -671,7 +1139,6 @@ TEST_F(ClientClassDefParserTest, invalidOptionData) { DhcpConfigError); } - // Verifies that a valid list of client classes will parse. TEST_F(ClientClassDefListParserTest, simpleValidList) { std::string cfg_text = @@ -714,6 +1181,54 @@ TEST_F(ClientClassDefListParserTest, simpleValidList) { EXPECT_FALSE(cclass); } +// Verifies that a valid list of client classes will parse. +TEST_F(ClientClassDefListParserTest, templateSimpleValidList) { + std::string cfg_text = + "[ \n" + " { \n" + " \"name\": \"one\", \n" + " \"template-class\": true \n" + " }, \n" + " { \n" + " \"name\": \"two\", \n" + " \"template-class\": true \n" + " }, \n" + " { \n" + " \"name\": \"three\", \n" + " \"template-class\": true \n" + " } \n" + "] \n"; + + // Parsing the list should succeed. + ClientClassDictionaryPtr dictionary; + ASSERT_NO_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET6)); + ASSERT_TRUE(dictionary); + + // We should have three classes in the dictionary. + EXPECT_EQ(3, dictionary->getClasses()->size()); + + // Make sure we can find all three. + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = dictionary->findClass("one")); + ASSERT_TRUE(cclass); + ASSERT_TRUE(dynamic_cast(cclass.get())); + EXPECT_EQ("one", cclass->getName()); + + ASSERT_NO_THROW(cclass = dictionary->findClass("two")); + ASSERT_TRUE(cclass); + ASSERT_TRUE(dynamic_cast(cclass.get())); + EXPECT_EQ("two", cclass->getName()); + + ASSERT_NO_THROW(cclass = dictionary->findClass("three")); + ASSERT_TRUE(cclass); + ASSERT_TRUE(dynamic_cast(cclass.get())); + EXPECT_EQ("three", cclass->getName()); + + // For good measure, make sure we can't find a non-existent class. + ASSERT_NO_THROW(cclass = dictionary->findClass("bogus")); + EXPECT_FALSE(cclass); +} + // Verifies that class list containing a duplicate class entries, fails // to parse. TEST_F(ClientClassDefListParserTest, duplicateClass) { @@ -735,10 +1250,33 @@ TEST_F(ClientClassDefListParserTest, duplicateClass) { DhcpConfigError); } +// Verifies that class list containing a duplicate class entries, fails +// to parse. +TEST_F(ClientClassDefListParserTest, templateDuplicateClass) { + std::string cfg_text = + "[ \n" + " { \n" + " \"name\": \"one\", \n" + " \"template-class\": true \n" + " }, \n" + " { \n" + " \"name\": \"two\", \n" + " \"template-class\": true \n" + " }, \n" + " { \n" + " \"name\": \"two\", \n" + " \"template-class\": true \n" + " } \n" + "] \n"; + + ClientClassDictionaryPtr dictionary; + ASSERT_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET), + DhcpConfigError); +} + // Test verifies that without any class specified, the fixed fields have their // default, empty value. -// @todo same with AF_INET6 -TEST_F(ClientClassDefParserTest, noFixedFields) { +TEST_F(ClientClassDefParserTest, noFixedFields4) { std::string cfg_text = "{ \n" @@ -767,6 +1305,37 @@ TEST_F(ClientClassDefParserTest, noFixedFields) { ASSERT_TRUE(cfg->getAll(DHCP4_OPTION_SPACE)->empty()); } +// Test verifies that without any class specified, the fixed fields have their +// default, empty value. +TEST_F(ClientClassDefParserTest, noFixedFields6) { + + std::string cfg_text = + "{ \n" + " \"name\": \"MICROSOFT\", \n" + " \"option-data\": [ \n" + " { \n" + " \"name\": \"sip-server-addr\", \n" + " \"data\": \"2003:db8::1, 2003:db8::2\" \n" + " } \n" + " ] \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, AF_INET6)); + + // We should find our class. + ASSERT_TRUE(cclass); + + // And it should not have any fixed fields set + EXPECT_EQ(0, cclass->getPreferred()); + EXPECT_EQ(0, cclass->getPreferred().getMin()); + EXPECT_EQ(0, cclass->getPreferred().getMax()); + + // Nor option definitions + CfgOptionDefPtr cfg = cclass->getCfgOptionDef(); + ASSERT_TRUE(cfg->getAll(DHCP6_OPTION_SPACE)->empty()); +} + // Test verifies option-def for a bad option fails to parse. TEST_F(ClientClassDefParserTest, badOptionDef) { std::string cfg_text = @@ -867,7 +1436,6 @@ TEST_F(ClientClassDefParserTest, option43Def) { EXPECT_EQ(0, std::memcmp(expected, &opt->getData()[0], 4)); } - // Test verifies that it is possible to define next-server field and it // is actually set in the class properly. TEST_F(ClientClassDefParserTest, nextServer) { @@ -985,7 +1553,6 @@ TEST_F(ClientClassDefParserTest, serverNameInvalid) { EXPECT_THROW(parseClientClassDef(cfg_too_long, AF_INET), DhcpConfigError); } - // Test verifies that it is possible to define boot-file-name field and it // is actually set in the class properly. TEST_F(ClientClassDefParserTest, filename) { @@ -1443,12 +2010,12 @@ TEST_F(ClientClassDefParserTest, invalidUserContext) { std::string cfg_text = "{ \n" " \"name\": \"one\", \n" - " \"user-context\": \"i am not a map\" \n" + " \"user-context\": \"i am not a map\" \n" "} \n"; ClientClassDefPtr cclass; ASSERT_THROW_MSG(cclass = parseClientClassDef(cfg_text, AF_INET), - DhcpConfigError, "User context has to be a map (:3:20)"); + DhcpConfigError, "User context has to be a map (:3:21)"); } } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc index 21c2e07164..f0f0e8451f 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc @@ -173,7 +173,6 @@ TEST(ClientClassDef, cfgOptionBasics) { // Verifies copy constructor and equality tools (methods/operators) TEST(ClientClassDef, copyAndEquality) { - boost::scoped_ptr cclass; ExpressionPtr expr; CfgOptionPtr test_options; @@ -200,11 +199,11 @@ TEST(ClientClassDef, copyAndEquality) { // The allocated Expression pointers should not match EXPECT_TRUE(cclass->getMatchExpr().get() != - cclass2->getMatchExpr().get()); + cclass2->getMatchExpr().get()); // The allocated CfgOption pointers should not match EXPECT_TRUE(cclass->getCfgOption().get() != - cclass2->getCfgOption().get()); + cclass2->getCfgOption().get()); // Verify the equality tools reflect that the classes are equal. EXPECT_TRUE(cclass->equals(*cclass2)); @@ -308,7 +307,6 @@ TEST(ClientClassDef, dependency) { EXPECT_FALSE(cclass->dependOnClass("bar")); } - // Tests the basic operation of ClientClassDictionary // This includes adding, finding, and removing classes TEST(ClientClassDictionary, basics) { @@ -334,11 +332,19 @@ TEST(ClientClassDictionary, basics) { ASSERT_NO_THROW(dictionary->addClass("cc2", expr, "", false, false, cfg_option)); - // Verify duplicate add attempt throws + // Verify duplicate add attempt throws using regular class ASSERT_THROW(dictionary->addClass("cc2", expr, "", false, false, cfg_option), DuplicateClientClassDef); + // Verify duplicate add attempt throws using template class + ASSERT_THROW(dictionary->addClass("cc2", expr, "", false, + false, cfg_option, CfgOptionDefPtr(), + ConstElementPtr(), IOAddress("0.0.0.0"), + "", "", Triplet(), + Triplet(), true), + DuplicateClientClassDef); + // Verify that you cannot add a class with no name. ASSERT_THROW(dictionary->addClass("", expr, "", false, false, cfg_option), @@ -655,7 +661,6 @@ TEST(ClientClassDef, fixedFieldsBasics) { EXPECT_EQ(filename, cclass->getFilename()); } - // Verifies the unparse method of option class definitions TEST(ClientClassDef, unparseDef) { CfgMgr::instance().setFamily(AF_INET); @@ -803,4 +808,861 @@ TEST(ClientClassDictionary, createOptions) { option->getData()); } +// Tests basic construction of TemplateClientClassDef +TEST(TemplateClientClassDef, construction) { + boost::scoped_ptr cclass; + + std::string name = "class1"; + ExpressionPtr expr; + CfgOptionPtr cfg_option; + + // Classes cannot have blank names + ASSERT_THROW(cclass.reset(new TemplateClientClassDef("", expr, cfg_option)), + BadValue); + + // Verify we can create a class with a name, expression, and no cfg_option + ASSERT_NO_THROW(cclass.reset(new TemplateClientClassDef(name, expr))); + EXPECT_EQ(name, cclass->getName()); + ASSERT_FALSE(cclass->getMatchExpr()); + EXPECT_FALSE(cclass->getCfgOptionDef()); + + // Verify we get an empty collection of cfg_option + cfg_option = cclass->getCfgOption(); + ASSERT_TRUE(cfg_option); + EXPECT_TRUE(cfg_option->empty()); + + // Verify we don't depend on something. + EXPECT_FALSE(cclass->dependOnClass("foobar")); + EXPECT_FALSE(cclass->dependOnClass("")); +} + +// Test that template client class is copied using the copy constructor. +TEST(TemplateClientClassDef, copyConstruction) { + auto expr = boost::make_shared(); + + auto cfg_option = boost::make_shared(); + auto option = boost::make_shared