----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22284/#review45202 -----------------------------------------------------------
Ship it! Good stuff. I'm just a bit confused about how the ReceiveOffers Message uses ANY/NONE principals in the master ACL vs. a specific request. src/master/flags.hpp <https://reviews.apache.org/r/22284/#comment79922> Deprecate this in favor of '--acls' too? src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79923> Why is this wrapped now? I only count 77 chars, and it doesn't look modified otherwise. src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79925> Please correct me if I'm misinterpreting the ReceiveOffers Message. In the master ACL: global-permissive: ANY Principal can ReceiveOffers for Role foo global-restrictive: NONE Principal can ReceiveOffers for Role foo But when calling authorize(ReceiveOffers request), Use ANY Principal when the framework has No Principal (framework authentication disabled?), since that's the only time this framework will still be authorized. Use NONE Principal never?!? src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79926> These Option<Error>'s read weird to me. I would expect future.get().isSome() to be a positive case, not an error case. Using Try<Nothing>'s instead gives "if(future.get().isError()) { //handle error }", which reads a lot better to me. src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79927> Log 'from'? src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79928> Log frameworkInfo.id()? src/tests/master_authorization_tests.cpp <https://reviews.apache.org/r/22284/#comment79929> Why confirm the first rereg with an EXPECT_CALL(sched, reregistered) and the second rereg with FUTURE_PROTOBUF(FrameworkReregisteredMessage)? - Adam B On June 9, 2014, 3:09 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22284/ > ----------------------------------------------------------- > > (Updated June 9, 2014, 3:09 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1307 > https://issues.apache.org/jira/browse/MESOS-1307 > > > Repository: mesos-git > > > Description > ------- > > Added authorization for roles during framework (re-)registration. > > > Diffs > ----- > > src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149 > src/master/master.hpp e2448310aa714b5fae49b9bf4fc95859ae9d7ec3 > src/master/master.cpp 89f426c14de365369b900864f1983b1f9260953f > src/tests/master_authorization_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/22284/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
