You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2018/12/27 03:19:15 UTC
Re: Review Request 68019: Added a parser for the Docker Seccomp config
format.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68019/#review211534
-----------------------------------------------------------
src/linux/seccomp/seccomp_parser.hpp
Lines 22 (patched)
<https://reviews.apache.org/r/68019/#comment296855>
This is should be before `#include <stout/try.hpp>`.
src/linux/seccomp/seccomp_parser.hpp
Lines 30 (patched)
<https://reviews.apache.org/r/68019/#comment296854>
One more newline here.
src/linux/seccomp/seccomp_parser.cpp
Lines 39-40 (patched)
<https://reviews.apache.org/r/68019/#comment296856>
Can we change it to `constexpr char SCMP_PREFIX[] = "SCMP_`?
src/linux/seccomp/seccomp_parser.cpp
Lines 484 (patched)
<https://reviews.apache.org/r/68019/#comment296860>
Can you please elaborate on why we need to do the parsing manually rather than using `protobuf::parse`? Is it because we need to manually handle the `SCMP_` prefix? Any other reasons?
src/linux/seccomp/seccomp_parser.cpp
Lines 485 (patched)
<https://reviews.apache.org/r/68019/#comment296858>
Kill this newline, and ditto for the other two places below this code.
src/linux/seccomp/seccomp_parser.cpp
Lines 504 (patched)
<https://reviews.apache.org/r/68019/#comment296857>
Just a question, this will not affect agent process since we do not call `seccompFilter.get()->load()`, right?
- 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/68019/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9105
> https://issues.apache.org/jira/browse/MESOS-9105
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Docker Seccomp config is a JSON file containing Seccomp filtering
> rules. This patch introduces a parser for Docker Seccomp config format.
> This parser accepts a JSON-string, parses and validates it, then
> returns a prepared `ContainerSeccompProfile` message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8
> src/Makefile.am 2d9c81b149a5764dc82593bef102f5568847daa2
> src/linux/seccomp/seccomp_parser.hpp PRE-CREATION
> src/linux/seccomp/seccomp_parser.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68019/diff/7/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68019: Added a parser for the Docker Seccomp config
format.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Dec. 27, 2018, 3:19 a.m., Qian Zhang wrote:
> > src/linux/seccomp/seccomp_parser.cpp
> > Lines 484 (patched)
> > <https://reviews.apache.org/r/68019/diff/7/?file=2109138#file2109138line484>
> >
> > Can you please elaborate on why we need to do the parsing manually rather than using `protobuf::parse`? Is it because we need to manually handle the `SCMP_` prefix? Any other reasons?
`SCMP_ARCH_X86` is a c-style macro. So, we need to avoid names starting with `SCMP_`.
>Is it because we need to manually handle the SCMP_ prefix?
Yes, it is.
>Any other reasons?
Nope.
> On Dec. 27, 2018, 3:19 a.m., Qian Zhang wrote:
> > src/linux/seccomp/seccomp_parser.cpp
> > Lines 504 (patched)
> > <https://reviews.apache.org/r/68019/diff/7/?file=2109138#file2109138line504>
> >
> > Just a question, this will not affect agent process since we do not call `seccompFilter.get()->load()`, right?
Yes, correct.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68019/#review211534
-----------------------------------------------------------
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/68019/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9105
> https://issues.apache.org/jira/browse/MESOS-9105
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Docker Seccomp config is a JSON file containing Seccomp filtering
> rules. This patch introduces a parser for Docker Seccomp config format.
> This parser accepts a JSON-string, parses and validates it, then
> returns a prepared `ContainerSeccompProfile` message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa
> src/linux/seccomp/seccomp_parser.hpp PRE-CREATION
> src/linux/seccomp/seccomp_parser.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68019/diff/9/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>