You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2014/06/04 22:45:01 UTC
git commit: Pulled common Framework (re-)registration validation into
a helper method.
Repository: mesos
Updated Branches:
refs/heads/master bda31070b -> 99c86dc1f
Pulled common Framework (re-)registration validation into a helper method.
Review: https://reviews.apache.org/r/22162
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/99c86dc1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/99c86dc1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/99c86dc1
Branch: refs/heads/master
Commit: 99c86dc1fdefad5c99f88f3594f9d00d0654905c
Parents: bda3107
Author: Jiang Yan Xu <ya...@jxu.me>
Authored: Thu May 22 23:33:02 2014 -0700
Committer: Jiang Yan Xu <ya...@jxu.me>
Committed: Wed Jun 4 13:44:18 2014 -0700
----------------------------------------------------------------------
src/master/master.cpp | 108 ++++++++++++++++++---------------------------
src/master/master.hpp | 10 +++++
2 files changed, 54 insertions(+), 64 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/99c86dc1/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 91dc1fd..89f426c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -986,6 +986,40 @@ void Master::detected(const Future<Option<MasterInfo> >& _leader)
}
+Try<Nothing> Master::validate(
+ const FrameworkInfo& frameworkInfo,
+ const UPID& from)
+{
+ if (flags.authenticate_frameworks) {
+ if (!authenticated.contains(from)) {
+ // This could happen if another authentication request came
+ // through before we are here or if a framework tried to
+ // (re-)register without authentication.
+ return Error("Framework at " + stringify(from) + " is not authenticated");
+ } else if (frameworkInfo.has_principal() &&
+ frameworkInfo.principal() != authenticated[from]) {
+ return Error(
+ "Framework principal '" + frameworkInfo.principal() +
+ "' does not match authenticated principal '" + authenticated[from] +
+ "'");
+ } else if (!frameworkInfo.has_principal()) {
+ // We allow an authenticated framework to not specify a
+ // principal in FrameworkInfo but we'd prefer if it did so we log
+ // a WARNING here when this happens.
+ LOG(WARNING) << "Framework at " << from << " (authenticated as '"
+ << authenticated[from]
+ << "') does not specify principal in its FrameworkInfo";
+ }
+ }
+
+ if (!roles.contains(frameworkInfo.role())) {
+ return Error("Role '" + frameworkInfo.role() + "' is not valid.");
+ }
+
+ return Nothing();
+}
+
+
void Master::registerFramework(
const UPID& from,
const FrameworkInfo& frameworkInfo)
@@ -1001,39 +1035,12 @@ void Master::registerFramework(
return;
}
- if (flags.authenticate_frameworks) {
- if (!authenticated.contains(from)) {
- // This could happen if another authentication request came
- // through before we are here or if a framework tried to register
- // without authentication.
- LOG(WARNING) << "Refusing registration of framework at " << from
- << " because it is not authenticated";
- FrameworkErrorMessage message;
- message.set_message("Framework at " + stringify(from) +
- " is not authenticated.");
- send(from, message);
- return;
- } else if (frameworkInfo.has_principal() &&
- frameworkInfo.principal() != authenticated[from]) {
- LOG(WARNING) << "Refusing registration of framework at " << from
- << " because its principal '" << frameworkInfo.principal()
- << "' does not match what it used in authentication: '"
- << authenticated[from] << "'";
- FrameworkErrorMessage message;
- message.set_message("Framework principal " + frameworkInfo.principal() +
- " does not match what was used in authentication: " +
- authenticated[from]);
- send(from, message);
- return;
- } else if (!frameworkInfo.has_principal()) {
- LOG(WARNING) << "Framework at " << from
- << " does not specify principal in its FrameworkInfo";
- }
- }
-
- if (!roles.contains(frameworkInfo.role())) {
+ Try<Nothing> valid = validate(frameworkInfo, from);
+ if (valid.isError()) {
+ LOG(WARNING) << "Refusing registration of framework at " << from << ": "
+ << valid.error();
FrameworkErrorMessage message;
- message.set_message("Role '" + frameworkInfo.role() + "' is not valid.");
+ message.set_message(valid.error());
send(from, message);
return;
}
@@ -1102,39 +1109,12 @@ void Master::reregisterFramework(
return;
}
- if (flags.authenticate_frameworks) {
- if (!authenticated.contains(from)) {
- // This could happen if another authentication request came
- // through before we are here or if a framework tried to
- // re-register without authentication.
- LOG(WARNING) << "Refusing re-registration of framework at " << from
- << " because it is not authenticated";
- FrameworkErrorMessage message;
- message.set_message("Framework '" + frameworkInfo.id().value() + "' at " +
- stringify(from) + " is not authenticated.");
- send(from, message);
- return;
- } else if (frameworkInfo.has_principal() &&
- frameworkInfo.principal() != authenticated[from]) {
- LOG(WARNING) << "Refusing re-registration of framework at " << from
- << " because its principal '" << frameworkInfo.principal()
- << "' does not match what it used in authentication: '"
- << authenticated[from] << "'";
- FrameworkErrorMessage message;
- message.set_message("Framework principal " + frameworkInfo.principal() +
- " does not match what was used in authentication: " +
- authenticated[from]);
- send(from, message);
- return;
- } else if (!frameworkInfo.has_principal()) {
- LOG(WARNING) << "Framework at " << from
- << " does not specify principal in its FrameworkInfo";
- }
- }
-
- if (!roles.contains(frameworkInfo.role())) {
+ Try<Nothing> valid = validate(frameworkInfo, from);
+ if (valid.isError()) {
+ LOG(WARNING) << "Refusing re-registration of framework at " << from << ": "
+ << valid.error();
FrameworkErrorMessage message;
- message.set_message("Role '" + frameworkInfo.role() + "' is not valid.");
+ message.set_message(valid.error());
send(from, message);
return;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/99c86dc1/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index d4ef4be..e244831 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -618,6 +618,16 @@ private:
process::Time startTime; // Start time used to calculate uptime.
Option<process::Time> electedTime; // Time when this master is elected.
+
+ // Helper method for common FrameworkInfo and PID validation shared
+ // by Framework registration and re-registration.
+ // An error return value indicates the request is invalid and a
+ // FrameworkErrorMessage should be returned.
+ // Note that registration/re-registration specific checking is not
+ // handled here.
+ Try<Nothing> validate(
+ const FrameworkInfo& frameworkInfo,
+ const process::UPID& from);
};