mesos git commit: Validated FrameworkInfo role.

[vi...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 5cd32bf81 -> 13e453789


Validated FrameworkInfo role.

Review: https://reviews.apache.org/r/46214/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/13e45378
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/13e45378
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/13e45378

Branch: refs/heads/master
Commit: 13e453789e467c60ac3e4c0ae75cec556812e6f0
Parents: 5cd32bf
Author: Klaus Ma <kl...@gmail.com>
Authored: Fri Apr 15 12:52:32 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Apr 15 12:52:32 2016 -0500

----------------------------------------------------------------------
 src/master/master.cpp                  |  4 +--
 src/tests/master_tests.cpp             | 26 ++++++++++++++
 src/tests/scheduler_http_api_tests.cpp | 53 +++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/13e45378/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 781402c..f9962a9 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2097,7 +2097,7 @@ void Master::subscribe(
   LOG(INFO) << "Received subscription request for"
             << " HTTP framework '" << frameworkInfo.name() << "'";
 
-  Option<Error> validationError = None();
+  Option<Error> validationError = roles::validate(frameworkInfo.role());
 
   if (validationError.isNone() && !isWhitelistedRole(frameworkInfo.role())) {
     validationError = Error("Role '" + frameworkInfo.role() + "' is not" +
@@ -2314,7 +2314,7 @@ void Master::subscribe(
     return;
   }
 
-  Option<Error> validationError = None();
+  Option<Error> validationError = roles::validate(frameworkInfo.role());
 
   if (validationError.isNone() && !isWhitelistedRole(frameworkInfo.role())) {
     validationError = Error("Role '" + frameworkInfo.role() + "' is not" +

http://git-wip-us.apache.org/repos/asf/mesos/blob/13e45378/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 1ae7260..cdd69b6 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -4062,6 +4062,32 @@ TEST_F(MasterTest, FrameworkInfoLabels)
 }
 
 
+// This test ensures that if a framework scheduler provides invalid
+// role in its FrameworkInfo message, the master will reject it.
+TEST_F(MasterTest, RejectFrameworkWithInvalidRole)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
+
+  // Add invalid role to the FrameworkInfo.
+  framework.set_role("/test/test1");
+
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, framework, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  Future<string> error;
+  EXPECT_CALL(sched, error(&driver, _))
+    .WillOnce(FutureArg<1>(&error));
+
+  driver.start();
+
+  AWAIT_READY(error);
+}
+
+
 TEST_F(MasterTest, FrameworksEndpointWithoutFrameworks)
 {
   master::Flags flags = CreateMasterFlags();

http://git-wip-us.apache.org/repos/asf/mesos/blob/13e45378/src/tests/scheduler_http_api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/scheduler_http_api_tests.cpp b/src/tests/scheduler_http_api_tests.cpp
index d469adf..f314d0a 100644
--- a/src/tests/scheduler_http_api_tests.cpp
+++ b/src/tests/scheduler_http_api_tests.cpp
@@ -331,6 +331,59 @@ TEST_P(SchedulerHttpApiTest, Subscribe)
 }
 
 
+// This test verifies if the role is invalid in scheduler's framework message,
+// the event is error on the stream in response to a Subscribe call request.
+TEST_P(SchedulerHttpApiTest, RejectFrameworkWithInvalidRole)
+{
+  // HTTP schedulers cannot yet authenticate.
+  master::Flags flags = CreateMasterFlags();
+  flags.authenticate_frameworks = false;
+
+  Try<Owned<cluster::Master>> master = StartMaster(flags);
+  ASSERT_SOME(master);
+
+  Call call;
+  call.set_type(Call::SUBSCRIBE);
+
+  Call::Subscribe* subscribe = call.mutable_subscribe();
+  v1::FrameworkInfo framework = DEFAULT_V1_FRAMEWORK_INFO;
+  // Set invalid role.
+  framework.set_role("/test/test1");
+  subscribe->mutable_framework_info()->CopyFrom(framework);
+
+  // Retrieve the parameter passed as content type to this test.
+  const string contentType = GetParam();
+  process::http::Headers headers;
+  headers["Accept"] = contentType;
+
+  Future<Response> response = process::http::streaming::post(
+      master.get()->pid,
+      "api/v1/scheduler",
+      headers,
+      serialize(call, contentType),
+      contentType);
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  AWAIT_EXPECT_RESPONSE_HEADER_EQ("chunked", "Transfer-Encoding", response);
+  ASSERT_EQ(Response::PIPE, response.get().type);
+
+  Option<Pipe::Reader> reader = response.get().reader;
+  ASSERT_SOME(reader);
+
+  auto deserializer = lambda::bind(
+      &SchedulerHttpApiTest::deserialize, this, contentType, lambda::_1);
+
+  Reader<Event> responseDecoder(Decoder<Event>(deserializer), reader.get());
+
+  Future<Result<Event>> event = responseDecoder.read();
+  AWAIT_READY(event);
+  ASSERT_SOME(event.get());
+
+  // Check event type is error.
+  ASSERT_EQ(Event::ERROR, event.get().get().type());
+}
+
+
 // This test verifies that the client will receive a `BadRequest` response if it
 // includes a stream ID header with a subscribe call.
 TEST_P(SchedulerHttpApiTest, SubscribeWithStreamId)