Re: Review Request 58964: Added authorization support for operator endpoints.

[Till Toenshoff <to...@me.com>]

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




docs/authorization.md
Lines 290 (patched)
<https://reviews.apache.org/r/58964/#comment250012>

    s/my mesos/by Mesos/



docs/authorization.md
Lines 298 (patched)
<https://reviews.apache.org/r/58964/#comment250013>

    s/my mesos/by Mesos/



docs/authorization.md
Lines 306 (patched)
<https://reviews.apache.org/r/58964/#comment250014>

    s/in a/on a/



docs/authorization.md
Lines 313 (patched)
<https://reviews.apache.org/r/58964/#comment250039>

    s/use types/use the types/



docs/authorization.md
Lines 316 (patched)
<https://reviews.apache.org/r/58964/#comment250015>

    s/in a/on a/



docs/authorization.md
Lines 321 (patched)
<https://reviews.apache.org/r/58964/#comment250040>

    s/use types/use the types/



src/authorizer/local/authorizer.cpp
Line 661 (original)
<https://reviews.apache.org/r/58964/#comment250016>

    Thanks for fixing those.



src/tests/authorization_tests.cpp
Lines 4947 (patched)
<https://reviews.apache.org/r/58964/#comment250018>

    Can we have a short leading comment on all the new tests please?
    I do understand it wont add huge value but I like having those in general - without exceptions.



src/tests/authorization_tests.cpp
Lines 4972 (patched)
<https://reviews.apache.org/r/58964/#comment250054>

    The term "whitelist" confuses me - especially given that we are now using quotes for both, principals as well as something like semantic matches. Maybe the following would be less irritating?
    
    ```
    // "foo" should be allowed to update the schedule.
    ```
    
    Can you update all others accordingly please?



src/tests/authorization_tests.cpp
Lines 4981 (patched)
<https://reviews.apache.org/r/58964/#comment250055>

    ```
    // "bar" should not be allowed to update the schedule.
    ```


- Till Toenshoff


On June 1, 2017, 2:56 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> -----------------------------------------------------------
> 
> (Updated June 1, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
>     https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`,
> `STOP_MAINTENANCE` and `GET_MAINTENANCE_STATUS` to the authorizer
> API as well as the necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing
> the semantics of allow maintenance on all nodes or none. This was done
> to extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -----
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.hpp 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>