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


Fix it, then Ship it!





docs/authorization.md
Lines 334-365 (patched)
<https://reviews.apache.org/r/67501/#comment288039>

    Let's move these up to the other actions related to offer operations, e.g., 
right below `resize_volume`.



docs/authorization.md
Lines 340 (patched)
<https://reviews.apache.org/r/67501/#comment288040>

    Can you use basic forms here and below (create, destroy)?



docs/csi.md
Lines 788-793 (original), 788-800 (patched)
<https://reviews.apache.org/r/67501/#comment288041>

    Not sure we should mention these operation ACLs is this document concerned 
with low-level CSI setup aspects.
    
    Suggest to just drop the changes here.



include/mesos/authorizer/authorizer.proto
Lines 261-262 (patched)
<https://reviews.apache.org/r/67501/#comment288043>

    These comments are incorrect as we will set a `Resource` object. Change to 
    
       // `FOO_BAR` will have an object with `Resource` set.
       
    Here and below.



src/authorizer/local/authorizer.cpp
Lines 730 (patched)
<https://reviews.apache.org/r/67501/#comment288045>

    We might want to add the comment you also added to `acls.proto`,
    
        // TODO(nfnt): Consider allowing granular permission to act upon
        // SOME resource provider types and names.



src/master/master.hpp
Lines 901 (patched)
<https://reviews.apache.org/r/67501/#comment288046>

    Let's make the first sentence here more specific, e.g.,
    
        Returns whether the `CREATE_VOLUME` operation ...
        
    Here and below.


- Benjamin Bannier


On June 20, 2018, 12:44 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 12:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-7329
>     https://issues.apache.org/jira/browse/MESOS-7329
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Framework operations `CREATE_VOLUME`, `DESTROY_VOLUME`, `CREATE_BLOCK`,
> `DESTROY_BLOCK` are authorized. Respective ACL actions have been added
> to the local authorizer. Currently access can only be given to either
> 'ANY' or 'NONE' resource providers.
> 
> 
> Diffs
> -----
> 
>   docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
>   docs/csi.md 7c38fc10633aa28d012606150099ab5cc4b60cb6 
>   include/mesos/authorizer/acls.proto 
> e4889939481dabe6c1c2876a54d654f98d00dec8 
>   include/mesos/authorizer/authorizer.proto 
> bb1010d7eb97de17807b0a730ce16a4b28bc2aa3 
>   src/authorizer/local/authorizer.cpp 
> 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
>   src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c 
>   src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Reply via email to