[3367] Implemented unit test for checking ciaddr.

src/bin/dhcp4/tests/dhcp4_client.cc

@@ -45,6 +45,7 @@ Dhcp4Client::Configuration::reset() {
 
 Dhcp4Client::Dhcp4Client(const Dhcp4Client::State& state) :
     config_(),
+    ciaddr_(IOAddress("0.0.0.0")),
     curr_transid_(0),
     dest_addr_("255.255.255.255"),
     hwaddr_(generateHWAddr()),
@@ -59,6 +60,7 @@ Dhcp4Client::Dhcp4Client(const Dhcp4Client::State& state) :
 Dhcp4Client::Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv>& srv,
                          const Dhcp4Client::State& state) :
     config_(),
+    ciaddr_(IOAddress("0.0.0.0")),
     curr_transid_(0),
     dest_addr_("255.255.255.255"),
     hwaddr_(generateHWAddr()),
@@ -151,6 +153,10 @@ Dhcp4Client::doDiscover(const boost::shared_ptr<IOAddress>& requested_addr) {
     if (requested_addr) {
         addRequestedAddress(*requested_addr);
     }
+    // Override the default ciaddr if specified by a test.
+    if (ciaddr_.isSpecified()) {
+        context_.query_->setCiaddr(ciaddr_.get());
+    }
     // Send the message to the server.
     sendMsg(context_.query_);
     // Expect response.
@@ -195,8 +201,10 @@ void
 Dhcp4Client::doRequest() {
     context_.query_ = createMsg(DHCPREQUEST);
 
-    // Set ciaddr.
-    if ((state_ == SELECTING) || (state_ == INIT_REBOOT)) {
+    // Override the default ciaddr if specified by a test.
+    if (ciaddr_.isSpecified()) {
+        context_.query_->setCiaddr(ciaddr_.get());
+    } else if ((state_ == SELECTING) || (state_ == INIT_REBOOT)) {
         context_.query_->setCiaddr(IOAddress("0.0.0.0"));
     } else {
         context_.query_->setCiaddr(IOAddress(config_.lease_.addr_));

src/bin/dhcp4/tests/dhcp4_client.h

@@ -34,6 +34,73 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Simple class representing an optional value.
+///
+/// This class encapsulates a value of any type. An additional flag held
+/// by this class indicates if the value is specified, i.e. should be used
+/// by the @c Dhcp4Client class. This is used in cases when the caller
+/// needs to override some values used by the client. For example, the
+/// client sets ciaddr according to the RFC2131, depending on the client's
+/// state and using available lease information. However, a test may need
+/// to override the value being used by the client to test some negative
+/// scenarios (e.g. invalid ciaddr). To do this, the test needs to set
+/// the optional value to the desired value and mark it specified. If the
+/// @c Dhcp4Client finds that the value is specified, it will use this
+/// value in the outgoing message. Otherwise, it will disregard the
+/// value and use the defaults.
+///
+/// @tparam Type of the encapsulated value.
+template<typename T>
+class OptionalValue {
+public:
+
+    /// @brief Constructor
+    ///
+    /// Creates optional value. The value defaults to "unspecified".
+    OptionalValue(const T& value)
+        : value_(value),
+          specified_(false) {
+    }
+
+    /// @brief Retrieves the actual value.
+    T get() const {
+        return (value_);
+    }
+
+    /// @brief Sets the actual value.
+    ///
+    /// @param value New value.
+    void set(const T& value) {
+        value_ = value;
+    }
+
+    /// @brief Sets the new value and marks it specified.
+    ///
+    /// @param value New actual value.
+    void specify(const T& value) {
+        set(value);
+        specify(true);
+    }
+
+    /// @brief Sets the value to "specified".
+    ///
+    /// It does not alter the actual value. It only marks it "specified".
+    void specify(const bool specified) {
+        specified_ = specified;
+    }
+
+    /// @brief Checks if the value is specified or unspecified.
+    ///
+    /// @return true if the value is specified, false otherwise.
+    bool isSpecified() const {
+        return (specified_);
+    }
+
+private:
+    T value_;         ///< Encapsulated value.
+    bool specified_;  ///< Flag which indicates if the value is specified.
+};
+
 /// @brief DHCPv4 client used for unit testing.
 ///
 /// This class implements a DHCPv4 "client" which interoperates with the
@@ -294,6 +361,13 @@ public:
     /// @brief Current client's configuration obtained from the server.
     Configuration config_;
 
+    /// @brief Specific ciaddr to be used in client's messages.
+    ///
+    /// If this value is "unspecified" the default values will be used
+    /// by the client. If this value is specified, it will override ciaddr
+    /// in the client's messages.
+    OptionalValue<asiolink::IOAddress> ciaddr_;
+
 private:
 
     /// @brief Creates and adds Requested IP Address option to the client's
@@ -373,6 +447,7 @@ private:
 
     /// @brief Enable relaying messages to the server.
     bool use_relay_;
+
 };
 
 } // end of namespace isc::dhcp::test

src/bin/dhcp4/tests/dora_unittest.cc

@@ -302,7 +302,7 @@ TEST_F(DORATest, selectingRequestNonMatchingAddress) {
 // Test that the client in the INIT-REBOOT state can request the IP
 // address it has and the address is returned. Also, check that if
 // if the client requests in valid address the server sends a DHCPNAK.
-TEST_F(DORATest, InitRebootRequest) {
+TEST_F(DORATest, initRebootRequest) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
     // Configure DHCP server.
     configure(DORA_CONFIGS[0], *client.getServer());
@@ -349,4 +349,57 @@ TEST_F(DORATest, InitRebootRequest) {
     EXPECT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
 }
 
+// Check that the ciaddr returned by the server is correct for DHCPOFFER and
+// DHCPNAK according to RFC2131, section 4.3.1.
+TEST_F(DORATest, ciaddr) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+    // Force ciaddr of Discover message to be non-zero.
+    client.ciaddr_.specify(IOAddress("10.0.0.50"));
+    // Obtain a lease from the server using the 4-way exchange.
+    ASSERT_NO_THROW(client.doDiscover(boost::shared_ptr<
+                                      IOAddress>(new IOAddress("10.0.0.50"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPOFFER.
+    ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
+    // Make sure ciaddr is not set for DHCPOFFER.
+    EXPECT_EQ("0.0.0.0", resp->getCiaddr().toText());
+
+    // Obtain a lease from the server using the 4-way exchange.
+    ASSERT_NO_THROW(client.doRequest());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+
+    // Let's transition the client to Renewing state.
+    client.setState(Dhcp4Client::RENEWING);
+
+    // Set the unicast destination address to indicate that it is a renewal.
+    client.setDestAddress(IOAddress("10.0.0.1"));
+    ASSERT_NO_THROW(client.doRequest());
+    // The client is sending invalid ciaddr so the server should send a NAK.
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // For DHCPACK the ciaddr may be 0 or may be set to the ciaddr value
+    // from the client's message. Kea sets it to the latter.
+    EXPECT_EQ("10.0.0.50", resp->getCiaddr().toText());
+
+    // Replace the address held by the client. The client will request
+    // the assignment of this address but the server has a different
+    // address for this client.
+    client.ciaddr_.specify(IOAddress("192.168.0.30"));
+    ASSERT_NO_THROW(client.doRequest());
+    // The client is sending invalid ciaddr so the server should send a NAK.
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+    // For DHCPNAK the ciaddr is always 0 (should not be copied) from the
+    // client's message.
+    EXPECT_EQ("0.0.0.0", resp->getCiaddr().toText());
+}
+
 } // end of anonymous namespace