You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/12/18 23:24:47 UTC
[mesos] branch 1.7.x updated (5b7055c -> 53b212d)
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 5b7055c Validated that resource providers use correct ID in operation states.
new a316ea2 Made master process all authorization results for `LAUNCH_GROUP`.
new f3e4de6 Made master to authorize `CREATE_DISK` and `DESTROY_DISK` properly.
new 3462c93 Documented `Object.value` authorization field for certain operations.
new 0b543e4 Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.
new 53b212d Added MESOS-9474, MESOS-9480 and MESOS-9486 to the 1.7.1 CHANGELOG.
The 5 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 | 3 +
include/mesos/authorizer/authorizer.proto | 15 +++
src/master/master.cpp | 178 +++++++++++++++++++++++-------
3 files changed, 154 insertions(+), 42 deletions(-)
[mesos] 04/05: Set up `Object.value` for `CREATE_DISK` and
`DESTROY_DISK` authorization.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 0b543e4764a2766fc9fee60e789c6a91d8a06f95
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Mon Dec 17 22:11:52 2018 -0800
Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.
This patch sets up `Object.value` to the role of the resource for
authorization actions `CREATE_BLOCK_DISK`, `DESTROY_BLOCK_DISK`,
`CREATE_MOUNT_DISK` and `DESTROY_MOUNT_DISK` so an old-school authorizer
can rely on the field to perform authorization.
Review: https://reviews.apache.org/r/69578
---
src/master/master.cpp | 43 +++++++++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 6d2c954..f33a5aa 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3812,8 +3812,9 @@ Future<bool> Master::authorizeCreateDisk(
return true; // Authorization is disabled.
}
- Option<authorization::Action> action;
+ const Resource& resource = createDisk.source();
+ Option<authorization::Action> action;
switch (createDisk.target_type()) {
case Resource::DiskInfo::Source::MOUNT: {
action = authorization::CREATE_MOUNT_DISK;
@@ -3830,8 +3831,7 @@ Future<bool> Master::authorizeCreateDisk(
"Failed to authorize principal '" +
(principal.isSome() ? stringify(principal.get()) : "ANY") +
"' to create a " + stringify(createDisk.target_type()) +
- " disk from '" + stringify(createDisk.source()) +
- "': Unsupported disk type");
+ " disk from '" + stringify(resource) + "': Unsupported disk type");
}
}
@@ -3843,7 +3843,20 @@ Future<bool> Master::authorizeCreateDisk(
request.mutable_subject()->CopyFrom(subject.get());
}
- request.mutable_object()->mutable_resource()->CopyFrom(createDisk.source());
+ request.mutable_object()->mutable_resource()->CopyFrom(resource);
+
+ // We set `object.value` in addition to `object.resource` to support legacy
+ // authorizers making only use of this deprecated field.
+ //
+ // NOTE: We rely on the master to ensure that the resource is in the
+ // post-reservation-refinement format and set the value to the most refined
+ // role, or default to '*' for consistency if there is no reservation.
+ CHECK(!resource.has_role()) << resource;
+ CHECK(!resource.has_reservation()) << resource;
+ request.mutable_object()->set_value(
+ resource.reservations().empty()
+ ? "*"
+ : resource.reservations().rbegin()->role());
LOG(INFO) << "Authorizing principal '"
<< (principal.isSome() ? stringify(principal.get()) : "ANY")
@@ -3862,9 +3875,10 @@ Future<bool> Master::authorizeDestroyDisk(
return true; // Authorization is disabled.
}
- Option<authorization::Action> action;
+ const Resource& resource = destroyDisk.source();
- switch (destroyDisk.source().disk().source().type()) {
+ Option<authorization::Action> action;
+ switch (resource.disk().source().type()) {
case Resource::DiskInfo::Source::MOUNT: {
action = authorization::DESTROY_MOUNT_DISK;
break;
@@ -3879,7 +3893,7 @@ Future<bool> Master::authorizeDestroyDisk(
return Failure(
"Failed to authorize principal '" +
(principal.isSome() ? stringify(principal.get()) : "ANY") +
- "' to destroy disk '" + stringify(destroyDisk.source()) +
+ "' to destroy disk '" + stringify(resource) +
"': Unsupported disk type");
}
}
@@ -3892,7 +3906,20 @@ Future<bool> Master::authorizeDestroyDisk(
request.mutable_subject()->CopyFrom(subject.get());
}
- request.mutable_object()->mutable_resource()->CopyFrom(destroyDisk.source());
+ request.mutable_object()->mutable_resource()->CopyFrom(resource);
+
+ // We set `object.value` in addition to `object.resource` to support legacy
+ // authorizers making only use of this deprecated field.
+ //
+ // NOTE: We rely on the master to ensure that the resource is in the
+ // post-reservation-refinement format and set the value to the most refined
+ // role, or default to '*' for consistency if there is no reservation.
+ CHECK(!resource.has_role()) << resource;
+ CHECK(!resource.has_reservation()) << resource;
+ request.mutable_object()->set_value(
+ resource.reservations().empty()
+ ? "*"
+ : resource.reservations().rbegin()->role());
LOG(INFO) << "Authorizing principal '"
<< (principal.isSome() ? stringify(principal.get()) : "ANY")
[mesos] 02/05: Made master to authorize `CREATE_DISK` and
`DESTROY_DISK` properly.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f3e4de63def9efb1b0e6140851292ba26d8bc644
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Fri Dec 14 11:10:13 2018 -0800
Made master to authorize `CREATE_DISK` and `DESTROY_DISK` properly.
This patch fixes a bug where the master does not respect the
authorization result for `CREATE_DISK` or`DESTROY_DISK`, causing
subsequent operations to drop if the authorization is denied.
Review: https://reviews.apache.org/r/69563
---
src/master/master.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 39ccc06..6d2c954 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5630,6 +5630,34 @@ void Master::_accept(
}
case Offer::Operation::CREATE_DISK: {
+ const Resource::DiskInfo::Source::Type diskType =
+ operation.create_disk().target_type();
+
+ CHECK(!authorizations.empty());
+ Future<bool> authorization = authorizations.front();
+ authorizations.pop_front();
+
+ CHECK(!authorization.isDiscarded());
+
+ if (authorization.isFailed()) {
+ // TODO(greggomann): We may want to retry this failed authorization
+ // request rather than dropping it immediately.
+ drop(framework,
+ operation,
+ "Authorization of principal '" + framework->info.principal() +
+ "' to create a " + stringify(diskType) + " disk failed: " +
+ authorization.failure());
+
+ continue;
+ } else if (!authorization.get()) {
+ drop(framework,
+ operation,
+ "Not authorized to create a " + stringify(diskType) +
+ " disk as '" + framework->info.principal() + "'");
+
+ continue;
+ }
+
if (!slave->capabilities.resourceProvider) {
drop(framework,
operation,
@@ -5669,6 +5697,34 @@ void Master::_accept(
}
case Offer::Operation::DESTROY_DISK: {
+ const Resource::DiskInfo::Source::Type diskType =
+ operation.destroy_disk().source().disk().source().type();
+
+ CHECK(!authorizations.empty());
+ Future<bool> authorization = authorizations.front();
+ authorizations.pop_front();
+
+ CHECK(!authorization.isDiscarded());
+
+ if (authorization.isFailed()) {
+ // TODO(greggomann): We may want to retry this failed authorization
+ // request rather than dropping it immediately.
+ drop(framework,
+ operation,
+ "Authorization of principal '" + framework->info.principal() +
+ "' to destroy a " + stringify(diskType) + " disk failed: " +
+ authorization.failure());
+
+ continue;
+ } else if (!authorization.get()) {
+ drop(framework,
+ operation,
+ "Not authorized to destroy a " + stringify(diskType) +
+ " disk as '" + framework->info.principal() + "'");
+
+ continue;
+ }
+
if (!slave->capabilities.resourceProvider) {
drop(framework,
operation,
[mesos] 03/05: Documented `Object.value` authorization field for
certain operations.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 3462c935f9d0887a34b5b27577824b6e2a5d3a8c
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Mon Dec 17 22:10:31 2018 -0800
Documented `Object.value` authorization field for certain operations.
Review: https://reviews.apache.org/r/69577
---
include/mesos/authorizer/authorizer.proto | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto
index 7330416..ab30c20 100644
--- a/include/mesos/authorizer/authorizer.proto
+++ b/include/mesos/authorizer/authorizer.proto
@@ -256,18 +256,33 @@ enum Action {
PRUNE_IMAGES = 41;
// `RESIZE_VOLUME` will have an object with `Resource` set.
+ //
+ // NOTE: For consistency, the `value` field will be set with the most refined
+ // role until all `*_WITH_ROLE` aliases are removed.
RESIZE_VOLUME = 42;
// `CREATE_BLOCK_DISK` will have an object with `Resource` set.
+ //
+ // NOTE: For consistency, the `value` field will be set with the most refined
+ // role until all `*_WITH_ROLE` aliases are removed.
CREATE_BLOCK_DISK = 43;
// `DESTROY_BLOCK_DISK` will have an object with `Resource` set.
+ //
+ // NOTE: For consistency, the `value` field will be set with the most refined
+ // role until all `*_WITH_ROLE` aliases are removed.
DESTROY_BLOCK_DISK = 44;
// `CREATE_MOUNT_DISK` will have an object with `Resource` set.
+ //
+ // NOTE: For consistency, the `value` field will be set with the most refined
+ // role until all `*_WITH_ROLE` aliases are removed.
CREATE_MOUNT_DISK = 45;
// `DESTROY_MOUNT_DISK` will have an object with `Resource` set.
+ //
+ // NOTE: For consistency, the `value` field will be set with the most refined
+ // role until all `*_WITH_ROLE` aliases are removed.
DESTROY_MOUNT_DISK = 46;
// This action will not fill in any object fields. A principal is either
[mesos] 05/05: Added MESOS-9474,
MESOS-9480 and MESOS-9486 to the 1.7.1 CHANGELOG.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 53b212d3fd6fd28452a6c64aebf3a5e4f6f44b5e
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Tue Dec 18 14:37:04 2018 -0800
Added MESOS-9474, MESOS-9480 and MESOS-9486 to the 1.7.1 CHANGELOG.
---
CHANGELOG | 3 +++
1 file changed, 3 insertions(+)
diff --git a/CHANGELOG b/CHANGELOG
index 6f102a2..caf3767 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,8 @@ Release Notes - Mesos - Version 1.7.1 (WIP)
* [MESOS-9411] - Validation of JWT tokens using HS256 hashing algorithm is not thread safe.
* [MESOS-9418] - Add support for the `Discard` blkio operation type.
* [MESOS-9419] - Executor to framework message crashes master if framework has not re-registered.
+ * [MESOS-9474] - Master does not respect authorization result for `CREATE_DISK` and `DESTROY_DISK`.
+ * [MESOS-9480] - Master may skip processing authorization results for `LAUNCH_GROUP`.
** Improvement:
* [MESOS-6765] - Make the Resources wrapper "copy-on-write" to improve performance.
@@ -41,6 +43,7 @@ Release Notes - Mesos - Version 1.7.1 (WIP)
* [MESOS-9305] - Create cgoup recursively to workaround systemd deleting cgroups_root.
* [MESOS-9321] - Add an optional `vendor` field in `Resource.DiskInfo.Source`.
* [MESOS-9340] - Log all socket errors in libprocess.
+ * [MESOS-9486] - Set up `object.value` for `CREATE_DISK` and `DESTROY_DISK` authorizations.
Release Notes - Mesos - Version 1.7.0
[mesos] 01/05: Made master process all authorization results for
`LAUNCH_GROUP`.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit a316ea27c686d7023d007a9617ce54f417d87f1e
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Fri Dec 14 11:01:29 2018 -0800
Made master process all authorization results for `LAUNCH_GROUP`.
This patch fixes a bug where the master does not complete the processing
of authorization results for `LAUNCH_GROUP`, causing a subsequent
operation to drop if one of the remaining authorization is denied.
Review: https://reviews.apache.org/r/69571
---
src/master/master.cpp | 79 +++++++++++++++++++++++++++++----------------------
1 file changed, 45 insertions(+), 34 deletions(-)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index fc29781..39ccc06 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -4711,8 +4711,8 @@ void Master::_accept(
// The order of the conversions is important and preserved.
vector<ResourceConversion> conversions;
- // The order of `authorizations` must match the order of the operations in
- // `accept.operations()`, as they are iterated through simultaneously.
+ // The order of `authorizations` must match the order of the operations and/or
+ // tasks in `accept.operations()` as they are iterated through simultaneously.
CHECK_READY(_authorizations);
std::deque<Future<bool>> authorizations(
_authorizations->begin(), _authorizations->end());
@@ -4721,6 +4721,7 @@ void Master::_accept(
switch (operation.type()) {
// The RESERVE operation allows a principal to reserve resources.
case Offer::Operation::RESERVE: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -4796,6 +4797,7 @@ void Master::_accept(
// The UNRESERVE operation allows a principal to unreserve resources.
case Offer::Operation::UNRESERVE: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -4861,6 +4863,7 @@ void Master::_accept(
}
case Offer::Operation::CREATE: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -4938,6 +4941,7 @@ void Master::_accept(
}
case Offer::Operation::DESTROY: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -5031,6 +5035,7 @@ void Master::_accept(
}
case Offer::Operation::GROW_VOLUME: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -5113,6 +5118,7 @@ void Master::_accept(
}
case Offer::Operation::SHRINK_VOLUME: {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -5196,6 +5202,7 @@ void Master::_accept(
case Offer::Operation::LAUNCH: {
foreach (const TaskInfo& task, operation.launch().task_infos()) {
+ CHECK(!authorizations.empty());
Future<bool> authorization = authorizations.front();
authorizations.pop_front();
@@ -5431,51 +5438,50 @@ void Master::_accept(
// TODO(bmahler): Consider injecting some default (cpus, mem, disk)
// resources when the framework omits the executor resources.
- // See if there are any validation or authorization errors.
+ // See if there are any authorization or validation errors.
// Note that we'll only report the first error we encounter
// for the group.
//
// TODO(anindya_sinha): If task group uses shared resources, this
// validation needs to be enhanced to accommodate multiple copies
// of shared resources across tasks within the task group.
- Option<Error> error = validation::task::group::validate(
- taskGroup, executor, framework, slave, _offeredResources);
+ Option<Error> error;
+ Option<TaskStatus::Reason> reason;
- Option<TaskStatus::Reason> reason = None();
-
- if (error.isSome()) {
- reason = TaskStatus::REASON_TASK_GROUP_INVALID;
- } else {
- foreach (const TaskInfo& task, taskGroup.tasks()) {
- Future<bool> authorization = authorizations.front();
- authorizations.pop_front();
-
- CHECK(!authorization.isDiscarded());
-
- if (authorization.isFailed()) {
- error = Error("Failed to authorize task"
- " '" + stringify(task.task_id()) + "'"
- ": " + authorization.failure());
+ // NOTE: We check for the authorization errors first and never break the
+ // loop to ensure that all authorization futures for this task group are
+ // iterated through.
+ foreach (const TaskInfo& task, taskGroup.tasks()) {
+ CHECK(!authorizations.empty());
+ Future<bool> authorization = authorizations.front();
+ authorizations.pop_front();
- reason = TaskStatus::REASON_TASK_GROUP_UNAUTHORIZED;
+ CHECK(!authorization.isDiscarded());
- break;
+ if (authorization.isFailed()) {
+ error = Error("Failed to authorize task"
+ " '" + stringify(task.task_id()) + "'"
+ ": " + authorization.failure());
+ } else if (!authorization.get()) {
+ string user = framework->info.user(); // Default user.
+ if (task.has_command() && task.command().has_user()) {
+ user = task.command().user();
}
- if (!authorization.get()) {
- string user = framework->info.user(); // Default user.
- if (task.has_command() && task.command().has_user()) {
- user = task.command().user();
- }
-
- error = Error("Task '" + stringify(task.task_id()) + "'"
- " is not authorized to launch as"
- " user '" + user + "'");
+ error = Error("Task '" + stringify(task.task_id()) + "'"
+ " is not authorized to launch as"
+ " user '" + user + "'");
+ }
+ }
- reason = TaskStatus::REASON_TASK_GROUP_UNAUTHORIZED;
+ if (error.isSome()) {
+ reason = TaskStatus::REASON_TASK_GROUP_UNAUTHORIZED;
+ } else {
+ error = validation::task::group::validate(
+ taskGroup, executor, framework, slave, _offeredResources);
- break;
- }
+ if (error.isSome()) {
+ reason = TaskStatus::REASON_TASK_GROUP_INVALID;
}
}
@@ -5708,6 +5714,11 @@ void Master::_accept(
}
}
+ CHECK(authorizations.empty())
+ << "Authorization results not processed: "
+ << stringify(
+ vector<Future<bool>>(authorizations.begin(), authorizations.end()));
+
// Update the allocator based on the operations.
if (!conversions.empty()) {
allocator->updateAllocation(