diff options
author | Shaun Verch <shaun.verch@mongodb.com> | 2014-05-07 18:01:04 -0400 |
---|---|---|
committer | Shaun Verch <shaun.verch@mongodb.com> | 2014-05-13 17:08:50 -0400 |
commit | a503b4b57e57f81bebddd07ed75cf116f23de350 (patch) | |
tree | ae0db9e6b4474142a3862ddf550554f1af09ba39 /src/mongo | |
parent | b3e8e45ea6f346f804161e1fe4043ba3e5850ba8 (diff) | |
download | mongo-a503b4b57e57f81bebddd07ed75cf116f23de350.tar.gz |
SERVER-13439 Make sure values explicitly set to false in config file show up in parsed result
Diffstat (limited to 'src/mongo')
-rw-r--r-- | src/mongo/db/db.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/mongod_options.cpp | 120 | ||||
-rw-r--r-- | src/mongo/db/server_options_helpers.cpp | 48 | ||||
-rw-r--r-- | src/mongo/s/mongos_options.cpp | 25 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_options.cpp | 13 | ||||
-rw-r--r-- | src/mongo/util/options_parser/option_description.h | 3 | ||||
-rw-r--r-- | src/mongo/util/options_parser/option_section.cpp | 25 | ||||
-rw-r--r-- | src/mongo/util/options_parser/options_parser.cpp | 65 | ||||
-rw-r--r-- | src/mongo/util/options_parser/options_parser_test.cpp | 61 |
9 files changed, 251 insertions, 112 deletions
diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp index 5d9c01ea63c..2ee1b36f091 100644 --- a/src/mongo/db/db.cpp +++ b/src/mongo/db/db.cpp @@ -867,7 +867,8 @@ static void startupConfigActions(const std::vector<std::string>& args) { #endif // _WIN32 #ifdef __linux__ - if (moe::startupOptionsParsed.count("shutdown")){ + if (moe::startupOptionsParsed.count("shutdown") && + moe::startupOptionsParsed["shutdown"].as<bool>() == true) { bool failed = false; string name = (boost::filesystem::path(storageGlobalParams.dbpath) / "mongod.lock").string(); diff --git a/src/mongo/db/mongod_options.cpp b/src/mongo/db/mongod_options.cpp index bdccddb670b..d0ea508d3d0 100644 --- a/src/mongo/db/mongod_options.cpp +++ b/src/mongo/db/mongod_options.cpp @@ -118,9 +118,6 @@ namespace mongo { // Network Options - general_options.addOptionChaining("net.ipv6", "ipv6", moe::Switch, - "enable IPv6 support (disabled by default)"); - general_options.addOptionChaining("net.http.JSONPEnabled", "jsonp", moe::Switch, "allow JSONP access via http (has security implications)"); @@ -458,17 +455,20 @@ namespace mongo { bool handlePreValidationMongodOptions(const moe::Environment& params, const std::vector<std::string>& args) { - if (params.count("help")) { + if (params.count("help") && + params["help"].as<bool>() == true) { printMongodHelp(moe::startupOptions); return false; } - if (params.count("version")) { + if (params.count("version") && + params["version"].as<bool>() == true) { cout << mongodVersion() << endl; printGitVersion(); printOpenSSLVersion(); return false; } - if (params.count("sysinfo")) { + if (params.count("sysinfo") && + params["sysinfo"].as<bool>() == true) { sysRuntimeInfo(); return false; } @@ -491,7 +491,8 @@ namespace mongo { // SERVER-10019 Enabling rest/jsonp without --httpinterface should break in all cases in the // future - if (params.count("net.http.RESTInterfaceEnabled")) { + if (params.count("net.http.RESTInterfaceEnabled") && + params["net.http.RESTInterfaceEnabled"].as<bool>() == true) { // If we are explicitly setting httpinterface to false in the config file (the source of // "net.http.enabled") and not overriding it on the command line (the source of @@ -505,7 +506,8 @@ namespace mongo { } } - if (params.count("net.http.JSONPEnabled")) { + if (params.count("net.http.JSONPEnabled") && + params["net.http.JSONPEnabled"].as<bool>() == true) { // If we are explicitly setting httpinterface to false in the config file (the source of // "net.http.enabled") and not overriding it on the command line (the source of @@ -527,7 +529,8 @@ namespace mongo { // Need to handle this before canonicalizing the general "server options", since // httpinterface and nohttpinterface are shared between mongos and mongod, but mongod has // extra validation required. - if (params->count("net.http.RESTInterfaceEnabled")) { + if (params->count("net.http.RESTInterfaceEnabled") && + (*params)["net.http.RESTInterfaceEnabled"].as<bool>() == true) { bool httpEnabled = false; if (params->count("net.http.enabled")) { Status ret = params->get("net.http.enabled", &httpEnabled); @@ -551,7 +554,8 @@ namespace mongo { } } - if (params->count("net.http.JSONPEnabled")) { + if (params->count("net.http.JSONPEnabled") && + (*params)["net.http.JSONPEnabled"].as<bool>() == true) { if (params->count("nohttpinterface")) { log() << "** WARNING: Should not specify both --jsonp and --nohttpinterface" << startupWarningsLog; @@ -581,12 +585,20 @@ namespace mongo { // "storage.journal.enabled" comes from the config file, so override it if any of "journal", // "nojournal", "dur", and "nodur" are set, since those come from the command line. - if (params->count("nodur") || params->count("nojournal")) { - Status ret = params->set("storage.journal.enabled", moe::Value(false)); + if (params->count("journal")) { + Status ret = params->set("storage.journal.enabled", + moe::Value((*params)["journal"].as<bool>())); if (!ret.isOK()) { return ret; } - ret = params->remove("nodur"); + ret = params->remove("journal"); + if (!ret.isOK()) { + return ret; + } + } + if (params->count("nojournal")) { + Status ret = params->set("storage.journal.enabled", + moe::Value(!(*params)["nojournal"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -595,9 +607,9 @@ namespace mongo { return ret; } } - - if (params->count("dur") || params->count("journal")) { - Status ret = params->set("storage.journal.enabled", moe::Value(true)); + if (params->count("dur")) { + Status ret = params->set("storage.journal.enabled", + moe::Value((*params)["dur"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -605,7 +617,14 @@ namespace mongo { if (!ret.isOK()) { return ret; } - ret = params->remove("journal"); + } + if (params->count("nodur")) { + Status ret = params->set("storage.journal.enabled", + moe::Value(!(*params)["nodur"].as<bool>())); + if (!ret.isOK()) { + return ret; + } + ret = params->remove("nodur"); if (!ret.isOK()) { return ret; } @@ -633,7 +652,9 @@ namespace mongo { // "auth" are set since those come from the command line. if (params->count("noauth")) { Status ret = params->set("security.authorization", - moe::Value(std::string("disabled"))); + (*params)["noauth"].as<bool>() ? + moe::Value(std::string("disabled")) : + moe::Value(std::string("enabled"))); if (!ret.isOK()) { return ret; } @@ -644,7 +665,9 @@ namespace mongo { } if (params->count("auth")) { Status ret = params->set("security.authorization", - moe::Value(std::string("enabled"))); + (*params)["auth"].as<bool>() ? + moe::Value(std::string("enabled")) : + moe::Value(std::string("disabled"))); if (!ret.isOK()) { return ret; } @@ -657,7 +680,8 @@ namespace mongo { // "storage.preallocDataFiles" comes from the config file, so override it if "noprealloc" is // set since that comes from the command line. if (params->count("noprealloc")) { - Status ret = params->set("storage.preallocDataFiles", moe::Value(false)); + Status ret = params->set("storage.preallocDataFiles", + moe::Value(!(*params)["noprealloc"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -670,7 +694,8 @@ namespace mongo { // "sharding.archiveMovedChunks" comes from the config file, so override it if // "noMoveParanoia" or "moveParanoia" are set since those come from the command line. if (params->count("noMoveParanoia")) { - Status ret = params->set("sharding.archiveMovedChunks", moe::Value(false)); + Status ret = params->set("sharding.archiveMovedChunks", + moe::Value(!(*params)["noMoveParanoia"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -680,7 +705,8 @@ namespace mongo { } } if (params->count("moveParanoia")) { - Status ret = params->set("sharding.archiveMovedChunks", moe::Value(true)); + Status ret = params->set("sharding.archiveMovedChunks", + moe::Value((*params)["moveParanoia"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -693,6 +719,12 @@ namespace mongo { // "sharding.clusterRole" comes from the config file, so override it if "configsvr" or // "shardsvr" are set since those come from the command line. if (params->count("configsvr")) { + if ((*params)["configsvr"].as<bool>() == false) { + // Handle the case where "configsvr" comes from the legacy config file and is set to + // false. This option is not allowed in the YAML config. + return Status(ErrorCodes::BadValue, + "configsvr option cannot be set to false in config file"); + } Status ret = params->set("sharding.clusterRole", moe::Value(std::string("configsvr"))); if (!ret.isOK()) { return ret; @@ -703,6 +735,12 @@ namespace mongo { } } if (params->count("shardsvr")) { + if ((*params)["shardsvr"].as<bool>() == false) { + // Handle the case where "shardsvr" comes from the legacy config file and is set to + // false. This option is not allowed in the YAML config. + return Status(ErrorCodes::BadValue, + "shardsvr option cannot be set to false in config file"); + } Status ret = params->set("sharding.clusterRole", moe::Value(std::string("shardsvr"))); if (!ret.isOK()) { return ret; @@ -748,7 +786,8 @@ namespace mongo { // "storage.indexBuildRetry" comes from the config file, so override it if // "noIndexBuildRetry" is set since that comes from the command line. if (params->count("noIndexBuildRetry")) { - Status ret = params->set("storage.indexBuildRetry", moe::Value(false)); + Status ret = params->set("storage.indexBuildRetry", + moe::Value(!(*params)["noIndexBuildRetry"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -840,10 +879,10 @@ namespace mongo { } if (params.count("storage.directoryPerDB")) { - storageGlobalParams.directoryperdb = true; + storageGlobalParams.directoryperdb = params["storage.directoryPerDB"].as<bool>(); } if (params.count("cpu")) { - serverGlobalParams.cpu = true; + serverGlobalParams.cpu = params["cpu"].as<bool>(); } if (params.count("security.authorization") && params["security.authorization"].as<std::string>() == "disabled") { @@ -854,7 +893,7 @@ namespace mongo { getGlobalAuthorizationManager()->setAuthEnabled(true); } if (params.count("storage.quota.enforced")) { - storageGlobalParams.quota = true; + storageGlobalParams.quota = params["storage.quota.enforced"].as<bool>(); } if (params.count("storage.quota.maxFilesPerDB")) { storageGlobalParams.quota = true; @@ -881,27 +920,27 @@ namespace mongo { storageGlobalParams.durOptions = params["storage.journal.debugFlags"].as<int>(); } if (params.count("nohints")) { - storageGlobalParams.useHints = false; + storageGlobalParams.useHints = !params["nohints"].as<bool>(); } if (params.count("nopreallocj")) { - storageGlobalParams.preallocj = false; + storageGlobalParams.preallocj = params["nopreallocj"].as<bool>(); } if (params.count("net.http.RESTInterfaceEnabled")) { - serverGlobalParams.rest = true; + serverGlobalParams.rest = params["net.http.RESTInterfaceEnabled"].as<bool>(); } if (params.count("net.http.JSONPEnabled")) { - serverGlobalParams.jsonp = true; + serverGlobalParams.jsonp = params["net.http.JSONPEnabled"].as<bool>(); } if (params.count("noscripting")) { - mongodGlobalParams.scriptingEnabled = false; + mongodGlobalParams.scriptingEnabled = !params["noscripting"].as<bool>(); } if (params.count("storage.preallocDataFiles")) { storageGlobalParams.prealloc = params["storage.preallocDataFiles"].as<bool>(); cout << "note: noprealloc may hurt performance in many applications" << endl; } if (params.count("storage.smallFiles")) { - storageGlobalParams.smallfiles = true; + storageGlobalParams.smallfiles = params["storage.smallFiles"].as<bool>(); } if (params.count("diaglog")) { warning() << "--diaglog is deprecated and will be removed in a future release"; @@ -918,31 +957,31 @@ namespace mongo { "Can't have journaling enabled when using --repair option."); } - if (params.count("repair")) { + if (params.count("repair") && params["repair"].as<bool>() == true) { mongodGlobalParams.upgrade = 1; // --repair implies --upgrade mongodGlobalParams.repair = 1; storageGlobalParams.dur = false; } - if (params.count("upgrade")) { + if (params.count("upgrade") && params["upgrade"].as<bool>() == true) { mongodGlobalParams.upgrade = 1; } if (params.count("notablescan")) { - storageGlobalParams.noTableScan = true; + storageGlobalParams.noTableScan = params["notablescan"].as<bool>(); } if (params.count("master")) { - replSettings.master = true; + replSettings.master = params["master"].as<bool>(); } - if (params.count("slave")) { + if (params.count("slave") && params["slave"].as<bool>() == true) { replSettings.slave = SimpleSlave; } if (params.count("slavedelay")) { replSettings.slavedelay = params["slavedelay"].as<int>(); } if (params.count("fastsync")) { - replSettings.fastsync = true; + replSettings.fastsync = params["fastsync"].as<bool>(); } if (params.count("autoresync")) { - replSettings.autoresync = true; + replSettings.autoresync = params["autoresync"].as<bool>(); } if (params.count("source")) { /* specifies what the source in local.sources should be */ @@ -1043,9 +1082,6 @@ namespace mongo { if (!params.count("replication.oplogSizeMB")) replSettings.oplogSize = 5 * 1024 * 1024; } - if (params.count("net.ipv6")) { - enableIPv6(); - } if (params.count("sharding.archiveMovedChunks")) { serverGlobalParams.moveParanoia = params["sharding.archiveMovedChunks"].as<bool>(); diff --git a/src/mongo/db/server_options_helpers.cpp b/src/mongo/db/server_options_helpers.cpp index 2d000598eb0..5c789660283 100644 --- a/src/mongo/db/server_options_helpers.cpp +++ b/src/mongo/db/server_options_helpers.cpp @@ -159,6 +159,9 @@ namespace { options->addOptionChaining("net.bindIp", "bind_ip", moe::String, "comma separated list of ip addresses to listen on - all local ips by default"); + options->addOptionChaining("net.ipv6", "ipv6", moe::Switch, + "enable IPv6 support (disabled by default)"); + options->addOptionChaining("net.maxIncomingConnections", "maxConns", moe::Int, maxConnInfoBuilder.str().c_str()); @@ -181,7 +184,7 @@ namespace { #ifndef _WIN32 options->addOptionChaining("syslog", "syslog", moe::Switch, "log to system's syslog facility instead of file or stdout") - .incompatibleWith("systemLog.logpath") + .incompatibleWith("logpath") .setSources(moe::SourceAllLegacy); options->addOptionChaining("systemLog.syslogFacility", "syslogFacility", moe::String, @@ -405,7 +408,8 @@ namespace { // "net.wireObjectCheck" comes from the config file, so override it if either "objcheck" or // "noobjcheck" are set, since those come from the command line. if (params->count("objcheck")) { - Status ret = params->set("net.wireObjectCheck", moe::Value(true)); + Status ret = params->set("net.wireObjectCheck", + moe::Value((*params)["objcheck"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -416,7 +420,8 @@ namespace { } if (params->count("noobjcheck")) { - Status ret = params->set("net.wireObjectCheck", moe::Value(false)); + Status ret = params->set("net.wireObjectCheck", + moe::Value(!(*params)["noobjcheck"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -429,7 +434,8 @@ namespace { // "net.http.enabled" comes from the config file, so override it if "nohttpinterface" or // "httpinterface" are set since those come from the command line. if (params->count("nohttpinterface")) { - Status ret = params->set("net.http.enabled", moe::Value(false)); + Status ret = params->set("net.http.enabled", + moe::Value(!(*params)["nohttpinterface"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -439,7 +445,8 @@ namespace { } } if (params->count("httpinterface")) { - Status ret = params->set("net.http.enabled", moe::Value(true)); + Status ret = params->set("net.http.enabled", + moe::Value((*params)["httpinterface"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -452,7 +459,8 @@ namespace { // "net.unixDomainSocket.enabled" comes from the config file, so override it if // "nounixsocket" is set since that comes from the command line. if (params->count("nounixsocket")) { - Status ret = params->set("net.unixDomainSocket.enabled", moe::Value(false)); + Status ret = params->set("net.unixDomainSocket.enabled", + moe::Value(!(*params)["nounixsocket"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -466,7 +474,7 @@ namespace { // that we ensure that we set the log level to the maximum of the options provided int logLevel = -1; for (std::string s = ""; s.length() <= 14; s.append("v")) { - if (!s.empty() && params->count(s)) { + if (!s.empty() && params->count(s) && (*params)[s].as<bool>() == true) { logLevel = s.length(); } @@ -523,7 +531,7 @@ namespace { // "systemLog.destination" comes from the config file, so override it if "syslog" is set // since that comes from the command line. - if (params->count("syslog")) { + if (params->count("syslog") && (*params)["syslog"].as<bool>() == true) { Status ret = params->set("systemLog.destination", moe::Value(std::string("syslog"))); if (!ret.isOK()) { return ret; @@ -578,10 +586,12 @@ namespace { } if (params.count("enableExperimentalIndexStatsCmd")) { - serverGlobalParams.experimental.indexStatsCmdEnabled = true; + serverGlobalParams.experimental.indexStatsCmdEnabled = + params["enableExperimentalIndexStatsCmd"].as<bool>(); } if (params.count("enableExperimentalStorageDetailsCmd")) { - serverGlobalParams.experimental.storageDetailsCmdEnabled = true; + serverGlobalParams.experimental.storageDetailsCmdEnabled = + params["enableExperimentalStorageDetailsCmd"].as<bool>(); } if (params.count("net.port")) { @@ -592,6 +602,10 @@ namespace { serverGlobalParams.bind_ip = params["net.bindIp"].as<std::string>(); } + if (params.count("net.ipv6") && params["net.ipv6"].as<bool>() == true) { + enableIPv6(); + } + if (params.count("net.http.enabled")) { serverGlobalParams.isHttpInterfaceEnabled = params["net.http.enabled"].as<bool>(); } @@ -624,11 +638,11 @@ namespace { } if (params.count("systemLog.quiet")) { - serverGlobalParams.quiet = true; + serverGlobalParams.quiet = params["systemLog.quiet"].as<bool>(); } if (params.count("systemLog.traceAllExceptions")) { - DBException::traceExceptions = true; + DBException::traceExceptions = params["systemLog.traceAllExceptions"].as<bool>(); } if (params.count("net.maxIncomingConnections")) { @@ -661,7 +675,9 @@ namespace { serverGlobalParams.noUnixSocket = !params["net.unixDomainSocket.enabled"].as<bool>(); } - if (params.count("processManagement.fork") && !params.count("shutdown")) { + if ((params.count("processManagement.fork") && + params["processManagement.fork"].as<bool>() == true) && + (!params.count("shutdown") || params["shutdown"].as<bool>() == false)) { serverGlobalParams.doFork = true; } #endif // _WIN32 @@ -745,7 +761,11 @@ namespace { } #endif // _WIN32 - serverGlobalParams.logAppend = params.count("systemLog.logAppend"); + if (params.count("systemLog.logAppend") && + params["systemLog.logAppend"].as<bool>() == true) { + serverGlobalParams.logAppend = true; + } + if (!serverGlobalParams.logpath.empty() && serverGlobalParams.logWithSyslog) { return Status(ErrorCodes::BadValue, "Cant use both a logpath and syslog "); } diff --git a/src/mongo/s/mongos_options.cpp b/src/mongo/s/mongos_options.cpp index 52aeb4def09..d30b86feb5a 100644 --- a/src/mongo/s/mongos_options.cpp +++ b/src/mongo/s/mongos_options.cpp @@ -91,9 +91,6 @@ namespace mongo { sharding_options.addOptionChaining("sharding.chunkSize", "chunkSize", moe::Int, "maximum amount of data per chunk"); - sharding_options.addOptionChaining("net.ipv6", "ipv6", moe::Switch, - "enable IPv6 support (disabled by default)"); - sharding_options.addOptionChaining("net.http.JSONPEnabled", "jsonp", moe::Switch, "allow JSONP access via http (has security implications)") .setSources(moe::SourceAllLegacy); @@ -134,15 +131,18 @@ namespace mongo { bool handlePreValidationMongosOptions(const moe::Environment& params, const std::vector<std::string>& args) { - if (params.count("help")) { + if (params.count("help") && + params["help"].as<bool>() == true) { printMongosHelp(moe::startupOptions); return false; } - if (params.count("version")) { + if (params.count("version") && + params["version"].as<bool>() == true) { printShardingVersionInfo(true); return false; } - if ( params.count( "test" ) ) { + if (params.count("test") && + params["test"].as<bool>() == true) { ::mongo::logger::globalLogDomain()->setMinimumLoggedSeverity( ::mongo::logger::LogSeverity::Debug(5)); StartupTest::runTests(); @@ -179,7 +179,8 @@ namespace mongo { // "sharding.autoSplit" comes from the config file, so override it if "noAutoSplit" is set // since that comes from the command line. if (params->count("noAutoSplit")) { - Status ret = params->set("sharding.autoSplit", moe::Value(false)); + Status ret = params->set("sharding.autoSplit", + moe::Value(!(*params)["noAutoSplit"].as<bool>())); if (!ret.isOK()) { return ret; } @@ -226,12 +227,8 @@ namespace mongo { params["replication.localPingThresholdMs"].as<int>(); } - if ( params.count( "net.ipv6" ) ) { - enableIPv6(); - } - if (params.count("net.http.JSONPEnabled")) { - serverGlobalParams.jsonp = true; + serverGlobalParams.jsonp = params["net.http.JSONPEnabled"].as<bool>(); } if (params.count("noscripting")) { @@ -260,7 +257,9 @@ namespace mongo { << "and is not recommended for production" << endl; } - mongosGlobalParams.upgrade = params.count("upgrade"); + if (params.count("upgrade")) { + mongosGlobalParams.upgrade = params["upgrade"].as<bool>(); + } return Status::OK(); } diff --git a/src/mongo/util/net/ssl_options.cpp b/src/mongo/util/net/ssl_options.cpp index 18551b84b85..38538f6019b 100644 --- a/src/mongo/util/net/ssl_options.cpp +++ b/src/mongo/util/net/ssl_options.cpp @@ -98,7 +98,8 @@ namespace mongo { Status canonicalizeSSLServerOptions(moe::Environment* params) { - if (params->count("net.ssl.sslOnNormalPorts")) { + if (params->count("net.ssl.sslOnNormalPorts") && + (*params)["net.ssl.sslOnNormalPorts"].as<bool>() == true) { Status ret = params->set("net.ssl.mode", moe::Value(std::string("requireSSL"))); if (!ret.isOK()) { return ret; @@ -164,13 +165,15 @@ namespace mongo { } if (params.count("net.ssl.weakCertificateValidation")) { - sslGlobalParams.sslWeakCertificateValidation = true; + sslGlobalParams.sslWeakCertificateValidation = + params["net.ssl.weakCertificateValidation"].as<bool>(); } if (params.count("net.ssl.allowInvalidCertificates")) { - sslGlobalParams.sslAllowInvalidCertificates = true; + sslGlobalParams.sslAllowInvalidCertificates = + params["net.ssl.allowInvalidCertificates"].as<bool>(); } if (params.count("net.ssl.FIPSMode")) { - sslGlobalParams.sslFIPSMode = true; + sslGlobalParams.sslFIPSMode = params["net.ssl.FIPSMode"].as<bool>(); } if (sslGlobalParams.sslMode.load() != SSLGlobalParams::SSLMode_disabled) { @@ -223,7 +226,7 @@ namespace mongo { } Status storeSSLClientOptions(const moe::Environment& params) { - if (params.count("ssl")) { + if (params.count("ssl") && params["ssl"].as<bool>() == true) { sslGlobalParams.sslMode.store(SSLGlobalParams::SSLMode_requireSSL); } if (params.count("ssl.PEMKeyFile")) { diff --git a/src/mongo/util/options_parser/option_description.h b/src/mongo/util/options_parser/option_description.h index 0604f79ced4..40be5f552aa 100644 --- a/src/mongo/util/options_parser/option_description.h +++ b/src/mongo/util/options_parser/option_description.h @@ -27,6 +27,9 @@ namespace optionenvironment { /** * An OptionType is an enum of all the types we support in the OptionsParser + * + * NOTE(sverch): The semantics of "Switch" options are completely identical to "Bool" options, + * except that on the command line they do not take a value. */ enum OptionType { StringVector, // po::value< std::vector<std::string> > diff --git a/src/mongo/util/options_parser/option_section.cpp b/src/mongo/util/options_parser/option_section.cpp index 2addd8d5d77..d3bacb04b95 100644 --- a/src/mongo/util/options_parser/option_section.cpp +++ b/src/mongo/util/options_parser/option_section.cpp @@ -82,7 +82,8 @@ namespace optionenvironment { Status typeToBoostType(std::auto_ptr<po::value_semantic>* boostType, OptionType type, const Value defaultValue = Value(), - const Value implicitValue = Value()) { + const Value implicitValue = Value(), + bool getSwitchAsBool = false) { switch (type) { case StringVector: { @@ -124,6 +125,20 @@ namespace optionenvironment { return Status::OK(); } + case Switch: + { + // In boost, switches default to false which makes it impossible to tell if + // a switch in a config file is not present or was explicitly set to false. + // + // Because of this, and because of the fact that we use the same set of + // options for the legacy key=value config file, we need a way to control + // whether we are telling boost that an option is a switch type or that an + // option is a bool type. + if (!getSwitchAsBool) { + *boostType = std::auto_ptr<po::value_semantic>(po::bool_switch()); + return Status::OK(); + } + } case Bool: { std::auto_ptr<po::typed_value<bool> > boostTypeBuilder(po::value<bool>()); @@ -338,11 +353,6 @@ namespace optionenvironment { return Status::OK(); } - case Switch: - { - *boostType = std::auto_ptr<po::value_semantic>(po::bool_switch()); - return Status::OK(); - } default: { StringBuilder sb; @@ -369,7 +379,8 @@ namespace optionenvironment { Status ret = typeToBoostType(&boostType, oditerator->_type, includeDefaults ? oditerator->_default : Value(), - oditerator->_implicit); + oditerator->_implicit, + !(sources & SourceCommandLine)); if (!ret.isOK()) { StringBuilder sb; sb << "Error getting boost type for option \"" diff --git a/src/mongo/util/options_parser/options_parser.cpp b/src/mongo/util/options_parser/options_parser.cpp index 00b3b5d09ff..42c5c2cdd0c 100644 --- a/src/mongo/util/options_parser/options_parser.cpp +++ b/src/mongo/util/options_parser/options_parser.cpp @@ -221,10 +221,7 @@ namespace optionenvironment { return Status::OK(); } else if (stringVal == "false") { - // XXX: Don't set switches that are false, to maintain backwards - // compatibility with the old behavior since some code depends on this - // behavior - *value = Value(); + *value = Value(false); return Status::OK(); } else { @@ -345,19 +342,6 @@ namespace optionenvironment { return ret; } - // XXX: Don't set switches that are false, to maintain backwards compatibility - // with the old behavior during the transition to the new parser - if (iterator->_type == Switch) { - bool value; - ret = optionValue.get(&value); - if (!ret.isOK()) { - return ret; - } - if (!value) { - continue; - } - } - // If this is really a StringMap, try to split on "key=value" for each element // in our StringVector if (iterator->_type == StringMap) { @@ -595,6 +579,45 @@ namespace optionenvironment { return Status::OK(); } + /** + * Remove any options of type "Switch" that are set to false. This is needed because boost + * defaults switches to false, and we need to be able to tell the difference between + * whether an option is set explicitly to false in config files or not present at all. + */ + Status removeFalseSwitches(const OptionSection& options, Environment* environment) { + std::vector<OptionDescription> options_vector; + Status ret = options.getAllOptions(&options_vector); + if (!ret.isOK()) { + return ret; + } + + for (std::vector<OptionDescription>::const_iterator iterator = options_vector.begin(); + iterator != options_vector.end(); iterator++) { + + if (iterator->_type == Switch) { + bool switchValue; + Status ret = environment->get(iterator->_dottedName, &switchValue); + if (!ret.isOK() && ret != ErrorCodes::NoSuchKey) { + StringBuilder sb; + sb << "Error getting switch value for option: " << iterator->_dottedName + << " from source: " << ret.toString(); + return Status(ErrorCodes::InternalError, sb.str()); + } + else if (ret.isOK() && switchValue == false) { + Status ret = environment->remove(iterator->_dottedName); + if (!ret.isOK()) { + StringBuilder sb; + sb << "Error removing false flag: " << iterator->_dottedName << ": " + << ret.toString(); + return Status(ErrorCodes::InternalError, sb.str()); + } + } + } + } + + return Status::OK(); + } + } // namespace /** @@ -673,6 +696,14 @@ namespace optionenvironment { sb << "Error parsing command line: " << e.what(); return Status(ErrorCodes::BadValue, sb.str()); } + + // This is needed because "switches" default to false in boost, and we don't want to + // erroneously think that they were present but set to false in a config file. + ret = removeFalseSwitches(options, environment); + if (!ret.isOK()) { + return ret; + } + return Status::OK(); } diff --git a/src/mongo/util/options_parser/options_parser_test.cpp b/src/mongo/util/options_parser/options_parser_test.cpp index 7a90663f218..dcbdf1527cf 100644 --- a/src/mongo/util/options_parser/options_parser_test.cpp +++ b/src/mongo/util/options_parser/options_parser_test.cpp @@ -964,6 +964,46 @@ namespace { ASSERT_EQUALS(str, "NotCommented"); } + // Ensure switches can be set to false in INI config files and handled differently from the "not + // present" case. + TEST(INIConfigFile, Switches) { + OptionsParserTester parser; + moe::Environment environment; + + moe::OptionSection testOpts; + testOpts.addOptionChaining("config", "config", moe::String, "Config file to parse"); + testOpts.addOptionChaining("switch1", "switch1", moe::Switch, "switch1"); + testOpts.addOptionChaining("switch2", "switch2", moe::Switch, "switch2"); + testOpts.addOptionChaining("switch3", "switch3", moe::Switch, "switch3"); + testOpts.addOptionChaining("switch4", "switch4", moe::Switch, "switch4"); + + std::vector<std::string> argv; + argv.push_back("binaryname"); + argv.push_back("--config"); + argv.push_back("default.conf"); + argv.push_back("--switch1"); + std::map<std::string, std::string> env_map; + + parser.setConfig("default.conf", "switch2=true\nswitch3=false"); + + // Switches have the following semantics: + // - Present on the command line -> set to true + // - Present in the config file -> set to value in config file + // - Not present -> not set to any value + ASSERT_OK(parser.run(testOpts, argv, env_map, &environment)); + bool switch1; + ASSERT_OK(environment.get(moe::Key("switch1"), &switch1)); + ASSERT_TRUE(switch1); + bool switch2; + ASSERT_OK(environment.get(moe::Key("switch2"), &switch2)); + ASSERT_TRUE(switch2); + bool switch3; + ASSERT_OK(environment.get(moe::Key("switch3"), &switch3)); + ASSERT_FALSE(switch3); + bool switch4; + ASSERT_NOT_OK(environment.get(moe::Key("switch4"), &switch4)); + } + TEST(INIConfigFile, Monkeys) { OptionsParserTester parser; moe::Environment environment; @@ -987,7 +1027,10 @@ namespace { ASSERT_OK(parser.run(testOpts, argv, env_map, &environment)); moe::Value value; - ASSERT_NOT_OK(environment.get(moe::Key("this"), &value)); + ASSERT_OK(environment.get(moe::Key("this"), &value)); + bool thisValue; + ASSERT_OK(value.get(&thisValue)); + ASSERT_FALSE(thisValue); ASSERT_NOT_OK(environment.get(moe::Key("that"), &value)); ASSERT_NOT_OK(environment.get(moe::Key("another"), &value)); ASSERT_OK(environment.get(moe::Key("other"), &value)); @@ -1527,12 +1570,8 @@ namespace { environment = moe::Environment(); parser.setConfig("config.json", "{ switchVal : false }"); ASSERT_OK(parser.run(testOpts, argv, env_map, &environment)); - - // A switch not being present results in it not getting added to the map for legacy reasons. - // The downside of this is that switches can't override a default value of "true" in a - // config file. We should change this once we elminate the places in the code that depend - // on this behavior. - ASSERT_NOT_OK(environment.get(moe::Key("switchVal"), &value)); + ASSERT_OK(environment.get(moe::Key("switchVal"), &switchVal)); + ASSERT_FALSE(switchVal); } TEST(JSONConfigFile, Nested) { @@ -3525,12 +3564,8 @@ namespace { environment = moe::Environment(); parser.setConfig("config.json", "switchVal : false"); ASSERT_OK(parser.run(testOpts, argv, env_map, &environment)); - - // A switch not being present results in it not getting added to the map for legacy reasons. - // The downside of this is that switches can't override a default value of "true" in a - // config file. We should change this once we elminate the places in the code that depend - // on this behavior. - ASSERT_NOT_OK(environment.get(moe::Key("switchVal"), &value)); + ASSERT_OK(environment.get(moe::Key("switchVal"), &switchVal)); + ASSERT_FALSE(switchVal); } TEST(YAMLConfigFile, Nested) { |