Skip to content

Commit a48e9ee

Browse files
author
Razvan Becheriu
committed
[#1790] add support for top level maps in global CB parameters
1 parent 4d04d91 commit a48e9ee

26 files changed

+376
-106
lines changed

‎src/bin/dhcp4/json_config_parser.cc‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ processDhcp4Config(isc::data::ConstElementPtr config_set) {
459459
if (expiration_cfg) {
460460
parameter_name = "expired-leases-processing";
461461
ExpirationConfigParser parser;
462-
parser.parse(expiration_cfg);
462+
parser.parse(expiration_cfg, CfgMgr::instance().getStagingCfg()->getCfgExpiration());
463463
}
464464

465465
// The hooks-libraries configuration must be parsed after parsing
@@ -577,32 +577,8 @@ processDhcp4Config(isc::data::ConstElementPtr config_set) {
577577

578578
ConstElementPtr compatibility = mutable_cfg->get("compatibility");
579579
if (compatibility) {
580-
for (auto const& kv : compatibility->mapValue()) {
581-
if (!kv.second || (kv.second->getType() != Element::boolean)) {
582-
isc_throw(DhcpConfigError,
583-
"compatibility parameter values must be "
584-
<< "boolean (" << kv.first << " at "
585-
<< kv.second->getPosition() << ")");
586-
}
587-
if (kv.first == "lenient-option-parsing") {
588-
CfgMgr::instance().getStagingCfg()->setLenientOptionParsing(
589-
kv.second->boolValue());
590-
} else if (kv.first == "ignore-dhcp-server-identifier") {
591-
CfgMgr::instance().getStagingCfg()->setIgnoreServerIdentifier(
592-
kv.second->boolValue());
593-
} else if (kv.first == "ignore-rai-link-selection") {
594-
CfgMgr::instance().getStagingCfg()->setIgnoreRAILinkSelection(
595-
kv.second->boolValue());
596-
} else if (kv.first == "exclude-first-last-24") {
597-
CfgMgr::instance().getStagingCfg()->setExcludeFirstLast24(
598-
kv.second->boolValue());
599-
} else {
600-
isc_throw(DhcpConfigError,
601-
"unsupported compatibility parameter: "
602-
<< kv.first << " (" << kv.second->getPosition()
603-
<< ")");
604-
}
605-
}
580+
CompatibilityParser parser;
581+
parser.parse(compatibility, *CfgMgr::instance().getStagingCfg());
606582
}
607583

608584
// Make parsers grouping.

‎src/bin/dhcp6/json_config_parser.cc‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ processDhcp6Config(isc::data::ConstElementPtr config_set) {
584584
if (expiration_cfg) {
585585
parameter_name = "expired-leases-processing";
586586
ExpirationConfigParser parser;
587-
parser.parse(expiration_cfg);
587+
parser.parse(expiration_cfg, CfgMgr::instance().getStagingCfg()->getCfgExpiration());
588588
}
589589

590590
// The hooks-libraries configuration must be parsed after parsing
@@ -709,23 +709,8 @@ processDhcp6Config(isc::data::ConstElementPtr config_set) {
709709

710710
ConstElementPtr compatibility = mutable_cfg->get("compatibility");
711711
if (compatibility) {
712-
for (auto const& kv : compatibility->mapValue()) {
713-
if (!kv.second || (kv.second->getType() != Element::boolean)) {
714-
isc_throw(DhcpConfigError,
715-
"compatibility parameter values must be "
716-
<< "boolean (" << kv.first << " at "
717-
<< kv.second->getPosition() << ")");
718-
}
719-
if (kv.first == "lenient-option-parsing") {
720-
CfgMgr::instance().getStagingCfg()->setLenientOptionParsing(
721-
kv.second->boolValue());
722-
} else {
723-
isc_throw(DhcpConfigError,
724-
"unsupported compatibility parameter: "
725-
<< kv.first << " (" << kv.second->getPosition()
726-
<< ")");
727-
}
728-
}
712+
CompatibilityParser parser;
713+
parser.parse(compatibility, *CfgMgr::instance().getStagingCfg());
729714
}
730715

731716
// Make parsers grouping.

‎src/lib/cc/stamped_value.cc‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,35 @@ StampedValue::validateConstruct() const {
163163
<< name_ << "' parameter is NULL");
164164
}
165165

166-
if ((value_->getType() != Element::string) &&
167-
(value_->getType() != Element::integer) &&
168-
(value_->getType() != Element::boolean) &&
169-
(value_->getType() != Element::real)) {
166+
auto type = value_->getType();
167+
if ((type != Element::string) &&
168+
(type != Element::integer) &&
169+
(type != Element::boolean) &&
170+
(type != Element::real) &&
171+
(type != Element::map)) {
170172
isc_throw(TypeError, "StampedValue: provided value of the '"
171173
<< name_ << "' parameter has invalid type: "
172-
<< Element::typeToName(value_->getType()));
174+
<< Element::typeToName(type));
175+
}
176+
177+
if (type == Element::map) {
178+
size_t count = value_->mapValue().size();
179+
if (count > 1) {
180+
isc_throw(BadValue, "StampedValue: provided value of the '"
181+
<< name_ << "' parameter has more than one element in the map");
182+
}
183+
if (count == 1) {
184+
type = value_->mapValue().begin()->second->getType();
185+
if ((type != Element::string) &&
186+
(type != Element::integer) &&
187+
(type != Element::boolean) &&
188+
(type != Element::real)) {
189+
isc_throw(BadValue, "StampedValue: provided value of the '"
190+
<< name_ << "/" << value_->mapValue().begin()->first
191+
<< "' parameter has invalid type: "
192+
<< Element::typeToName(type));
193+
}
194+
}
173195
}
174196
}
175197

‎src/lib/cc/stamped_value.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ typedef boost::shared_ptr<StampedValue> StampedValuePtr;
2929
/// e.g. global parameter of the DHCP server.
3030
///
3131
/// Global configuration elements having simple types, e.g. DHCP
32-
/// timers, need to be associatied with modification timestamps.
32+
/// timers, need to be associated with modification timestamps.
3333
/// This association is made by deriving from @c StampedElement.
3434
/// The values can be strings, integers, booleans or real numbers.
3535
///
@@ -53,7 +53,7 @@ class StampedValue : public StampedElement {
5353
///
5454
/// @throw BadValue if the value is null.
5555
/// @throw TypeError if the value is neither a string, integer,
56-
/// bool nor real.
56+
/// bool, real or a map with only one element of these types.
5757
StampedValue(const std::string& name, const ElementPtr& value);
5858

5959
/// @brief Constructor creating a string value.
@@ -76,7 +76,7 @@ class StampedValue : public StampedElement {
7676
///
7777
/// @throw BadValue if the value is null.
7878
/// @throw TypeError if the value is neither a string, integer,
79-
/// bool nor real.
79+
/// bool, real or a map with only one element of these types.
8080
static StampedValuePtr create(const std::string& name,
8181
const ElementPtr& value);
8282

@@ -170,8 +170,8 @@ class StampedValue : public StampedElement {
170170
/// This is called from the constructors.
171171
///
172172
/// @throw BadValue if the value is null.
173-
/// @throw TypeError if the value type is neither a string,
174-
/// integer, boolean nor real.
173+
/// @throw TypeError if the value type is neither a string, integer,
174+
/// boolean, real or a map with only one element of these types.
175175
void validateConstruct() const;
176176

177177
/// @brief Checks if the value is accessed correctly.

‎src/lib/cc/tests/stamped_value_unittest.cc‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,82 @@ TEST(StampedValueTest, convertStringToDouble) {
161161
EXPECT_THROW(StampedValue::create("bar", "hoho", Element::real), BadValue);
162162
}
163163

164+
// Tests that stamped value from map can be created, but only with at most one element.
165+
TEST(StampedValueTest, createFromMap) {
166+
StampedValuePtr value;
167+
ElementPtr map = Element::createMap();
168+
ASSERT_NO_THROW(value = StampedValue::create("bar", map));
169+
EXPECT_FALSE(value->amNull());
170+
EXPECT_EQ(Element::map, value->getType());
171+
EXPECT_EQ("bar", value->getName());
172+
ASSERT_THROW(value->getValue(), TypeError);
173+
EXPECT_EQ(value->getElementValue()->getType(), Element::map);
174+
ASSERT_EQ(value->getElementValue()->mapValue().size(), 0);
175+
176+
EXPECT_THROW(value->getIntegerValue(), TypeError);
177+
EXPECT_THROW(value->getBoolValue(), TypeError);
178+
EXPECT_THROW(value->getDoubleValue(), TypeError);
179+
180+
map->set("foo", Element::create("0"));
181+
ASSERT_NO_THROW(value = StampedValue::create("bar", map));
182+
EXPECT_FALSE(value->amNull());
183+
EXPECT_EQ(Element::map, value->getType());
184+
EXPECT_EQ("bar", value->getName());
185+
ASSERT_THROW(value->getValue(), TypeError);
186+
EXPECT_EQ(value->getElementValue()->getType(), Element::map);
187+
ASSERT_EQ(value->getElementValue()->mapValue().size(), 1);
188+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->first, "foo");
189+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->getType(), Element::string);
190+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->stringValue(), "0");
191+
192+
map->set("foo", Element::create(true));
193+
ASSERT_NO_THROW(value = StampedValue::create("bar", map));
194+
EXPECT_FALSE(value->amNull());
195+
EXPECT_EQ(Element::map, value->getType());
196+
EXPECT_EQ("bar", value->getName());
197+
ASSERT_THROW(value->getValue(), TypeError);
198+
EXPECT_EQ(value->getElementValue()->getType(), Element::map);
199+
ASSERT_EQ(value->getElementValue()->mapValue().size(), 1);
200+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->first, "foo");
201+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->getType(), Element::boolean);
202+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->boolValue(), true);
203+
204+
map->set("foo", Element::create(0));
205+
ASSERT_NO_THROW(value = StampedValue::create("bar", map));
206+
EXPECT_FALSE(value->amNull());
207+
EXPECT_EQ(Element::map, value->getType());
208+
EXPECT_EQ("bar", value->getName());
209+
ASSERT_THROW(value->getValue(), TypeError);
210+
EXPECT_EQ(value->getElementValue()->getType(), Element::map);
211+
ASSERT_EQ(value->getElementValue()->mapValue().size(), 1);
212+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->first, "foo");
213+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->getType(), Element::integer);
214+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->intValue(), 0);
215+
216+
map->set("foo", Element::create(0.0));
217+
ASSERT_NO_THROW(value = StampedValue::create("bar", map));
218+
EXPECT_FALSE(value->amNull());
219+
EXPECT_EQ(Element::map, value->getType());
220+
EXPECT_EQ("bar", value->getName());
221+
ASSERT_THROW(value->getValue(), TypeError);
222+
EXPECT_EQ(value->getElementValue()->getType(), Element::map);
223+
ASSERT_EQ(value->getElementValue()->mapValue().size(), 1);
224+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->first, "foo");
225+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->getType(), Element::real);
226+
EXPECT_EQ(value->getElementValue()->mapValue().begin()->second->doubleValue(), 0.0);
227+
}
228+
164229
// Tests that the value must have an allowed type.
165230
TEST(StampedValueTest, createFailures) {
166231
EXPECT_THROW(StampedValue::create("bar", ElementPtr()), BadValue);
167-
EXPECT_THROW(StampedValue::create("bar", Element::createMap()), TypeError);
168232
EXPECT_THROW(StampedValue::create("bar", Element::createList()), TypeError);
233+
ElementPtr map = Element::createMap();
234+
map->set("foo", Element::create("0"));
235+
map->set("test", Element::create("true"));
236+
EXPECT_THROW(StampedValue::create("bar", map), BadValue);
237+
map = Element::createMap();
238+
map->set("foo", Element::createMap());
239+
EXPECT_THROW(StampedValue::create("bar", map), BadValue);
169240

170241
EXPECT_THROW(StampedValue::create("bar", "1", Element::map), TypeError);
171242
EXPECT_THROW(StampedValue::create("bar", "1", Element::list), TypeError);

‎src/lib/config_backend/tests/config_backend_mgr_unittest.cc‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -376,16 +376,16 @@ class ConfigBackendMgrTest : public ::testing::Test {
376376
void addTestData() {
377377
// Add two properties with different names into the first backend.
378378
config_mgr_.getPool()->createProperty(std::make_pair("dogs", 1),
379-
BackendSelector(BackendSelector::Type::MYSQL));
379+
BackendSelector(BackendSelector::Type::MYSQL));
380380
config_mgr_.getPool()->createProperty(std::make_pair("wolves", 3),
381-
BackendSelector(BackendSelector::Type::MYSQL));
381+
BackendSelector(BackendSelector::Type::MYSQL));
382382

383383
// Add two properties into the second backend. Both properties share the
384384
// name so as we can test retrieving multiple records from the same backend.
385385
config_mgr_.getPool()->createProperty(std::make_pair("cats", 2),
386-
BackendSelector(BackendSelector::Type::POSTGRESQL));
386+
BackendSelector(BackendSelector::Type::POSTGRESQL));
387387
config_mgr_.getPool()->createProperty(std::make_pair("cats", 4),
388-
BackendSelector(BackendSelector::Type::POSTGRESQL));
388+
BackendSelector(BackendSelector::Type::POSTGRESQL));
389389
}
390390

391391
/// Instance of the test configuration manager.
@@ -448,25 +448,25 @@ TEST_F(ConfigBackendMgrTest, getSingleProperty) {
448448

449449
// No dogs in the postgresql backend and no cats in mysql backend.
450450
EXPECT_EQ(0, config_mgr_.getPool()->getProperty("dogs",
451-
BackendSelector(BackendSelector::Type::POSTGRESQL)));
451+
BackendSelector(BackendSelector::Type::POSTGRESQL)));
452452
EXPECT_EQ(0, config_mgr_.getPool()->getProperty("cats",
453-
BackendSelector(BackendSelector::Type::MYSQL)));
453+
BackendSelector(BackendSelector::Type::MYSQL)));
454454

455455
// If the selectors are pointing to the right databases, the dogs and cats
456456
// should be returned properly.
457457
EXPECT_EQ(1, config_mgr_.getPool()->getProperty("dogs",
458-
BackendSelector(BackendSelector::Type::MYSQL)));
458+
BackendSelector(BackendSelector::Type::MYSQL)));
459459
EXPECT_EQ(2, config_mgr_.getPool()->getProperty("cats",
460-
BackendSelector(BackendSelector::Type::POSTGRESQL)));
460+
BackendSelector(BackendSelector::Type::POSTGRESQL)));
461461

462462
// Also make sure that the variant of getProperty function taking two arguments
463463
// would return the value.
464464
EXPECT_EQ(1, config_mgr_.getPool()->getProperty("dogs", 1,
465-
BackendSelector(BackendSelector::Type::MYSQL)));
465+
BackendSelector(BackendSelector::Type::MYSQL)));
466466

467467
// If the value is not matching it should return 0.
468468
EXPECT_EQ(0, config_mgr_.getPool()->getProperty("dogs", 2,
469-
BackendSelector(BackendSelector::Type::MYSQL)));
469+
BackendSelector(BackendSelector::Type::MYSQL)));
470470

471471
// Try to use the backend that is not present.
472472
EXPECT_THROW(config_mgr_.getPool()->getProperty("cats",
@@ -483,18 +483,18 @@ TEST_F(ConfigBackendMgrTest, getMultipleProperties) {
483483
// There is one dogs entry in mysql.
484484
PropertiesList mysql_list =
485485
config_mgr_.getPool()->getProperties("dogs",
486-
BackendSelector(BackendSelector::Type::MYSQL));
486+
BackendSelector(BackendSelector::Type::MYSQL));
487487
ASSERT_EQ(1, mysql_list.size());
488488

489489
// There is also one wolves entry in mysql.
490490
mysql_list = config_mgr_.getPool()->getProperties("wolves",
491-
BackendSelector(BackendSelector::Type::MYSQL));
491+
BackendSelector(BackendSelector::Type::MYSQL));
492492
ASSERT_EQ(1, mysql_list.size());
493493

494494
// There are two cats entries in postgresql.
495495
PropertiesList postgresql_list =
496496
config_mgr_.getPool()->getProperties("cats",
497-
BackendSelector(BackendSelector::Type::POSTGRESQL));
497+
BackendSelector(BackendSelector::Type::POSTGRESQL));
498498
ASSERT_EQ(2, postgresql_list.size());
499499

500500
// Try to use the backend that is not present.
@@ -550,7 +550,7 @@ TEST_F(ConfigBackendMgrTest, unregister) {
550550

551551
// Try to use the backend that is not present.
552552
EXPECT_THROW(config_mgr_.getPool()->getProperties("cats",
553-
BackendSelector(BackendSelector::Type::MYSQL)),
553+
BackendSelector(BackendSelector::Type::MYSQL)),
554554
NoSuchDatabase);
555555
}
556556

‎src/lib/dhcpsrv/cb_ctl_dhcp.h‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,25 +34,41 @@ class CBControlDHCP : public process::CBControlBase<ConfigBackendMgrType> {
3434

3535
protected:
3636

37-
/// @brief Adds globals fetched from config backend(s) to a SrvConfig instance
37+
/// @brief It translates the top level map parameters from flat naming
38+
/// format (e.g. map-name/element-name) to proper ElementMap objects and
39+
/// adds all globals fetched from config backend(s) to a SrvConfig instance
40+
///
41+
/// Iterates over the given collection of global parameters and adds them to
42+
/// the given configuration's list of configured globals.
3843
///
39-
/// Iterates over the given collection of global parameters and adds them to the
40-
/// given configuration's list of configured globals.
4144
///
4245
/// @param external_cfg SrvConfig instance to update
4346
/// @param cb_globals collection of global parameters supplied by configuration
4447
/// backend
45-
void addGlobalsToConfig(SrvConfigPtr external_cfg,
46-
data::StampedValueCollection& cb_globals) const {
48+
void translateAndAddGlobalsToConfig(SrvConfigPtr external_cfg,
49+
data::StampedValueCollection& cb_globals) const {
4750
auto const& index = cb_globals.get<data::StampedValueNameIndexTag>();
4851
for (auto const& cb_global : index) {
4952

5053
if (cb_global->amNull()) {
5154
continue;
5255
}
5356

54-
external_cfg->addConfiguredGlobal(cb_global->getName(),
55-
cb_global->getElementValue());
57+
std::string name = cb_global->getName();
58+
auto pos = name.find('/');
59+
if (pos != std::string::npos) {
60+
const std::string sub_elem(name.substr(pos + 1));
61+
name = name.substr(0, pos);
62+
data::ElementPtr sub_param = boost::const_pointer_cast<data::Element>(external_cfg->getConfiguredGlobal(name));
63+
if (!sub_param) {
64+
sub_param = data::Element::createMap();
65+
}
66+
sub_param->set(sub_elem, cb_global->getElementValue());
67+
external_cfg->addConfiguredGlobal(name, sub_param);
68+
} else {
69+
// Reuse name and value.
70+
external_cfg->addConfiguredGlobal(name, cb_global->getElementValue());
71+
}
5672
}
5773
}
5874
};

0 commit comments

Comments
 (0)