[509-improve-disabling-client-id] [509-improve-disabling-client-id-] Factored subnetsIncludeMatchClientId and subnetsAllHRGlobal

src/lib/database/db_log.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -100,7 +100,7 @@ public:
 /// @brief Database logger stack
 typedef std::list<DbLogger> DbLoggerStack;
 
-/// @brief Global database logger stack (initialized to dhcpsrv logger)
+/// @brief Global database logger stack (initialized to database logger)
 extern DbLoggerStack db_logger_stack;
 
 /// @brief Check database logger stack

src/lib/database/dbaccess_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2019 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -9,7 +9,6 @@
 #include <database/database_connection.h>
 #include <database/db_exceptions.h>
 #include <database/dbaccess_parser.h>
-#include <dhcpsrv/parsers/dhcp_parsers.h>
 
 #include <boost/lexical_cast.hpp>
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -2844,17 +2844,9 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
 
     // Client identifier is optional. First check if we can try to lookup
     // by client-id.
-    bool try_clientid_lookup = false;
+    bool try_clientid_lookup = (ctx.clientid_ &&
-    if (ctx.clientid_) {
+        SharedNetwork4::subnetsIncludeMatchClientId(original_subnet,
-        for (Subnet4Ptr subnet = original_subnet; subnet;
+                                                    ctx.query_->getClasses()));
-             subnet = subnet->getNextSubnet(original_subnet,
-                                            ctx.query_->getClasses())) {
-            if (subnet->getMatchClientId()) {
-                try_clientid_lookup = true;
-                break;
-            }
-        }
-    }
 
     // If it is possible to use client identifier to try to find client's lease.
     if (try_clientid_lookup) {

src/lib/dhcpsrv/shared_network.cc

@@ -358,6 +358,28 @@ SharedNetwork4::getPreferredSubnet(const Subnet4Ptr& selected_subnet) const {
                                                  Lease::TYPE_V4));
 }
 
+bool
+SharedNetwork4::subnetsIncludeMatchClientId(const Subnet4Ptr& first_subnet,
+                                            const ClientClasses& client_classes) {
+    for (Subnet4Ptr subnet = first_subnet; subnet;
+         subnet = subnet->getNextSubnet(first_subnet, client_classes)) {
+        if (subnet->getMatchClientId()) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
+void
+SharedNetwork4::subnetsAllHRGlobal(Subnet4Ptr& bad_subnet) const {
+    for (auto subnet : *getAllSubnets()) {
+        if (subnet->getHostReservationMode() != Network::HR_GLOBAL) {
+            bad_subnet = subnet;
+            return;
+        }
+    }
+}
+
 ElementPtr
 SharedNetwork4::toElement() const {
     ElementPtr map = Network4::toElement();
@@ -441,6 +463,16 @@ SharedNetwork6::getPreferredSubnet(const Subnet6Ptr& selected_subnet,
     return (Impl::getPreferredSubnet(subnets_, selected_subnet, lease_type));
 }
 
+void
+SharedNetwork6::subnetsAllHRGlobal(Subnet6Ptr& bad_subnet) const {
+    for (auto subnet : *getAllSubnets()) {
+        if (subnet->getHostReservationMode() != Network::HR_GLOBAL) {
+            bad_subnet = subnet;
+            return;
+        }
+    }
+}
+
 ElementPtr
 SharedNetwork6::toElement() const {
     ElementPtr map = Network6::toElement();

src/lib/dhcpsrv/shared_network.h

@@ -175,6 +175,25 @@ public:
     /// if no better subnet was found.
     Subnet4Ptr getPreferredSubnet(const Subnet4Ptr& selected_subnet) const;
 
+    /// @brief Checks if the shared network includes a subnet with
+    /// the match client ID flag set to true.
+    ///
+    /// @param first_subnet Pointer to the subnet from which iteration starts.
+    /// @param client_classes List of classes that the client belongs to.
+    /// @return true if the shared network includes at least a subnet
+    /// guarded by a given class with the match client ID flag set to true.
+    /// False otherwise.
+    static
+    bool subnetsIncludeMatchClientId(const Subnet4Ptr& first_subnet,
+                                     const ClientClasses& client_classes);
+
+    /// @brief Check if the shared network includes a subnet with
+    /// not global host reservation mode.
+    ///
+    /// @param [out] bad_subnet First subnet which has not a global host
+    /// reservation mode.
+    void subnetsAllHRGlobal(Subnet4Ptr& bad_subnet) const;
+
     /// @brief Unparses shared network object.
     ///
     /// @return A pointer to unparsed shared network configuration.
@@ -365,6 +384,13 @@ public:
     Subnet6Ptr getPreferredSubnet(const Subnet6Ptr& selected_subnet,
                                   const Lease::Type& lease_type) const;
 
+    /// @brief Check if the shared network includes a subnet with
+    /// not global host reservation mode.
+    ///
+    /// @param [out] bad_subnet First subnet which has not a global host
+    /// reservation mode.
+    void subnetsAllHRGlobal(Subnet6Ptr& bad_subnet) const;
+
     /// @brief Unparses shared network object.
     ///
     /// @return A pointer to unparsed shared network configuration.

src/lib/dhcpsrv/tests/shared_network_unittest.cc

@@ -410,6 +410,73 @@ TEST(SharedNetwork4Test, getPreferredSubnet) {
     EXPECT_EQ(subnet3->getID(), preferred->getID());
 }
 
+// This test verifies that subnetsIncludeMatchClientId() works as expected.
+TEST(SharedNetwork4Test, subnetsIncludeMatchClientId) {
+    SharedNetwork4Ptr network(new SharedNetwork4("frog"));
+    ClientClasses classes;
+
+    // Create a subnet and add it to the shared network.
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("10.0.0.0"), 8, 10, 20, 30,
+                                   SubnetID(1)));
+    subnet1->setMatchClientId(false);
+    ASSERT_NO_THROW(network->add(subnet1));
+
+    // The subnet does not match client id.
+    EXPECT_FALSE(SharedNetwork4::subnetsIncludeMatchClientId(subnet1, classes));
+
+    // Create a second subnet and add it.
+    Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.0"), 24, 10, 20, 30,
+                                   SubnetID(2)));
+    ASSERT_NO_THROW(network->add(subnet2));
+
+    // Default is to match client id.
+    EXPECT_TRUE(SharedNetwork4::subnetsIncludeMatchClientId(subnet1, classes));
+
+    // Add a class.
+    classes.insert("class1");
+
+    //The second subnet is not guarded so matches.
+    EXPECT_TRUE(SharedNetwork4::subnetsIncludeMatchClientId(subnet1, classes));
+
+    // Put the second subnet in another class
+    subnet2->allowClientClass("class2");
+    EXPECT_FALSE(SharedNetwork4::subnetsIncludeMatchClientId(subnet1, classes));
+
+    // Put the second subnet in the class.
+    subnet2->allowClientClass("class1");
+    EXPECT_TRUE(SharedNetwork4::subnetsIncludeMatchClientId(subnet1, classes));
+}
+
+// This test verifies that subnetsAllHRGlobal() works as expected.
+TEST(SharedNetwork4Test, subnetsAllHRGlobal) {
+    SharedNetwork4Ptr network(new SharedNetwork4("frog"));
+    Subnet4Ptr bad;
+
+    // Empty shared network is right.
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    EXPECT_FALSE(bad);
+
+    // Create a subnet and add it to the shared network.
+    Subnet4Ptr subnet(new Subnet4(IOAddress("10.0.0.0"), 8, 10, 20, 30,
+                                  SubnetID(1)));
+    ASSERT_NO_THROW(network->add(subnet));
+
+    // Default host reservation mode is ALL.
+    bad.reset();
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    ASSERT_TRUE(bad);
+    EXPECT_EQ(1, bad->getID());
+    EXPECT_EQ("10.0.0.0/8", bad->toText());
+
+    // Set the HR mode to global.
+    subnet->setHostReservationMode(Network::HR_GLOBAL);
+
+    // Now the shared network is all global.
+    bad.reset();
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    EXPECT_FALSE(bad);
+}
+
 // This test verifies operations on the network's relay list
 TEST(SharedNetwork4Test, relayInfoList) {
     SharedNetwork4Ptr network(new SharedNetwork4("frog"));
@@ -968,6 +1035,36 @@ TEST(SharedNetwork6Test, getPreferredSubnet) {
     EXPECT_EQ(subnet3->getID(), preferred->getID());
 }
 
+// This test verifies that subnetsAllHRGlobal() works as expected.
+TEST(SharedNetwork6Test, subnetsAllHRGlobal) {
+    SharedNetwork6Ptr network(new SharedNetwork6("frog"));
+    Subnet6Ptr bad;
+
+    // Empty shared network is right.
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    EXPECT_FALSE(bad);
+
+    // Create a subnet and add it to the shared network.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 64, 10, 20, 30,
+                                  40, SubnetID(1)));
+    ASSERT_NO_THROW(network->add(subnet));
+
+    // Default host reservation mode is ALL.
+    bad.reset();
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    ASSERT_TRUE(bad);
+    EXPECT_EQ(1, bad->getID());
+    EXPECT_EQ("2001:db8:1::/64", bad->toText());
+
+    // Set the HR mode to global.
+    subnet->setHostReservationMode(Network::HR_GLOBAL);
+
+    // Now the shared network is all global.
+    bad.reset();
+    ASSERT_NO_THROW(network->subnetsAllHRGlobal(bad));
+    EXPECT_FALSE(bad);
+}
+
 // This test verifies operations on the network's relay list
 TEST(SharedNetwork6Test, relayInfoList) {
     SharedNetwork6Ptr network(new SharedNetwork6("frog"));