> On Dec. 13, 2017, 11:32 a.m., James Peach wrote: > > src/master/master.cpp > > Line 3876 (original), 3881 (patched) > > <https://reviews.apache.org/r/64515/diff/1/?file=1912811#file1912811line3881> > > > > You can now log the `SlaveID` here.
The `SlaveInfo` here may not have a `SlaveID` if it's called from `registerSlave`. Here to consistently identity the agent, we would need to pass in the pid of the agent, (`SlaveInfo` also has the hostname). This info is already logged once the authorization is done. Other `Master::authorize*` methods all log the data passed to the authorizer here, I'll add the agent's resources for consistency. > On Dec. 13, 2017, 11:32 a.m., James Peach wrote: > > src/master/master.cpp > > Lines 3905 (patched) > > <https://reviews.apache.org/r/64515/diff/1/?file=1912811#file1912811line3905> > > > > So what are the semantics for dynamic resources? If this new ACL > > semantics is expressing that resources in role `Foo` may only be provided > > on agents which principal `Bar`, it doesn't seem correct that we would > > allow dynamic reservations to violate that. The semantics of `reserve_resources` ACL is still the same: it determines whether a principal can reserve resources of certain roles, regardless of whether it's static or dynamic. What's new is the usage that considers the static reservation to be initiated by the agent's principal and we are authorizing it here. I think in reality we use ACLs to prevent "bad actors" to do harmful things so in the case of dynamic reservations we don't need to addtinoally check against the agent's principal because the agent is not initiating the reservation. In any case, it shouldn't be authorized at agent registration time but reservation request time. I adjusted the comments. Do you think it's clearer? - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64515/#review193690 ----------------------------------------------------------- On Jan. 3, 2018, 11 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64515/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2018, 11 a.m.) > > > Review request for mesos, Alexander Rojas and James Peach. > > > Bugs: MESOS-8306 > https://issues.apache.org/jira/browse/MESOS-8306 > > > Repository: mesos > > > Description > ------- > > Used `reserve_resources` ACL for static reservations. > > > Diffs > ----- > > include/mesos/authorizer/acls.proto > d869820491f1b81f81e1157493aa73e32735b9c3 > src/master/master.hpp 8fe9420dbe03ea2cefc6a40b0f64284aa9fe7915 > src/master/master.cpp 282fdf8ac38e3613c621c1c8e5c50f27bde9dafd > src/tests/master_authorization_tests.cpp > 676543a5ad1bb5d47011fc2a8b05dfaaeef18c64 > > > Diff: https://reviews.apache.org/r/64515/diff/3/ > > > Testing > ------- > > make check. > > > Thanks, > > Jiang Yan Xu > >
