mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 66531: Added new authorization for `UpdateVolume`.
Date Tue, 17 Apr 2018 23:19:56 GMT

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




src/master/master.cpp
Lines 3816-3819 (patched)
<https://reviews.apache.org/r/66531/#comment282554>

    Does this comment make sense here? I see that it's included for other operations like
CREATE above, but it looks to me like this comment only makes sense for RESERVE. The fact
that users can push just one reservation at a time only implies that authorization of the
RESERVE operation has been performed for all prior reservations.
    
    I would recommend removing this comment, but I think the code here can remain the same.
    
    i.e., if a user is attempting to update a volume in role '/foo/bar', we can just authorize
'/foo/bar' and leave it up to the authorizer if it wants to evaluate just '/foo/bar', or to
also evaluate role '/foo'.


- Greg Mann


On April 10, 2018, 7:24 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66531/
> -----------------------------------------------------------
> 
> (Updated April 10, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
>     https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The new authorization action is modelled after `CreateVolume`, and will
> be shared by both `GROW_VOLUME` and `SHRINK_VOLUME` operations and
> corresponding operator APIs.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/acls.proto 8ef33210644e7d2924b402a3158b1b38c1fdb464 
>   include/mesos/authorizer/authorizer.proto 1508c01130013d74ed2b2574a2428f38e3d2c064

>   src/authorizer/local/authorizer.cpp c098ba9ded1b29a2e079cf09ab80b61f6fa4af05 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
> 
> 
> Diff: https://reviews.apache.org/r/66531/diff/1/
> 
> 
> Testing
> -------
> 
> Manually created ACL and verified that untrusted principals will not allow to grow/shrink
volumes.
> Also created unit test in next patch.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


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