Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review95328 --- Ship it! src/local/local.cpp (line 219) https://reviews.apache.org/r/36049/#comment150242 s/authenticator/authorizer/ - will fix while committing. src/local/local.cpp (line 222) https://reviews.apache.org/r/36049/#comment150243 See above. src/master/main.cpp (line 349) https://reviews.apache.org/r/36049/#comment150244 See above. src/master/main.cpp (line 352) https://reviews.apache.org/r/36049/#comment150245 See above. - Till Toenshoff On Aug. 12, 2015, 1:52 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 12, 2015, 1:52 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 12, 2015, 3:52 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 10, 2015, 5:29 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Changes --- mcpark review. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs (updated) - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/#review94664
---
src/local/local.cpp (lines 217 - 219)
https://reviews.apache.org/r/36049/#comment149269
Does it make sense to log a warning for this?
```
if (flags.authorizers != master::DEFAULT_AUTHORIZER flags.acls.isSome())
{
LOG(WARNNING) /* acls are being ignored! */;
}
```
src/local/local.cpp (lines 238 - 248)
https://reviews.apache.org/r/36049/#comment149268
This behavior is noticeably different, since we used to simply
`EXIT(EXIT_FAILURE)` on this error. What's the rationale here?
My thoughts: if an authorizer fails to initialize given some ACLs, it's
seems likely that the ACLs are ill-formed. In which case, the intent from the
user is clear that they __want__ authorization, but they made a mistake in
their ACL construction. Therefore, it seems more reasonable for us to exit with
an error rather than ignoring their intent and skipping authorization
altogether.
src/master/flags.cpp (line 417)
https://reviews.apache.org/r/36049/#comment149270
Can we expand a little upon `Authorizer implementation to use.`?
Maybe something like: `Authorizer implementation to use when authorizating
actions that required it.`?
src/master/flags.cpp (line 422)
https://reviews.apache.org/r/36049/#comment149271
`s/than default/than the default`
`s/contents//`
src/master/flags.cpp (line 425)
https://reviews.apache.org/r/36049/#comment149272
I think we can follow what we do for `authenticators` in
`src/master/master.cpp` to indicate the lack of support for multiple
authorizers.
(1) split `flags.authorizers` by `,`
(2) if empty - no authorizers specified
(3) if size 1, multiple authorizers not supported
(4) proceed with the first name.
src/master/main.cpp (lines 346 - 379)
https://reviews.apache.org/r/36049/#comment149273
Same comments as `src/local/local.cpp`
- Michael Park
On Aug. 5, 2015, 9:15 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/
---
(Updated Aug. 5, 2015, 9:15 a.m.)
Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till
Toenshoff.
Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947
Repository: mesos
Description
---
Adds and integrates helper classes needed to support an `Authorizer` module.
Also adds a flag to the master, allowing the selection of an `Authorizer`
module.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/module/authorizer.hpp PRE-CREATION
src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec
src/authorizer/authorizer.cpp PRE-CREATION
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce
src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756
src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce
src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670
src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b
Diff: https://reviews.apache.org/r/36049/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review94667 --- src/master/flags.cpp (line 415) https://reviews.apache.org/r/36049/#comment149277 Could we please add this flag to the documentation i.e. configuration.md - Joerg Schad On Aug. 5, 2015, 9:15 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 5, 2015, 9:15 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
On Aug. 3, 2015, 4:34 p.m., Bernd Mathiske wrote: src/local/local.cpp, line 221 https://reviews.apache.org/r/36049/diff/8/?file=1017149#file1017149line221 That's a bit too subtle for me. Proposals, either: - pass the acls to the custom authorizer and let it decide what to do with them - or disallow the presence of both flags, terminate the slave with an error. The latter would also greatly simplify the control flow here. The ideal would be to remove the whole `Authorizer::initialize()`, however we cannot disallow the presence of both flags since `--authorizers` is always set (with a default value). True, the code ain't pretty, but instead of giving it a make up, I would rather push for a long term solution (removing the initialize method at least with that signature), and then entirely remove this code. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review93906 --- On Aug. 4, 2015, 6:26 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 4, 2015, 6:26 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review94207 --- Ship it! src/local/local.cpp (line 226) https://reviews.apache.org/r/36049/#comment148761 Wrong spacing. - Jan Schlicht On Aug. 4, 2015, 6:26 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 4, 2015, 6:26 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review94023 --- Ship it! Ship It! - Jan Schlicht On Aug. 3, 2015, 11:47 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 3, 2015, 11:47 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 4, 2015, 6:26 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Changes --- Rebase, changes from reviewers. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review93906 --- src/local/local.cpp (line 221) https://reviews.apache.org/r/36049/#comment148332 That's a bit too subtle for me. Proposals, either: - pass the acls to the custom authorizer and let it decide what to do with them - or disallow the presence of both flags, terminate the slave with an error. The latter would also greatly simplify the control flow here. src/master/flags.cpp (line 230) https://reviews.apache.org/r/36049/#comment148333 See above. IMHO better to disallow having both flags simultaneously. If the user gives contradictory instructions, the slave should not start. src/master/flags.cpp (line 421) https://reviews.apache.org/r/36049/#comment148335 See above. src/master/main.cpp (line 305) https://reviews.apache.org/r/36049/#comment148336 See above. - Bernd Mathiske On Aug. 3, 2015, 2:47 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 3, 2015, 2:47 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review93898 --- Ship it! Ship It! - Till Toenshoff On Aug. 3, 2015, 9:47 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 3, 2015, 9:47 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
On July 8, 2015, 12:12 a.m., Till Toenshoff wrote:
src/local/local.cpp, line 241
https://reviews.apache.org/r/36049/diff/7/?file=1000249#file1000249line241
I am assuming that the `LocalAuthorizer` should be considered unusable
should its initialize function ever fail.
My most favored solution here would be to log the failure and make sure
that `authorizer` remains unset so that we can operate without any
authorization. That would be following the approach of the authenticator
`initialize` failure handling.
```
TryNothing initialize =
authorizer.get()-initialize(flags.acls.get());
if (initialize.isError()) {
// A failure to initialize the authorizer does lead to unusable
authorization
// but allows actions to skip authorization.
LOG(WARNING) Authorization is disabled: Failed to initialize '
flags.authorizers ' authorizer:
initialize.error();
delete authorizer.get();
authorizer = None();
}
```
Inherited from
https://github.com/apache/mesos/blob/master/src/master/master.cpp#L484
As a note, please don't use links to the master branch, use a specific review
since any update on the master invalidates the line you want to show. e.g.
https://github.com/apache/mesos/blob/bfe6c07b79550bb3d1f2ab6f5344d740e6eb6f60/src/master/master.cpp#L484
- Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/#review90774
---
On July 7, 2015, 9:34 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/
---
(Updated July 7, 2015, 9:34 a.m.)
Review request for mesos, Adam B and Till Toenshoff.
Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947
Repository: mesos
Description
---
Adds and integrates helper classes needed to support an `Authorizer` module.
Also adds a flag to the master, allowing the selection of an `Authorizer`
module.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/module/authorizer.hpp PRE-CREATION
src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851
src/authorizer/authorizer.cpp PRE-CREATION
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce
src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756
src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9
src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670
src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26
Diff: https://reviews.apache.org/r/36049/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 7, 2015, 9:34 a.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Update JIRA entry. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/#review90774
---
There are some nits and slight inconsistencies but overall I think we are in
good shape here.
src/local/local.cpp (line 217)
https://reviews.apache.org/r/36049/#comment143899
Capital The please.
src/local/local.cpp (line 220)
https://reviews.apache.org/r/36049/#comment143898
Please start with a capital Add after that colon.
src/local/local.cpp (lines 227 - 228)
https://reviews.apache.org/r/36049/#comment143900
I think we should rephrase the message here;
```
Could not create ' flags.authorizers ' authorizer:
create.error()
```
src/local/local.cpp (line 232)
https://reviews.apache.org/r/36049/#comment143903
For validating the configuration, I always found it very helpful that we
were showing the activated authenticator name/s in the master log -- hence I
would like to suggest to do the same here as well;
```
LOG(INFO) Using ' flags.authorizers ' authorizer;
```
src/local/local.cpp (line 234)
https://reviews.apache.org/r/36049/#comment143909
I am assuming that the `LocalAuthorizer` should be considered unusable
should its initialize function ever fail.
My most favored solution here would be to log the failure and make sure
that `authorizer` remains unset so that we can operate without any
authorization. That would be following the approach of the authenticator
`initialize` failure handling.
```
TryNothing initialize = authorizer.get()-initialize(flags.acls.get());
if (initialize.isError()) {
// A failure to initialize the authorizer does lead to unusable
authorization
// but allows actions to skip authorization.
LOG(WARNING) Authorization is disabled: Failed to initialize '
flags.authorizers ' authorizer:
initialize.error();
delete authorizer.get();
authorizer = None();
}
```
Inherited from
https://github.com/apache/mesos/blob/master/src/master/master.cpp#L484
src/master/flags.cpp (line 230)
https://reviews.apache.org/r/36049/#comment143910
s/authorizer/authorizers/
src/master/flags.cpp (line 231)
https://reviews.apache.org/r/36049/#comment143911
Lets make sure we match the flag name and also replace that default by
the actual implementation name.
```
Note that if the flag --authorizers is provided with a value different\n
than ' + DEFAULT_AUTHORIZER + ', the ACLs contents will be ignored.\n
\n
```
src/master/flags.cpp (line 421)
https://reviews.apache.org/r/36049/#comment143912
s/authorizer/authorizers/
Please sure you check if you properly renamed that flag in all references.
Thanks Alexander :)
src/master/flags.cpp (lines 423 - 424)
https://reviews.apache.org/r/36049/#comment143913
That looks like weird wrapping to me.
src/master/main.cpp (lines 301 - 317)
https://reviews.apache.org/r/36049/#comment143916
See my comments on local.cpp starting at line 217 ff. regarding this entire
block.
- Till Toenshoff
On July 7, 2015, 7:34 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/
---
(Updated July 7, 2015, 7:34 a.m.)
Review request for mesos, Adam B and Till Toenshoff.
Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947
Repository: mesos
Description
---
Adds and integrates helper classes needed to support an `Authorizer` module.
Also adds a flag to the master, allowing the selection of an `Authorizer`
module.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/module/authorizer.hpp PRE-CREATION
src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851
src/authorizer/authorizer.cpp PRE-CREATION
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce
src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756
src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9
src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670
src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26
Diff: https://reviews.apache.org/r/36049/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 6, 2015, 5:42 p.m.) Review request for mesos, Adam B and Till Toenshoff. Changes --- Proper use of factory. Includes cleanup. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 3, 2015, 3:17 p.m.) Review request for mesos and Till Toenshoff. Changes --- No longer assumes that `Authorizer::create()` can return a null pointer. Adds documentation on the usage of the `--authorizers` flag. Adds a **TODO** for support of multiple authorizers. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description (updated) --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. It also adds a flag which allows to select the module name. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
src/authorizer/authorizer.cpp, lines 17-28
https://reviews.apache.org/r/36049/diff/5/?file=998603#file998603line17
Great update! But it seems the comment is not applying, no?
The built-in authorizer factory `TryAuthorizer*
LocalAuthorizer::create()` does not seem to test for `nullptr`. You seem to
do those checks in master.cpp and local.cpp instead. Or am I missing the
point here?
`LocalAuthorizer::create()` now returns an error if new fails, i.e. returns a
null pointer. Still not sure if I should remove the check in main.
On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
src/local/local.cpp, lines 227-228
https://reviews.apache.org/r/36049/diff/5/?file=998604#file998604line227
You may reach this without an error-message, no?
How about:
```
if (create.isError() || create.get() == nullptr) {
EXIT(EXIT_FAILURE) Could not create authorizer module '
flags.authorizer '
(create.isError() ? : + create.error() : );
;
}
```
Or actually, that factory can not return a `nullptr` anymore, can it?
Now the comment from the previos issue is true, so create never returns a
nullptr but instead sets the `Try` to an `Error`.
On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
src/master/flags.cpp, lines 415-424
https://reviews.apache.org/r/36049/diff/5/?file=998608#file998608line415
We may want to follow the pattern used for the authenticator - as in,
call the flag authorizers and prepare the CLI interface for allowing the
activation of multiple, concurrently active authorizers.
What do you think?
as bespoken.
- Alexander
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/#review90317
---
On July 3, 2015, 3:17 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36049/
---
(Updated July 3, 2015, 3:17 p.m.)
Review request for mesos and Till Toenshoff.
Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945
Repository: mesos
Description
---
Adds and integrates helper classes needed to support an `Authorizer` module.
Also adds a flag to the master, allowing the selection of an `Authorizer`
module.
It also adds a flag which allows to select the module name.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/module/authorizer.hpp PRE-CREATION
src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851
src/authorizer/authorizer.cpp PRE-CREATION
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce
src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756
src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9
src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670
Diff: https://reviews.apache.org/r/36049/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
On July 3, 2015, 8:29 a.m., Till Toenshoff wrote: Let's also update the description of this RR; ``` Adds and integrates helper classes needed to support an Authorizer module. Also adds a flag to the master, allowing the selection of an Authorizer module. ``` or maybe better `.. Authorizer modules.` - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review90317 --- On July 2, 2015, 11:34 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 2, 2015, 11:34 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds all the helper classes needed to support an Authorizer module. It also adds a flag which allows to select the module name. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review90341 --- src/master/flags.cpp (line 422) https://reviews.apache.org/r/36049/#comment143353 s/default/ + DEFAULT_AUTHORIZER + / - Till Toenshoff On July 2, 2015, 11:34 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 2, 2015, 11:34 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds all the helper classes needed to support an Authorizer module. It also adds a flag which allows to select the module name. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 2, 2015, 12:25 p.m.) Review request for mesos and Till Toenshoff. Changes --- Fixes include path. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds all the helper classes needed to support an Authorizer module. It also adds a flag which allows to select the module name. Diffs (updated) - include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated July 3, 2015, 1:34 a.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-2945 https://issues.apache.org/jira/browse/MESOS-2945 Repository: mesos Description --- Adds all the helper classes needed to support an Authorizer module. It also adds a flag which allows to select the module name. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
