mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Toenshoff <toensh...@me.com>
Subject Re: Review Request 58964: Added authorization support for operator endpoints.
Date Mon, 12 Jun 2017 09:48:15 GMT

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


Fix it, then Ship it!




Looks great - one nit on the proto message member naming.


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

    Not yours, but it seems we are missing a bunch here -- we should get them complete - mind
doing this here or in a follow-up?
    
    ```
      optional quota.QuotaInfo quota_info = 6;
      optional WeightInfo weight_info = 7;
      optional Resource resource = 8;
      optional CommandInfo command_info = 9;
      optional ContainerID container_id = 10;
    ```



include/mesos/authorizer/acls.proto
Lines 472-476 (patched)
<https://reviews.apache.org/r/58964/#comment251284>

    We are using repeated here, hence we are allowing for multiple elements and we do always
use plural for the naming of such proto message arrays.
    
    In this case, maybe we regard each of the members as an ACL; add "es" for a shortened
way of saying ACLs.
    
    ```
      repeated ACL.UpdateMaintenanceSchedule update_maintenance_schedules = 35;
      repeated ACL.GetMaintenanceSchedule get_maintenance_schedules = 36;
      repeated ACL.StartMaintenance start_maintenances = 37;
      repeated ACL.StopMaintenance stop_maintenances = 38;
      repeated ACL.GetMaintenanceStatus get_maintenance_statuses = 39;
    ```


- Till Toenshoff


On June 2, 2017, 12:32 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> -----------------------------------------------------------
> 
> (Updated June 2, 2017, 12:32 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 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
>   include/mesos/authorizer/authorizer.hpp 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto c9184d151befa4cea9bdebb36a315c760e6424b2

>   src/authorizer/local/authorizer.cpp 1f2a9902e88705cf412af834c127c3afe6d3bef4 
>   src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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