This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit e9edd205a09fa10df2c6e36e42b7db7e0ad7a005 Author: Benjamin Mahler <[email protected]> AuthorDate: Wed Oct 9 00:06:36 2019 -0400 Rejected invalid quota configs from registry operation. The registry operation for updating quota was assuming that the quota configs were valid. This validates them and fails the operation if invalid. Note that the caller should be validating this already, but this serves as an extra guard. Review: https://reviews.apache.org/r/71597 --- src/master/quota.cpp | 18 ++++++++++++++++++ src/master/quota_handler.cpp | 12 +++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/master/quota.cpp b/src/master/quota.cpp index a9ae3aa..dea2ef6 100644 --- a/src/master/quota.cpp +++ b/src/master/quota.cpp @@ -57,6 +57,24 @@ UpdateQuota::UpdateQuota( Try<bool> UpdateQuota::perform( Registry* registry, hashset<SlaveID>* /*slaveIDs*/) { + // Sanity check that we're not writing any invalid configs. + foreach (const QuotaConfig& config, configs) { + Option<Error> error = validate(config); + + if (error.isSome()) { + // We also log it since this error won't currently surface + // in logging or to the end client that is attempting the + // update. + LOG(ERROR) << "Invalid quota config" + << " '" << jsonify(JSON::Protobuf(config)) << "'" + << ": " + error->message; + + return Error("Invalid quota config" + " '" + string(jsonify(JSON::Protobuf(config))) + "'" + ": " + error->message); + } + } + google::protobuf::RepeatedPtrField<QuotaConfig>& registryConfigs = *registry->mutable_quota_configs(); diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index 083ee30..9cfe76b 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -572,7 +572,9 @@ Future<http::Response> Master::QuotaHandler::_update( ->apply(Owned<RegistryOperation>(new quota::UpdateQuota(configs))) .then(defer(master->self(), [=](bool result) -> Future<http::Response> { // Currently, quota registry entry mutation never fails. - CHECK(result); + CHECK(result) + << "An invalid quota config was supplied to the registry " + << JSON::protobuf(configs); foreach (const QuotaConfig& config, configs) { master->quotas[config.role()] = Quota(config); @@ -899,7 +901,9 @@ Future<http::Response> Master::QuotaHandler::__set( ->apply(Owned<RegistryOperation>(new quota::UpdateQuota(configs))) .then(defer(master->self(), [=](bool result) -> Future<http::Response> { // See the top comment in "master/quota.hpp" for why this check is here. - CHECK(result); + CHECK(result) + << "An invalid quota config was supplied to the registry " + << JSON::protobuf(configs); master->allocator->updateQuota(quotaInfo.role(), quota); @@ -1052,7 +1056,9 @@ Future<http::Response> Master::QuotaHandler::__remove(const string& role) const ->apply(Owned<RegistryOperation>(new quota::UpdateQuota(configs))) .then(defer(master->self(), [=](bool result) -> Future<http::Response> { // See the top comment in "master/quota.hpp" for why this check is here. - CHECK(result); + CHECK(result) + << "An invalid quota config was supplied to the registry " + << JSON::protobuf(configs); master->allocator->updateQuota(role, DEFAULT_QUOTA);
