----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18730/#review42577 -----------------------------------------------------------
Ship it! src/authorizer/authorizer.hpp <https://reviews.apache.org/r/18730/#comment76378> Run mesos-style.py. ;-) src/authorizer/authorizer.hpp <https://reviews.apache.org/r/18730/#comment76379> The caveat with this one is that two different ACLs might satisfy this request in aggregate. Consider these ACLs: users: [ benh ] action: http_get urls: [ /secret ] users: [ vinod ] action: http_get urls: [ /secret ] And now this request: users: [ benh, vinod ] action: http_get urls: [ /secret ] So if we aren't going to do any aggregation then we need to call this out explicitly in the documentation (and add a TODO to relax this after we add aggregation). But to be perfectly honest, I don't think this kind of aggregation is that difficult. src/authorizer/authorizer.hpp <https://reviews.apache.org/r/18730/#comment76380> mesos-style.py src/authorizer/authorizer.hpp <https://reviews.apache.org/r/18730/#comment76381> For the SOME cases, by "every element is allowed" I think using the phrase "the request values are a subset of the ACL values" might be helpful for people to understand the logic (same goes for the logic in 'matches' too). Also, this suffers from the same fate as above with respect to disaggregated ACLs. src/tests/authorization_tests.cpp <https://reviews.apache.org/r/18730/#comment76383> Tests all look great! Can we also add some tests that handle the aggregation case to capture our intended semantics please? Thanks! src/tests/authorization_tests.cpp <https://reviews.apache.org/r/18730/#comment76382> principal - Benjamin Hindman On May 9, 2014, 6:22 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18730/ > ----------------------------------------------------------- > > (Updated May 9, 2014, 6:22 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen. > > > Bugs: MESOS-911 > https://issues.apache.org/jira/browse/MESOS-911 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/mesos.proto a5826d7d732b32b31802b3ed9a1e34b234bea061 > src/Makefile.am f461a1515e7bafac677f2d0bcdd499f57ba3f029 > src/authorizer/authorizer.hpp PRE-CREATION > src/tests/authorization_tests.cpp PRE-CREATION > src/tests/master_contender_detector_tests.cpp > 42051bfc7c698e2e80cfe23686ee11ef722b679e > > Diff: https://reviews.apache.org/r/18730/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
