[#2077] Init match expressions

Match expressions are now initialized for client classes fetched from the
configuration backend.

diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
index 6b3c36773e7aa1fc5779bb9dbc0e84ad0aa09c3c..3c53d0522331863061cb5a0b973dd061fcd6a333 100644 (file)
--- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
+++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
@@ -203,6 +203,9 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
     if (audit_entries.empty() || !updated_entries.empty()) {
         ClientClassDictionary client_classes = getMgr().getPool()->getAllClientClasses4(backend_selector,
                                                                                         server_selector);
+        // Match expressions are not initialized for classes returned from the config backend.
+        // We have to ensure to initialize them before they can be used by the server.
+        client_classes.initMatchExpr(AF_INET);
         external_cfg->setClientClassDictionary(boost::make_shared<ClientClassDictionary>(client_classes));
     }
 

diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
index d2096045f32824540fab41b33c421c7c0f0ca6df..2713db2201f3609d662138291c45fe6472188125 100644 (file)
--- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
+++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
@@ -202,6 +202,9 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
     if (audit_entries.empty() || !updated_entries.empty()) {
         ClientClassDictionary client_classes = getMgr().getPool()->getAllClientClasses6(backend_selector,
                                                                                         server_selector);
+        // Match expressions are not initialized for classes returned from the config backend.
+        // We have to ensure to initialize them before they can be used by the server.
+        client_classes.initMatchExpr(AF_INET6);
         external_cfg->setClientClassDictionary(boost::make_shared<ClientClassDictionary>(client_classes));
     }
 

diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc
index 6a3b3c10cd093ebc97435cdb929384d4acf1d3e8..8666f9432ae1564a0fee3d8d98c992bda69fa16a 100644 (file)
--- a/src/lib/dhcpsrv/client_class_def.cc
+++ b/src/lib/dhcpsrv/client_class_def.cc
@@ -9,8 +9,11 @@
 #include <eval/dependency.h>
 #include <dhcpsrv/client_class_def.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/parsers/client_class_def_parser.h>
 #include <boost/foreach.hpp>
 
+#include <queue>
+
 using namespace isc::data;
 
 namespace isc {
@@ -384,6 +387,25 @@ ClientClassDictionary::equals(const ClientClassDictionary& other) const {
     return (true);
 }
 
+void
+ClientClassDictionary::initMatchExpr(uint16_t family) {
+    std::queue<ExpressionPtr> expressions;
+    for (auto c : *list_) {
+        ExpressionPtr match_expr = boost::make_shared<Expression>();
+        if (!c->getTest().empty()) {
+            ExpressionParser parser;
+            parser.parse(match_expr, Element::create(c->getTest()), family);
+        }
+        expressions.push(match_expr);
+    }
+    // All expressions successfully initialied. Let's set them for the
+    // client classes in the dictionary.
+    for (auto c : *list_) {
+        c->setMatchExpr(expressions.front());
+        expressions.pop();
+    }
+}
+
 ElementPtr
 ClientClassDictionary::toElement() const {
     ElementPtr result = Element::createList();

diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h
index 89542b02947c2ea33371966860b03c1532fc0e0f..012f3e9c39c4f878bb1614ad6f70782193237d49 100644 (file)
--- a/src/lib/dhcpsrv/client_class_def.h
+++ b/src/lib/dhcpsrv/client_class_def.h
@@ -397,6 +397,12 @@ public:
     /// @return true if descriptors equal, false otherwise.
     bool equals(const ClientClassDictionary& other) const;
 
+    /// @brief Iterates over the classes in the dictionary and ensures that
+    /// that match expressions are initialized.
+    ///
+    /// @param family Class universe, e.g. AF_INET or AF_INET6.
+    void initMatchExpr(uint16_t family);
+
     /// @brief Equality operator.
     ///
     /// @param other Other client class dictionary to compare to.

diff --git a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
index 8bd10b8082de8b9795663769c3419e93ed3205f7..e6318b1336adfb904a481ed87a829e5df365bbfd 100644 (file)
--- a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
@@ -382,6 +382,7 @@ public:
         // Insert client classes into the database.
         auto expression = boost::make_shared<Expression>();
         ClientClassDefPtr client_class = boost::make_shared<ClientClassDef>("first-class", expression);
+        client_class->setTest("substring(option[1].hex, 0, 8) == 'my-value'");
         client_class->setId(1);
         client_class->setModificationTime(getTimestamp("dhcp4_client_class"));
         mgr.getPool()->createUpdateClientClass4(BackendSelector::UNSPEC(), ServerSelector::ALL(),
@@ -592,6 +593,8 @@ public:
         if (hasConfigElement("dhcp4_client_class") &&
             (getTimestamp("dhcp4_client_class") > lb_modification_time)) {
             ASSERT_TRUE(found_class);
+            ASSERT_TRUE(found_class->getMatchExpr());
+            EXPECT_GT(found_class->getMatchExpr()->size(), 0);
             EXPECT_EQ("first-class", found_class->getName());
 
         } else {
@@ -1175,6 +1178,7 @@ public:
         // Insert client classes into the database.
         auto expression = boost::make_shared<Expression>();
         ClientClassDefPtr client_class = boost::make_shared<ClientClassDef>("first-class", expression);
+        client_class->setTest("substring(option[1].hex, 0, 8) == 'my-value'");
         client_class->setId(1);
         client_class->setModificationTime(getTimestamp("dhcp6_client_class"));
         mgr.getPool()->createUpdateClientClass6(BackendSelector::UNSPEC(), ServerSelector::ALL(),
@@ -1385,6 +1389,8 @@ public:
         if (hasConfigElement("dhcp6_client_class") &&
             (getTimestamp("dhcp6_client_class") > lb_modification_time)) {
             ASSERT_TRUE(found_class);
+            ASSERT_TRUE(found_class->getMatchExpr());
+            EXPECT_GT(found_class->getMatchExpr()->size(), 0);
             EXPECT_EQ("first-class", found_class->getName());
 
         } else {

diff --git a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc
index 5532dd9050335fd62c78666d2ca4d3c47c3f6a47..b93c7198eabacd87d1955a209258cfd95de81c59 100644 (file)
--- a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc
+++ b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc
@@ -542,6 +542,62 @@ TEST(ClientClassDictionary, dependency) {
     EXPECT_EQ("cc4", depend);
 }
 
+// Tests that match expressions are set for all client classes in the
+// dictionary.
+TEST(ClientClassDictionary, initMatchExpr) {
+    ClientClassDictionaryPtr dictionary(new ClientClassDictionary());
+    ExpressionPtr expr;
+    CfgOptionPtr cfg_option;
+
+    // Add several classes.
+    ASSERT_NO_THROW(dictionary->addClass("foo", expr, "", false,
+                                         false, cfg_option));
+    ASSERT_NO_THROW(dictionary->addClass("bar", expr, "member('KNOWN') or member('foo')", false,
+                                         false, cfg_option));
+    ASSERT_NO_THROW(dictionary->addClass("baz", expr, "substring(option[61].hex,0,3) == 'foo'", false,
+                                         false, cfg_option));
+
+    // Create match expressions for all of them.
+    ASSERT_NO_THROW(dictionary->initMatchExpr(AF_INET));
+
+    // Ensure that the expressions were created.
+    auto classes = *(dictionary->getClasses());
+    EXPECT_TRUE(classes[0]->getMatchExpr());
+    EXPECT_EQ(0, classes[0]->getMatchExpr()->size());
+
+    EXPECT_TRUE(classes[1]->getMatchExpr());
+    EXPECT_EQ(3, classes[1]->getMatchExpr()->size());
+
+    EXPECT_TRUE(classes[2]->getMatchExpr());
+    EXPECT_EQ(6, classes[2]->getMatchExpr()->size());
+}
+
+// Tests that an error is returned when any of the test expressions is
+// invalid, and that no expressions are initialized if there is an error
+// for a single expresion.
+TEST(ClientClassDictionary, initMatchExprError) {
+    ClientClassDictionaryPtr dictionary(new ClientClassDictionary());
+    ExpressionPtr expr;
+    CfgOptionPtr cfg_option;
+
+    // Add several classes. One of them has invalid test expression.
+    ASSERT_NO_THROW(dictionary->addClass("foo", expr, "member('KNOWN')", false,
+                                         false, cfg_option));
+    ASSERT_NO_THROW(dictionary->addClass("bar", expr, "wrong expression", false,
+                                         false, cfg_option));
+    ASSERT_NO_THROW(dictionary->addClass("baz", expr, "substring(option[61].hex,0,3) == 'foo'", false,
+                                         false, cfg_option));
+
+    // An attempt to initialize match expressions should fail because the
+    // test expression for the second class is invalid.
+    ASSERT_THROW(dictionary->initMatchExpr(AF_INET), std::exception);
+
+    // Ensure that no classes have their match expressions modified.
+    for (auto c : (*dictionary->getClasses())) {
+        EXPECT_FALSE(c->getMatchExpr());
+    }
+}
+
 // Tests the default constructor regarding fixed fields
 TEST(ClientClassDef, fixedFieldsDefaults) {
     boost::scoped_ptr<ClientClassDef> cclass;