You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2018/11/20 19:16:45 UTC
[mesos] branch 1.6.x updated (639d4f2 -> 4b5aa1c)
This is an automated email from the ASF dual-hosted git repository.
tillt pushed a change to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 639d4f2 Fixed an issue about inheriting user for nested containers.
new 35869e5 Added collectAuthorizations helper to master.hpp.
new 4b5aa1c Added MESOS-9317 to the 1.6.2 CHANGELOG.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGELOG | 1 +
src/master/master.cpp | 54 ++----------------
src/master/master.hpp | 15 +++++
src/master/weights_handler.cpp | 12 +---
src/tests/master_tests.cpp | 122 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 144 insertions(+), 60 deletions(-)
[mesos] 01/02: Added collectAuthorizations helper to master.hpp.
Posted by ti...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tillt pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 35869e5966c59a4047b8e409e7795e38a6b6d5c0
Author: Till Toenshoff <to...@me.com>
AuthorDate: Tue Nov 20 14:45:50 2018 +0100
Added collectAuthorizations helper to master.hpp.
Adds the helper function 'collectAuthorizations' to master.hpp. This
function allows for a simple way to collect authorization futures and
only if all supplied futures result in an approved authorization will
the returned future return true.
All identified areas that were formally triggering MESOS-9317 are
being updated to make use of this new helper.
A helper function has been chosen and preferred over copying this
pattern into the areas that needed a fix to allow for an efficient and
complete test coverage.
Additionally we are adding a test validating that new helper.
Review: https://reviews.apache.org/r/69369/
** Backported for 1.6.2 **
---
src/master/master.cpp | 54 ++----------------
src/master/master.hpp | 15 +++++
src/master/weights_handler.cpp | 12 +---
src/tests/master_tests.cpp | 122 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 143 insertions(+), 60 deletions(-)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 3fdc791..3f923a7 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3649,17 +3649,7 @@ Future<bool> Master::authorizeReserveResources(
return authorizer.get()->authorized(request);
}
- return await(authorizations)
- .then([](const list<Future<bool>>& authorizations)
- -> Future<bool> {
- // Compute a disjunction.
- foreach (const Future<bool>& authorization, authorizations) {
- if (!authorization.get()) {
- return false;
- }
- }
- return true;
- });
+ return collectAuthorizations(authorizations);
}
@@ -3712,17 +3702,7 @@ Future<bool> Master::authorizeUnreserveResources(
return authorizer.get()->authorized(request);
}
- return await(authorizations)
- .then([](const list<Future<bool>>& authorizations)
- -> Future<bool> {
- // Compute a disjunction.
- foreach (const Future<bool>& authorization, authorizations) {
- if (!authorization.get()) {
- return false;
- }
- }
- return true;
- });
+ return collectAuthorizations(authorizations);
}
@@ -3779,17 +3759,7 @@ Future<bool> Master::authorizeCreateVolume(
return authorizer.get()->authorized(request);
}
- return await(authorizations)
- .then([](const list<Future<bool>>& authorizations)
- -> Future<bool> {
- // Compute a disjunction.
- foreach (const Future<bool>& authorization, authorizations) {
- if (!authorization.get()) {
- return false;
- }
- }
- return true;
- });
+ return collectAuthorizations(authorizations);
}
@@ -3831,17 +3801,7 @@ Future<bool> Master::authorizeDestroyVolume(
return authorizer.get()->authorized(request);
}
- return await(authorizations)
- .then([](const list<Future<bool>>& authorizations)
- -> Future<bool> {
- // Compute a disjunction.
- foreach (const Future<bool>& authorization, authorizations) {
- if (!authorization.get()) {
- return false;
- }
- }
- return true;
- });
+ return collectAuthorizations(authorizations);
}
@@ -3924,11 +3884,7 @@ Future<bool> Master::authorizeSlave(
authorizeReserveResources(slaveInfo.resources(), principal));
}
- return collect(authorizations)
- .then([](const list<bool>& results)
- -> Future<bool> {
- return std::find(results.begin(), results.end(), false) == results.end();
- });
+ return collectAuthorizations(authorizations);
}
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 5ec764b..f01b743 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -19,6 +19,7 @@
#include <stdint.h>
+#include <algorithm>
#include <list>
#include <memory>
#include <set>
@@ -44,6 +45,8 @@
#include <mesos/scheduler/scheduler.hpp>
+#include <process/collect.hpp>
+#include <process/future.hpp>
#include <process/limiter.hpp>
#include <process/http.hpp>
#include <process/owned.hpp>
@@ -2204,6 +2207,18 @@ private:
};
+// Collects authorization results. Any discarded or failed future
+// results in a failure; any false future results in false.
+inline process::Future<bool> collectAuthorizations(
+ const std::list<process::Future<bool>>& authorizations)
+{
+ return process::collect(authorizations)
+ .then([](const std::list<bool>& results) -> process::Future<bool> {
+ return std::find(results.begin(), results.end(), false) == results.end();
+ });
+}
+
+
inline std::ostream& operator<<(
std::ostream& stream,
const Framework& framework);
diff --git a/src/master/weights_handler.cpp b/src/master/weights_handler.cpp
index 1053652..cd3a919 100644
--- a/src/master/weights_handler.cpp
+++ b/src/master/weights_handler.cpp
@@ -346,17 +346,7 @@ Future<bool> Master::WeightsHandler::authorizeUpdateWeights(
return master->authorizer.get()->authorized(request);
}
- return await(authorizations)
- .then([](const list<Future<bool>>& authorizations)
- -> Future<bool> {
- // Compute a disjunction.
- foreach (const Future<bool>& authorization, authorizations) {
- if (!authorization.get()) {
- return false;
- }
- }
- return true;
- });
+ return collectAuthorizations(authorizations);
}
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 6a3a503..69bcbad 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -105,6 +105,7 @@ using process::PID;
using process::Promise;
using process::http::Accepted;
+using process::http::InternalServerError;
using process::http::OK;
using process::http::Response;
using process::http::Unauthorized;
@@ -9754,6 +9755,127 @@ TEST_P(MasterTestPrePostReservationRefinement, CreateAndDestroyVolumesV1)
AWAIT_EXPECT_RESPONSE_STATUS_EQ(Accepted().status, v1DestroyVolumesResponse);
}
+
+// This test validates that an authorization error when requesting
+// volume creation does result in an internal server error.
+// See MESOS-9317.
+TEST_F(MasterTest, CreateVolumesV1AuthorizationFailure)
+{
+ MockAuthorizer authorizer;
+ Try<Owned<cluster::Master>> master = StartMaster(&authorizer);
+ ASSERT_SOME(master);
+
+ // For capturing the `SlaveID` so we can use it in the create/destroy
+ // volumes API call.
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ // Do static reservation so we can create persistent volumes from it.
+ slaveFlags.resources = "disk(role1):1024";
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
+
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+ SlaveID slaveId = slaveRegisteredMessage->slave_id();
+
+ // Create the persistent volume.
+ v1::master::Call v1CreateVolumesCall;
+ v1CreateVolumesCall.set_type(v1::master::Call::CREATE_VOLUMES);
+ v1::master::Call_CreateVolumes* createVolumes =
+ v1CreateVolumesCall.mutable_create_volumes();
+
+ Resources volume = createPersistentVolume(
+ Megabytes(64),
+ "role1",
+ "id1",
+ "path1",
+ None(),
+ None(),
+ DEFAULT_CREDENTIAL.principal());
+
+ createVolumes->mutable_agent_id()->CopyFrom(evolve(slaveId));
+ createVolumes->mutable_volumes()->CopyFrom(v1::Resources(evolve(volume)));
+
+ ContentType contentType = ContentType::PROTOBUF;
+
+ Promise<bool> promise;
+ EXPECT_CALL(authorizer, authorized(_))
+ .WillOnce(Return(promise.future()));
+
+ Future<Response> response = process::http::post(
+ master.get()->pid,
+ "api/v1",
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+ serialize(contentType, v1CreateVolumesCall),
+ stringify(contentType));
+
+ promise.fail("Authorizer failure");
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(InternalServerError().status, response);
+}
+
+
+// Test for the authorization collect helper.
+TEST_F(MasterTest, CollectAuthorizations)
+{
+ {
+ Promise<bool> promise1;
+ Promise<bool> promise2;
+
+ Future<bool> result =
+ master::collectAuthorizations({promise1.future(), promise2.future()});
+
+ promise1.set(true);
+ promise2.set(false);
+
+ AWAIT_EXPECT_FALSE(result);
+ }
+
+ {
+ Promise<bool> promise1;
+ Promise<bool> promise2;
+
+ Future<bool> result =
+ master::collectAuthorizations({promise1.future(), promise2.future()});
+
+ promise1.set(true);
+ promise2.fail("Authorization failure");
+
+ AWAIT_EXPECT_FAILED(result);
+ }
+
+ {
+ Promise<bool> promise1;
+ Promise<bool> promise2;
+
+ Future<bool> result =
+ master::collectAuthorizations({promise1.future(), promise2.future()});
+
+ promise1.set(true);
+ promise2.discard();
+
+ AWAIT_EXPECT_FAILED(result);
+ }
+
+ {
+ Promise<bool> promise1;
+ Promise<bool> promise2;
+
+ Future<bool> result =
+ master::collectAuthorizations({promise1.future(), promise2.future()});
+
+ promise1.set(true);
+ promise2.set(true);
+
+ AWAIT_EXPECT_TRUE(result);
+ }
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {
[mesos] 02/02: Added MESOS-9317 to the 1.6.2 CHANGELOG.
Posted by ti...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tillt pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 4b5aa1c39f649a8760db5d58b3f2c05994c7902c
Author: Till Toenshoff <to...@me.com>
AuthorDate: Tue Nov 20 17:35:15 2018 +0100
Added MESOS-9317 to the 1.6.2 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index d8b3bc6..d1112bf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,7 @@ Release Notes - Mesos - Version 1.6.2 (WIP)
* [MESOS-9279] - Docker Containerizer 'usage' call might be expensive if mount table is big.
* [MESOS-9283] - Docker containerizer actor can get backlogged with large number of containers.
* [MESOS-9308] - URI disk profile adaptor could deadlock.
+ * [MESOS-9317] - Some master endpoints do not handle failed authorization properly.
* [MESOS-9332] - Nested container should run as the same user of its parent container by default.
* [MESOS-9334] - Container stuck at ISOLATING state due to libevent poll never returns.