Repository: mesos Updated Branches: refs/heads/master a90ed2bf5 -> 89c6cea11
Validated that quota requests contain unique resource names. This also adds a test. Review: https://reviews.apache.org/r/52284/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/89c6cea1 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/89c6cea1 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/89c6cea1 Branch: refs/heads/master Commit: 89c6cea11427fef36b0161620c451d0587f1d205 Parents: a90ed2b Author: Zhitao Li <zhitaoli...@gmail.com> Authored: Wed Sep 27 13:46:11 2017 -0700 Committer: Benjamin Mahler <bmah...@apache.org> Committed: Wed Sep 27 13:46:11 2017 -0700 ---------------------------------------------------------------------- src/master/quota.cpp | 10 ++++++++++ src/tests/master_quota_tests.cpp | 32 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/89c6cea1/src/master/quota.cpp ---------------------------------------------------------------------- diff --git a/src/master/quota.cpp b/src/master/quota.cpp index 79e4ad8..58bab6a 100644 --- a/src/master/quota.cpp +++ b/src/master/quota.cpp @@ -133,6 +133,8 @@ Option<Error> quotaInfo(const QuotaInfo& quotaInfo) return Error("QuotaInfo with empty 'guarantee'"); } + hashset<string> names; + foreach (const Resource& resource, quotaInfo.guarantee()) { // Check that `resource` does not contain fields that are // irrelevant for quota. @@ -151,6 +153,14 @@ Option<Error> quotaInfo(const QuotaInfo& quotaInfo) if (resource.type() != Value::SCALAR) { return Error("QuotaInfo must not include non-scalar resources"); } + + // Check that resource names do not repeat. + if (names.contains(resource.name())) { + return Error("QuotaInfo contains duplicate resource name" + " '" + resource.name() + "'"); + } + + names.insert(resource.name()); } return None(); http://git-wip-us.apache.org/repos/asf/mesos/blob/89c6cea1/src/tests/master_quota_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp index 5e3f2d2..f9feb67 100644 --- a/src/tests/master_quota_tests.cpp +++ b/src/tests/master_quota_tests.cpp @@ -31,6 +31,7 @@ #include <process/owned.hpp> #include <process/pid.hpp> +#include <stout/format.hpp> #include <stout/protobuf.hpp> #include <stout/stringify.hpp> #include <stout/strings.hpp> @@ -159,7 +160,6 @@ protected: // * Role is absent. // * Role is an empty string. // * Role is '*'? -// * Resources with the same name are present. // Verifies that a request for a non-existent role is rejected when // using an explicitly configured list of role names. @@ -314,6 +314,36 @@ TEST_F(MasterQuotaTest, SetRequestWithInvalidData) << response->body; } + // A quota set request with a duplicate resource name should + // return '400 Bad Request'. + { + Resource cpusResource = Resources::parse("cpus", "1", "*").get(); + cpusResource.clear_role(); + + // Manually construct a bad request because `Resources` class merges + // resources with same name so we cannot use it. + const string badRequest = strings::format( + "{\"role\": \"%s\", \"guarantee\":[%s, %s]}", + ROLE1, + stringify(JSON::protobuf(cpusResource)), + stringify(JSON::protobuf(cpusResource))).get(); + + Future<Response> response = process::http::post( + master.get()->pid, + "quota", + createBasicAuthHeaders(DEFAULT_CREDENTIAL), + badRequest); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); + + AWAIT_EXPECT_RESPONSE_BODY_EQ( + "Failed to validate set quota request:" + " QuotaInfo contains duplicate resource name 'cpus'", + response) + << response->body; + } + + // A quota set request with the `DiskInfo` field set should return // '400 Bad Request'. {
