Re: Review Request 68020: Added Seccomp-related flags to the agent.

2019-01-21 Thread Andrei Budnik 


> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp
> > Lines 1397 (patched)
> > 
> >
> > I would like to re-visit the naming seccomp_profile V.S. 
> > seccomp_profile_name. WDYT?
> > 
> > cc @qianzhang

`seccomp_profile` suggests that its value can be a Seccomp profile.
`seccomp_profile_name` emphasizes that it's not a JSON value.

Note, that seccomp profile is a ~4k characters long string. Passing the whole 
JSON as an argument might be problematic.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the name of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2019-01-18 Thread Gilbert Song 

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


Ship it!





src/slave/flags.cpp
Lines 1397 (patched)


I would like to re-visit the naming seccomp_profile V.S. 
seccomp_profile_name. WDYT?

cc @qianzhang


- Gilbert Song


On Nov. 8, 2018, 7:24 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 7:24 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the name of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2019-01-15 Thread Andrei Budnik 


> On Jan. 15, 2019, 2:31 a.m., Qian Zhang wrote:
> > src/slave/flags.cpp
> > Lines 1399 (patched)
> > 
> >
> > Path or name?

Well.. technically it is a path. For example, if 
`--seccomp_config_dir=/home/nobody/seccomp`, then I can specify 
`--seccomp_profile_name=myseccomp/default.json`, resulting in a file path 
`/home/nobody/seccomp/myseccomp/default.json`.
Renaming `seccomp_profile_name` to `seccomp_profile_path` adds more confusion 
as we already have `seccomp_profile_dir`.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the name of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2019-01-14 Thread Qian Zhang 

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


Fix it, then Ship it!





src/slave/flags.cpp
Lines 1399 (patched)


Path or name?


- Qian Zhang


On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the name of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2019-01-03 Thread Andrei Budnik 


> On Dec. 27, 2018, 7:47 a.m., Qian Zhang wrote:
> > In the commit message of this patch, I would suggest to change `the path of 
> > the default Seccomp profile` to `the name of the default Seccomp profile`.

Fixed.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the name of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 330283f4e3957075dd4310de4a841feac23de36c 
>   src/slave/flags.hpp 494ae02ab5eb365e2cda5017be573691107c3f28 
>   src/slave/flags.cpp 6bac8e1409f04d639204c45eda8a90c098e3dbd0 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2018-12-27 Thread Qian Zhang 


> On Dec. 27, 2018, 3:44 p.m., Qian Zhang wrote:
> > src/slave/flags.hpp
> > Lines 192 (patched)
> > 
> >
> > Why not make this flag as optional too?

If we make this flag as optional, then we do not need to use empty value to 
indicate disabling default profile, instead we will disable default profile 
when this flag is not specified which seems more intuitive to me.


- Qian


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


On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the path of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2018-12-26 Thread Qian Zhang 

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



In the commit message of this patch, I would suggest to change `the path of the 
default Seccomp profile` to `the name of the default Seccomp profile`.

- Qian Zhang


On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the path of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2018-12-26 Thread Qian Zhang 

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




docs/configuration/agent.md
Lines 1645-1646 (patched)


Should we move this note into the description of `--seccomp_profile_name`?



docs/configuration/agent.md
Lines 1655 (patched)


Should it be `Path of the default Seccomp profile`?



docs/configuration/agent.md
Lines 1659 (patched)


What's the behavior differece between this flag specified with an empty 
value and this flag not specified at all?



src/slave/flags.hpp
Lines 192 (patched)


Why not make this flag as optional too?


- Qian Zhang


On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68020/
> ---
> 
> (Updated Nov. 8, 2018, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `--seccomp_config_dir` and `--seccomp_profile_name` flags have been
> added to the agent. These flags are used by the `linux/seccomp`
> isolator to specify the path of the directory containing Seccomp
> profiles and the path of the default Seccomp profile.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
> 
> 
> Diff: https://reviews.apache.org/r/68020/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

2018-11-08 Thread Andrei Budnik 

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

(Updated Nov. 8, 2018, 3:24 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Summary (updated)
-

Added Seccomp-related flags to the agent.


Repository: mesos


Description (updated)
---

`--seccomp_config_dir` and `--seccomp_profile_name` flags have been
added to the agent. These flags are used by the `linux/seccomp`
isolator to specify the path of the directory containing Seccomp
profiles and the path of the default Seccomp profile.


Diffs (updated)
-

  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 


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

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


Testing
---


Thanks,

Andrei Budnik