]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1132] addressed review comments
authorWlodek Wencel <wlodek@isc.org>
Fri, 17 Apr 2020 14:17:04 +0000 (16:17 +0200)
committerFrancis Dupont <fdupont@isc.org>
Thu, 23 Apr 2020 13:36:12 +0000 (15:36 +0200)
src/bin/dhcp4/dhcp4_srv.cc
src/bin/dhcp4/dhcp4_srv.h
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
src/bin/dhcp4/tests/inform_unittest.cc
src/bin/dhcp4/tests/shared_network_unittest.cc

index 1a4f68a5bb7f5ca01d40efce982091bc3ec1692d..852432e986f99770d0f18429f62953cb7280376b 100644 (file)
@@ -588,7 +588,7 @@ void Dhcpv4Exchange::evaluateClasses(const Pkt4Ptr& pkt, bool depend_on_known) {
 
 const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
-bool Dhcpv4Srv::Dhcpv4Srv::test_send_responses_to_source_(false);
+bool Dhcpv4Srv::test_send_responses_to_source_(false);
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t server_port, uint16_t client_port,
                      const bool use_bcast, const bool direct_response_desired)
@@ -600,10 +600,8 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t server_port, uint16_t client_port,
 
     const char* env = std::getenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE");
     if (env) {
-        if (strncmp(env, "ENABLED", 7) == 0) {
-            LOG_WARN(dhcp4_logger, DHCP4_TESTING_MODE_SEND_TO_SOURCE_ENABLED);
-            setSendResponsesToSource(true);
-        }
+        LOG_WARN(dhcp4_logger, DHCP4_TESTING_MODE_SEND_TO_SOURCE_ENABLED);
+        test_send_responses_to_source_ = true;
     }
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET)
@@ -2726,11 +2724,6 @@ Dhcpv4Srv::adjustRemoteAddr(Dhcpv4Exchange& ex) {
         } else {
             response->setRemoteAddr(query->getRemoteAddr());
         }
-
-        if (getSendResponsesToSource()) {
-            response->setRemoteAddr(query->getRemoteAddr());
-        }
-
         // Remote address is now set so return.
         return;
     }
@@ -2789,7 +2782,7 @@ Dhcpv4Srv::adjustRemoteAddr(Dhcpv4Exchange& ex) {
 
     }
 
-    if (getSendResponsesToSource()) {
+    if (Dhcpv4Srv::test_send_responses_to_source_) {
         response->setRemoteAddr(query->getRemoteAddr());
     }
 }
index 107d6b4906d332429551f49f24152c6a3a12f954..f6c9353e8552107da2996c7bbed738ad69cc8373 100644 (file)
@@ -425,6 +425,13 @@ public:
     /// of all packets.  Called during reconfigure and shutdown.
     void discardPackets();
 
+    /// @brief returns value of test_send_responses_to_source_
+    ///
+    /// @return bool value of test_send_responses_to_source_
+    static bool getSendResponsesToSource() {
+        return test_send_responses_to_source_;
+    }
+
 protected:
 
     /// @name Functions filtering and sanity-checking received messages.
@@ -741,24 +748,6 @@ public:
     /// will cause the packet to be assigned to class VENDOR_CLASS_FOO.
     static const std::string VENDOR_CLASS_PREFIX;
 
-    /// @brief This function set test_send_responses_to_source_ value
-    ///
-    /// If environment variable KEA_TEST_SEND_RESPONSES_TO_SOURCE will be
-    /// set to ENABLED this function will set value true to
-    /// test_send_responses_to_source_.
-    ///
-    /// @param bool value of test_send_responses_to_source_
-    void setSendResponsesToSource(const bool value) {
-        test_send_responses_to_source_ = value;
-    }
-
-    /// @brief returns value of test_send_responses_to_source_
-    ///
-    /// @return bool value of test_send_responses_to_source_
-    static bool getSendResponsesToSource() {
-        return test_send_responses_to_source_;
-    }
-
 private:
     /// @brief Process Client FQDN %Option sent by a client.
     ///
@@ -1080,8 +1069,8 @@ protected:
 
 private:
 
-    /// @brief value that define is send response to source mode is enabled
-    /// holds ture if it is.
+    /// @brief store value that defines if kea will send responses
+    /// to a source address of incoming packet. Only for testing.
     static bool test_send_responses_to_source_;
 
 public:
index ac9273082bd7c4e23652ab33108ccc622f3262ad..06239f36e12206deb93dc1332d8fb5fec0bc55ba 100644 (file)
@@ -3913,4 +3913,55 @@ TEST_F(Dhcpv4SrvTest, userContext) {
 /// @todo: Implement proper tests for MySQL lease/host database,
 ///        see ticket #4214.
 
+
+TEST_F(Dhcpv4SrvTest, SendToSourceMode) {
+    string config  = "{"
+    "    \"interfaces-config\": {"
+    "        \"interfaces\": [ \"*\" ]"
+    "    },"
+    "    \"valid-lifetime\": 600,"
+    "    \"shared-networks\": ["
+    "        {"
+    "            \"name\": \"frog\","
+    "            \"relay\": {"
+    "                \"ip-address\": \"192.3.5.6\""
+    "            },"
+    "            \"subnet4\": ["
+    "                {"
+    "                    \"subnet\": \"192.0.2.0/26\","
+    "                    \"id\": 10,"
+    "                    \"pools\": ["
+    "                        {"
+    "                            \"pool\": \"192.0.2.63 - 192.0.2.63\""
+    "                        }"
+    "                    ]"
+    "                }"
+    "            ]"
+    "        }"
+    "    ],"
+    "    \"subnet4\": ["
+    "        {"
+    "            \"subnet\": \"192.0.2.64/26\","
+    "            \"id\": 1000,"
+    "            \"relay\": {"
+    "                \"ip-address\": \"192.1.2.3\""
+    "            },"
+    "            \"pools\": ["
+    "                {"
+    "                    \"pool\": \"192.0.2.65 - 192.0.2.65\""
+    "                }"
+    "            ]"
+    "        }"
+    "    ]"
+    "}";
+
+    // Set env variable that put kea into testing mode
+    //setenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE", "ENABLED", 1);
+    Dhcp4Client client1(Dhcp4Client::SELECTING);
+    configure(config, *client1.getServer());
+    // Check if send to source testing mode is enabled
+    EXPECT_TRUE(isc::dhcp::test::NakedDhcpv4Srv::getSendResponsesToSource());
+    unsetenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE");
+}
+
 }  // namespace
index 591d41924614e3539a1869b0792b9bda4a9dc162..1be526b4c3e18fed6c4e324390695f7e51ac729b 100644 (file)
@@ -485,60 +485,4 @@ TEST_F(InformTest, statisticsInform) {
     EXPECT_EQ(5, pkt4_sent->getInteger().first);
 }
 
-// This test checks that the server receiving DHCPINFORM via relay, should
-// unicasts the DHCPACK to the client (ciaddr) but sending to source address
-// because of testing mode enabled with unaltred content.
-TEST_F(InformTest, relayedClientSendToSourceTestingMode) {
-    setenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE", "ENABLED", 1);
-    Dhcp4Client client;
-    // Configure DHCP server.
-    configure(INFORM_CONFIGS[1], *client.getServer());
-    // Message is relayed.
-    EXPECT_TRUE(isc::dhcp::test::NakedDhcpv4Srv::getSendResponsesToSource());
-    client.useRelay();
-    // Request some configuration when DHCPINFORM is sent.
-    client.requestOptions(DHO_LOG_SERVERS, DHO_COOKIE_SERVERS);
-    // Preconfigure the client with the IP address.
-    client.createLease(IOAddress("192.0.2.56"), 600);
-    // Send DHCPINFORM message to the server.
-    ASSERT_NO_THROW(client.doInform());
-    // Make sure that the server responded.
-    ASSERT_TRUE(client.getContext().response_);
-    Pkt4Ptr resp = client.getContext().response_;
-    Pkt4Ptr query = client.getContext().query_;
-    // Make sure that the server has responded with DHCPACK.
-    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
-    // Response should NOT have been unicast to the ciaddr.
-    EXPECT_NE(IOAddress("192.0.2.56"), resp->getLocalAddr());
-    // Destiantion address of response should be equal to source
-    // address of query.
-    EXPECT_EQ(query->getLocalAddr(), resp->getLocalAddr());
-    // The ciaddr should have been copied.
-    EXPECT_EQ(IOAddress("192.0.2.56"), resp->getCiaddr());
-    // Response is unicast to the client, so it must not be relayed.
-    EXPECT_FALSE(resp->isRelayed());
-    EXPECT_EQ(DHCP4_CLIENT_PORT, resp->getLocalPort());
-    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getRemotePort());
-    // Make sure that the server id is present.
-    EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
-    // Make sure that the Routers option has been received.
-    ASSERT_EQ(2, client.config_.routers_.size());
-    EXPECT_EQ("192.0.2.200", client.config_.routers_[0].toText());
-    EXPECT_EQ("192.0.2.201", client.config_.routers_[1].toText());
-    // Make sure that the DNS Servers option has been received.
-    ASSERT_EQ(2, client.config_.dns_servers_.size());
-    EXPECT_EQ("192.0.2.202", client.config_.dns_servers_[0].toText());
-    EXPECT_EQ("192.0.2.203", client.config_.dns_servers_[1].toText());
-    // Make sure that the Quotes Servers option has been received.
-    ASSERT_EQ(2, client.config_.quotes_servers_.size());
-    EXPECT_EQ("10.0.0.202", client.config_.quotes_servers_[0].toText());
-    EXPECT_EQ("10.0.0.203", client.config_.quotes_servers_[1].toText());
-    // Make sure that the Log Servers option has been received.
-    ASSERT_EQ(2, client.config_.log_servers_.size());
-    EXPECT_EQ("10.0.0.200", client.config_.log_servers_[0].toText());
-    EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText());
-
-    unsetenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE");
-}
-
 } // end of anonymous namespace
index 47abaaf09719a9247b62a054e5da77d69689a04c..88a63ecdbfcd87df45dfd9aeeaefc89c835b8d8d 100644 (file)
@@ -2199,18 +2199,6 @@ TEST_F(Dhcpv4SharedNetworkTest, poolInSubnetSelectedByClass) {
     });
 }
 
-// Check if Kea starts with send to source mode enabled
-TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkCheckIfSendToSourceTestingModeEnabled) {
-    // Set env variable that put kea into testing mode
-    setenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE", "ENABLED", 1);
-    Dhcp4Client client1(Dhcp4Client::SELECTING);
-    configure(NETWORKS_CONFIG[1], *client1.getServer());
-    // Check if send to source testing mode is enabled
-    EXPECT_TRUE(isc::dhcp::test::NakedDhcpv4Srv::getSendResponsesToSource());
-
-    unsetenv("KEA_TEST_SEND_RESPONSES_TO_SOURCE");
-}
-
 // Shared network is selected based on giaddr value (relay specified
 // on shared network level, but response is send to source address.
 TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSendToSourceTestingModeEnabled) {