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 <y...@jxu.me> Authored: Thu May 22 23:33:02 2014 -0700 Committer: Jiang Yan Xu <y...@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); };