BIND 10 trac3282, updated. c75896605bb0224250ff5a460d4bb7de55e18751 [3282] Use DDNS configuration values in b10-dhcp4
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jan 20 15:35:16 UTC 2014
The branch, trac3282 has been updated
via c75896605bb0224250ff5a460d4bb7de55e18751 (commit)
from 4289cec8c39d8ed8411c1b2718c5d8f63aa3f122 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c75896605bb0224250ff5a460d4bb7de55e18751
Author: Thomas Markwalder <tmark at isc.org>
Date: Mon Jan 20 10:32:32 2014 -0500
[3282] Use DDNS configuration values in b10-dhcp4
Replaced hard-coded constants in dhcp4_srv.cc with methods and
logic in D2ClientMgr to implement behavior changes based on
dhcp-ddns configuration values.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_srv.cc | 206 ++++++-----------
src/bin/dhcp4/tests/fqdn_unittest.cc | 402 ++++++++++++++++++++++++++++------
2 files changed, 394 insertions(+), 214 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 22ab653..4c061fa 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -79,36 +79,6 @@ Dhcp4Hooks Hooks;
namespace isc {
namespace dhcp {
-namespace {
-
-// @todo The following constants describe server's behavior with respect to the
-// DHCPv4 Client FQDN Option sent by a client. They will be removed
-// when DDNS parameters for DHCPv4 are implemented with the ticket #3033.
-
-// @todo Additional configuration parameter which we may consider is the one
-// that controls whether the DHCP server sends the removal NameChangeRequest
-// if it discovers that the entry for the particular client exists or that
-// it always updates the DNS.
-
-// Should server always include the FQDN option in its response, regardless
-// if it has been requested in Parameter Request List Option (Disabled).
-const bool FQDN_ALWAYS_INCLUDE = false;
-// Enable A RR update delegation to the client (Disabled).
-const bool FQDN_ALLOW_CLIENT_UPDATE = false;
-// Globally enable updates (Enabled).
-const bool FQDN_ENABLE_UPDATE = true;
-// Do update, even if client requested no updates with N flag (Disabled).
-const bool FQDN_OVERRIDE_NO_UPDATE = false;
-// Server performs an update when client requested delegation (Enabled).
-const bool FQDN_OVERRIDE_CLIENT_UPDATE = true;
-// The fully qualified domain-name suffix if partial name provided by
-// a client.
-const char* FQDN_PARTIAL_SUFFIX = "example.com";
-// Should server replace the domain-name supplied by the client (Disabled).
-const bool FQDN_REPLACE_CLIENT_NAME = false;
-
-}
-
Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
const bool direct_response_desired)
: shutdown_(true), alloc_engine_(), port_(port),
@@ -580,8 +550,8 @@ Dhcpv4Srv::appendServerID(const Pkt4Ptr& response) {
// The source address for the outbound message should have been set already.
// This is the address that to the best of the server's knowledge will be
// available from the client.
- // @todo: perhaps we should consider some more sophisticated server id
- // generation, but for the current use cases, it should be ok.
+ /// @todo: perhaps we should consider some more sophisticated server id
+ /// generation, but for the current use cases, it should be ok.
response->addOption(OptionPtr(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
response->getLocalAddr()))
);
@@ -650,8 +620,8 @@ Dhcpv4Srv::appendRequestedVendorOptions(const Pkt4Ptr& question, Pkt4Ptr& answer
uint32_t vendor_id = vendor_req->getVendorId();
// Let's try to get ORO within that vendor-option
- /// @todo This is very specific to vendor-id=4491 (Cable Labs). Other vendors
- /// may have different policies.
+ /// @todo This is very specific to vendor-id=4491 (Cable Labs). Other
+ /// vendors may have different policies.
OptionUint8ArrayPtr oro =
boost::dynamic_pointer_cast<OptionUint8Array>(vendor_req->getOption(DOCSIS3_V4_ORO));
@@ -757,68 +727,19 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
// response to a client.
Option4ClientFqdnPtr fqdn_resp(new Option4ClientFqdn(*fqdn));
- // RFC4702, section 4 - set 'NOS' flags to 0.
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_S, 0);
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_O, 0);
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_N, 0);
-
- // Conditions when N flag has to be set to indicate that server will not
- // perform DNS updates:
- // 1. Updates are globally disabled,
- // 2. Client requested no update and server respects it,
- // 3. Client requested that the forward DNS update is delegated to the
- // client but server neither respects requests for forward update
- // delegation nor it is configured to send update on its own when
- // client requested delegation.
- if (!FQDN_ENABLE_UPDATE ||
- (fqdn->getFlag(Option4ClientFqdn::FLAG_N) &&
- !FQDN_OVERRIDE_NO_UPDATE) ||
- (!fqdn->getFlag(Option4ClientFqdn::FLAG_S) &&
- !FQDN_ALLOW_CLIENT_UPDATE && !FQDN_OVERRIDE_CLIENT_UPDATE)) {
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_N, true);
-
- // Conditions when S flag is set to indicate that server will perform DNS
- // update on its own:
- // 1. Client requested that server performs DNS update and DNS updates are
- // globally enabled.
- // 2. Client requested that server delegates forward update to the client
- // but server doesn't respect requests for delegation and it is
- // configured to perform an update on its own when client requested the
- // delegation.
- } else if (fqdn->getFlag(Option4ClientFqdn::FLAG_S) ||
- (!fqdn->getFlag(Option4ClientFqdn::FLAG_S) &&
- !FQDN_ALLOW_CLIENT_UPDATE && FQDN_OVERRIDE_CLIENT_UPDATE)) {
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_S, true);
- }
-
- // Server MUST set the O flag if it has overriden the client's setting
- // of S flag.
- if (fqdn->getFlag(Option4ClientFqdn::FLAG_S) !=
- fqdn_resp->getFlag(Option4ClientFqdn::FLAG_S)) {
- fqdn_resp->setFlag(Option4ClientFqdn::FLAG_O, true);
- }
+ // Set the server S, N, and O flags based on client's flags and
+ // current configuration.
+ D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
+ d2_mgr.adjustFqdnFlags<Option4ClientFqdn>(*fqdn, *fqdn_resp);
- // If client suppled partial or empty domain-name, server should generate
- // one.
- if (fqdn->getDomainNameType() == Option4ClientFqdn::PARTIAL) {
- std::ostringstream name;
- if (fqdn->getDomainName().empty() || FQDN_REPLACE_CLIENT_NAME) {
- fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL);
+ // Carry over the client's E flag.
+ fqdn_resp->setFlag(Option4ClientFqdn::FLAG_E,
+ fqdn->getFlag(Option4ClientFqdn::FLAG_E));
- } else {
- name << fqdn->getDomainName();
- name << "." << FQDN_PARTIAL_SUFFIX;
- fqdn_resp->setDomainName(name.str(), Option4ClientFqdn::FULL);
- }
-
- // Server may be configured to replace a name supplied by a client, even if
- // client supplied fully qualified domain-name. The empty domain-name is
- // is set to indicate that the name must be generated when the new lease
- // is acquired.
- } else if(FQDN_REPLACE_CLIENT_NAME) {
- fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL);
- }
+ // Adjust the domain name based on domain name value and type sent by the
+ // client and current configuration.
+ d2_mgr.adjustDomainName<Option4ClientFqdn>(*fqdn, *fqdn_resp);
// Add FQDN option to the response message. Note that, there may be some
// cases when server may choose not to include the FQDN option in a
@@ -838,15 +759,20 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
void
Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
Pkt4Ptr& answer) {
+ // Fetch D2 configuration.
+ D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
+
// Do nothing if the DNS updates are disabled.
- if (!FQDN_ENABLE_UPDATE) {
+ if (!d2_mgr.ddnsEnabled()) {
return;
}
std::string hostname = isc::util::str::trim(opt_hostname->readString());
unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname);
// The hostname option sent by the client should be at least 1 octet long.
- // If it isn't we ignore this option.
+ // If it isn't we ignore this option. (Per RFC 2131, section 3.14)
+ /// @todo It would be more liberal to accept this and let it fall into
+ /// the case of replace or less than two below.
if (label_count == 0) {
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_EMPTY_HOSTNAME);
return;
@@ -864,21 +790,20 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
// By checking the number of labels present in the hostname we may infer
// whether client has sent the fully qualified or unqualified hostname.
- // @todo We may want to reconsider whether it is appropriate for the
- // client to send a root domain name as a Hostname. There are
- // also extensions to the auto generation of the client's name,
- // e.g. conversion to the puny code which may be considered at some point.
- // For now, we just remain liberal and expect that the DNS will handle
- // conversion if needed and possible.
- if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) {
+ /// @todo We may want to reconsider whether it is appropriate for the
+ /// client to send a root domain name as a Hostname. There are
+ /// also extensions to the auto generation of the client's name,
+ /// e.g. conversion to the puny code which may be considered at some point.
+ /// For now, we just remain liberal and expect that the DNS will handle
+ /// conversion if needed and possible.
+ if ((d2_mgr.getD2ClientConfig()->getReplaceClientName()) ||
+ (label_count < 2)) {
opt_hostname_resp->writeString("");
- // If there are two labels, it means that the client has specified
- // the unqualified name. We have to concatenate the unqalified name
- // with the domain name.
} else if (label_count == 2) {
- std::ostringstream resp_hostname;
- resp_hostname << hostname << "." << FQDN_PARTIAL_SUFFIX << ".";
- opt_hostname_resp->writeString(resp_hostname.str());
+ // If there are two labels, it means that the client has specified
+ // the unqualified name. We have to concatenate the unqalified name
+ // with the domain name.
+ opt_hostname_resp->writeString(d2_mgr.qualifyName(hostname));
}
answer->addOption(opt_hostname_resp);
@@ -969,9 +894,9 @@ queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
void
Dhcpv4Srv::sendNameChangeRequests() {
while (!name_change_reqs_.empty()) {
- // @todo Once next NameChangeRequest is picked from the queue
- // we should send it to the b10-dhcp_ddns module. Currently we
- // just drop it.
+ /// @todo Once next NameChangeRequest is picked from the queue
+ /// we should send it to the b10-dhcp_ddns module. Currently we
+ /// just drop it.
name_change_reqs_.pop();
}
}
@@ -990,8 +915,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// thing this client can get is some global information (like DNS
// servers).
- // perhaps this should be logged on some higher level? This is most likely
- // configuration bug.
+ // perhaps this should be logged on some higher level? This is most
+ // likely configuration bug.
LOG_ERROR(dhcp4_logger, DHCP4_SUBNET_SELECTION_FAILED)
.arg(question->getRemoteAddr().toText())
.arg(serverReceivedPacketName(question->getType()));
@@ -1004,7 +929,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// as siaddr has nothing to do with a lease, but otherwise we would have
// to select subnet twice (performance hit) or update too many functions
// at once.
- // @todo: move subnet selection to a common code
+ /// @todo: move subnet selection to a common code
answer->setSiaddr(subnet->getSiaddr());
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED)
@@ -1047,8 +972,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
(answer->getOption(DHO_HOST_NAME));
if (opt_hostname) {
hostname = opt_hostname->readString();
- // @todo It could be configurable what sort of updates the server
- // is doing when Hostname option was sent.
+ /// @todo It could be configurable what sort of updates the
+ /// server is doing when Hostname option was sent.
fqdn_fwd = true;
fqdn_rev = true;
}
@@ -1058,7 +983,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// will try to honour the hint, but it is just a hint - some other address
// may be used instead. If fake_allocation is set to false, the lease will
// be inserted into the LeaseMgr as well.
- // @todo pass the actual FQDN data.
+ /// @todo pass the actual FQDN data.
Lease4Ptr old_lease;
Lease4Ptr lease = alloc_engine_->allocateLease4(subnet, client_id, hwaddr,
hint, fqdn_fwd, fqdn_rev,
@@ -1083,14 +1008,9 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// generating the entire hostname for the client. The example of the
// client's name, generated from the IP address is: host-192-0-2-3.
if ((fqdn || opt_hostname) && lease->hostname_.empty()) {
- hostname = lease->addr_.toText();
- // Replace dots with hyphens.
- std::replace(hostname.begin(), hostname.end(), '.', '-');
- ostringstream stream;
- // The partial suffix will need to be replaced with the actual
- // domain-name for the client when configuration is implemented.
- stream << "host-" << hostname << "." << FQDN_PARTIAL_SUFFIX << ".";
- lease->hostname_ = stream.str();
+ lease->hostname_ = CfgMgr::instance()
+ .getD2ClientMgr().generateFqdn(lease->addr_);
+
// The operations below are rather safe, but we want to catch
// any potential exceptions (e.g. invalid lease database backend
// implementation) and log an error.
@@ -1122,22 +1042,18 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// Subnet mask (type 1)
answer->addOption(getNetmaskOption(subnet));
- // @todo: send renew timer option (T1, option 58)
- // @todo: send rebind timer option (T2, option 59)
+ /// @todo: send renew timer option (T1, option 58)
+ /// @todo: send rebind timer option (T2, option 59)
- // @todo Currently the NameChangeRequests are always generated if
- // real (not fake) allocation is being performed. Should we have
- // control switch to enable/disable NameChangeRequest creation?
- // Perhaps we need a way to detect whether the b10-dhcp-ddns module
- // is up an running?
- if (!fake_allocation) {
+ // Create NameChangeRequests if DDNS is enabled and this is a
+ // real allocation.
+ if (!fake_allocation && CfgMgr::instance().ddnsEnabled()) {
try {
createNameChangeRequests(lease, old_lease);
} catch (const Exception& ex) {
LOG_ERROR(dhcp4_logger, DHCP4_NCR_CREATION_FAILED)
.arg(ex.what());
}
-
}
} else {
@@ -1178,8 +1094,8 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
// address for the response. Instead, we have to check what address our
// socket is bound to and use it as a source address. This operation
// may throw if for some reason the socket is closed.
- // @todo Consider an optimization that we use local address from
- // the query if this address is not broadcast.
+ /// @todo Consider an optimization that we use local address from
+ /// the query if this address is not broadcast.
SocketInfo sock_info = IfaceMgr::instance().getSocket(*query);
// Set local adddress, port and interface.
response->setLocalAddr(sock_info.addr_);
@@ -1294,7 +1210,7 @@ Pkt4Ptr
Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
/// @todo Uncomment this (see ticket #3116)
- // sanityCheck(request, MANDATORY);
+ /// sanityCheck(request, MANDATORY);
Pkt4Ptr ack = Pkt4Ptr
(new Pkt4(DHCPACK, request->getTransid()));
@@ -1336,7 +1252,7 @@ void
Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
/// @todo Uncomment this (see ticket #3116)
- // sanityCheck(release, MANDATORY);
+ /// sanityCheck(release, MANDATORY);
// Try to find client-id
ClientIdPtr client_id;
@@ -1361,7 +1277,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
// Does the hardware address match? We don't want one client releasing
// second client's leases.
if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
- // @todo: Print hwaddr from lease as part of ticket #2589
+ /// @todo: Print hwaddr from lease as part of ticket #2589
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
.arg(release->getCiaddr().toText())
.arg(client_id ? client_id->toText() : "(no client-id)")
@@ -1419,9 +1335,11 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
.arg(client_id ? client_id->toText() : "(no client-id)")
.arg(release->getHWAddr()->toText());
- // Remove existing DNS entries for the lease, if any.
- queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);
-
+ if (CfgMgr::instance().ddnsEnabled() &&
+ CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew()) {
+ // Remove existing DNS entries for the lease, if any.
+ queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);
+ }
} else {
// Release failed -
LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
@@ -1662,8 +1580,8 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port,
}
// Let's reopen active sockets. openSockets4 will check internally whether
// sockets are marked active or inactive.
- // @todo Optimization: we should not reopen all sockets but rather select
- // those that have been affected by the new configuration.
+ /// @todo Optimization: we should not reopen all sockets but rather select
+ /// those that have been affected by the new configuration.
isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
boost::bind(&Dhcpv4Srv::ifaceMgrSocket4ErrorHandler, _1);
if (!IfaceMgr::instance().openSockets4(port, use_bcast, error_handler)) {
diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index d3bf9ae..73f15a7 100644
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -18,6 +18,7 @@
#include <dhcp/option_int_array.h>
#include <dhcp4/tests/dhcp4_test_utils.h>
#include <dhcp_ddns/ncr_msg.h>
+#include <dhcpsrv/cfgmgr.h>
#include <gtest/gtest.h>
#include <boost/scoped_ptr.hpp>
@@ -32,13 +33,55 @@ namespace {
class NameDhcpv4SrvTest : public Dhcpv4SrvFakeIfaceTest {
public:
+
+ // Bit Constants for turning on and off DDNS configuration options.
+ static const uint16_t REMOVE_ON_RENEW = 1;
+ static const uint16_t ALWAYS_INCLUDE_FQDN = 2;
+ static const uint16_t OVERRIDE_NO_UPDATE = 4;
+ static const uint16_t OVERRIDE_CLIENT_UPDATE = 8;
+ static const uint16_t REPLACE_CLIENT_NAME = 16;
+
NameDhcpv4SrvTest() : Dhcpv4SrvFakeIfaceTest() {
srv_ = new NakedDhcpv4Srv(0);
+ // Config DDNS to be enabled, all controls off
+ enableD2();
}
+
virtual ~NameDhcpv4SrvTest() {
delete srv_;
}
+ /// @brief Sets the server's DDNS configuration to ddns updates disabled.
+ void disableD2() {
+ // Default constructor creates a config with DHCP-DDNS updates
+ // disabled.
+ D2ClientConfigPtr cfg(new D2ClientConfig());
+ CfgMgr::instance().setD2ClientConfig(cfg);
+ }
+
+ /// @brief Enables DHCP-DDNS updates with the given options enabled.
+ ///
+ /// Replaces the current D2ClientConfiguration with a configuration
+ /// which as updates enabled and the control options set based upon
+ /// the bit mask of options.
+ ///
+ /// @param mask Bit mask of configuration options that should be enabled.
+ void enableD2(const uint16_t mask = 0) {
+ D2ClientConfigPtr cfg;
+
+ ASSERT_NO_THROW(cfg.reset(new D2ClientConfig(true,
+ isc::asiolink::IOAddress("192.0.2.1"), 477,
+ dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON,
+ (mask & REMOVE_ON_RENEW),
+ (mask & ALWAYS_INCLUDE_FQDN),
+ (mask & OVERRIDE_NO_UPDATE),
+ (mask & OVERRIDE_CLIENT_UPDATE),
+ (mask & REPLACE_CLIENT_NAME),
+ "myhost", "example.com")));
+
+ CfgMgr::instance().setD2ClientConfig(cfg);
+ }
+
// Create a lease to be used by various tests.
Lease4Ptr createLease(const isc::asiolink::IOAddress& addr,
const std::string& hostname,
@@ -78,15 +121,18 @@ public:
return (opt_hostname);
}
- // Generates partial hostname from the address. The format of the
- // generated address is: host-A-B-C-D, where A.B.C.D is an IP
- // address.
+ /// @brief Convenience method for generating an FQDN from an IP address.
+ ///
+ /// This is just a wrapper method around the D2ClientMgr's method for
+ /// generating domain names from the configured prefix, suffix, and a
+ /// given IP address. This is useful for verifying that fully generated
+ /// names are correct.
+ ///
+ /// @param addr IP address used in the lease.
+ ///
+ /// @return An std::string contained the generated FQDN.
std::string generatedNameFromAddress(const IOAddress& addr) {
- std::string gen_name = addr.toText();
- std::replace(gen_name.begin(), gen_name.end(), '.', '-');
- std::ostringstream hostname;
- hostname << "host-" << gen_name;
- return (hostname.str());
+ return(CfgMgr::instance().getD2ClientMgr().generateFqdn(addr));
}
// Get the Client FQDN Option from the given message.
@@ -182,6 +228,21 @@ public:
Option4ClientFqdnPtr fqdn = getClientFqdnOption(answer);
ASSERT_TRUE(fqdn);
+ checkFqdnFlags(answer, exp_flags);
+
+ EXPECT_EQ(exp_domain_name, fqdn->getDomainName());
+ EXPECT_EQ(exp_domain_type, fqdn->getDomainNameType());
+
+ }
+
+ /// @brief Checks the packet's FQDN option flags against a given mask
+ ///
+ /// @param pkt IPv4 packet whose FQDN flags should be checked.
+ /// @param exp_flags Bit mask of flags that are expected to be true.
+ void checkFqdnFlags(const Pkt4Ptr& pkt, const uint8_t exp_flags) {
+ Option4ClientFqdnPtr fqdn = getClientFqdnOption(pkt);
+ ASSERT_TRUE(fqdn);
+
const bool flag_n = (exp_flags & Option4ClientFqdn::FLAG_N) != 0;
const bool flag_s = (exp_flags & Option4ClientFqdn::FLAG_S) != 0;
const bool flag_o = (exp_flags & Option4ClientFqdn::FLAG_O) != 0;
@@ -191,12 +252,9 @@ public:
EXPECT_EQ(flag_s, fqdn->getFlag(Option4ClientFqdn::FLAG_S));
EXPECT_EQ(flag_o, fqdn->getFlag(Option4ClientFqdn::FLAG_O));
EXPECT_EQ(flag_e, fqdn->getFlag(Option4ClientFqdn::FLAG_E));
-
- EXPECT_EQ(exp_domain_name, fqdn->getDomainName());
- EXPECT_EQ(exp_domain_type, fqdn->getDomainNameType());
-
}
+
// Processes the Hostname option in the client's message and returns
// the hostname option which would be sent to the client. It will
// throw NULL pointer if the hostname option is not to be included
@@ -221,8 +279,8 @@ public:
}
- // Test that the client message holding an FQDN is processed and the
- // NameChangeRequests are generated.
+ // Test that the client message holding an FQDN is processed and
+ // that the response packet is as expected.
void testProcessMessageWithFqdn(const uint8_t msg_type,
const std::string& hostname) {
Pkt4Ptr req = generatePktWithFqdn(msg_type, Option4ClientFqdn::FLAG_S |
@@ -287,6 +345,50 @@ public:
srv_->name_change_reqs_.pop();
}
+
+ /// @brief Tests processing a request with the given client flags
+ ///
+ /// This method creates a request with its FQDN flags set to the given
+ /// value and submits it to the server for processing. It then checks
+ /// the following:
+ /// 1. Did the server generate an ACK with the correct FQDN flags
+ /// 2. If the server should have generated an NCR, did it? and If
+ /// so was it correct?
+ ///
+ /// @param client_flags Mask of client FQDN flags which are true
+ /// @param response_flags Mask of expected FQDN flags in the response
+ void flagVsConfigScenario(const uint8_t client_flags,
+ const uint8_t response_flags) {
+ Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, client_flags,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+
+ // Process the request.
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+ // Verify the response and flags.
+ checkResponse(reply, DHCPACK, 1234);
+ checkFqdnFlags(reply, response_flags);
+
+ // There should be an NCR only if response S flag is 1.
+ /// @todo This logic will need to change if forward and reverse
+ /// updates are ever controlled independently.
+ if ((response_flags & Option4ClientFqdn::FLAG_S) == 0) {
+ ASSERT_EQ(0, srv_->name_change_reqs_.size());
+ } else {
+ // Verify that there is one NameChangeRequest generated as expected.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(),
+ "myhost.example.com.",
+ "", // empty DHCID means don't check it
+ time(NULL) + subnet_->getValid(),
+ subnet_->getValid(), true);
+ }
+ }
+
+
NakedDhcpv4Srv* srv_;
};
@@ -348,21 +450,101 @@ TEST_F(NameDhcpv4SrvTest, dhcidComputeFromHWAddr) {
EXPECT_EQ(dhcid_ref, dhcid.toStr());
}
+// Tests the following scenario:
+// - Updates are enabled
+// - All overrides are off
+// - Client requests forward update (N = 0, S = 1)
+//
+// Server should perform the update:
+// - Reponse flags should N = 0, S = 1, O = 0
+// - Should queue an NCR
+TEST_F(NameDhcpv4SrvTest, updatesEnabled) {
+ flagVsConfigScenario((Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_S),
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_S));
+}
-// Test that server confirms to perform the forward and reverse DNS update,
-// when client asks for it.
-TEST_F(NameDhcpv4SrvTest, serverUpdateForwardFqdn) {
- Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
- Option4ClientFqdn::FLAG_E |
- Option4ClientFqdn::FLAG_S,
- "myhost.example.com.",
- Option4ClientFqdn::FULL,
- true);
+// Tests the following scenario
+// - Updates are disabled
+// - Client requests forward update (N = 0, S = 1)
+//
+// Server should NOT perform updates:
+// - Response flags should N = 1, S = 0, O = 1
+// - Should not queue any NCRs
+TEST_F(NameDhcpv4SrvTest, updatesDisabled) {
+ disableD2();
+ flagVsConfigScenario((Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_S),
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_N |
+ Option4ClientFqdn::FLAG_O));
+}
- testProcessFqdn(query,
- Option4ClientFqdn::FLAG_E | Option4ClientFqdn::FLAG_S,
- "myhost.example.com.");
+// Tests the following scenario:
+// - Updates are enabled
+// - All overrides are off.
+// - Client requests no updates (N = 1, S = 0)
+//
+// Server should NOT perform updates:
+// - Response flags should N = 1, S = 0, O = 0
+// - Should not queue any NCRs
+TEST_F(NameDhcpv4SrvTest, respectNoUpdate) {
+ flagVsConfigScenario((Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_N),
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_N));
+}
+// Tests the following scenario:
+// - Updates are enabled
+// - override-no-update is on
+// - Client requests no updates (N = 1, S = 0)
+//
+// Server should override "no update" request and perform updates:
+// - Response flags should be N = 0, S = 1, O = 1
+// - Should queue an NCR
+TEST_F(NameDhcpv4SrvTest, overrideNoUpdate) {
+ enableD2(OVERRIDE_NO_UPDATE);
+ flagVsConfigScenario((Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_N),
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_O));
+}
+
+// Tests the following scenario:
+// - Updates are enabled
+// - All overrides are off.
+// - Client requests delegation (N = 0, S = 0)
+//
+// Server should respect client's delegation request and NOT do updates:
+
+// - Response flags should be N = 1, S = 0, O = 0
+// - Should not queue any NCRs
+TEST_F(NameDhcpv4SrvTest, respectClientDelegation) {
+
+ flagVsConfigScenario(Option4ClientFqdn::FLAG_E,
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_N));
+}
+
+// Tests the following scenario:
+// - Updates are enabled
+// - override-client-update is on.
+// - Client requests delegation (N = 0, S = 0)
+//
+// Server should override client's delegation request and do updates:
+// - Response flags should be N = 0, S = 1, O = 1
+// - Should queue an NCR
+TEST_F(NameDhcpv4SrvTest, overrideClientDelegation) {
+ // Turn on override-client-update.
+ enableD2(OVERRIDE_CLIENT_UPDATE);
+
+ flagVsConfigScenario(Option4ClientFqdn::FLAG_E,
+ (Option4ClientFqdn::FLAG_E |
+ Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_O));
}
// Test that server processes the Hostname option sent by a client and
@@ -447,34 +629,6 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) {
}
-// Test server's response when client requests no DNS update.
-TEST_F(NameDhcpv4SrvTest, noUpdateFqdn) {
- Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
- Option4ClientFqdn::FLAG_E |
- Option4ClientFqdn::FLAG_N,
- "myhost.example.com.",
- Option4ClientFqdn::FULL,
- true);
- testProcessFqdn(query, Option4ClientFqdn::FLAG_E |
- Option4ClientFqdn::FLAG_N,
- "myhost.example.com.");
-}
-
-// Test that server does not accept delegation of the forward DNS update
-// to a client.
-TEST_F(NameDhcpv4SrvTest, clientUpdateNotAllowedFqdn) {
- Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
- Option4ClientFqdn::FLAG_E,
- "myhost.example.com.",
- Option4ClientFqdn::FULL,
- true);
-
- testProcessFqdn(query, Option4ClientFqdn::FLAG_E |
- Option4ClientFqdn::FLAG_S | Option4ClientFqdn::FLAG_O,
- "myhost.example.com.");
-
-}
-
// Test that exactly one NameChangeRequest is generated when the new lease
// has been acquired (old lease is NULL).
TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNewLease) {
@@ -600,18 +754,42 @@ TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) {
// Verify that there is one NameChangeRequest generated.
ASSERT_EQ(1, srv_->name_change_reqs_.size());
+
// The hostname is generated from the IP address acquired (yiaddr).
- std::ostringstream hostname;
- hostname << generatedNameFromAddress(reply->getYiaddr())
- << ".example.com.";
+ std::string hostname = generatedNameFromAddress(reply->getYiaddr());
+
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
- reply->getYiaddr().toText(), hostname.str(),
+ reply->getYiaddr().toText(), hostname,
"", // empty DHCID forces that it is not checked
time(NULL) + subnet_->getValid(),
subnet_->getValid(), true);
}
// Test that server generates client's hostname from the IP address assigned
+// to it when DHCPv4 Client FQDN option specifies an empty domain-name AND
+// ddns updates are disabled.
+TEST_F(NameDhcpv4SrvTest, processRequestEmptyDomainNameDisabled) {
+ disableD2();
+ Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "", Option4ClientFqdn::PARTIAL, true);
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ Option4ClientFqdnPtr fqdn = getClientFqdnOption(reply);
+ ASSERT_TRUE(fqdn);
+
+ // The hostname is generated from the IP address acquired (yiaddr).
+ std::string hostname = generatedNameFromAddress(reply->getYiaddr());
+
+ EXPECT_EQ(hostname, fqdn->getDomainName());
+ EXPECT_EQ(Option4ClientFqdn::FULL, fqdn->getDomainNameType());
+}
+
+
+// Test that server generates client's hostname from the IP address assigned
// to it when Hostname option carries the top level domain-name.
TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
Pkt4Ptr req = generatePktWithHostname(DHCPREQUEST, ".");
@@ -626,11 +804,12 @@ TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
// Verify that there is one NameChangeRequest generated.
ASSERT_EQ(1, srv_->name_change_reqs_.size());
+
// The hostname is generated from the IP address acquired (yiaddr).
- std::ostringstream hostname;
- hostname << generatedNameFromAddress(reply->getYiaddr()) << ".example.com.";
+ std::string hostname = generatedNameFromAddress(reply->getYiaddr());
+
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
- reply->getYiaddr().toText(), hostname.str(),
+ reply->getYiaddr().toText(), hostname,
"", // empty DHCID forces that it is not checked
time(NULL), subnet_->getValid(), true);
}
@@ -742,21 +921,26 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
time(NULL), subnet_->getValid(), true);
}
-// Test that when the Release message is sent for the previously acquired
-// lease, then server genenerates a NameChangeRequest to remove the entries
-// corresponding to the lease being released.
-TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
+// Test that when a release message is sent for a previously acquired lease,
+// DDNS updates are enabled and remove-on-renew is true that the server
+// genenerates a NameChangeRequest to remove entries corresponding to the
+// released lease.
+TEST_F(NameDhcpv4SrvTest, processRequestReleaseRemoveOn) {
+ // Enable ddns updates and remove-on-renew .
+ enableD2(REMOVE_ON_RENEW);
+ ASSERT_TRUE(CfgMgr::instance().ddnsEnabled());
+ ASSERT_TRUE(CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew());
+
+ // Create and process a lease request so we have a lease to release.
Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
Option4ClientFqdn::FLAG_E,
"myhost.example.com.",
Option4ClientFqdn::FULL, true);
-
Pkt4Ptr reply;
ASSERT_NO_THROW(reply = srv_->processRequest(req));
-
checkResponse(reply, DHCPACK, 1234);
- // Verify that there is one NameChangeRequest generated.
+ // Verify that there is one NameChangeRequest generated for lease.
ASSERT_EQ(1, srv_->name_change_reqs_.size());
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
reply->getYiaddr().toText(), "myhost.example.com.",
@@ -764,18 +948,96 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
"965B68B6D438D98E680BF10B09F3BCF",
time(NULL), subnet_->getValid(), true);
- // Create a Release message.
+ // Create and process the Release message.
Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
rel->setCiaddr(reply->getYiaddr());
rel->setRemoteAddr(IOAddress("192.0.2.3"));
rel->addOption(generateClientId());
rel->addOption(srv_->getServerID());
-
ASSERT_NO_THROW(srv_->processRelease(rel));
// The lease has been removed, so there should be a NameChangeRequest to
// remove corresponding DNS entries.
ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
+ reply->getYiaddr().toText(), "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ time(NULL), subnet_->getValid(), true);
}
+// Test that when a Release message is sent for a previously acquired
+// lease and DDNS updates are enabled but remove-on-renew is off that server
+// does NOT generate NameChangeRequest to remove entries corresponding to
+// the released lease.
+TEST_F(NameDhcpv4SrvTest, processRequestReleaseRemoveOff) {
+ // Verify the config is as expected.
+ ASSERT_TRUE(CfgMgr::instance().ddnsEnabled());
+ ASSERT_FALSE(CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew());
+
+ // Create and process a lease request so we have a lease to release.
+ Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+ checkResponse(reply, DHCPACK, 1234);
+
+ // Verify that there is one NameChangeRequest generated for the new lease.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(), "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ time(NULL), subnet_->getValid(), true);
+
+ // Create and process the Release message.
+ Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
+ rel->setCiaddr(reply->getYiaddr());
+ rel->setRemoteAddr(IOAddress("192.0.2.3"));
+ rel->addOption(generateClientId());
+ rel->addOption(srv_->getServerID());
+ ASSERT_NO_THROW(srv_->processRelease(rel));
+
+ // With remove-on-renew off, there should be not be a NameChangeRequest
+ // for the remove.
+ ASSERT_EQ(0, srv_->name_change_reqs_.size());
+}
+
+// Test that when the Release message is sent for a previously acquired lease
+// and DDNS updates are disabled that server does NOT generate a
+// NameChangeRequest to remove entries corresponding to the released lease.
+TEST_F(NameDhcpv4SrvTest, processRequestReleaseUpdatesDisabled) {
+ // Disable DDNS.
+ disableD2();
+ ASSERT_FALSE(CfgMgr::instance().ddnsEnabled());
+
+ // Create and process a lease request so we have a lease to release.
+ Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+ checkResponse(reply, DHCPACK, 1234);
+
+ // With DDNS updates disabled, there should be not be a NameChangeRequest
+ // for the add.
+ ASSERT_EQ(0, srv_->name_change_reqs_.size());
+
+ // Create and process the Release message.
+ Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
+ rel->setCiaddr(reply->getYiaddr());
+ rel->setRemoteAddr(IOAddress("192.0.2.3"));
+ rel->addOption(generateClientId());
+ rel->addOption(srv_->getServerID());
+ ASSERT_NO_THROW(srv_->processRelease(rel));
+
+ // With DDNS updates disabled, there should be not be a NameChangeRequest
+ // for the remove.
+ ASSERT_EQ(0, srv_->name_change_reqs_.size());
+}
+
+
} // end of anonymous namespace
More information about the bind10-changes
mailing list