You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2018/08/06 13:39:11 UTC
Re: Review Request 68020: Added new `--seccomp_config_dir` flag to
the agent.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68020/
-----------------------------------------------------------
(Updated Aug. 6, 2018, 1:39 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Repository: mesos
Description
-------
This flag is used to specify a path to the directory containing
Seccomp profiles. This flag is used by the `linux/seccomp` isolator.
Diffs
-----
src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe
src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478
Diff: https://reviews.apache.org/r/68020/diff/1/
Testing
-------
Thanks,
Andrei Budnik
Re: Review Request 68020: Added Seccomp-related flags to the agent.
Posted by Qian Zhang <zh...@gmail.com>.
> On Dec. 27, 2018, 3:44 p.m., Qian Zhang wrote:
> > src/slave/flags.hpp
> > Lines 192 (patched)
> > <https://reviews.apache.org/r/68020/diff/9/?file=2111366#file2111366line192>
> >
> > 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.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/68020/#comment296862>
Should we move this note into the description of `--seccomp_profile_name`?
docs/configuration/agent.md
Lines 1655 (patched)
<https://reviews.apache.org/r/68020/#comment296863>
Should it be `Path of the default Seccomp profile`?
docs/configuration/agent.md
Lines 1659 (patched)
<https://reviews.apache.org/r/68020/#comment296864>
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)
<https://reviews.apache.org/r/68020/#comment296866>
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.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 15, 2019, 2:31 a.m., Qian Zhang wrote:
> > src/slave/flags.cpp
> > Lines 1399 (patched)
> > <https://reviews.apache.org/r/68020/diff/10/?file=2117329#file2117329line1399>
> >
> > 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.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/68020/#comment297561>
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.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> 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.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp
> > Lines 1397 (patched)
> > <https://reviews.apache.org/r/68020/diff/10/?file=2117329#file2117329line1397>
> >
> > 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.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/68020/#comment297821>
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.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
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