----------------------------------------------------------- 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 > >
