Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-29 Thread Benjamin Bannier 

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

(Updated Sept. 29, 2016, 6:20 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This change introduces Linux capability-based security the Mesos
exector. A new flag `capabilities` is introduced to optionally specify
the capabilities tasks launched by the Mesos executor are allowed to
use.


Diffs (updated)
-

  src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
  src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
  src/launcher/posix/executor.cpp 0f3fed3f117d150a1020a3b2987f9763c6a343b9 

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


Testing
---

Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
gcc-4.8.4 w/o optimizations.


Thanks,

Benjamin Bannier

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-22 Thread Benjamin Bannier 

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

(Updated Sept. 22, 2016, 10:24 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

This change introduces Linux capability-based security the Mesos
exector. A new flag `capabilities` is introduced to optionally specify
the capabilities tasks launched by the Mesos executor are allowed to
use.


Diffs (updated)
-

  src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
  src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
  src/launcher/posix/executor.cpp 0f3fed3f117d150a1020a3b2987f9763c6a343b9 

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


Testing
---

Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
gcc-4.8.4 w/o optimizations.


Thanks,

Benjamin Bannier

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-22 Thread Jie Yu 


> On Sept. 20, 2016, 12:36 a.m., Jie Yu wrote:
> > src/launcher/executor.cpp, line 817
> > 
> >
> > It looks wierd that we have ifdef here but not have that for the field 
> > member. I'd suggest we don't do any ifdef for flags as they are optional 
> > anyway. Otherwise, you'll have to change all the constructors as well, 
> > which is a little messy.
> 
> Benjamin Bannier wrote:
> Hmm, that would mean that that flag would be there under e.g., macos or 
> windows, but have no effect which we'd need to patch by either documentation 
> or adding a validation step.
> 
> I am not sure I understand your comment re:constructors; the member is 
> there and can be default constructed. I think the code should be fine as is.

All I care about is consistency. For instance, why not wrap ifndef windows on 
'user' and 'rootfs'? Why only do it for 'capabilities'. Either you fix all of 
them, or make it consistent and add some TODO. I don't want to be in in the 
middle ground.


- Jie


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


On Sept. 20, 2016, 4:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51930/
> ---
> 
> (Updated Sept. 20, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces Linux capability-based security the Mesos
> exector. A new flag `capabilities` is introduced to optionally specify
> the capabilities tasks launched by the Mesos executor are allowed to
> use.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
>   src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
>   src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 
> 
> Diff: https://reviews.apache.org/r/51930/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
> gcc-4.8.4 w/o optimizations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-20 Thread Benjamin Bannier 


> On Sept. 20, 2016, 2:36 a.m., Jie Yu wrote:
> > src/launcher/executor.cpp, line 817
> > 
> >
> > It looks wierd that we have ifdef here but not have that for the field 
> > member. I'd suggest we don't do any ifdef for flags as they are optional 
> > anyway. Otherwise, you'll have to change all the constructors as well, 
> > which is a little messy.

Hmm, that would mean that that flag would be there under e.g., macos or 
windows, but have no effect which we'd need to patch by either documentation or 
adding a validation step.

I am not sure I understand your comment re:constructors; the member is there 
and can be default constructed. I think the code should be fine as is.


- Benjamin


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


On Sept. 19, 2016, 4:19 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51930/
> ---
> 
> (Updated Sept. 19, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces Linux capability-based security the Mesos
> exector. A new flag `capabilities` is introduced to optionally specify
> the capabilities tasks launched by the Mesos executor are allowed to
> use.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
>   src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
>   src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 
> 
> Diff: https://reviews.apache.org/r/51930/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
> gcc-4.8.4 w/o optimizations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-19 Thread Jie Yu 

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




src/launcher/executor.cpp (line 817)


It looks wierd that we have ifdef here but not have that for the field 
member. I'd suggest we don't do any ifdef for flags as they are optional 
anyway. Otherwise, you'll have to change all the constructors as well, which is 
a little messy.


- Jie Yu


On Sept. 19, 2016, 2:19 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51930/
> ---
> 
> (Updated Sept. 19, 2016, 2:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces Linux capability-based security the Mesos
> exector. A new flag `capabilities` is introduced to optionally specify
> the capabilities tasks launched by the Mesos executor are allowed to
> use.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
>   src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
>   src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 
> 
> Diff: https://reviews.apache.org/r/51930/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
> gcc-4.8.4 w/o optimizations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-19 Thread Jie Yu 

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


Ship it!




Ship It!

- Jie Yu


On Sept. 19, 2016, 2:19 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51930/
> ---
> 
> (Updated Sept. 19, 2016, 2:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces Linux capability-based security the Mesos
> exector. A new flag `capabilities` is introduced to optionally specify
> the capabilities tasks launched by the Mesos executor are allowed to
> use.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
>   src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
>   src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 
> 
> Diff: https://reviews.apache.org/r/51930/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
> gcc-4.8.4 w/o optimizations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 51930: Introduced Linux capabilities support for Mesos executor.

2016-09-19 Thread Benjamin Bannier 

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This change introduces Linux capability-based security the Mesos
exector. A new flag `capabilities` is introduced to optionally specify
the capabilities tasks launched by the Mesos executor are allowed to
use.


Diffs
-

  src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
  src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
  src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 

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


Testing
---

Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
gcc-4.8.4 w/o optimizations.


Thanks,

Benjamin Bannier