Commit f7ee3792 authored by Francis Dupont's avatar Francis Dupont
Browse files

[5533a] Addressed comments

parent 019d977b
......@@ -13,6 +13,7 @@
#include <dhcpsrv/cfg_option.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcp4/json_config_parser.h>
#include <dhcpsrv/db_type.h>
#include <dhcpsrv/parsers/client_class_def_parser.h>
#include <dhcpsrv/parsers/dbaccess_parser.h>
#include <dhcpsrv/parsers/dhcp_parsers.h>
......@@ -412,14 +413,14 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
// Please move at the end when migration will be finished.
if (config_pair.first == "lease-database") {
DbAccessParser parser(CfgDbAccess::LEASE_DB);
DbAccessParser parser(DBType::LEASE_DB);
CfgDbAccessPtr cfg_db_access = srv_cfg->getCfgDbAccess();
parser.parse(cfg_db_access, config_pair.second);
continue;
}
if (config_pair.first == "hosts-database") {
DbAccessParser parser(CfgDbAccess::HOSTS_DB);
DbAccessParser parser(DBType::HOSTS_DB);
CfgDbAccessPtr cfg_db_access = srv_cfg->getCfgDbAccess();
parser.parse(cfg_db_access, config_pair.second);
continue;
......@@ -427,9 +428,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
if (config_pair.first == "hosts-databases") {
CfgDbAccessPtr cfg_db_access = srv_cfg->getCfgDbAccess();
for (size_t i = 0; i < config_pair.second->size(); ++i) {
DbAccessParser parser(CfgDbAccess::HOSTS_DB + i);
parser.parse(cfg_db_access, config_pair.second->get(i));
DbAccessParser parser(DBType::HOSTS_DB);
auto list = config_pair.second->listValue();
for (auto it : list) {
parser.parse(cfg_db_access, it);
}
continue;
}
......
......@@ -5790,11 +5790,11 @@ TEST_F(Dhcp4ParserTest, hostsDatabases) {
ConstCfgDbAccessPtr cfgdb =
CfgMgr::instance().getStagingCfg()->getCfgDbAccess();
ASSERT_TRUE(cfgdb);
const std::vector<std::string>& hal = cfgdb->getHostDbAccessStringList();
const std::list<std::string>& hal = cfgdb->getHostDbAccessStringList();
ASSERT_EQ(2, hal.size());
// Keywords are in alphabetical order
EXPECT_EQ("name=keatest1 password=keatest type=mysql user=keatest", hal[0]);
EXPECT_EQ("name=keatest2 password=keatest type=mysql user=keatest", hal[1]);
EXPECT_EQ("name=keatest1 password=keatest type=mysql user=keatest", hal.front());
EXPECT_EQ("name=keatest2 password=keatest type=mysql user=keatest", hal.back());
}
// This test checks comments. Please keep it last.
......
......@@ -16,6 +16,7 @@
#include <dhcp/iface_mgr.h>
#include <dhcpsrv/cfg_option.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/db_type.h>
#include <dhcpsrv/pool.h>
#include <dhcpsrv/subnet.h>
#include <dhcpsrv/timer_mgr.h>
......@@ -525,25 +526,25 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
// Please move at the end when migration will be finished.
if (config_pair.first == "lease-database") {
DbAccessParser parser(CfgDbAccess::LEASE_DB);
DbAccessParser parser(DBType::LEASE_DB);
CfgDbAccessPtr cfg_db_access = srv_config->getCfgDbAccess();
parser.parse(cfg_db_access, config_pair.second);
continue;
}
if (config_pair.first == "hosts-database") {
DbAccessParser parser(CfgDbAccess::HOSTS_DB);
DbAccessParser parser(DBType::HOSTS_DB);
CfgDbAccessPtr cfg_db_access = srv_config->getCfgDbAccess();
parser.parse(cfg_db_access, config_pair.second);
continue;
}
// For now only support empty or singleton, ignoring extra entries.
if (config_pair.first == "hosts-databases") {
CfgDbAccessPtr cfg_db_access = srv_config->getCfgDbAccess();
for (size_t i = 0; i < config_pair.second->size(); ++i) {
DbAccessParser parser(CfgDbAccess::HOSTS_DB + i);
parser.parse(cfg_db_access, config_pair.second->get(i));
DbAccessParser parser(DBType::HOSTS_DB);
auto list = config_pair.second->listValue();
for (auto it : list) {
parser.parse(cfg_db_access, it);
}
continue;
}
......
......@@ -6348,11 +6348,11 @@ TEST_F(Dhcp6ParserTest, hostsDatabases) {
ConstCfgDbAccessPtr cfgdb =
CfgMgr::instance().getStagingCfg()->getCfgDbAccess();
ASSERT_TRUE(cfgdb);
const std::vector<std::string>& hal = cfgdb->getHostDbAccessStringList();
const std::list<std::string>& hal = cfgdb->getHostDbAccessStringList();
ASSERT_EQ(2, hal.size());
// Keywords are in alphabetical order
EXPECT_EQ("name=keatest1 password=keatest type=mysql user=keatest", hal[0]);
EXPECT_EQ("name=keatest2 password=keatest type=mysql user=keatest", hal[1]);
EXPECT_EQ("name=keatest1 password=keatest type=mysql user=keatest", hal.front());
EXPECT_EQ("name=keatest2 password=keatest type=mysql user=keatest", hal.back());
}
// This test checks comments. Please keep it last.
......
......@@ -116,6 +116,7 @@ libkea_dhcpsrv_la_SOURCES += daemon.cc daemon.h
libkea_dhcpsrv_la_SOURCES += database_connection.cc database_connection.h
libkea_dhcpsrv_la_SOURCES += db_exceptions.h
libkea_dhcpsrv_la_SOURCES += db_log.cc db_log.h
libkea_dhcpsrv_la_SOURCES += db_type.h
libkea_dhcpsrv_la_SOURCES += dhcp4o6_ipc.cc dhcp4o6_ipc.h
libkea_dhcpsrv_la_SOURCES += dhcpsrv_log.cc dhcpsrv_log.h
libkea_dhcpsrv_la_SOURCES += dhcpsrv_db_log.cc dhcpsrv_db_log.h
......@@ -275,6 +276,7 @@ libkea_dhcpsrv_include_HEADERS = \
daemon.h \
database_connection.h \
db_exceptions.h \
db_type.h \
dhcp4o6_ipc.h \
dhcpsrv_log.h \
host.h \
......
......@@ -6,6 +6,7 @@
#include <config.h>
#include <dhcpsrv/cfg_db_access.h>
#include <dhcpsrv/db_type.h>
#include <dhcpsrv/host_data_source_factory.h>
#include <dhcpsrv/host_mgr.h>
#include <dhcpsrv/lease_mgr_factory.h>
......@@ -21,27 +22,31 @@ namespace isc {
namespace dhcp {
CfgDbAccess::CfgDbAccess()
: appended_parameters_(), db_access_(2) {
db_access_[LEASE_DB] = "type=memfile";
: appended_parameters_(), lease_db_access_("type=memfile"),
host_db_access_() {
}
std::string
CfgDbAccess::getLeaseDbAccessString() const {
return (getAccessString(db_access_[LEASE_DB]));
return (getAccessString(lease_db_access_));
}
std::string
CfgDbAccess::getHostDbAccessString() const {
return (getAccessString(db_access_[HOSTS_DB]));
if (host_db_access_.empty()) {
return ("");
} else {
return (getAccessString(host_db_access_.front()));
}
}
std::vector<std::string>
std::list<std::string>
CfgDbAccess::getHostDbAccessStringList() const {
std::vector<std::string> ret;
for (size_t idx = HOSTS_DB; idx < db_access_.size(); ++idx) {
if (!db_access_[idx].empty()) {
ret.push_back(getAccessString(db_access_[idx]));
std::list<std::string> ret;
for (const std::string& dbaccess : host_db_access_) {
if (!dbaccess.empty()) {
ret.push_back(getAccessString(dbaccess));
}
}
return (ret);
......@@ -55,14 +60,13 @@ CfgDbAccess::createManagers() const {
// Recreate host data source.
HostMgr::create();
auto host_db_access_list = getHostDbAccessStringList();
for (auto it = host_db_access_list.begin();
it != host_db_access_list.end(); ++it) {
HostMgr::addSource(*it);
std::list<std::string> host_db_access_list = getHostDbAccessStringList();
for (std::string& hds : host_db_access_list) {
HostMgr::addBackend(hds);
}
// Check for a host cache.
HostMgr::checkCacheSource(true);
HostMgr::checkCacheBackend(true);
}
std::string
......
......@@ -10,7 +10,7 @@
#include <cc/cfg_to_element.h>
#include <boost/shared_ptr.hpp>
#include <string>
#include <vector>
#include <list>
namespace isc {
namespace dhcp {
......@@ -22,10 +22,6 @@ namespace dhcp {
/// passed to the @ref isc::dhcp::LeaseMgrFactory::create function.
class CfgDbAccess {
public:
/// @brief Specifies the database types
static const size_t LEASE_DB = 0;
static const size_t HOSTS_DB = 1;
/// @brief Constructor.
CfgDbAccess();
......@@ -48,7 +44,7 @@ public:
///
/// @param lease_db_access New lease database access string.
void setLeaseDbAccessString(const std::string& lease_db_access) {
db_access_[LEASE_DB] = lease_db_access;
lease_db_access_ = lease_db_access;
}
/// @brief Retrieves host database access string.
......@@ -60,22 +56,21 @@ public:
/// @brief Sets host database access string.
///
/// @param host_db_access New host database access string.
void setHostDbAccessString(const std::string& host_db_access) {
db_access_[HOSTS_DB] = host_db_access;
/// @param front Add at front if true, at back if false (default).
void setHostDbAccessString(const std::string& host_db_access,
bool front = false) {
if (front) {
host_db_access_.push_front(host_db_access);
} else {
host_db_access_.push_back(host_db_access);
}
}
/// @brief Retrieves host database access string.
///
/// @return Database access strings with additional parameters
/// specified with @ref CfgDbAccess::setAppendedParameters
std::vector<std::string> getHostDbAccessStringList() const;
/// @brief Pushes host database access string.
///
/// @param db_access New host database access string.
void pushHostDbAccessString(const std::string& db_access) {
db_access_.push_back(db_access);
}
std::list<std::string> getHostDbAccessStringList() const;
/// @brief Creates instance of lease manager and host data sources
/// according to the configuration specified.
......@@ -99,8 +94,11 @@ protected:
/// strings.
std::string appended_parameters_;
/// @brief Holds database access strings.
std::vector<std::string> db_access_;
/// @brief Holds lease database access string.
std::string lease_db_access_;
/// @brief Holds host database access strings.
std::list<std::string> host_db_access_;
};
......@@ -121,7 +119,7 @@ struct CfgLeaseDbAccess : public CfgDbAccess, public isc::data::CfgToElement {
///
/// @result a pointer to a configuration
virtual isc::data::ElementPtr toElement() const {
return (CfgDbAccess::toElementDbAccessString(db_access_[LEASE_DB]));
return (CfgDbAccess::toElementDbAccessString(lease_db_access_));
}
};
......@@ -136,9 +134,9 @@ struct CfgHostDbAccess : public CfgDbAccess, public isc::data::CfgToElement {
/// @result a pointer to a configuration
virtual isc::data::ElementPtr toElement() const {
isc::data::ElementPtr result = isc::data::Element::createList();
for (size_t idx = HOSTS_DB; idx < db_access_.size(); ++idx) {
for (const std::string& dbaccess : host_db_access_) {
isc::data::ElementPtr entry =
CfgDbAccess::toElementDbAccessString(db_access_[idx]);
CfgDbAccess::toElementDbAccessString(dbaccess);
if (entry->size() > 0) {
result->add(entry);
}
......
......@@ -105,4 +105,25 @@
For details, see @ref isc::dhcp::CqlConnection::openDatabase().
@section dhcpdb-host Host Backends.
Host backends (known also as host data sources) are similar to lease
backends with a few differences:
- host backends are optional (so it is allowed to have none) because
the first source of host reservations is the server configuration,
others are alternate backends.
- there may be more than one host backend. In such a case for lookups
returning a collection all results are appended, for lookups returning
at most one entry the first found is returned. Add operation is submitted
to all alternate backends which can ignore it, add the entry or throw
if the new entry conflicts with an already existing one. Delete
operations are submitted in sequence to all alternate backends until
one finds the entry, deletes it and returns true.
- the first alternate backend can be a cache (host cache hook library
is a premium feature) which avoids to lookup slow databases.
For subnet ID and identifier negative caching is optionally supported.
*/
// Copyright (C) 2018 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
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#ifndef DB_TYPE_H
#define DB_TYPE_H
namespace isc {
namespace dhcp {
/// @brief Specifies the database type.
enum class DBType {
LEASE_DB = 1,
HOSTS_DB = 2
};
} // namespace isc
} // namespace dhcp
#endif
......@@ -548,12 +548,15 @@ public:
/// @brief Sets the negative cached flag.
///
/// @param negative sets whether this is a negative cached host.
/// @param negative sets whether this is a negative cached host,
/// i.e. a fake host in the cache which indicates non-existence
/// and avoids to lookup in a slow backend.
void setNegative(bool negative) {
negative_ = negative;
}
/// @brief Return the negative cache flag value.
/// When true standard lookup methods return null host pointer instead.
bool getNegative() const {
return (negative_);
}
......
......@@ -96,6 +96,8 @@ HostDataSourceFactory::registerFactory(const string& db_type,
return (false);
}
map_.insert(pair<string, Factory>(db_type, factory));
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, HOSTS_BACKEND_REGISTER)
.arg(db_type);
return (true);
}
......@@ -104,6 +106,8 @@ HostDataSourceFactory::deregisterFactory(const string& db_type) {
auto index = map_.find(db_type);
if (index != map_.end()) {
map_.erase(index);
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, HOSTS_BACKEND_DEREGISTER)
.arg(db_type);
return (true);
} else {
return (false);
......
......@@ -42,17 +42,17 @@ HostMgr::create() {
}
void
HostMgr::addSource(const std::string& access) {
HostMgr::addBackend(const std::string& access) {
HostDataSourceFactory::add(getHostMgrPtr()->alternate_sources_, access);
}
bool
HostMgr::delSource(const std::string& db_type) {
HostMgr::delBackend(const std::string& db_type) {
return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_, db_type));
}
void
HostMgr::delAllSources() {
HostMgr::delAllBackends() {
getHostMgrPtr()->alternate_sources_.clear();
}
......@@ -65,7 +65,7 @@ HostMgr::getHostDataSource() const {
}
bool
HostMgr::checkCacheSource(bool logging) {
HostMgr::checkCacheBackend(bool logging) {
if (getHostMgrPtr()->cache_ptr_) {
return (true);
}
......@@ -98,9 +98,8 @@ HostMgr::instance() {
ConstHostCollection
HostMgr::getAll(const HWAddrPtr& hwaddr, const DuidPtr& duid) const {
ConstHostCollection hosts = getCfgHosts()->getAll(hwaddr, duid);
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
ConstHostCollection hosts_plus = (*it)->getAll(hwaddr, duid);
for (auto source : alternate_sources_) {
ConstHostCollection hosts_plus = source->getAll(hwaddr, duid);
hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end());
}
return (hosts);
......@@ -113,10 +112,9 @@ HostMgr::getAll(const Host::IdentifierType& identifier_type,
ConstHostCollection hosts = getCfgHosts()->getAll(identifier_type,
identifier_begin,
identifier_len);
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
for (auto source : alternate_sources_) {
ConstHostCollection hosts_plus =
(*it)->getAll(identifier_type, identifier_begin, identifier_len);
source->getAll(identifier_type, identifier_begin, identifier_len);
hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end());
}
return (hosts);
......@@ -126,9 +124,8 @@ HostMgr::getAll(const Host::IdentifierType& identifier_type,
ConstHostCollection
HostMgr::getAll4(const IOAddress& address) const {
ConstHostCollection hosts = getCfgHosts()->getAll4(address);
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
ConstHostCollection hosts_plus = (*it)->getAll4(address);
for (auto source : alternate_sources_) {
ConstHostCollection hosts_plus = source->getAll4(address);
hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end());
}
return (hosts);
......@@ -147,7 +144,8 @@ HostMgr::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
.arg(hwaddr ? hwaddr->toText() : "(no-hwaddr)")
.arg(duid ? duid->toText() : "(duid)");
for (auto it = alternate_sources_.begin();
!host && it != alternate_sources_.end(); ++it) {
!host && // stop at first found
it != alternate_sources_.end(); ++it) {
if (hwaddr) {
host = (*it)->get4(subnet_id, hwaddr, DuidPtr());
}
......@@ -212,12 +210,7 @@ HostMgr::get4Any(const SubnetID& subnet_id,
.arg(subnet_id)
.arg(Host::getIdentifierAsText(identifier_type, identifier_begin,
identifier_len));
// @todo: remove this
if (negative_caching_) {
cacheNegative(subnet_id, SubnetID(0),
identifier_type, identifier_begin, identifier_len);
}
return (host);
return (ConstHostPtr());
}
ConstHostPtr
......@@ -229,6 +222,9 @@ HostMgr::get4(const SubnetID& subnet_id,
identifier_begin, identifier_len);
if (host && host->getNegative()) {
return (ConstHostPtr());
} else if (!host && negative_caching_) {
cacheNegative(subnet_id, SubnetID(0),
identifier_type, identifier_begin, identifier_len);
}
return (host);
}
......@@ -245,7 +241,8 @@ HostMgr::get4(const SubnetID& subnet_id,
.arg(subnet_id)
.arg(address.toText());
for (auto it = alternate_sources_.begin();
!host && it != alternate_sources_.end(); ++it) {
!host && // stop at first found
it != alternate_sources_.end(); ++it) {
host = (*it)->get4(subnet_id, address);
if (host && cache_ptr_ && (it != alternate_sources_.begin())) {
cache(host);
......@@ -272,7 +269,8 @@ HostMgr::get6(const SubnetID& subnet_id, const DuidPtr& duid,
.arg(hwaddr ? hwaddr->toText() : "(no-hwaddr)");
for (auto it = alternate_sources_.begin();
!host && it != alternate_sources_.end(); ++it) {
!host && // stop at first found
it != alternate_sources_.end(); ++it) {
if (duid) {
host = (*it)->get6(subnet_id, duid, HWAddrPtr());
}
......@@ -300,7 +298,8 @@ HostMgr::get6(const IOAddress& prefix, const uint8_t prefix_len) const {
.arg(prefix.toText())
.arg(static_cast<int>(prefix_len));
for (auto it = alternate_sources_.begin();
!host && it != alternate_sources_.end(); ++it) {
!host && // stop at first found
it != alternate_sources_.end(); ++it) {
host = (*it)->get6(prefix, prefix_len);
if (host && cache_ptr_ && (it != alternate_sources_.begin())) {
cache(host);
......@@ -358,13 +357,7 @@ HostMgr::get6Any(const SubnetID& subnet_id,
.arg(Host::getIdentifierAsText(identifier_type, identifier_begin,
identifier_len));
// @todo: remove this
if (negative_caching_) {
cacheNegative(SubnetID(0), subnet_id,
identifier_type, identifier_begin, identifier_len);
}
return (host);
return (ConstHostPtr());
}
ConstHostPtr
......@@ -376,6 +369,9 @@ HostMgr::get6(const SubnetID& subnet_id,
identifier_begin, identifier_len);
if (host && host->getNegative()) {
return (ConstHostPtr());
} else if (!host && negative_caching_) {
cacheNegative(SubnetID(0), subnet_id,
identifier_type, identifier_begin, identifier_len);
}
return (host);
}
......@@ -392,7 +388,8 @@ HostMgr::get6(const SubnetID& subnet_id,
.arg(subnet_id)
.arg(addr.toText());
for (auto it = alternate_sources_.begin();
!host && it != alternate_sources_.end(); ++it) {
!host && // stop at first found
it != alternate_sources_.end(); ++it) {
host = (*it)->get6(subnet_id, addr);
if (host && cache_ptr_ && (it != alternate_sources_.begin())) {
cache(host);
......@@ -410,11 +407,10 @@ HostMgr::add(const HostPtr& host) {
isc_throw(NoHostDataSourceManager, "Unable to add new host because there is "
"no hosts-database configured.");
}
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
(*it)->add(host);
for (auto source : alternate_sources_) {
source->add(host);
}
// If no backend throws it should be cached.
// If no backend throws the host should be cached.
if (cache_ptr_) {
cache(host);
}
......@@ -427,9 +423,8 @@ HostMgr::del(const SubnetID& subnet_id, const asiolink::IOAddress& addr) {
"no hosts-database configured.");
}
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
if ((*it)->del(subnet_id, addr)) {
for (auto source : alternate_sources_) {
if (source->del(subnet_id, addr)) {
return (true);
}
}
......@@ -444,10 +439,9 @@ HostMgr::del4(const SubnetID& subnet_id, const Host::IdentifierType& identifier_
"no hosts-database configured.");
}
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
if ((*it)->del4(subnet_id, identifier_type,
identifier_begin, identifier_len)) {
for (auto source : alternate_sources_) {
if (source->del4(subnet_id, identifier_type,
identifier_begin, identifier_len)) {
return (true);
}
}
......@@ -462,10 +456,9 @@ HostMgr::del6(const SubnetID& subnet_id, const Host::IdentifierType& identifier_
"no alternate host data source present");
}
for (auto it = alternate_sources_.begin();
it != alternate_sources_.end(); ++it) {
if ((*it)->del6(subnet_id, identifier_type,
identifier_begin, identifier_len)) {
for (auto source : alternate_sources_) {
if (source->del6(subnet_id, identifier_type,
identifier_begin, identifier_len)) {
return (true);
}
}
......
......@@ -63,33 +63,33 @@ public:
///
static void create();
/// @brief Add an alternate host data source.
/// @brief Add an alternate host backend (aka host data source).
///
/// @param access Host data source access parameters for the alternate
/// host data source. It holds "keyword=value" pairs, separated by spaces.
/// The supported values are specific to the alternate data source in use.
/// @param access Host backend access parameters for the alternate
/// host backend. It holds "keyword=value" pairs, separated by spaces.
/// The supported values are specific to the alternate backend in use.
/// However, the "type" parameter will be common and it will specify which
/// data source is to be used. Currently, no parameters are supported
/// backend is to be used. Currently, no parameters are supported
/// and the parameter is ignored.
static void addSource(const std::string& access);
static void addBackend(const std::string& access);