----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57473/#review168590 -----------------------------------------------------------
src/authorizer/local/authorizer.cpp Lines 423 (patched) <https://reviews.apache.org/r/57473/#comment240853> Redundant `break` here. src/authorizer/local/authorizer.cpp Line 477 (original), 429 (patched) <https://reviews.apache.org/r/57473/#comment240854> Not your's, but another redundant `break`. src/authorizer/local/authorizer.cpp Lines 765 (patched) <https://reviews.apache.org/r/57473/#comment240849> Not clear what you meant here, _matches any ACL, any order_? Also `splitted` -> `split`. src/authorizer/local/authorizer.cpp Lines 846-848 (patched) <https://reviews.apache.org/r/57473/#comment240850> We must expand the full list of values here, case authorization::ACCESS_MESOS_LOG: case authorization::ACCESS_SANDBOX: case authorization::ATTACH_CONTAINER_INPUT: case authorization::ATTACH_CONTAINER_OUTPUT: case authorization::DESTROY_VOLUME: case authorization::GET_ENDPOINT_WITH_PATH: case authorization::KILL_NESTED_CONTAINER: case authorization::LAUNCH_NESTED_CONTAINER: case authorization::LAUNCH_NESTED_CONTAINER_SESSION: case authorization::RUN_TASK: case authorization::SET_LOG_LEVEL: case authorization::TEARDOWN_FRAMEWORK: case authorization::UNKNOWN: case authorization::UNRESERVE_RESOURCES: case authorization::VIEW_CONTAINER: case authorization::VIEW_EXECUTOR: case authorization::VIEW_FLAGS: case authorization::VIEW_FRAMEWORK: case authorization::VIEW_TASK: case authorization::WAIT_NESTED_CONTAINER: UNREACHABLE(); src/authorizer/local/authorizer.cpp Lines 686-698 (original), 961-973 (patched) <https://reviews.apache.org/r/57473/#comment240852> Let's pull these into the switch and just put an `UNREACHABLE` here. src/authorizer/local/authorizer.cpp Line 990 (original), 1193 (patched) <https://reviews.apache.org/r/57473/#comment240855> Not yours, but it looks like most `break` statements in this `switch` are redundant since they directly follow a `return`. We should clean this up. - Benjamin Bannier On March 10, 2017, 9:18 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57473/ > ----------------------------------------------------------- > > (Updated March 10, 2017, 9:18 a.m.) > > > Review request for mesos, Adam B and Benjamin Bannier. > > > Repository: mesos > > > Description > ------- > > Adds mechanisms to support authorization of hierarchical roles, > that is, it allows operators to write ACLs of the form `role/%` > which will enforce the rule for any nested role, e.g. `role/a`, > `role/b` and such. > > > Diffs > ----- > > src/authorizer/local/authorizer.cpp > 2227b241eab1606815fa6464e3d6b1345624fd22 > > > Diff: https://reviews.apache.org/r/57473/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Alexander Rojas > >
