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 Fri, 10 Mar 2017 12:51:22 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57473/#review168590
-----------------------------------------------------------




src/authorizer/local/authorizer.cpp
Lines 423 (patched)
<https://reviews.apache.org/r/57473/#comment240853>

    Redundant `break` here.



src/authorizer/local/authorizer.cpp
Line 477 (original), 429 (patched)
<https://reviews.apache.org/r/57473/#comment240854>

    Not your's, but another redundant `break`.



src/authorizer/local/authorizer.cpp
Lines 765 (patched)
<https://reviews.apache.org/r/57473/#comment240849>

    Not clear what you meant here, _matches any ACL, any order_? Also `splitted` -> `split`.



src/authorizer/local/authorizer.cpp
Lines 846-848 (patched)
<https://reviews.apache.org/r/57473/#comment240850>

    We must expand the full list of values here,
    
        case authorization::ACCESS_MESOS_LOG:
        case authorization::ACCESS_SANDBOX:
        case authorization::ATTACH_CONTAINER_INPUT:
        case authorization::ATTACH_CONTAINER_OUTPUT:
        case authorization::DESTROY_VOLUME:
        case authorization::GET_ENDPOINT_WITH_PATH:
        case authorization::KILL_NESTED_CONTAINER:
        case authorization::LAUNCH_NESTED_CONTAINER:
        case authorization::LAUNCH_NESTED_CONTAINER_SESSION:
        case authorization::RUN_TASK:
        case authorization::SET_LOG_LEVEL:
        case authorization::TEARDOWN_FRAMEWORK:
        case authorization::UNKNOWN:
        case authorization::UNRESERVE_RESOURCES:
        case authorization::VIEW_CONTAINER:
        case authorization::VIEW_EXECUTOR:
        case authorization::VIEW_FLAGS:
        case authorization::VIEW_FRAMEWORK:
        case authorization::VIEW_TASK:
        case authorization::WAIT_NESTED_CONTAINER:
          UNREACHABLE();



src/authorizer/local/authorizer.cpp
Lines 686-698 (original), 961-973 (patched)
<https://reviews.apache.org/r/57473/#comment240852>

    Let's pull these into the switch and just put an `UNREACHABLE` here.



src/authorizer/local/authorizer.cpp
Line 990 (original), 1193 (patched)
<https://reviews.apache.org/r/57473/#comment240855>

    Not yours, but it looks like most `break` statements in this `switch` are redundant since
they directly follow a `return`.
    
    We should clean this up.


- Benjamin Bannier


On March 10, 2017, 9:18 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57473/
> -----------------------------------------------------------
> 
> (Updated March 10, 2017, 9:18 a.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 2227b241eab1606815fa6464e3d6b1345624fd22 
> 
> 
> Diff: https://reviews.apache.org/r/57473/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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