This is an automated email from the ASF dual-hosted git repository. asekretenko pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 889aa4c1f75af4294173a99a7c1df97534a83ded Author: Andrei Sekretenko <[email protected]> AuthorDate: Tue Jan 21 21:58:23 2020 +0100 Converted UPDATE_FRAMEWORK to synchronous authorization. Review: https://reviews.apache.org/r/72097 --- src/master/master.cpp | 72 ++++++++++++++++----------------------------------- src/master/master.hpp | 9 ------- 2 files changed, 22 insertions(+), 59 deletions(-) diff --git a/src/master/master.cpp b/src/master/master.cpp index 9d1e541..07685fe 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -2235,17 +2235,6 @@ void Master::detected(const Future<Option<MasterInfo>>& _leader) } -Future<bool> Master::authorizeFramework( - const FrameworkInfo& frameworkInfo) -{ - return authorize( - frameworkInfo.has_principal() - ? Option<Principal>(frameworkInfo.principal()) - : Option<Principal>::none(), - ActionObject::frameworkRegistration(frameworkInfo)); -} - - Option<Error> Master::validateFrameworkAuthentication( const FrameworkInfo& frameworkInfo, const UPID& from) @@ -3210,6 +3199,12 @@ void Master::updateFramework( Future<process::http::Response> Master::updateFramework( mesos::scheduler::Call::UpdateFramework&& call) { + Framework* const framework = + CHECK_NOTNULL(getFramework(call.framework_info().id())); + + LOG(INFO) << "Processing UPDATE_FRAMEWORK call for framework " + << call.framework_info().id(); + Option<Error> error = validateFramework(call.framework_info(), call.suppressed_roles()); @@ -3218,52 +3213,29 @@ Future<process::http::Response> Master::updateFramework( "Supplied FrameworkInfo is not valid: " + error->message); } - LOG(INFO) << "Processing UPDATE_FRAMEWORK call for framework " - << call.framework_info().id(); - - Future<bool> authorized = authorizeFramework(call.framework_info()); - - return authorized - .then(defer(self(), &Self::_updateFramework, std::move(call), lambda::_1)); -} - + error = validation::framework::validateUpdate( + framework->info, call.framework_info()); -Future<process::http::Response> Master::_updateFramework( - mesos::scheduler::Call::UpdateFramework&& call, - const Future<bool>& authorized) -{ - if (authorized.isFailed()) { + if (error.isSome()) { return process::http::BadRequest( - "Authorization failure: " + authorized.failure()); - } - - // TODO(asekretenko): We should make it possible for the authorizer to return - // the exact cause of declined authorization, after which we can simply - // forward it here. - if (!authorized.get()) { - return process::http::Forbidden( - "Not authorized to use roles '" + - stringify(protobuf::framework::getRoles(call.framework_info())) + "'"); + "FrameworkInfo update is not valid: " + error->message); } - Framework* framework = getFramework(call.framework_info().id()); + ActionObject actionObject = + ActionObject::frameworkRegistration(call.framework_info()); - // TODO(asekretenko): Test against the race with framework removal. - if (framework == nullptr) { - return process::http::Conflict( - "Framework was removed while this call was being processed"); + Try<bool> approved = framework->approved(actionObject); + if (approved.isError()) { + return process::http::BadRequest( + "Authorization failure: " + approved.error()); } - // We need to validate the FrameworkInfo update here - // to avoid races with SUBSCRIBE or another UPDATE_FRAMEWORK call. - // - // TODO(asekretenko): Test against these races. - Option<Error> error = validation::framework::validateUpdate( - framework->info, call.framework_info()); - - if (error.isSome()) { - return process::http::BadRequest( - "FrameworkInfo update is not valid: " + error->message); + if (!*approved) { + // TODO(asekretenko): We should make it possible for the objectApprover to + // return the exact cause of declined authorization, after which we can + // simply forward it here. + return process::http::Forbidden( + "Not authorized to " + stringify(actionObject)); } set<string> suppressedRoles( diff --git a/src/master/master.hpp b/src/master/master.hpp index e0ef1cd..f766b5c 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -762,11 +762,6 @@ protected: const Option<process::http::authentication::Principal>& principal, std::vector<authorization::ActionObject>&& actionObjects); - // Returns whether the framework is authorized. - // Returns failure for transient authorization failures. - process::Future<bool> authorizeFramework( - const FrameworkInfo& frameworkInfo); - // Determine if a new executor needs to be launched. bool isLaunchExecutor ( const ExecutorID& executorId, @@ -947,10 +942,6 @@ private: process::Future<process::http::Response> updateFramework( mesos::scheduler::Call::UpdateFramework&& call); - process::Future<process::http::Response> _updateFramework( - mesos::scheduler::Call::UpdateFramework&& call, - const process::Future<bool>& authorized); - // Subscribes a client to the 'api/vX' endpoint. void subscribe( const StreamingHttpConnection<v1::master::Event>& http,
