> On June 10, 2014, 8:37 a.m., Adam B wrote: > > src/master/master.cpp, lines 1050-1051 > > <https://reviews.apache.org/r/22284/diff/2/?file=606172#file606172line1050> > > > > 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?!?
The acls.permissive bit only comes into play when none of the ACLs that were setup matches the authorization request. You can think of it as the default case in a switch statement. An authorization request will likely never have "NONE". I can't imagine why someone would ask that question from authorizer. In this particular case, we ask if ANY principal is allowed because the framework didn't set its principal (eg. auth disabled and FrameworkInfo.principal is not set). In the future, we might make FrameworkInfo.principal 'required' instead of 'optional' in which case we wont be making such request. For now we make it optional for smooth upgrade. Hope that clears things up. > On June 10, 2014, 8:37 a.m., Adam B wrote: > > src/master/master.cpp, lines 1097-1100 > > <https://reviews.apache.org/r/22284/diff/2/?file=606172#file606172line1097> > > > > 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. See the discussion in the "authorize tasks" ticket regarding this. Since the type is Option<Error>, a '.isSome()' tells one that it is an error. We have used this format before (see status update manager, group). > On June 10, 2014, 8:37 a.m., Adam B wrote: > > src/tests/master_authorization_tests.cpp, lines 881-882 > > <https://reviews.apache.org/r/22284/diff/2/?file=606173#file606173line881> > > > > Why confirm the first rereg with an EXPECT_CALL(sched, reregistered) > > and the second rereg with FUTURE_PROTOBUF(FrameworkReregisteredMessage)? yes because the second re-registered message is dropped by the driver as the driver is already registered. > On June 10, 2014, 8:37 a.m., Adam B wrote: > > src/master/flags.hpp, line 146 > > <https://reviews.apache.org/r/22284/diff/2/?file=606170#file606170line146> > > > > Deprecate this in favor of '--acls' too? Interesting idea, but our current ACLs do not support this. Also, there is currently work being done by Alexandra Sava around node drain which might change this behavior. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22284/#review45202 ----------------------------------------------------------- On June 9, 2014, 10: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, 10: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 > >
