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:32 UTC
[mesos] branch 1.5.x updated (742471c -> 9c28b26)
This is an automated email from the ASF dual-hosted git repository.
tillt pushed a change to branch 1.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 742471c Fixed an issue about inheriting user for nested containers.
new 0ef48eb Added collectAuthorizations helper to master.hpp.
new 9c28b26 Added MESOS-9317 to the 1.5.3 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 | 48 ++--------------
src/master/master.hpp | 17 +++++-
src/master/weights_handler.cpp | 12 +---
src/tests/master_tests.cpp | 122 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 144 insertions(+), 56 deletions(-)
[mesos] 02/02: Added MESOS-9317 to the 1.5.3 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 9c28b265804269de9411f04cfcccefe3bdd8ef1a
Author: Till Toenshoff <to...@me.com>
AuthorDate: Tue Nov 20 17:35:52 2018 +0100
Added MESOS-9317 to the 1.5.3 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index 0c68284..8798f91 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ Release Notes - Mesos - Version 1.5.3 (WIP)
** Bug
* [MESOS-7474] - Mesos fetcher cache doesn't retry when missed.
+ * [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] 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 0ef48eb254a69ab9d492cbec03736fc88bb8012a
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.5.x **
---
src/master/master.cpp | 48 ++--------------
src/master/master.hpp | 17 +++++-
src/master/weights_handler.cpp | 12 +---
src/tests/master_tests.cpp | 122 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 143 insertions(+), 56 deletions(-)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 5fcdd63..0229d1b 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3706,17 +3706,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);
}
@@ -3769,17 +3759,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);
}
@@ -3836,17 +3816,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);
}
@@ -3888,17 +3858,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);
}
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 43676bd..c1db276 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,8 +45,10 @@
#include <mesos/scheduler/scheduler.hpp>
-#include <process/limiter.hpp>
+#include <process/collect.hpp>
+#include <process/future.hpp>
#include <process/http.hpp>
+#include <process/limiter.hpp>
#include <process/owned.hpp>
#include <process/process.hpp>
#include <process/protobuf.hpp>
@@ -2164,6 +2167,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 59104a8..ce07271 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 dcd9088..9c46e0c 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;
@@ -9333,6 +9334,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 {