----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57473/#review169445 -----------------------------------------------------------
Fix it, then Ship it! src/authorizer/local/authorizer.cpp Lines 62-67 (patched) <https://reviews.apache.org/r/57473/#comment241794> I am still not convinced this abstraction is needed from looking at only at this patch. All it does is move one `string` comparison from `approved` time to ACL build time, but we still do a lot of string comparisons at `approved` time when matching. I also don't see a lot of potential for reuse as we already reuse a lot of code via `createHierarchicalACLs`. I believe this could be replaced by a simple string check at approved time (possibly encapsulated in a function). src/authorizer/local/authorizer.cpp Lines 401-407 (patched) <https://reviews.apache.org/r/57473/#comment241789> Let's sort these alphanumerically. src/authorizer/local/authorizer.cpp Lines 607 (patched) <https://reviews.apache.org/r/57473/#comment241797> I believe we resolved this as `+1`. src/authorizer/local/authorizer.cpp Lines 612-632 (patched) <https://reviews.apache.org/r/57473/#comment241790> Let's sort these. src/authorizer/local/authorizer.cpp Lines 750 (patched) <https://reviews.apache.org/r/57473/#comment241791> `matches`. - Benjamin Bannier On March 17, 2017, 12:54 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57473/ > ----------------------------------------------------------- > > (Updated March 17, 2017, 12:54 p.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 > be8037299601427e5d5e79f58f77eea3f89579d0 > > > Diff: https://reviews.apache.org/r/57473/diff/3/ > > > Testing > ------- > > `make check` > > > Thanks, > > Alexander Rojas > >
