You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2018/06/08 10:50:20 UTC

Review Request 67501: Added authorization for resource provider operations.

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

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/1/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for resource provider operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.

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



Patch looks great!

Reviews applied: [67501]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 8, 2018, 4:20 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 8, 2018, 4:20 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/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for resource provider operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

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



PASS: Mesos patch 67501 was successfully built and tested.

Reviews applied: `['67501']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501

- Mesos Reviewbot Windows


On June 8, 2018, 10:50 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 8, 2018, 10:50 a.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/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Benjamin Bannier <be...@mesosphere.io>.

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


Ship it!




Ship It!

- Benjamin Bannier


On June 21, 2018, 2:34 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 2:34 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.


> On June 25, 2018, 9:08 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/authorizer/acls.proto
> > Line 542 (original), 538 (patched)
> > <https://reviews.apache.org/r/67501/diff/3-4/?file=2044102#file2044102line542>
> >
> >     Is it because that implementing explicit objects takes significant efforts and has a lower priority, so we decided to leave it as a TODO?

Yes, exactly. See the next reviews in the chain by Benjamin that add a principal to resources created by resource providers and in the end add support for `SOME` to `DestroyBlockDisk` and `DestroyMountDisk`.


- Jan


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


On June 25, 2018, 3:58 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 25, 2018, 3:58 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

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




include/mesos/authorizer/acls.proto
Line 542 (original), 538 (patched)
<https://reviews.apache.org/r/67501/#comment288239>

    Is it because that implementing explicit objects takes significant efforts and has a lower priority, so we decided to leave it as a TODO?


- Chun-Hung Hsiao


On June 25, 2018, 1:58 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 25, 2018, 1:58 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.

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



Patch looks great!

Reviews applied: [67501]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 25, 2018, 1:58 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 25, 2018, 1:58 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['67501']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501

Relevant logs:

- [stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501/logs/stout-tests-cmake-stdout.log):

```
       "D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj" (default target) (6) ->
       (CustomBuild target) -> 
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]


       "D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj" (default target) (3) ->
       "D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (4) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]


       "D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj" (default target) (8) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]


       "D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (10) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]


       "D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) (14) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]

    1 Warning(s)
    5 Error(s)

Time Elapsed 00:03:15.35
```

- [libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501/logs/libprocess-tests-cmake-stdout.log):

```
       (CustomBuild target) -> 
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]


       "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj" (default target) (14) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]


       "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\libprocess\examples\example.vcxproj" (default target) (8) ->
       "D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (19) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]


       "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (3) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]


       "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\libprocess\examples\example.vcxproj" (default target) (8) ->
       "D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) (20) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]

    113 Warning(s)
    5 Error(s)

Time Elapsed 00:03:29.23
```

- [mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501/logs/mesos-tests-cmake-stdout.log):

```


       "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (3) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]


       "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj" (default target) (11) ->
       "D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (17) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]


       "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (18) ->
       "D:\DCOS\mesos\src\slave\containerizer\mesos\mesos-containerizer.vcxproj" (default target) (26) ->
       "D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj" (default target) (31) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]


       "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj" (default target) (15) ->
       "D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) (23) ->
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 3. [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]

    1 Warning(s)
    7 Error(s)

Time Elapsed 00:00:17.90
```

- Mesos Reviewbot Windows


On June 25, 2018, 1:58 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 25, 2018, 1:58 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Benjamin Bannier <be...@mesosphere.io>.

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




include/mesos/authorizer/acls.proto
Lines 542 (patched)
<https://reviews.apache.org/r/67501/#comment288405>

    I had some discussions around using principals to control these `DESTROY_*` operations with Jie and Greg.
    
    The take-away was that using principals instead of roles (like e.g., already suggested by Chun in this RR) is not something we should do going forward. It is already tech debt in the destruction of persistent volumes (which is pretty hard to remove in a backwards-compatible way), and we should not add it here.
    
    This is related to the fact that when a resource is reserved to a certain role, every framework in that role already has access _to the data_ in that volume (they could e.g., read and modify the data from a task). By using a principal to control destruction we protect the metadata (_some volume `xyz` exists_) while we do not protect the actual data (access to the data is based on roles). That means using principals to authorized `UNRESERVE` operations is useful (it changes data visibility), but does not add real benefits for the `DESTROY_*` operations.
    
    Could you use a `required Entity roles` here and below instead? We'd also need to update the documentation and ACL validation.


- Benjamin Bannier


On June 27, 2018, 12:40 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 27, 2018, 12:40 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Benjamin Bannier <be...@mesosphere.io>.

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


Ship it!




Ship It!

- Benjamin Bannier


On June 28, 2018, 10:50 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 10:50 a.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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/7/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

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

(Updated June 28, 2018, 10:50 a.m.)


Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


Changes
-------

Updated documentation to reflect latest changes.


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 (updated)
-----

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


Diff: https://reviews.apache.org/r/67501/diff/7/

Changes: https://reviews.apache.org/r/67501/diff/6-7/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

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

(Updated June 28, 2018, 10:47 a.m.)


Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


Changes
-------

Use `role` instead of `principal` as authorization object in `DESTROY_BLOCK_DISK` and `DESTROY_MOUNT_DISK`.


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 (updated)
-----

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


Diff: https://reviews.apache.org/r/67501/diff/6/

Changes: https://reviews.apache.org/r/67501/diff/5-6/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

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

(Updated June 27, 2018, 12:40 p.m.)


Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


Changes
-------

Addressed issues.


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 (updated)
-----

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


Diff: https://reviews.apache.org/r/67501/diff/5/

Changes: https://reviews.apache.org/r/67501/diff/4-5/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for storage operations.

Posted by Benjamin Bannier <be...@mesosphere.io>.

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


Fix it, then Ship it!





src/authorizer/local/authorizer.cpp
Line 1657 (original), 1703 (patched)
<https://reviews.apache.org/r/67501/#comment288321>

    Remove change in indention here.


- Benjamin Bannier


On June 25, 2018, 3:58 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 25, 2018, 3:58 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

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

(Updated June 25, 2018, 3:58 p.m.)


Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


Changes
-------

Changed authorization objects. Roles for create operations, principals for destroy operations.


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 (updated)
-----

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


Diff: https://reviews.apache.org/r/67501/diff/4/

Changes: https://reviews.apache.org/r/67501/diff/3-4/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for storage operations.

Posted by Chun-Hung Hsiao <ch...@apache.org>.


> On June 21, 2018, 3:39 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/authorizer/acls.proto
> > Lines 534 (patched)
> > <https://reviews.apache.org/r/67501/diff/3/?file=2044102#file2044102line534>
> >
> >     The patch looks good overall. But I'd like to raise a discussion about the object entity. Why are we planning to use resource providers but not roles, like the other offer operations?

It seems to me that `roles` fit better here since the resource only has a provider ID, which is a dynamically generated value, and would be hard for the operator to set it up. Thoughts?


- Chun-Hung


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


On June 21, 2018, 12:34 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 12:34 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.


> On June 21, 2018, 5:39 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/authorizer/acls.proto
> > Lines 534 (patched)
> > <https://reviews.apache.org/r/67501/diff/3/?file=2044102#file2044102line534>
> >
> >     The patch looks good overall. But I'd like to raise a discussion about the object entity. Why are we planning to use resource providers but not roles, like the other offer operations?
> 
> Chun-Hung Hsiao wrote:
>     It seems to me that `roles` fit better here since the resource only has a provider ID, which is a dynamically generated value, and would be hard for the operator to set it up. Thoughts?

I agree, thanks for catching this! Similar to resource reservation, create operations should be authorized per role, destroy operations authorized per creator principal.


- Jan


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


On June 21, 2018, 2:34 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 2:34 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

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


Fix it, then Ship it!





include/mesos/authorizer/acls.proto
Lines 534 (patched)
<https://reviews.apache.org/r/67501/#comment288120>

    The patch looks good overall. But I'd like to raise a discussion about the object entity. Why are we planning to use resource providers but not roles, like the other offer operations?


- Chun-Hung Hsiao


On June 21, 2018, 12:34 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 12:34 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

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



PASS: Mesos patch 67501 was successfully built and tested.

Reviews applied: `['67501']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501

- Mesos Reviewbot Windows


On June 21, 2018, 5:34 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 5:34 a.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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.

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



Patch looks great!

Reviews applied: [67501]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 21, 2018, 12:34 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 21, 2018, 12:34 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 
>   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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

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

(Updated June 21, 2018, 2:34 p.m.)


Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


Changes
-------

Addressed issues.


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 (updated)
-----

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  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 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


Diff: https://reviews.apache.org/r/67501/diff/3/

Changes: https://reviews.apache.org/r/67501/diff/2-3/


Testing
-------

make check


Thanks,

Jan Schlicht

Re: Review Request 67501: Added authorization for storage operations.

Posted by Benjamin Bannier <be...@mesosphere.io>.

-----------------------------------------------------------
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
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.

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



Patch looks great!

Reviews applied: [67501]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 20, 2018, 10:44 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 10:44 a.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
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

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



PASS: Mesos patch 67501 was successfully built and tested.

Reviews applied: `['67501']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67501

- Mesos Reviewbot Windows


On June 20, 2018, 10:44 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 10:44 a.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
> 
>

Re: Review Request 67501: Added authorization for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

-----------------------------------------------------------
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.


Changes
-------

Renamed authorization actions.


Summary (updated)
-----------------

Added authorization for storage operations.


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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/67501/diff/1-2/


Testing
-------

make check


Thanks,

Jan Schlicht