This is an automated email from the ASF dual-hosted git repository. tillt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 176f5c2fb998b2327059893e180501b32f9450e0 Author: Till Toenshoff <[email protected]> AuthorDate: Tue Nov 20 14:46:00 2018 +0100 Introduced common/authorization and refactored collectAuthorizations. Adds a new collection of authorization specific helper/s to reduce code duplication and increase efficient test coverage. Moves the newly introduced 'collectAuthorizations' helper into this new authorization source unit. Review: https://reviews.apache.org/r/69384/ --- src/CMakeLists.txt | 1 + src/Makefile.am | 2 ++ src/common/authorization.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ src/common/authorization.hpp | 35 +++++++++++++++++++++++++++++++++++ src/master/master.cpp | 11 ++++++----- src/master/master.hpp | 13 ------------- src/master/weights_handler.cpp | 4 +++- src/tests/master_tests.cpp | 17 +++++++++-------- 8 files changed, 97 insertions(+), 27 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7323682..bde0704 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -221,6 +221,7 @@ set(AUTHORIZER_SRC authorizer/local/authorizer.cpp) set(COMMON_SRC + common/authorization.cpp common/attributes.cpp common/build.cpp common/command_utils.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 2d9c81b..8da1a05 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -996,6 +996,7 @@ libmesos_no_3rdparty_la_SOURCES += \ checks/checker_process.cpp \ checks/health_checker.cpp \ common/attributes.cpp \ + common/authorization.cpp \ common/command_utils.cpp \ common/http.cpp \ common/protobuf_utils.cpp \ @@ -1145,6 +1146,7 @@ libmesos_no_3rdparty_la_SOURCES += \ checks/checks_runtime.hpp \ checks/checks_types.hpp \ checks/health_checker.hpp \ + common/authorization.hpp \ common/build.hpp \ common/command_utils.hpp \ common/http.hpp \ diff --git a/src/common/authorization.cpp b/src/common/authorization.cpp new file mode 100644 index 0000000..5064ad2 --- /dev/null +++ b/src/common/authorization.cpp @@ -0,0 +1,41 @@ +// 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 "common/authorization.hpp" + +#include <algorithm> + +#include <process/collect.hpp> + +#include <stout/foreach.hpp> + +using std::vector; + +using process::Future; + +namespace mesos { +namespace authorization { + +Future<bool> collectAuthorizations(const vector<Future<bool>>& authorizations) +{ + return process::collect(authorizations) + .then([](const vector<bool>& results) -> Future<bool> { + return std::find(results.begin(), results.end(), false) == results.end(); + }); +} + +} // namespace authorization { +} // namespace mesos { diff --git a/src/common/authorization.hpp b/src/common/authorization.hpp new file mode 100644 index 0000000..439996a --- /dev/null +++ b/src/common/authorization.hpp @@ -0,0 +1,35 @@ +// 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 __COMMON_AUTHORIZATION_HPP__ +#define __COMMON_AUTHORIZATION_HPP__ + +#include <vector> + +#include <process/future.hpp> + +namespace mesos { +namespace authorization { + +// Collects authorization results. Any discarded or failed future +// results in a failure; any false future results in 'false'. +process::Future<bool> collectAuthorizations( + const std::vector<process::Future<bool>>& authorizations); + +} // namespace authorization { +} // namespace mesos { + +#endif // __COMMON_AUTHORIZATION_HPP__ diff --git a/src/master/master.cpp b/src/master/master.cpp index 6e34cc4..0b43dc7 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -75,6 +75,7 @@ #include "authentication/cram_md5/authenticator.hpp" +#include "common/authorization.hpp" #include "common/build.hpp" #include "common/http.hpp" #include "common/protobuf_utils.hpp" @@ -3641,7 +3642,7 @@ Future<bool> Master::authorizeReserveResources( return authorizer.get()->authorized(request); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } @@ -3694,7 +3695,7 @@ Future<bool> Master::authorizeUnreserveResources( return authorizer.get()->authorized(request); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } @@ -3751,7 +3752,7 @@ Future<bool> Master::authorizeCreateVolume( return authorizer.get()->authorized(request); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } @@ -3793,7 +3794,7 @@ Future<bool> Master::authorizeDestroyVolume( return authorizer.get()->authorized(request); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } @@ -3974,7 +3975,7 @@ Future<bool> Master::authorizeSlave( authorizeReserveResources(slaveInfo.resources(), principal)); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } diff --git a/src/master/master.hpp b/src/master/master.hpp index afd829e..baa6668 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -19,7 +19,6 @@ #include <stdint.h> -#include <algorithm> #include <list> #include <memory> #include <set> @@ -2353,18 +2352,6 @@ private: }; -// Collects authorization results. Any discarded or failed future -// results in a failure; any false future results in false. -inline process::Future<bool> collectAuthorizations( - const std::vector<process::Future<bool>>& authorizations) -{ - return process::collect(authorizations) - .then([](const std::vector<bool>& results) -> process::Future<bool> { - return std::find(results.begin(), results.end(), false) == results.end(); - }); -} - - inline std::ostream& operator<<( std::ostream& stream, const Framework& framework); diff --git a/src/master/weights_handler.cpp b/src/master/weights_handler.cpp index 1d34356..dfb6f06 100644 --- a/src/master/weights_handler.cpp +++ b/src/master/weights_handler.cpp @@ -28,6 +28,8 @@ #include <stout/strings.hpp> #include <stout/utils.hpp> +#include "common/authorization.hpp" + #include "master/weights.hpp" namespace http = process::http; @@ -345,7 +347,7 @@ Future<bool> Master::WeightsHandler::authorizeUpdateWeights( return master->authorizer.get()->authorized(request); } - return collectAuthorizations(authorizations); + return authorization::collectAuthorizations(authorizations); } diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index 658051f..651bb9b 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -46,6 +46,7 @@ #include <stout/strings.hpp> #include <stout/try.hpp> +#include "common/authorization.hpp" #include "common/build.hpp" #include "common/protobuf_utils.hpp" @@ -10160,8 +10161,8 @@ TEST_F(MasterTest, CollectAuthorizations) Promise<bool> promise1; Promise<bool> promise2; - Future<bool> result = - master::collectAuthorizations({promise1.future(), promise2.future()}); + Future<bool> result = authorization::collectAuthorizations( + {promise1.future(), promise2.future()}); promise1.set(true); promise2.set(false); @@ -10173,8 +10174,8 @@ TEST_F(MasterTest, CollectAuthorizations) Promise<bool> promise1; Promise<bool> promise2; - Future<bool> result = - master::collectAuthorizations({promise1.future(), promise2.future()}); + Future<bool> result = authorization::collectAuthorizations( + {promise1.future(), promise2.future()}); promise1.set(true); promise2.fail("Authorization failure"); @@ -10186,8 +10187,8 @@ TEST_F(MasterTest, CollectAuthorizations) Promise<bool> promise1; Promise<bool> promise2; - Future<bool> result = - master::collectAuthorizations({promise1.future(), promise2.future()}); + Future<bool> result = authorization::collectAuthorizations( + {promise1.future(), promise2.future()}); promise1.set(true); promise2.discard(); @@ -10199,8 +10200,8 @@ TEST_F(MasterTest, CollectAuthorizations) Promise<bool> promise1; Promise<bool> promise2; - Future<bool> result = - master::collectAuthorizations({promise1.future(), promise2.future()}); + Future<bool> result = authorization::collectAuthorizations( + {promise1.future(), promise2.future()}); promise1.set(true); promise2.set(true);
