You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2015/10/27 20:30:10 UTC
[2/3] mesos git commit: Added synchronous validation for Call in
Agent.
Added synchronous validation for Call in Agent.
Added validation for Call protobuf message in Agent /api/v1/executor
endpoint.
Review: https://reviews.apache.org/r/38577
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ea31fd04
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ea31fd04
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ea31fd04
Branch: refs/heads/master
Commit: ea31fd04ef66af2c04dd996dcb9849c20eede723
Parents: eb8f248
Author: Isabel Jimenez <co...@isabeljimenez.com>
Authored: Tue Oct 27 12:27:44 2015 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Tue Oct 27 12:29:35 2015 -0700
----------------------------------------------------------------------
src/CMakeLists.txt | 1 +
src/Makefile.am | 2 +
src/slave/http.cpp | 10 ++-
src/slave/validation.cpp | 108 +++++++++++++++++++++++++++++
src/slave/validation.hpp | 45 ++++++++++++
src/tests/executor_http_api_tests.cpp | 3 +-
6 files changed, 166 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index e6169a0..6c7519b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -183,6 +183,7 @@ if (NOT WIN32)
slave/slave.cpp
slave/state.cpp
slave/status_update_manager.cpp
+ slave/validation.cpp
slave/containerizer/containerizer.cpp
slave/containerizer/composing.cpp
slave/containerizer/composing.hpp
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index e797dac..d6eb302 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -533,6 +533,7 @@ libmesos_no_3rdparty_la_SOURCES = \
slave/slave.cpp \
slave/state.cpp \
slave/status_update_manager.cpp \
+ slave/validation.cpp \
slave/containerizer/containerizer.cpp \
slave/containerizer/composing.cpp \
slave/containerizer/composing.hpp \
@@ -846,6 +847,7 @@ libmesos_no_3rdparty_la_SOURCES += \
slave/slave.hpp \
slave/state.hpp \
slave/status_update_manager.hpp \
+ slave/validation.hpp \
slave/containerizer/containerizer.hpp \
slave/containerizer/external_containerizer.hpp \
slave/containerizer/fetcher.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 3f7f71b..d6df97f 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -51,6 +51,7 @@
#include "mesos/resources.hpp"
#include "slave/slave.hpp"
+#include "slave/validation.hpp"
using process::Clock;
@@ -260,8 +261,13 @@ Future<Response> Slave::Http::executor(const Request& request) const
const executor::Call call = devolve(v1Call);
- // TODO(anand): Validate the protobuf (MESOS-2906) before proceeding
- // further.
+
+ Option<Error> error = validation::executor::call::validate(call);
+
+ if (error.isSome()) {
+ return BadRequest("Failed to validate Executor::Call: " +
+ error.get().message);
+ }
ContentType responseContentType;
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/slave/validation.cpp
----------------------------------------------------------------------
diff --git a/src/slave/validation.cpp b/src/slave/validation.cpp
new file mode 100644
index 0000000..b46559d
--- /dev/null
+++ b/src/slave/validation.cpp
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "slave/validation.hpp"
+
+namespace mesos {
+namespace internal {
+namespace slave {
+namespace validation {
+namespace executor {
+namespace call {
+
+Option<Error> validate(const mesos::executor::Call& call)
+{
+ if (!call.IsInitialized()) {
+ return Error("Not initialized: " + call.InitializationErrorString());
+ }
+
+ // 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_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"
+ );
+ }
+
+ return None();
+ }
+
+ case mesos::executor::Call::MESSAGE: {
+ if (!call.has_message()) {
+ return Error("Expecting 'message' to be present");
+ }
+ return None();
+ }
+
+ default: {
+ return Error("Unknown call type");
+ }
+ }
+}
+
+} // namespace call {
+} // namespace executor {
+} // namespace validation {
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/slave/validation.hpp
----------------------------------------------------------------------
diff --git a/src/slave/validation.hpp b/src/slave/validation.hpp
new file mode 100644
index 0000000..bffa793
--- /dev/null
+++ b/src/slave/validation.hpp
@@ -0,0 +1,45 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef __SLAVE_VALIDATION_HPP__
+#define __SLAVE_VALIDATION_HPP__
+
+#include <mesos/executor/executor.hpp>
+
+#include <stout/error.hpp>
+#include <stout/option.hpp>
+
+namespace mesos {
+namespace internal {
+namespace slave {
+namespace validation {
+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 {
+} // namespace mesos {
+
+#endif // __SLAVE_VALIDATION_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/ea31fd04/src/tests/executor_http_api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/executor_http_api_tests.cpp b/src/tests/executor_http_api_tests.cpp
index 0908871..13a903b 100644
--- a/src/tests/executor_http_api_tests.cpp
+++ b/src/tests/executor_http_api_tests.cpp
@@ -363,6 +363,7 @@ TEST_P(ExecutorHttpApiTest, DefaultAccept)
Call call;
call.mutable_framework_id()->CopyFrom(evolve(frameworkId.get()));
call.mutable_executor_id()->CopyFrom(evolve(executorId));
+
call.set_type(Call::SUBSCRIBE);
call.mutable_subscribe();
@@ -439,7 +440,6 @@ TEST_P(ExecutorHttpApiTest, NoAcceptHeader)
// Only subscribe needs to 'Accept' JSON or protobuf.
Call call;
-
call.mutable_framework_id()->CopyFrom(evolve(frameworkId.get()));
call.mutable_executor_id()->CopyFrom(evolve(executorId));
@@ -496,6 +496,7 @@ TEST_P(ExecutorHttpApiTest, NotAcceptable)
Call call;
call.mutable_framework_id()->set_value("dummy_framework_id");
call.mutable_executor_id()->set_value("dummy_executor_id");
+
call.set_type(Call::SUBSCRIBE);
call.mutable_subscribe();