Commit e292f3a2 authored by Thomas Markwalder's avatar Thomas Markwalder

[#686,!403] LFC now sees sanity checking as disabled

Add CfgConsistency defaults to application level parsing

src/lib/dhcpsrv/cfg_consistency.h
    Changed constructor default mode to LEASE_CHECK_NONE

src/lib/dhcpsrv/lease_file_loader.h
    LeaseFileLoader::load() - modified to create and use a
    sanity_checker only if checking is enabled.  This avoids
    senselessly making the same decisions for every lease loaded.

src/lib/dhcpsrv/parsers/simple_parser4.*
src/lib/dhcpsrv/parsers/simple_parser6.*
    Added sanity checks defaults so they can be properly set at
    the application level.

src/lib/dhcpsrv/sanity_checker.h
    SanityChecker::leaseCheckingEnabled() - new static function
    to test if sanity checking for leases is enabled

src/lib/dhcpsrv/tests/sanity_checks_unittest.cc
src/lib/dhcpsrv/tests/srv_config_unittest.cc
    updated tests for new constructor default value
parent 6f71e2c6
1607. [bug] tmark
Corrected an initialization issue which caused lease sanity
checking to be enabled inside the Lease File Cleanup (LFC)
process. The LFC cannot meaningfully perform sanity checking
as it does not have access to the full server configuration.
(Gitlab #686,!403 git TBD)
1606. [bug] tmark
Corrected an error with retrieving DHCPv6 leases, whose IAID
values are larger than int32_t max, from Postgresql lease
......
// 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
......@@ -34,7 +34,7 @@ class CfgConsistency : public isc::data::UserContext, public isc::data::CfgToEle
/// @brief Constructor
CfgConsistency()
: lease_sanity_check_(LEASE_CHECK_WARN) {
: lease_sanity_check_(LEASE_CHECK_NONE) {
}
......
// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2015-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
......@@ -12,6 +12,7 @@
#include <util/versioned_csv_file.h>
#include <dhcpsrv/sanity_checker.h>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
namespace isc {
......@@ -86,7 +87,14 @@ public:
lease_file.close();
lease_file.open();
SanityChecker lease_checker;
// Create lease sanity checker if checking is enabled.
boost::scoped_ptr<SanityChecker> lease_checker;
if (SanityChecker::leaseCheckingEnabled(false)) {
// Since lease file is loaded during the configuration,
// we have to use staging config, rather than current
// config for this (false = staging).
lease_checker.reset(new SanityChecker());
}
boost::shared_ptr<LeaseObjectType> lease;
// Track the number of corrupted leases.
......@@ -125,12 +133,15 @@ public:
DHCPSRV_MEMFILE_LEASE_LOAD)
.arg(lease->toText());
// Now see if we need to sanitize this lease. As lease file is
// loaded during the configuration, we have to use staging config,
// rather than current config for this (false = staging).
lease_checker.checkLease(lease, false);
if (!lease) {
continue;
if (lease_checker) {
// If the lease is insane the checker will rese the lease pointer.
// As lease file is loaded during the configuration, we have
// to use staging config, rather than current config for this
// (false = staging).
lease_checker->checkLease(lease, false);
if (!lease) {
continue;
}
}
// Check if this lease exists.
......
......@@ -163,6 +163,18 @@ const SimpleDefaults SimpleParser4::IFACE4_DEFAULTS = {
{ "re-detect", Element::boolean, "true" }
};
/// @brief This table defines default values for dhcp-queue-control in DHCPv4.
const SimpleDefaults SimpleParser4::DHCP_QUEUE_CONTROL4_DEFAULTS = {
{ "enable-queue", Element::boolean, "false"},
{ "queue-type", Element::string, "kea-ring4"},
{ "capacity", Element::integer, "500"}
};
/// @brief This defines default values for sanity checking for DHCPv4.
const SimpleDefaults SimpleParser4::SANITY_CHECKS4_DEFAULTS = {
{ "lease-checks", Element::string, "warn" }
};
/// @brief List of parameters that can be inherited to subnet4 scope.
///
/// Some parameters may be defined on both global (directly in Dhcp4) and
......@@ -192,14 +204,6 @@ const ParamsList SimpleParser4::INHERIT_TO_SUBNET4 = {
"t2-percent"
};
/// @brief This table defines default values for dhcp-queue-control in DHCPv4.
const SimpleDefaults SimpleParser4::DHCP_QUEUE_CONTROL4_DEFAULTS = {
{ "enable-queue", Element::boolean, "false"},
{ "queue-type", Element::string, "kea-ring4"},
{ "capacity", Element::integer, "500"}
};
/// @}
/// ---------------------------------------------------------------------------
......@@ -266,6 +270,18 @@ size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) {
cnt += setDefaults(mutable_cfg, DHCP_QUEUE_CONTROL4_DEFAULTS);
// Set the defaults for sanity-checks. If the element isn't
// there we'll add it.
ConstElementPtr sanity_checks = global->get("sanity-checks");
if (sanity_checks) {
mutable_cfg = boost::const_pointer_cast<Element>(sanity_checks);
} else {
mutable_cfg = Element::createMap();
global->set("sanity-checks", mutable_cfg);
}
cnt += setDefaults(mutable_cfg, SANITY_CHECKS4_DEFAULTS);
return (cnt);
}
......
......@@ -46,6 +46,7 @@ public:
static const isc::data::SimpleDefaults SHARED_NETWORK4_DEFAULTS;
static const isc::data::SimpleDefaults IFACE4_DEFAULTS;
static const isc::data::SimpleDefaults DHCP_QUEUE_CONTROL4_DEFAULTS;
static const isc::data::SimpleDefaults SANITY_CHECKS4_DEFAULTS;
static const isc::data::ParamsList INHERIT_TO_SUBNET4;
};
......
......@@ -146,6 +146,18 @@ const SimpleDefaults SimpleParser6::IFACE6_DEFAULTS = {
{ "re-detect", Element::boolean, "true" }
};
/// @brief This table defines default values for dhcp-queue-control in DHCPv4.
const SimpleDefaults SimpleParser6::DHCP_QUEUE_CONTROL6_DEFAULTS = {
{ "enable-queue", Element::boolean, "false"},
{ "queue-type", Element::string, "kea-ring6"},
{ "capacity", Element::integer, "500"}
};
/// @brief This defines default values for sanity checking for DHCPv6.
const SimpleDefaults SimpleParser6::SANITY_CHECKS6_DEFAULTS = {
{ "lease-checks", Element::string, "warn" }
};
/// @brief List of parameters that can be inherited from the global to subnet6 scope.
///
/// Some parameters may be defined on both global (directly in Dhcp6) and
......@@ -175,14 +187,6 @@ const ParamsList SimpleParser6::INHERIT_TO_SUBNET6 = {
"t2-percent"
};
/// @brief This table defines default values for dhcp-queue-control in DHCPv4.
const SimpleDefaults SimpleParser6::DHCP_QUEUE_CONTROL6_DEFAULTS = {
{ "enable-queue", Element::boolean, "false"},
{ "queue-type", Element::string, "kea-ring6"},
{ "capacity", Element::integer, "500"}
};
/// @}
/// ---------------------------------------------------------------------------
......@@ -251,6 +255,18 @@ size_t SimpleParser6::setAllDefaults(isc::data::ElementPtr global) {
cnt += setDefaults(mutable_cfg, DHCP_QUEUE_CONTROL6_DEFAULTS);
// Set the defaults for sanity-checks. If the element isn't
// there we'll add it.
ConstElementPtr sanity_checks = global->get("sanity-checks");
if (sanity_checks) {
mutable_cfg = boost::const_pointer_cast<Element>(sanity_checks);
} else {
mutable_cfg = Element::createMap();
global->set("sanity-checks", mutable_cfg);
}
cnt += setDefaults(mutable_cfg, SANITY_CHECKS6_DEFAULTS);
return (cnt);
}
......
......@@ -47,6 +47,7 @@ public:
static const isc::data::SimpleDefaults SHARED_NETWORK6_DEFAULTS;
static const isc::data::SimpleDefaults IFACE6_DEFAULTS;
static const isc::data::SimpleDefaults DHCP_QUEUE_CONTROL6_DEFAULTS;
static const isc::data::SimpleDefaults SANITY_CHECKS6_DEFAULTS;
static const isc::data::ParamsList INHERIT_TO_SUBNET6;
};
......
......@@ -5,7 +5,6 @@
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#include <dhcpsrv/sanity_checker.h>
#include <dhcpsrv/cfg_consistency.h>
#include <dhcpsrv/cfg_subnets4.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/subnet_id.h>
#include <dhcpsrv/dhcpsrv_log.h>
......@@ -14,6 +13,22 @@
namespace isc {
namespace dhcp {
bool SanityChecker::leaseCheckingEnabled(bool current) {
SrvConfigPtr cfg;
if (current) {
cfg = CfgMgr::instance().getCurrentCfg();
} else {
cfg = CfgMgr::instance().getStagingCfg();
}
if (cfg) {
CfgConsistencyPtr sanity = cfg->getConsistency();
return (sanity && (sanity->getLeaseSanityCheck() != CfgConsistency::LEASE_CHECK_NONE));
}
return (false);
}
void SanityChecker::checkLease(Lease4Ptr& lease, bool current) {
SrvConfigPtr cfg;
if (current) {
......@@ -21,7 +36,13 @@ void SanityChecker::checkLease(Lease4Ptr& lease, bool current) {
} else {
cfg = CfgMgr::instance().getStagingCfg();
}
CfgConsistencyPtr sanity = cfg->getConsistency();
if (sanity->getLeaseSanityCheck() == CfgConsistency::LEASE_CHECK_NONE) {
// No sense going farther.
return;
}
CfgSubnets4Ptr subnets = cfg->getCfgSubnets4();
checkLeaseInternal(lease, sanity, subnets);
}
......@@ -39,6 +60,11 @@ void SanityChecker::checkLease(Lease6Ptr& lease, bool current) {
cfg = CfgMgr::instance().getStagingCfg();
}
CfgConsistencyPtr sanity = cfg->getConsistency();
if (sanity->getLeaseSanityCheck() == CfgConsistency::LEASE_CHECK_NONE) {
// No sense going farther.
return;
}
CfgSubnets6Ptr subnets = cfg->getCfgSubnets6();
checkLeaseInternal(lease, sanity, subnets);
}
......@@ -47,12 +73,7 @@ template<typename LeasePtrType, typename SubnetsType>
void SanityChecker::checkLeaseInternal(LeasePtrType& lease, const CfgConsistencyPtr& checks,
const SubnetsType& subnets) {
if (checks->getLeaseSanityCheck() == CfgConsistency::LEASE_CHECK_NONE) {
return;
}
auto subnet = subnets->getBySubnetId(lease->subnet_id_);
if (subnet && subnet->inRange(lease->addr_)) {
// If the subnet is defined and the address is in range, we're good.
......@@ -147,8 +168,6 @@ void SanityChecker::checkLeaseInternal(LeasePtrType& lease, const CfgConsistency
template<typename LeaseType, typename SubnetsType>
SubnetID SanityChecker::findSubnetId(const LeaseType& lease, const SubnetsType& subnets) {
//CfgSubnets4Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4();
auto subnet = subnets->selectSubnet(lease->addr_);
if (!subnet) {
return (0);
......
......@@ -44,6 +44,14 @@ class SanityChecker {
/// config
void checkLease(Lease6Ptr& lease, bool current = true);
/// @brief Indicates the specified configuration enables lease sanity checking.
///
/// @param current specifies whether to use current (true) or staging(false)
/// server configuration
/// @return true if the sanity-checks/lease-checks parameter value (see
/// @ref CfgConsistency for details) is not CfgConsistency::LEASE_CHECK_NONE.
static bool leaseCheckingEnabled(bool current = true);
private:
/// @brief Internal implementation for checkLease command
......
......@@ -159,9 +159,9 @@ TEST_F(SanityChecksTest, leaseCheck) {
SrvConfig cfg;
// The default should be to warn about inconsistency.
// The default should be to none.
EXPECT_EQ(cfg.getConsistency()->getLeaseSanityCheck(),
CfgConsistency::LEASE_CHECK_WARN);
CfgConsistency::LEASE_CHECK_NONE);
parserCheck(cfg, valid1, false, CfgConsistency::LEASE_CHECK_NONE);
parserCheck(cfg, valid2, false, CfgConsistency::LEASE_CHECK_WARN);
......
......@@ -513,7 +513,7 @@ TEST_F(SrvConfigTest, unparse) {
defaults += "\"lease-database\": { \"type\": \"memfile\" },\n";
defaults += "\"hooks-libraries\": [ ],\n";
defaults += "\"sanity-checks\": {\n";
defaults += " \"lease-checks\": \"warn\"\n";
defaults += " \"lease-checks\": \"none\"\n";
defaults += " },\n";
defaults += "\"dhcp-ddns\": \n";
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment