Repository: mesos Updated Branches: refs/heads/master 4e5e72616 -> 7b785c75b
Moved `executor::Call` validation to common validation library. `executor::Call` validation is used by both the agent and the executor driver, so move it to the common validation library. Review: https://reviews.apache.org/r/67830/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7b785c75 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7b785c75 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7b785c75 Branch: refs/heads/master Commit: 7b785c75bd4a48dea90a9265f7775a6e11310907 Parents: 4e5e726 Author: James Peach <jpe...@apache.org> Authored: Tue Jul 17 09:03:12 2018 -0700 Committer: James Peach <jpe...@apache.org> Committed: Tue Jul 17 09:03:12 2018 -0700 ---------------------------------------------------------------------- src/common/validation.cpp | 104 ++++++++++++++++++++++++++++++++++++++ src/common/validation.hpp | 9 ++++ src/executor/executor.cpp | 8 ++- src/slave/http.cpp | 3 +- src/slave/validation.cpp | 110 ----------------------------------------- src/slave/validation.hpp | 11 ----- 6 files changed, 118 insertions(+), 127 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/common/validation.cpp ---------------------------------------------------------------------- diff --git a/src/common/validation.cpp b/src/common/validation.cpp index f89b0ca..5f8f288 100644 --- a/src/common/validation.cpp +++ b/src/common/validation.cpp @@ -21,6 +21,8 @@ #include <algorithm> #include <cctype> +#include <mesos/executor/executor.hpp> + #include <mesos/resources.hpp> #include <stout/foreach.hpp> @@ -506,6 +508,108 @@ Option<Error> validateCheckStatusInfo(const CheckStatusInfo& checkStatusInfo) return None(); } +Option<Error> validateExecutorCall(const mesos::executor::Call& call) +{ + if (!call.IsInitialized()) { + return Error("Not initialized: " + call.InitializationErrorString()); + } + + if (!call.has_type()) { + return Error("Expecting 'type' to be present"); + } + + // All calls should have executor id set. + if (!call.has_executor_id()) { + return Error("Expecting 'executor_id' to be present"); + } + + // All calls should have framework id set. + if (!call.has_framework_id()) { + return Error("Expecting 'framework_id' to be present"); + } + + switch (call.type()) { + case mesos::executor::Call::SUBSCRIBE: { + if (!call.has_subscribe()) { + return Error("Expecting 'subscribe' to be present"); + } + return None(); + } + + case mesos::executor::Call::UPDATE: { + if (!call.has_update()) { + return Error("Expecting 'update' to be present"); + } + + const TaskStatus& status = call.update().status(); + + if (!status.has_uuid()) { + return Error("Expecting 'uuid' to be present"); + } + + Try<id::UUID> uuid = id::UUID::fromBytes(status.uuid()); + if (uuid.isError()) { + return uuid.error(); + } + + if (status.has_executor_id() && + status.executor_id().value() + != call.executor_id().value()) { + return Error("ExecutorID in Call: " + + call.executor_id().value() + + " does not match ExecutorID in TaskStatus: " + + call.update().status().executor_id().value() + ); + } + + if (status.source() != TaskStatus::SOURCE_EXECUTOR) { + return Error("Received Call from executor " + + call.executor_id().value() + + " of framework " + + call.framework_id().value() + + " with invalid source, expecting 'SOURCE_EXECUTOR'" + ); + } + + if (status.state() == TASK_STAGING) { + return Error("Received TASK_STAGING from executor " + + call.executor_id().value() + + " of framework " + + call.framework_id().value() + + " which is not allowed" + ); + } + + // TODO(alexr): Validate `check_status` is present if + // the corresponding `TaskInfo.check` has been defined. + + if (status.has_check_status()) { + Option<Error> validate = + common::validation::validateCheckStatusInfo(status.check_status()); + + if (validate.isSome()) { + return validate.get(); + } + } + + return None(); + } + + case mesos::executor::Call::MESSAGE: { + if (!call.has_message()) { + return Error("Expecting 'message' to be present"); + } + return None(); + } + + case mesos::executor::Call::UNKNOWN: { + return None(); + } + } + + UNREACHABLE(); +} + } // namespace validation { } // namespace common { } // namespace internal { http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/common/validation.hpp ---------------------------------------------------------------------- diff --git a/src/common/validation.hpp b/src/common/validation.hpp index fa7f4a7..5929ac2 100644 --- a/src/common/validation.hpp +++ b/src/common/validation.hpp @@ -25,6 +25,13 @@ #include <stout/option.hpp> namespace mesos { + +namespace executor { + +class Call; + +} // namespace executor { + namespace internal { namespace common { namespace validation { @@ -60,6 +67,8 @@ Option<Error> validateCheckInfo(const CheckInfo& checkInfo); Option<Error> validateCheckStatusInfo(const CheckStatusInfo& checkStatusInfo); +Option<Error> validateExecutorCall(const mesos::executor::Call& call); + } // namespace validation { } // namespace common { } // namespace internal { http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/executor/executor.cpp ---------------------------------------------------------------------- diff --git a/src/executor/executor.cpp b/src/executor/executor.cpp index ab67cae..a9e1fcf 100644 --- a/src/executor/executor.cpp +++ b/src/executor/executor.cpp @@ -45,14 +45,13 @@ #include "common/http.hpp" #include "common/recordio.hpp" +#include "common/validation.hpp" #include "internal/devolve.hpp" #include "logging/flags.hpp" #include "logging/logging.hpp" -#include "slave/validation.hpp" - #include "version/version.hpp" using namespace mesos; @@ -64,8 +63,6 @@ using std::string; using mesos::internal::recordio::Reader; -using mesos::internal::slave::validation::executor::call::validate; - using process::async; using process::Clock; using process::delay; @@ -302,7 +299,8 @@ public: void send(const Call& call) { - Option<Error> error = validate(devolve(call)); + Option<Error> error = + common::validation::validateExecutorCall(devolve(call)); if (error.isSome()) { drop(call, error->message); return; http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 51d670d..cf93c48 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -61,6 +61,7 @@ #include "common/http.hpp" #include "common/recordio.hpp" #include "common/resources_utils.hpp" +#include "common/validation.hpp" #include "internal/devolve.hpp" @@ -767,7 +768,7 @@ Future<Response> Http::executor( const executor::Call call = devolve(v1Call); - Option<Error> error = validation::executor::call::validate(call); + Option<Error> error = common::validation::validateExecutorCall(call); if (error.isSome()) { return BadRequest("Failed to validate Executor::Call: " + error->message); http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/validation.cpp ---------------------------------------------------------------------- diff --git a/src/slave/validation.cpp b/src/slave/validation.cpp index 3e333a7..df5e137 100644 --- a/src/slave/validation.cpp +++ b/src/slave/validation.cpp @@ -25,9 +25,6 @@ #include <stout/stringify.hpp> #include <stout/unreachable.hpp> -#include <stout/uuid.hpp> - -#include "checks/checker.hpp" #include "common/resources_utils.hpp" #include "common/validation.hpp" @@ -546,113 +543,6 @@ Option<Error> validate( } // namespace call { } // namespace agent { - -namespace executor { -namespace call { - -Option<Error> validate(const mesos::executor::Call& call) -{ - if (!call.IsInitialized()) { - return Error("Not initialized: " + call.InitializationErrorString()); - } - - if (!call.has_type()) { - return Error("Expecting 'type' to be present"); - } - - // All calls should have executor id set. - if (!call.has_executor_id()) { - return Error("Expecting 'executor_id' to be present"); - } - - // All calls should have framework id set. - if (!call.has_framework_id()) { - return Error("Expecting 'framework_id' to be present"); - } - - switch (call.type()) { - case mesos::executor::Call::SUBSCRIBE: { - if (!call.has_subscribe()) { - return Error("Expecting 'subscribe' to be present"); - } - return None(); - } - - case mesos::executor::Call::UPDATE: { - if (!call.has_update()) { - return Error("Expecting 'update' to be present"); - } - - const TaskStatus& status = call.update().status(); - - if (!status.has_uuid()) { - return Error("Expecting 'uuid' to be present"); - } - - Try<id::UUID> uuid = id::UUID::fromBytes(status.uuid()); - if (uuid.isError()) { - return uuid.error(); - } - - if (status.has_executor_id() && - status.executor_id().value() - != call.executor_id().value()) { - return Error("ExecutorID in Call: " + - call.executor_id().value() + - " does not match ExecutorID in TaskStatus: " + - call.update().status().executor_id().value() - ); - } - - if (status.source() != TaskStatus::SOURCE_EXECUTOR) { - return Error("Received Call from executor " + - call.executor_id().value() + - " of framework " + - call.framework_id().value() + - " with invalid source, expecting 'SOURCE_EXECUTOR'" - ); - } - - if (status.state() == TASK_STAGING) { - return Error("Received TASK_STAGING from executor " + - call.executor_id().value() + - " of framework " + - call.framework_id().value() + - " which is not allowed" - ); - } - - // TODO(alexr): Validate `check_status` is present if - // the corresponding `TaskInfo.check` has been defined. - - if (status.has_check_status()) { - Option<Error> validate = - common::validation::validateCheckStatusInfo(status.check_status()); - - if (validate.isSome()) { - return validate.get(); - } - } - - return None(); - } - - case mesos::executor::Call::MESSAGE: { - if (!call.has_message()) { - return Error("Expecting 'message' to be present"); - } - return None(); - } - - case mesos::executor::Call::UNKNOWN: { - return None(); - } - } - UNREACHABLE(); -} - -} // namespace call { -} // namespace executor { } // namespace validation { } // namespace slave { } // namespace internal { http://git-wip-us.apache.org/repos/asf/mesos/blob/7b785c75/src/slave/validation.hpp ---------------------------------------------------------------------- diff --git a/src/slave/validation.hpp b/src/slave/validation.hpp index 3a278e4..c10f615 100644 --- a/src/slave/validation.hpp +++ b/src/slave/validation.hpp @@ -19,8 +19,6 @@ #include <mesos/agent/agent.hpp> -#include <mesos/executor/executor.hpp> - #include <stout/error.hpp> #include <stout/option.hpp> @@ -47,15 +45,6 @@ Option<Error> validate( } // namespace call { } // namespace agent { -namespace executor { -namespace call { - -// Validates that an executor call is well-formed. -// TODO(ijimenez): Add unit tests. -Option<Error> validate(const mesos::executor::Call& call); - -} // namespace call { -} // namespace executor { } // namespace validation { } // namespace slave { } // namespace internal {