mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 57473: Added support for authorization of Hierachical roles.
Date Mon, 20 Mar 2017 15:54:23 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message