This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch 1.9.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d2381ebce99d490fcc93e6212b02fe42e02b6e47 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 415d616..3aabea1 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 f28eb27..dab912f 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -575,7 +575,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); @@ -913,7 +915,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); @@ -1066,7 +1070,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);
