Commit 6d6a3eed authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[#35,!517] Changed moveDdnsParams to modify element map instead of SrvConfig

Moving the parameters needs to be done before defaults are applied to the
config, so moveDdnsParams was changed to modify a mutable top level
element map instead of SrvConfig contents.

src/lib/dhcpsrv/parsers/simple_parser4.cc
src/lib/dhcpsrv/parsers/simple_parser6.cc
    Change ddns-send-updates default to true.

src/lib/dhcpsrv/srv_config.*
    SrvConfig::getConfiguredGlobal() - new method to fetch a
    global by name

    SrvConfig::moveDdnsParams() - changed to accept/modify
    a top-level Element map

src/lib/dhcpsrv/tests/srv_config_unittest.cc
    updated unit tests accordingly
parent d3eda7f2
...@@ -105,15 +105,15 @@ const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { ...@@ -105,15 +105,15 @@ const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = {
{ "calculate-tee-times", Element::boolean, "false" }, { "calculate-tee-times", Element::boolean, "false" },
{ "t1-percent", Element::real, ".50" }, { "t1-percent", Element::real, ".50" },
{ "t2-percent", Element::real, ".875" }, { "t2-percent", Element::real, ".875" },
{ "ddns-send-updates", Element::boolean, "false" }, { "ddns-send-updates", Element::boolean, "true" },
{ "ddns-override-no-update", Element::boolean, "false" }, { "ddns-override-no-update", Element::boolean, "false" },
{ "ddns-override-client-update", Element::boolean, "false" }, { "ddns-override-client-update", Element::boolean, "false" },
{ "ddns-replace-client-name", Element::string, "never" }, { "ddns-replace-client-name", Element::string, "never" },
{ "ddns-generated-prefix", Element::string, "myhost" }, { "ddns-generated-prefix", Element::string, "myhost" },
// TKM should this still be true? qualifying-suffix has no default ?? // TKM should this still be true? qualifying-suffix has no default ??
{ "ddns-generated-suffix", Element::string, "" }, { "ddns-qualifying-suffix", Element::string, "" },
{ "hostname-char-set", Element::string, "" }, { "hostname-char-set", Element::string, "" },
{ "hostname-char-replacement", Element::string, "" }, { "hostname-char-replacement", Element::string, "" }
}; };
/// @brief This table defines all option definition parameters. /// @brief This table defines all option definition parameters.
......
...@@ -100,15 +100,15 @@ const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { ...@@ -100,15 +100,15 @@ const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = {
{ "calculate-tee-times", Element::boolean, "true" }, { "calculate-tee-times", Element::boolean, "true" },
{ "t1-percent", Element::real, ".50" }, { "t1-percent", Element::real, ".50" },
{ "t2-percent", Element::real, ".80" }, { "t2-percent", Element::real, ".80" },
{ "ddns-send-updates", Element::boolean, "false" }, { "ddns-send-updates", Element::boolean, "true" },
{ "ddns-override-no-update", Element::boolean, "false" }, { "ddns-override-no-update", Element::boolean, "false" },
{ "ddns-override-client-update", Element::boolean, "false" }, { "ddns-override-client-update", Element::boolean, "false" },
{ "ddns-replace-client-name", Element::string, "never" }, { "ddns-replace-client-name", Element::string, "never" },
{ "ddns-generated-prefix", Element::string, "myhost" }, { "ddns-generated-prefix", Element::string, "myhost" },
// TKM should this still be true? qualifying-suffix has no default ?? // TKM should this still be true? qualifying-suffix has no default ??
{ "ddns-generated-suffix", Element::string, "" }, { "ddns-qualifying-suffix", Element::string, "" },
{ "hostname-char-set", Element::string, "" }, { "hostname-char-set", Element::string, "" },
{ "hostname-char-replacement", Element::string, "" }, { "hostname-char-replacement", Element::string, "" }
}; };
/// @brief This table defines all option definition parameters. /// @brief This table defines all option definition parameters.
......
...@@ -273,6 +273,16 @@ SrvConfig::updateStatistics() { ...@@ -273,6 +273,16 @@ SrvConfig::updateStatistics() {
} }
} }
isc::data::ConstElementPtr
SrvConfig::getConfiguredGlobal(std::string name) const {
isc::data::ConstElementPtr global;
if (configured_globals_->contains(name)) {
global = configured_globals_->get(name);
}
return (global);
}
void void
SrvConfig::clearConfiguredGlobals() { SrvConfig::clearConfiguredGlobals() {
configured_globals_ = isc::data::Element::createMap(); configured_globals_ = isc::data::Element::createMap();
...@@ -598,9 +608,19 @@ SrvConfig::getDdnsParams(const Subnet& subnet) const { ...@@ -598,9 +608,19 @@ SrvConfig::getDdnsParams(const Subnet& subnet) const {
} }
void void
SrvConfig::moveDdnsParams(isc::data::ElementPtr d2_cfg) { SrvConfig::moveDdnsParams(isc::data::ElementPtr srv_elem) {
if (!d2_cfg || (d2_cfg->getType() != Element::map)) { if (!srv_elem || (srv_elem->getType() != Element::map)) {
isc_throw(BadValue, "moveDdnsParams must be given a map element"); isc_throw(BadValue, "moveDdnsParams server config must be given a map element");
}
if (!srv_elem->contains("dhcp-ddns")) {
/* nothing to do */
return;
}
ElementPtr d2_elem = boost::const_pointer_cast<Element>(srv_elem->get("dhcp-ddns"));
if (!d2_elem || (d2_elem->getType() != Element::map)) {
isc_throw(BadValue, "moveDdnsParams dhcp-ddns is not a map");
} }
struct Param { struct Param {
...@@ -619,10 +639,10 @@ SrvConfig::moveDdnsParams(isc::data::ElementPtr d2_cfg) { ...@@ -619,10 +639,10 @@ SrvConfig::moveDdnsParams(isc::data::ElementPtr d2_cfg) {
}; };
for (auto param : params) { for (auto param : params) {
if (d2_cfg->contains(param.from_name)) { if (d2_elem->contains(param.from_name)) {
if (!configured_globals_->contains(param.to_name)) { if (!srv_elem->contains(param.to_name)) {
// No global value for it already, so let's add it. // No global value for it already, so let's add it.
addConfiguredGlobal(param.to_name, d2_cfg->get(param.from_name)); srv_elem->set(param.to_name, d2_elem->get(param.from_name));
LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_DDNS_PARAMETER_MOVED) LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_DDNS_PARAMETER_MOVED)
.arg(param.from_name).arg(param.to_name); .arg(param.from_name).arg(param.to_name);
} else { } else {
...@@ -632,7 +652,7 @@ SrvConfig::moveDdnsParams(isc::data::ElementPtr d2_cfg) { ...@@ -632,7 +652,7 @@ SrvConfig::moveDdnsParams(isc::data::ElementPtr d2_cfg) {
} }
// Now remove it from d2_data, so D2ClientCfg won't complain. // Now remove it from d2_data, so D2ClientCfg won't complain.
d2_cfg->remove(param.from_name); d2_elem->remove(param.from_name);
} }
} }
} }
......
...@@ -627,6 +627,12 @@ public: ...@@ -627,6 +627,12 @@ public:
return (isc::data::ConstElementPtr(configured_globals_)); return (isc::data::ConstElementPtr(configured_globals_));
} }
/// @brief Returns pointer to a given configured global parameter
/// @param name name of the parameter to fetch
/// @return Pointer to the parameter if it exists, otherwise an
/// empty pointer.
isc::data::ConstElementPtr getConfiguredGlobal(std::string name) const;
/// @brief Removes all configured global parameters. /// @brief Removes all configured global parameters.
/// @note This removes the default values too so either /// @note This removes the default values too so either
/// @c applyDefaultsConfiguredGlobals and @c mergeGlobals, /// @c applyDefaultsConfiguredGlobals and @c mergeGlobals,
...@@ -648,7 +654,30 @@ public: ...@@ -648,7 +654,30 @@ public:
configured_globals_->set(name, value); configured_globals_->set(name, value);
} }
void moveDdnsParams(isc::data::ElementPtr d2_cfg); /// @brief Moves deprecated parameters from dhcp-ddns element to global element
///
/// Given a server configuration element map, the following parameters are moved
/// from dhcp-ddns to top-level (i.e. global) element if they do not already
/// exist there:
///
/// @code
/// From dhcp-ddns: To (global):
/// ------------------------------------------------------
/// override-no-update ddns-override-no-update
/// override-client-update ddns-override-client-update
/// replace-client-name ddns-replace-client-name
/// generated-prefix ddns-generated-prefix
/// qualifying-suffix ddns-qualifying-suffix
/// hostname-char-set hostname-char-set
/// hostname-char-replacement hostname-char-replacement
/// @endcode
///
/// Note that the whether or not the deprecated parameters are added
/// to the global element, they are always removed from the dhcp-ddns
/// element.
///
/// @param srv_elem server top level map to alter
static void moveDdnsParams(isc::data::ElementPtr srv_elem);
/// @brief Unparse a configuration object /// @brief Unparse a configuration object
/// ///
......
...@@ -493,6 +493,20 @@ TEST_F(SrvConfigTest, configuredGlobals) { ...@@ -493,6 +493,20 @@ TEST_F(SrvConfigTest, configuredGlobals) {
ADD_FAILURE() << "unexpected element found:" << global->first; ADD_FAILURE() << "unexpected element found:" << global->first;
} }
} }
// Verify that using getConfiguredGlobal() to fetch an individual
// parameters works.
ConstElementPtr global;
// We should find global "astring".
ASSERT_NO_THROW(global = conf.getConfiguredGlobal("astring"));
ASSERT_TRUE(global);
ASSERT_EQ(Element::string, global->getType());
EXPECT_EQ("okay", global->stringValue());
// Not finding global "not-there" should return an empty pointer
// without throwing.
ASSERT_NO_THROW(global = conf.getConfiguredGlobal("not-there"));
ASSERT_FALSE(global);
} }
// Verifies that the toElement method works well (tests limited to // Verifies that the toElement method works well (tests limited to
...@@ -1108,8 +1122,10 @@ TEST_F(SrvConfigTest, mergeGlobals6) { ...@@ -1108,8 +1122,10 @@ TEST_F(SrvConfigTest, mergeGlobals6) {
} }
// Verifies that parameters formerly in dhcp-ddns{} are correctly moved // Validates SrvConfig::moveDdnsParams by ensuring that deprecated dhcp-ddns
// to configured globals. // parameters are:
// 1. Translated to their global counterparts if they do not exist globally
// 2. Removed from the dhcp-ddns element
TEST_F(SrvConfigTest, moveDdnsParamsTest) { TEST_F(SrvConfigTest, moveDdnsParamsTest) {
DdnsParamsPtr params; DdnsParamsPtr params;
...@@ -1117,166 +1133,135 @@ TEST_F(SrvConfigTest, moveDdnsParamsTest) { ...@@ -1117,166 +1133,135 @@ TEST_F(SrvConfigTest, moveDdnsParamsTest) {
struct Scenario { struct Scenario {
std::string description; std::string description;
std::string d2_input; std::string input_cfg;
std::string d2_output; std::string exp_cfg;
std::string input_globals;
std::string exp_globals;
}; };
std::vector<Scenario> scenarios { std::vector<Scenario> scenarios {
{ {
"scenario 1, move with no global conflicts", "scenario 1, move with no global conflicts",
// d2_input // input_cfg
"{\n" "{\n"
" \"enable-updates\": true, \n" " \"dhcp-ddns\": {\n"
" \"server-ip\" : \"192.0.2.0\",\n" " \"enable-updates\": true, \n"
" \"server-port\" : 3432,\n" " \"server-ip\" : \"192.0.2.0\",\n"
" \"sender-ip\" : \"192.0.2.1\",\n" " \"server-port\" : 3432,\n"
" \"sender-port\" : 3433,\n" " \"sender-ip\" : \"192.0.2.1\",\n"
" \"max-queue-size\" : 2048,\n" " \"sender-port\" : 3433,\n"
" \"ncr-protocol\" : \"UDP\",\n" " \"max-queue-size\" : 2048,\n"
" \"ncr-format\" : \"JSON\",\n" " \"ncr-protocol\" : \"UDP\",\n"
" \"user-context\": { \"foo\": \"bar\" },\n" " \"ncr-format\" : \"JSON\",\n"
" \"override-no-update\": true,\n" " \"user-context\": { \"foo\": \"bar\" },\n"
" \"override-client-update\": false,\n" " \"override-no-update\": true,\n"
" \"replace-client-name\": \"always\",\n" " \"override-client-update\": false,\n"
" \"generated-prefix\": \"prefix\",\n" " \"replace-client-name\": \"always\",\n"
" \"qualifying-suffix\": \"suffix.com.\",\n" " \"generated-prefix\": \"prefix\",\n"
" \"hostname-char-set\": \"[^A-Z]\",\n" " \"qualifying-suffix\": \"suffix.com.\",\n"
" \"hostname-char-replacement\": \"x\"\n" " \"hostname-char-set\": \"[^A-Z]\",\n"
"}\n", " \"hostname-char-replacement\": \"x\"\n"
// d2_output " }\n"
"{\n" "}\n",
" \"enable-updates\": true,\n" // exp_cfg
" \"server-ip\" : \"192.0.2.0\",\n" "{\n"
" \"server-port\" : 3432,\n" " \"dhcp-ddns\": {\n"
" \"sender-ip\" : \"192.0.2.1\",\n" " \"enable-updates\": true, \n"
" \"sender-port\" : 3433,\n" " \"server-ip\" : \"192.0.2.0\",\n"
" \"max-queue-size\" : 2048,\n" " \"server-port\" : 3432,\n"
" \"ncr-protocol\" : \"UDP\",\n" " \"sender-ip\" : \"192.0.2.1\",\n"
" \"ncr-format\" : \"JSON\",\n" " \"sender-port\" : 3433,\n"
" \"user-context\": { \"foo\": \"bar\" }\n" " \"max-queue-size\" : 2048,\n"
"}\n", " \"ncr-protocol\" : \"UDP\",\n"
// input_globals - no globals to start with " \"ncr-format\" : \"JSON\",\n"
"{\n" " \"user-context\": { \"foo\": \"bar\" }\n"
"}\n", " },\n"
// exp_globals " \"ddns-override-no-update\": true,\n"
"{\n" " \"ddns-override-client-update\": false,\n"
" \"ddns-override-no-update\": true,\n" " \"ddns-replace-client-name\": \"always\",\n"
" \"ddns-override-client-update\": false,\n" " \"ddns-generated-prefix\": \"prefix\",\n"
" \"ddns-replace-client-name\": \"always\",\n" " \"ddns-qualifying-suffix\": \"suffix.com.\",\n"
" \"ddns-generated-prefix\": \"prefix\",\n" " \"hostname-char-set\": \"[^A-Z]\",\n"
" \"ddns-qualifying-suffix\": \"suffix.com.\",\n" " \"hostname-char-replacement\": \"x\"\n"
" \"hostname-char-set\": \"[^A-Z]\",\n" "}\n"
" \"hostname-char-replacement\": \"x\"\n"
"}\n"
}, },
{ {
"scenario 2, globals already exist for all movable params", "scenario 2, globals already exist for all movable params",
// d2_input // input_cfg
"{\n" "{\n"
" \"enable-updates\": true, \n" " \"dhcp-ddns\" : {\n"
" \"override-no-update\": true,\n" " \"enable-updates\": true, \n"
" \"override-client-update\": true,\n" " \"override-no-update\": true,\n"
" \"replace-client-name\": \"always\",\n" " \"override-client-update\": true,\n"
" \"generated-prefix\": \"prefix\",\n" " \"replace-client-name\": \"always\",\n"
" \"qualifying-suffix\": \"suffix.com.\",\n" " \"generated-prefix\": \"prefix\",\n"
" \"hostname-char-set\": \"[^A-Z]\",\n" " \"qualifying-suffix\": \"suffix.com.\",\n"
" \"hostname-char-replacement\": \"x\"\n" " \"hostname-char-set\": \"[^A-Z]\",\n"
"}\n", " \"hostname-char-replacement\": \"x\"\n"
// d2_output " },\n"
"{\n" " \"ddns-override-no-update\": false,\n"
" \"enable-updates\": true \n" " \"ddns-override-client-update\": false,\n"
"}\n", " \"ddns-replace-client-name\": \"when-present\",\n"
// input_globals " \"ddns-generated-prefix\": \"org_prefix\",\n"
"{\n" " \"ddns-qualifying-suffix\": \"org_suffix.com.\",\n"
" \"ddns-override-no-update\": false,\n" " \"hostname-char-set\": \"[^a-z]\",\n"
" \"ddns-override-client-update\": false,\n" " \"hostname-char-replacement\": \"y\"\n"
" \"ddns-replace-client-name\": \"when-present\",\n" "}\n",
" \"ddns-generated-prefix\": \"org_prefix\",\n" // exp_cfg
" \"ddns-qualifying-suffix\": \"org_suffix.com.\",\n" "{\n"
" \"hostname-char-set\": \"[^a-z]\",\n" " \"dhcp-ddns\" : {\n"
" \"hostname-char-replacement\": \"y\"\n" " \"enable-updates\": true\n"
"}\n", " },\n"
// exp_globals " \"ddns-override-no-update\": false,\n"
"{\n" " \"ddns-override-client-update\": false,\n"
" \"ddns-override-no-update\": false,\n" " \"ddns-replace-client-name\": \"when-present\",\n"
" \"ddns-override-client-update\": false,\n" " \"ddns-generated-prefix\": \"org_prefix\",\n"
" \"ddns-replace-client-name\": \"when-present\",\n" " \"ddns-qualifying-suffix\": \"org_suffix.com.\",\n"
" \"ddns-generated-prefix\": \"org_prefix\",\n" " \"hostname-char-set\": \"[^a-z]\",\n"
" \"ddns-qualifying-suffix\": \"org_suffix.com.\",\n" " \"hostname-char-replacement\": \"y\"\n"
" \"hostname-char-set\": \"[^a-z]\",\n" "}\n"
" \"hostname-char-replacement\": \"y\"\n"
"}\n"
}, },
{ {
"scenario 3, nothing to move", "scenario 3, nothing to move",
// d2_input // input_cfg
"{\n" "{\n"
" \"enable-updates\": true, \n" " \"dhcp-ddns\" : {\n"
" \"server-ip\" : \"192.0.2.0\",\n" " \"enable-updates\": true, \n"
" \"server-port\" : 3432,\n" " \"server-ip\" : \"192.0.2.0\",\n"
" \"sender-ip\" : \"192.0.2.1\"\n" " \"server-port\" : 3432,\n"
"}\n", " \"sender-ip\" : \"192.0.2.1\"\n"
// d2_output " }\n"
"{\n" "}\n",
" \"enable-updates\": true,\n" // exp_output
" \"server-ip\" : \"192.0.2.0\",\n" "{\n"
" \"server-port\" : 3432,\n" " \"dhcp-ddns\" : {\n"
" \"sender-ip\" : \"192.0.2.1\"\n" " \"enable-updates\": true, \n"
"}\n", " \"server-ip\" : \"192.0.2.0\",\n"
// input_globals " \"server-port\" : 3432,\n"
"{\n" " \"sender-ip\" : \"192.0.2.1\"\n"
" \"hostname-char-set\": \"[^A-Z]\",\n" " }\n"
" \"hostname-char-replacement\": \"x\"\n" "}\n"
"}\n", }
// exp_globals
"{\n"
" \"hostname-char-set\": \"[^A-Z]\",\n"
" \"hostname-char-replacement\": \"x\"\n"
"}\n"
},
}; };
for (auto scenario : scenarios) { for (auto scenario : scenarios) {
SrvConfig conf(32); SrvConfig conf(32);
ConstElementPtr d2_input; ElementPtr input_cfg;
ConstElementPtr exp_d2_output; ConstElementPtr exp_cfg;
ConstElementPtr input_globals;
ConstElementPtr exp_globals;
{ {
SCOPED_TRACE(scenario.description); SCOPED_TRACE(scenario.description);
ASSERT_NO_THROW(d2_input = Element::fromJSON(scenario.d2_input)) // Parse the input cfg into a mutable Element map.
<< "d2_input didn't parse, test is broken"; ASSERT_NO_THROW(input_cfg = boost::const_pointer_cast<Element>
(Element::fromJSON(scenario.input_cfg)))
ASSERT_NO_THROW(exp_d2_output = Element::fromJSON(scenario.d2_output)) << "input_cfg didn't parse, test is broken";
<< "d2_output didn't parse, test is broken";
ASSERT_NO_THROW(input_globals = Element::fromJSON(scenario.input_globals))
<< "input_globals didn't parse, test is broken";
ASSERT_NO_THROW(exp_globals = Element::fromJSON(scenario.exp_globals))
<< "exp_globals didn't parse, test is broken";
// Create the original set of configured globals.
for (auto input_global : input_globals->mapValue()) {
conf.addConfiguredGlobal(input_global.first, input_global.second);
}
// We need a mutable copy of d2_input to pass into the move function. // Parse the expected cfg into an Element map.
ElementPtr mutable_d2 = boost::const_pointer_cast<Element>(d2_input); ASSERT_NO_THROW(exp_cfg = Element::fromJSON(scenario.exp_cfg))
// NOw call moveDdnsParams. << "exp_cfg didn't parse, test is broken";
ASSERT_NO_THROW(conf.moveDdnsParams(mutable_d2));
// Make sure the content of mutable_d2 is correct. // Now call moveDdnsParams.
EXPECT_TRUE(mutable_d2->equals(*exp_d2_output)); ASSERT_NO_THROW(SrvConfig::moveDdnsParams(input_cfg));
// Make sure the content of configured globals. // Make sure the resultant configuration is as expected.
EXPECT_TRUE(conf.getConfiguredGlobals()->equals(*exp_globals)); EXPECT_TRUE(input_cfg->equals(*exp_cfg));
} }
} }
} }
......
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