You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/02/20 02:08:28 UTC

Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

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

Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp e5aaf67e63996700b2cdcdd04055ad5b04bfb085 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 26, 2016, 6:58 p.m., Neil Conway wrote:
> > src/master/master.cpp, line 2930
> > <https://reviews.apache.org/r/43782/diff/5/?file=1272253#file1272253line2930>
> >
> >     Should talk about persistent volumes, not reservations.

Whoops, sorry. Fixed!


- Greg


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


On Feb. 26, 2016, 7:08 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43782/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
>     https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed object of `CreateVolume` ACL to `roles`.
> 
> This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 
> 
> Diff: https://reviews.apache.org/r/43782/diff/
> 
> 
> Testing
> -------
> 
> Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/#review120923
-----------------------------------------------------------


Fix it, then Ship it!





src/master/master.cpp (line 2930)
<https://reviews.apache.org/r/43782/#comment182448>

    Should talk about persistent volumes, not reservations.


- Neil Conway


On Feb. 26, 2016, 6:53 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43782/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 6:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
>     https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed object of `CreateVolume` ACL to `roles`.
> 
> This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 
> 
> Diff: https://reviews.apache.org/r/43782/diff/
> 
> 
> Testing
> -------
> 
> Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 26, 2016, 7:33 p.m., Jie Yu wrote:
> > src/master/master.cpp, line 2934
> > <https://reviews.apache.org/r/43782/diff/4/?file=1272138#file1272138line2934>
> >
> >     Ditto on using 'contains'

I think I probably pushed an update while you were reviewing, this should be in the latest diff :-) Thx!


- Greg


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


On Feb. 26, 2016, 7:08 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43782/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
>     https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed object of `CreateVolume` ACL to `roles`.
> 
> This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 
> 
> Diff: https://reviews.apache.org/r/43782/diff/
> 
> 
> Testing
> -------
> 
> Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/#review120895
-----------------------------------------------------------


Fix it, then Ship it!





src/master/master.cpp (line 2934)
<https://reviews.apache.org/r/43782/#comment182424>

    Ditto on using 'contains'


- Jie Yu


On Feb. 26, 2016, 7:08 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43782/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
>     https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed object of `CreateVolume` ACL to `roles`.
> 
> This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 
> 
> Diff: https://reviews.apache.org/r/43782/diff/
> 
> 
> Testing
> -------
> 
> Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 7:08 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Changes
-------

Addressed comment.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs (updated)
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 6:53 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Changes
-------

Made use of hashset::contains.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs (updated)
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 4:54 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs (updated)
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/
-----------------------------------------------------------

(Updated Feb. 24, 2016, 6:42 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Changes
-------

Rebase.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs (updated)
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/
-----------------------------------------------------------

(Updated Feb. 22, 2016, 7:55 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.


Changes
-------

Addressed comments.


Bugs: MESOS-4591
    https://issues.apache.org/jira/browse/MESOS-4591


Repository: mesos


Description
-------

Changed object of `CreateVolume` ACL to `roles`.

This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.


Diffs (updated)
-----

  include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547 
  src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 

Diff: https://reviews.apache.org/r/43782/diff/


Testing
-------

Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.


Thanks,

Greg Mann


Re: Review Request 43782: Changed object of `CreateVolume` ACL to `roles`.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43782/#review120030
-----------------------------------------------------------




src/tests/authorization_tests.cpp (line 565)
<https://reviews.apache.org/r/43782/#comment181403>

    s/can/can only



src/tests/authorization_tests.cpp (lines 575 - 576)
<https://reviews.apache.org/r/43782/#comment181404>

    Do we still need ` and principal "bar" will not be  allowed to create volumes for any other roles.` if update line 567 to `Principal "bar" can only create volumes for the "panda" role.`
    
    Or else move this comment to line 567?


- Guangya Liu


On 二月 20, 2016, 1:08 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43782/
> -----------------------------------------------------------
> 
> (Updated 二月 20, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
>     https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed object of `CreateVolume` ACL to `roles`.
> 
> This solves a problem in which any principal could create volumes for any role using the '/create-volumes' operator endpoint.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp e5aaf67e63996700b2cdcdd04055ad5b04bfb085 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp e169e1b141a38dc389eefd42c11a078c413123d5 
> 
> Diff: https://reviews.apache.org/r/43782/diff/
> 
> 
> Testing
> -------
> 
> Persistent volume tests and operation validation tests were altered to accomodate the new ACL object, and some new principal/role combinations were added to `AuthorizationTest.CreateVolume`.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>