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/07/23 13:56:20 UTC
Review Request 68017: Added Seccomp-related protobuf messages.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Bugs: MESOS-9033
https://issues.apache.org/jira/browse/MESOS-9033
Repository: mesos
Description
-------
See summary.
Diffs
-----
include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae
include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
Diff: https://reviews.apache.org/r/68017/diff/1/
Testing
-------
Thanks,
Andrei Budnik
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Qian Zhang <zh...@gmail.com>.
> On Jan. 15, 2019, 10:39 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> Gilbert Song wrote:
> I would prefer option #2.
>
> The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
> do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
So the semantic will be:
1. seccompinfo is None: The default seccomp profile (if any) will be applied for the container.
2. seccompinfo is Some but profile_name is None: No seccomp filtering for the container.
This is kind of unintuitive to me because none-seccompinfo and none-profile_name will have totally different result.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 14, 2019, 6:39 p.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
I would prefer option #2.
The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 14, 2019, 6:39 p.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
two options:
1. remove it
2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 14, 2019, 6:39 p.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> Gilbert Song wrote:
> I would prefer option #2.
>
> The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
>
> Qian Zhang wrote:
> > do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> So the semantic will be:
> 1. seccompinfo is None: The default seccomp profile (if any) will be applied for the container.
> 2. seccompinfo is Some but profile_name is None: No seccomp filtering for the container.
>
> This is kind of unintuitive to me because none-seccompinfo and none-profile_name will have totally different result.
>
> Qian Zhang wrote:
> Second thought, do we really need to support framework to disable seccomp filtering? I think that may not be what the operator wants. If the operator specifies a default seccomp profile, that means the operator cares about the security of the agent host, and he/she would expect the default seccomp profile to be applied for each container unless framework overrides it with another seccomp profile. Override does not mean disable because any seccomp profiles that framework can use are also set up by operator under `--seccomp_config_dir` (i.e., still under the operator's control), but if framework can disable seccomp filtering for its containers, that means the security of the agent host is out of the operator's control which seems kind of security hole.
>
> However, disabling seccomp filtering can still be supported (if some operators want it) by not specifying the default seccomp profile, and in this case if framework does not specify seccompinfo or profile_name, the seccomp filtering will be disabled. But if operator specifies a default seccomp profile, I think we should not support disabling seccomp filtering. And another way to support disabling seccomp filtering is, operator specifies the default seccomp profile and also put some other profiles under `--seccomp_config_dir` and names them like `strict`, `loose` and `none`, and there is no syscall filtering in the `none` profile which is equivalent to disabling seccomp filtering.
>
> Andrei Budnik wrote:
> I think we should adhere semantics for Seccomp configuration similar to Docker and Kubernetes:
> https://docs.docker.com/engine/security/seccomp/ (especially, `You can pass unconfined to run a container without the default seccomp profile.`)
> https://kubernetes.io/docs/concepts/policy/pod-security-policy/#seccomp
Probably we should think deeper on this API. docker/k8s API may not always make 100% sense.
1. From the perspective of cluster management, as an operator, if Dan sets a default seccomp profile for the cluster, Dan would expect the profile could be overwritten by framework for one container, but could not be disabled. If any framework want to support seccomp `unconfined`, it is the same effect with no-default profile, totally up to framework to set the profile name.
2. From user's perspective, there may not be sufficient communication between operators and users. when users realize that an app could not be launched due to the default seccomp profile, he/she would expect `unconfined`.
for concern #2, we need to design a better API than existing one (at least introducing an enum since mesos does not expect both field set at the same time). Also, it is not clear to me that how many frameworks will support the seccomp API in near term. probably most people would just consume the default profile in near term.
Either way, I think it does not hurt to remove `unconfined` for now and introduce it later when people ask.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 15, 2019, 2:39 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> Gilbert Song wrote:
> I would prefer option #2.
>
> The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
>
> Qian Zhang wrote:
> > do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> So the semantic will be:
> 1. seccompinfo is None: The default seccomp profile (if any) will be applied for the container.
> 2. seccompinfo is Some but profile_name is None: No seccomp filtering for the container.
>
> This is kind of unintuitive to me because none-seccompinfo and none-profile_name will have totally different result.
>
> Qian Zhang wrote:
> Second thought, do we really need to support framework to disable seccomp filtering? I think that may not be what the operator wants. If the operator specifies a default seccomp profile, that means the operator cares about the security of the agent host, and he/she would expect the default seccomp profile to be applied for each container unless framework overrides it with another seccomp profile. Override does not mean disable because any seccomp profiles that framework can use are also set up by operator under `--seccomp_config_dir` (i.e., still under the operator's control), but if framework can disable seccomp filtering for its containers, that means the security of the agent host is out of the operator's control which seems kind of security hole.
>
> However, disabling seccomp filtering can still be supported (if some operators want it) by not specifying the default seccomp profile, and in this case if framework does not specify seccompinfo or profile_name, the seccomp filtering will be disabled. But if operator specifies a default seccomp profile, I think we should not support disabling seccomp filtering. And another way to support disabling seccomp filtering is, operator specifies the default seccomp profile and also put some other profiles under `--seccomp_config_dir` and names them like `strict`, `loose` and `none`, and there is no syscall filtering in the `none` profile which is equivalent to disabling seccomp filtering.
I think we should adhere semantics for Seccomp configuration similar to Docker and Kubernetes:
https://docs.docker.com/engine/security/seccomp/ (especially, `You can pass unconfined to run a container without the default seccomp profile.`)
https://kubernetes.io/docs/concepts/policy/pod-security-policy/#seccomp
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 15, 2019, 2:39 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> Gilbert Song wrote:
> I would prefer option #2.
>
> The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
>
> Qian Zhang wrote:
> > do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> So the semantic will be:
> 1. seccompinfo is None: The default seccomp profile (if any) will be applied for the container.
> 2. seccompinfo is Some but profile_name is None: No seccomp filtering for the container.
>
> This is kind of unintuitive to me because none-seccompinfo and none-profile_name will have totally different result.
>
> Qian Zhang wrote:
> Second thought, do we really need to support framework to disable seccomp filtering? I think that may not be what the operator wants. If the operator specifies a default seccomp profile, that means the operator cares about the security of the agent host, and he/she would expect the default seccomp profile to be applied for each container unless framework overrides it with another seccomp profile. Override does not mean disable because any seccomp profiles that framework can use are also set up by operator under `--seccomp_config_dir` (i.e., still under the operator's control), but if framework can disable seccomp filtering for its containers, that means the security of the agent host is out of the operator's control which seems kind of security hole.
>
> However, disabling seccomp filtering can still be supported (if some operators want it) by not specifying the default seccomp profile, and in this case if framework does not specify seccompinfo or profile_name, the seccomp filtering will be disabled. But if operator specifies a default seccomp profile, I think we should not support disabling seccomp filtering. And another way to support disabling seccomp filtering is, operator specifies the default seccomp profile and also put some other profiles under `--seccomp_config_dir` and names them like `strict`, `loose` and `none`, and there is no syscall filtering in the `none` profile which is equivalent to disabling seccomp filtering.
>
> Andrei Budnik wrote:
> I think we should adhere semantics for Seccomp configuration similar to Docker and Kubernetes:
> https://docs.docker.com/engine/security/seccomp/ (especially, `You can pass unconfined to run a container without the default seccomp profile.`)
> https://kubernetes.io/docs/concepts/policy/pod-security-policy/#seccomp
>
> Gilbert Song wrote:
> Probably we should think deeper on this API. docker/k8s API may not always make 100% sense.
>
> 1. From the perspective of cluster management, as an operator, if Dan sets a default seccomp profile for the cluster, Dan would expect the profile could be overwritten by framework for one container, but could not be disabled. If any framework want to support seccomp `unconfined`, it is the same effect with no-default profile, totally up to framework to set the profile name.
>
> 2. From user's perspective, there may not be sufficient communication between operators and users. when users realize that an app could not be launched due to the default seccomp profile, he/she would expect `unconfined`.
>
> for concern #2, we need to design a better API than existing one (at least introducing an enum since mesos does not expect both field set at the same time). Also, it is not clear to me that how many frameworks will support the seccomp API in near term. probably most people would just consume the default profile in near term.
>
> Either way, I think it does not hurt to remove `unconfined` for now and introduce it later when people ask.
Removed `unconfined` flag. Updated the following patches in the patch chain.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
On Jan. 16, 2019, 1:41 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68017/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2019, 1:41 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/16/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Qian Zhang <zh...@gmail.com>.
> On Jan. 15, 2019, 10:39 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3158-3159 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158>
> >
> > Seems like this was added recently.
> >
> > Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
> >
> > (probably more comment needed)
> >
> > If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
>
> Gilbert Song wrote:
> after second thought, I would suggest to remove this field for now. since there is not use case yet (I understand your motivation: you want users could get rid of seccomp if there is a default one). We could add this field later if necessary. For now:
>
> two options:
> 1. remove it
> 2. do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> Gilbert Song wrote:
> I would prefer option #2.
>
> The reason we want to avoid introducing `unconfined` now is that framework could set both field at the same time and ideally we may need an `enum type`. given the use case is not clear yet (people may not necessary to use it yet). lets make it implicit for now.
>
> Qian Zhang wrote:
> > do it implicitly by using the optional profile_name: if seccompinfo isSome but profile_name is None. do not set seccomp filtering for container
>
> So the semantic will be:
> 1. seccompinfo is None: The default seccomp profile (if any) will be applied for the container.
> 2. seccompinfo is Some but profile_name is None: No seccomp filtering for the container.
>
> This is kind of unintuitive to me because none-seccompinfo and none-profile_name will have totally different result.
Second thought, do we really need to support framework to disable seccomp filtering? I think that may not be what the operator wants. If the operator specifies a default seccomp profile, that means the operator cares about the security of the agent host, and he/she would expect the default seccomp profile to be applied for each container unless framework overrides it with another seccomp profile. Override does not mean disable because any seccomp profiles that framework can use are also set up by operator under `--seccomp_config_dir` (i.e., still under the operator's control), but if framework can disable seccomp filtering for its containers, that means the security of the agent host is out of the operator's control which seems kind of security hole.
However, disabling seccomp filtering can still be supported (if some operators want it) by not specifying the default seccomp profile, and in this case if framework does not specify seccompinfo or profile_name, the seccomp filtering will be disabled. But if operator specifies a default seccomp profile, I think we should not support disabling seccomp filtering. And another way to support disabling seccomp filtering is, operator specifies the default seccomp profile and also put some other profiles under `--seccomp_config_dir` and names them like `strict`, `loose` and `none`, and there is no syscall filtering in the `none` profile which is equivalent to disabling seccomp filtering.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211986
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/mesos.proto
Lines 3158-3159 (patched)
<https://reviews.apache.org/r/68017/#comment297565>
Seems like this was added recently.
Is this field only used when there is a default agent wide seccomp profile provided from the agent flag? and we reply on it to give an opportunity for user/framework to get rid of seccomp?
(probably more comment needed)
If it is the case, do we have other options for naming? e.g., `no_seccomp` (maybe more explicit)
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review212088
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Jan. 16, 2019, 5:41 a.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68017/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2019, 5:41 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/16/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/
-----------------------------------------------------------
(Updated Jan. 16, 2019, 1:41 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Changes
-------
removed `unconfined`
Bugs: MESOS-9033
https://issues.apache.org/jira/browse/MESOS-9033
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
include/mesos/seccomp/seccomp.hpp PRE-CREATION
include/mesos/seccomp/seccomp.proto PRE-CREATION
include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
Diff: https://reviews.apache.org/r/68017/diff/16/
Changes: https://reviews.apache.org/r/68017/diff/15-16/
Testing
-------
Thanks,
Andrei Budnik
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 15, 2019, 8:41 a.m., Qian Zhang wrote:
> > include/mesos/seccomp/seccomp.proto
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118635#file2118635line79>
> >
> > So the value must be numeric? Do we support other types (like string value)?
> So the value must be numeric?
Yes.
> Do we support other types (like string value)?
No.
https://github.com/moby/moby/blob/8e610b2b55bfd1bfa9436ab110d311f5e8a74dcb/api/types/seccomp.go#L73
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211942
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211942
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/seccomp/seccomp.proto
Lines 22-23 (patched)
<https://reviews.apache.org/r/68017/#comment297504>
Here we just need one blank line rather than two.
include/mesos/seccomp/seccomp.proto
Lines 79 (patched)
<https://reviews.apache.org/r/68017/#comment297529>
So the value must be numeric? Do we support other types (like string value)?
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 2, 2019, 8:23 p.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 197 (patched)
> > <https://reviews.apache.org/r/68017/diff/11/?file=2116532#file2116532line197>
> >
> > As we discussed last time, could we move this protobuf message to `include/mesos/seccomp/` dir, since this is not user facing API in the future, nor container life cycle managerment (in containerizer.proto).
Fixed.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211613
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
>
>
> Diff: https://reviews.apache.org/r/68017/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211613
-----------------------------------------------------------
include/mesos/slave/containerizer.proto
Line 26 (original), 26 (patched)
<https://reviews.apache.org/r/68017/#comment296968>
See my comment below, and just add:
```
import "mesos/seccomp/seccomp.proto`;
```
include/mesos/slave/containerizer.proto
Lines 197 (patched)
<https://reviews.apache.org/r/68017/#comment296967>
As we discussed last time, could we move this protobuf message to `include/mesos/seccomp/` dir, since this is not user facing API in the future, nor container life cycle managerment (in containerizer.proto).
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
>
>
> Diff: https://reviews.apache.org/r/68017/diff/11/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Dec. 13, 2018, 8:24 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 197 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line197>
> >
> > When parsing a profile, I can see unavoidably we have to do some transforming in the parser helper, but probably we could do best effort to leverage protobuf::parse<>().
Our internal json format is incompatible with Docker Seccomp profile format. However, I use proto-generated helpers to parse enums, grep for `_Parse(` in `seccomp_parser.cpp`.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211275
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221
>
>
> Diff: https://reviews.apache.org/r/68017/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211275
-----------------------------------------------------------
include/mesos/slave/containerizer.proto
Lines 197 (patched)
<https://reviews.apache.org/r/68017/#comment296232>
When parsing a profile, I can see unavoidably we have to do some transforming in the parser helper, but probably we could do best effort to leverage protobuf::parse<>().
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221
>
>
> Diff: https://reviews.apache.org/r/68017/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 14, 2019, 3:38 p.m., Gilbert Song wrote:
> > include/mesos/seccomp/seccomp.proto
> > Lines 21-28 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118635#file2118635line21>
> >
> > This is my fault. Sorry, Andrei!
> >
> > Would you mind moving this .proto to /src/seccomp again? The previous suggestion was not appropriate after the second thought because:
> >
> > proto files under /include:
> > They are public protobuf files that may be used by external/3th party modules, while some of the proto files like v1/v2.proto and fetcher.proto should be moved out from /include dir. this is a tech debt.
> >
> > proto files under /src:
> > used for internal conversion.
>
> Andrei Budnik wrote:
> I can't move it to `src/seccomp`, because `message ContainerLaunchInfo` (from `include/mesos/slave/containerizer.proto`) depends on it!
Dropped.
Forgot ContainerLaunchInfo is also a public facing protobuf (isolator module uses it). so /include should be good place to locate then.
Thanks!
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211972
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 14, 2019, 11:38 p.m., Gilbert Song wrote:
> > include/mesos/seccomp/seccomp.proto
> > Lines 21-28 (patched)
> > <https://reviews.apache.org/r/68017/diff/14/?file=2118635#file2118635line21>
> >
> > This is my fault. Sorry, Andrei!
> >
> > Would you mind moving this .proto to /src/seccomp again? The previous suggestion was not appropriate after the second thought because:
> >
> > proto files under /include:
> > They are public protobuf files that may be used by external/3th party modules, while some of the proto files like v1/v2.proto and fetcher.proto should be moved out from /include dir. this is a tech debt.
> >
> > proto files under /src:
> > used for internal conversion.
I can't move it to `src/seccomp`, because `message ContainerLaunchInfo` (from `include/mesos/slave/containerizer.proto`) depends on it!
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211972
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
>
>
> Diff: https://reviews.apache.org/r/68017/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211972
-----------------------------------------------------------
include/mesos/seccomp/seccomp.proto
Lines 21-28 (patched)
<https://reviews.apache.org/r/68017/#comment297552>
This is my fault. Sorry, Andrei!
Would you mind moving this .proto to /src/seccomp again? The previous suggestion was not appropriate after the second thought because:
proto files under /include:
They are public protobuf files that may be used by external/3th party modules, while some of the proto files like v1/v2.proto and fetcher.proto should be moved out from /include dir. this is a tech debt.
proto files under /src:
used for internal conversion.
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/seccomp/seccomp.hpp PRE-CREATION
> include/mesos/seccomp/seccomp.proto PRE-CREATION
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am 188a47017221a931d8b965a4af5e033b77e6ce4e
>
>
> Diff: https://reviews.apache.org/r/68017/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Dec. 21, 2018, 1:23 a.m., Qian Zhang wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 258 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line258>
> >
> > Should it be `repeated CapabilityInfo.Capability caps`?
Good catch.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211492
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
>
>
> Diff: https://reviews.apache.org/r/68017/diff/11/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Dec. 21, 2018, 9:23 a.m., Qian Zhang wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 197 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line197>
> >
> > Can you please elaborate a bit why our internal json format is incompatible with Docker Seccomp profile format?
Our internal json format is designed *only* for marshalling/unmarshalling of a *Protobuf message* via JSON. You cannot take *any* JSON and then convert it to the protobuf message, because JSON -> Proto translation requires storing a SCHEMA into JSON. This schema is used to convert from JSON to proto message by our internal parser.
If we want to use our internal json format, we need to modify (to add SCHEMA) the Docker Seccomp profile.
> On Dec. 21, 2018, 9:23 a.m., Qian Zhang wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 250-254 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line250>
> >
> > If it is `Some` comparison operators accept two values, should this field be optional?
I could do that, but it overcomplicates the logic of Seccomp parser. In order to do that, I need to add a `switch` statement that sets the value for `value_two` field depending on `op` operator's type. Note, that the user of `ContainerSeccompProfile` is `class Seccomp` which ignores `value_two` field when it's not needed. Also, `value_two` is *always* defined in Docker's profile regardless of operator's type. Hence, making a single field optional by increasing complexity of the Seccomp parser (see `parseSyscallArgument()`) doesn't look appealing to me.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211492
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
>
>
> Diff: https://reviews.apache.org/r/68017/diff/11/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211492
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 3089 (patched)
<https://reviews.apache.org/r/68017/#comment296692>
Can we mention the name of the flag here?
include/mesos/slave/containerizer.proto
Lines 197 (patched)
<https://reviews.apache.org/r/68017/#comment296698>
Can you please elaborate a bit why our internal json format is incompatible with Docker Seccomp profile format?
include/mesos/slave/containerizer.proto
Lines 250-254 (patched)
<https://reviews.apache.org/r/68017/#comment296697>
If it is `Some` comparison operators accept two values, should this field be optional?
include/mesos/slave/containerizer.proto
Lines 256-257 (patched)
<https://reviews.apache.org/r/68017/#comment296694>
Can you please add a comment for this message?
include/mesos/slave/containerizer.proto
Lines 258 (patched)
<https://reviews.apache.org/r/68017/#comment296695>
Should it be `repeated CapabilityInfo.Capability caps`?
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221
>
>
> Diff: https://reviews.apache.org/r/68017/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 200-215 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line200>
> >
> > I saw you remove all `SCMP_` prefix and implemented a hashmap converter in the next patch. You do this for `AUDIT_` architecture on pupose?
Adding `SCMP_` prefix leads to compilation errors due to interference with libseccomp's constants.
> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 257-259 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line257>
> >
> > Do we want `Arches`? or we could leave a TODO here. I am think about whether we should have a linux specific proto enum for Architecture like S390X etc.
No, we don't. Filtering of seccomp rules by architecture happens during parsing of a Seccomp profile.
> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 262 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line262>
> >
> > Is an `action` repeated in this case?
No. See https://github.com/moby/moby/blob/master/profiles/seccomp/default.json#L363
> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 263-264 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line263>
> >
> > Even we do not use `comments` now, but it may be used in the future. I would suggest to add it now with no-ops, or add a TODO
_Why_ do we need `comments` in a protobuf message? The only user is a c'zer launcher process. Also, we want to keep this protobuf message as small as possible - it is serialized on disk via the `ContainerLaunchInfo` proto!
> On Dec. 13, 2018, 8:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 269 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line269>
> >
> > How do we add repeated `subArchitectures` under the current `Architecture` in the future?
We add `subArchitectures` to `architectures` field (when the current `Architecture` matches the native architecure) during parsing of a Seccomp profile. See `parseArchMap()` function.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211264
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221
>
>
> Diff: https://reviews.apache.org/r/68017/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
> On Dec. 13, 2018, 12:20 a.m., Gilbert Song wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 200-215 (patched)
> > <https://reviews.apache.org/r/68017/diff/10/?file=2109592#file2109592line200>
> >
> > I saw you remove all `SCMP_` prefix and implemented a hashmap converter in the next patch. You do this for `AUDIT_` architecture on pupose?
>
> Andrei Budnik wrote:
> Adding `SCMP_` prefix leads to compilation errors due to interference with libseccomp's constants.
Probably we should add some comments for getting rid of the `SCMP_` prefix (since people may ask the same question):
// `SCMP_` prefix is not included in `enum` due to compilation conflicts with libseccomp constants.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211264
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb
>
>
> Diff: https://reviews.apache.org/r/68017/diff/11/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review211264
-----------------------------------------------------------
include/mesos/slave/containerizer.proto
Lines 200-215 (patched)
<https://reviews.apache.org/r/68017/#comment296202>
I saw you remove all `SCMP_` prefix and implemented a hashmap converter in the next patch. You do this for `AUDIT_` architecture on pupose?
include/mesos/slave/containerizer.proto
Lines 257-259 (patched)
<https://reviews.apache.org/r/68017/#comment296207>
Do we want `Arches`? or we could leave a TODO here. I am think about whether we should have a linux specific proto enum for Architecture like S390X etc.
include/mesos/slave/containerizer.proto
Lines 262 (patched)
<https://reviews.apache.org/r/68017/#comment296206>
Is an `action` repeated in this case?
include/mesos/slave/containerizer.proto
Lines 263-264 (patched)
<https://reviews.apache.org/r/68017/#comment296205>
Even we do not use `comments` now, but it may be used in the future. I would suggest to add it now with no-ops, or add a TODO
include/mesos/slave/containerizer.proto
Lines 269 (patched)
<https://reviews.apache.org/r/68017/#comment296203>
How do we add repeated `subArchitectures` under the current `Architecture` in the future?
- 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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
> include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221
>
>
> Diff: https://reviews.apache.org/r/68017/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/
-----------------------------------------------------------
(Updated Nov. 8, 2018, 3:24 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Bugs: MESOS-9033
https://issues.apache.org/jira/browse/MESOS-9033
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
Diff: https://reviews.apache.org/r/68017/diff/6/
Changes: https://reviews.apache.org/r/68017/diff/5-6/
Testing
-------
Thanks,
Andrei Budnik
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Oct. 22, 2018, 5:18 p.m., James Peach wrote:
> > include/mesos/mesos.proto
> > Lines 3085 (patched)
> > <https://reviews.apache.org/r/68017/diff/5/?file=2101833#file2101833line3085>
> >
> > I'm not sure we should have a default here. The default seccomp profile should probably be defined by te operator, which implies an agent flag (which could have a default value). Specifying a default in the protobuf seems likely to cause interoperatbility problems.
I've removed default value here and added `--seccomp_profile_name` agent flag.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review209886
-----------------------------------------------------------
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/68017/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
>
>
> Diff: https://reviews.apache.org/r/68017/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review209886
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 3085 (patched)
<https://reviews.apache.org/r/68017/#comment294510>
I'm not sure we should have a default here. The default seccomp profile should probably be defined by te operator, which implies an agent flag (which could have a default value). Specifying a default in the protobuf seems likely to cause interoperatbility problems.
- James Peach
On Aug. 6, 2018, 1:38 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68017/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 1:38 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17
> include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
>
>
> Diff: https://reviews.apache.org/r/68017/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68017: Added Seccomp-related protobuf messages.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/
-----------------------------------------------------------
(Updated Aug. 6, 2018, 1:38 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Bugs: MESOS-9033
https://issues.apache.org/jira/browse/MESOS-9033
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae
include/mesos/slave/containerizer.proto 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9
Diff: https://reviews.apache.org/r/68017/diff/3/
Changes: https://reviews.apache.org/r/68017/diff/2-3/
Testing
-------
Thanks,
Andrei Budnik