You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Stephan Erb <st...@dev.static-void.de> on 2016/02/01 00:29:29 UTC
Re: Review Request 43027: Optionally enable setuid inside Docker
containers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43027/#review117162
-----------------------------------------------------------
With the new proposed option we'd get `--execute-as-user`, `--nosetuid`, and `--docker-setuid`. The last two are basically doing the same thing.
Would it make sense to resolve this by pushing things up the stack and allow cluster administrators to provide an executor config per containerizer?
- Stephan Erb
On Jan. 31, 2016, 7:50 a.m., Benjamin Staffin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43027/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2016, 7:50 a.m.)
>
>
> Review request for Aurora.
>
>
> Bugs: AURORA-1237
> https://issues.apache.org/jira/browse/AURORA-1237
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adds a flag to enable the new behavior. If enabled, also sets
> ownership of the sandbox directory appropriately.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/executor/bin/thermos_executor_main.py f4f5cd77b6444c225ec960c7e2cf5349a80bd344
> src/main/python/apache/aurora/executor/common/sandbox.py 4780232318ffdf8c6bbbe78bee518886cffd580a
> src/main/python/apache/aurora/executor/thermos_task_runner.py 3896e3841562600379705dbf78a6f62728246348
>
> Diff: https://reviews.apache.org/r/43027/diff/
>
>
> Testing
> -------
>
> TBD
>
>
> Thanks,
>
> Benjamin Staffin
>
>
Re: Review Request 43027: Optionally enable setuid inside Docker
containers
Posted by Benjamin Staffin <be...@gmail.com>.
> On Jan. 31, 2016, 3:29 p.m., Stephan Erb wrote:
> > With the new proposed option we'd get `--execute-as-user`, `--nosetuid`, and `--docker-setuid`. The last two are basically doing the same thing.
> >
> > Would it make sense to resolve this by pushing things up the stack and allow cluster administrators to provide an executor config per containerizer?
>
> Benjamin Staffin wrote:
> The last two are doing the same thing, except that the existing behaviour has the docker runner ignoring all setuid options and always running as root (or possibly as the user set in the image def, if set). I'm still trying to think up a better name for this new flag that doesn't require renaming the existing ones and breaking compatibility.
>
> What if we replaced all three of those with something like: `--setuid=[auto | off | always:<uid>][,nodocker]`
>
> With the default set to `--setuid=auto,nodocker` for the current behaviour,
>
> And perhaps aliases for the old flags during a deprecation period:
> `--execute-as-user=<uid>` aliased to `--setuid=always:<uid>,nodocker`
> `--nosetuid` aliased to `--setuid=off`
>
> If we want to push this further up the stack as you suggest, what might that interface look like?
>
> Stephan Erb wrote:
> My idea was in the line of: When starting the the Aurora scheduler, I can provide a different thermos command line for Docker tasks than for ordinary Mesos tasks.
>
> But that will probably a more complex change than the one you have proposed here.
I'll take a stab at implementing the --setuid=... approach tonight if that sounds sane enough for now. As far as I know it's the only place where the executor has inconsistent behavior between docker and not-docker, so in theory we won't need a bunch of special cases like this.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43027/#review117162
-----------------------------------------------------------
On Jan. 30, 2016, 10:50 p.m., Benjamin Staffin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43027/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2016, 10:50 p.m.)
>
>
> Review request for Aurora.
>
>
> Bugs: AURORA-1237
> https://issues.apache.org/jira/browse/AURORA-1237
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adds a flag to enable the new behavior. If enabled, also sets
> ownership of the sandbox directory appropriately.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/executor/bin/thermos_executor_main.py f4f5cd77b6444c225ec960c7e2cf5349a80bd344
> src/main/python/apache/aurora/executor/common/sandbox.py 4780232318ffdf8c6bbbe78bee518886cffd580a
> src/main/python/apache/aurora/executor/thermos_task_runner.py 3896e3841562600379705dbf78a6f62728246348
>
> Diff: https://reviews.apache.org/r/43027/diff/
>
>
> Testing
> -------
>
> TBD
>
>
> Thanks,
>
> Benjamin Staffin
>
>
Re: Review Request 43027: Optionally enable setuid inside Docker
containers
Posted by Stephan Erb <st...@dev.static-void.de>.
> On Feb. 1, 2016, 12:29 a.m., Stephan Erb wrote:
> > With the new proposed option we'd get `--execute-as-user`, `--nosetuid`, and `--docker-setuid`. The last two are basically doing the same thing.
> >
> > Would it make sense to resolve this by pushing things up the stack and allow cluster administrators to provide an executor config per containerizer?
>
> Benjamin Staffin wrote:
> The last two are doing the same thing, except that the existing behaviour has the docker runner ignoring all setuid options and always running as root (or possibly as the user set in the image def, if set). I'm still trying to think up a better name for this new flag that doesn't require renaming the existing ones and breaking compatibility.
>
> What if we replaced all three of those with something like: `--setuid=[auto | off | always:<uid>][,nodocker]`
>
> With the default set to `--setuid=auto,nodocker` for the current behaviour,
>
> And perhaps aliases for the old flags during a deprecation period:
> `--execute-as-user=<uid>` aliased to `--setuid=always:<uid>,nodocker`
> `--nosetuid` aliased to `--setuid=off`
>
> If we want to push this further up the stack as you suggest, what might that interface look like?
My idea was in the line of: When starting the the Aurora scheduler, I can provide a different thermos command line for Docker tasks than for ordinary Mesos tasks.
But that will probably a more complex change than the one you have proposed here.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43027/#review117162
-----------------------------------------------------------
On Jan. 31, 2016, 7:50 a.m., Benjamin Staffin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43027/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2016, 7:50 a.m.)
>
>
> Review request for Aurora.
>
>
> Bugs: AURORA-1237
> https://issues.apache.org/jira/browse/AURORA-1237
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adds a flag to enable the new behavior. If enabled, also sets
> ownership of the sandbox directory appropriately.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/executor/bin/thermos_executor_main.py f4f5cd77b6444c225ec960c7e2cf5349a80bd344
> src/main/python/apache/aurora/executor/common/sandbox.py 4780232318ffdf8c6bbbe78bee518886cffd580a
> src/main/python/apache/aurora/executor/thermos_task_runner.py 3896e3841562600379705dbf78a6f62728246348
>
> Diff: https://reviews.apache.org/r/43027/diff/
>
>
> Testing
> -------
>
> TBD
>
>
> Thanks,
>
> Benjamin Staffin
>
>
Re: Review Request 43027: Optionally enable setuid inside Docker
containers
Posted by Benjamin Staffin <be...@gmail.com>.
> On Jan. 31, 2016, 3:29 p.m., Stephan Erb wrote:
> > With the new proposed option we'd get `--execute-as-user`, `--nosetuid`, and `--docker-setuid`. The last two are basically doing the same thing.
> >
> > Would it make sense to resolve this by pushing things up the stack and allow cluster administrators to provide an executor config per containerizer?
The last two are doing the same thing, except that the existing behaviour has the docker runner ignoring all setuid options and always running as root (or possibly as the user set in the image def, if set). I'm still trying to think up a better name for this new flag that doesn't require renaming the existing ones and breaking compatibility.
What if we replaced all three of those with something like: `--setuid=[auto | off | always:<uid>][,nodocker]`
With the default set to `--setuid=auto,nodocker` for the current behaviour,
And perhaps aliases for the old flags during a deprecation period:
`--execute-as-user=<uid>` aliased to `--setuid=always:<uid>,nodocker`
`--nosetuid` aliased to `--setuid=off`
If we want to push this further up the stack as you suggest, what might that interface look like?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43027/#review117162
-----------------------------------------------------------
On Jan. 30, 2016, 10:50 p.m., Benjamin Staffin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43027/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2016, 10:50 p.m.)
>
>
> Review request for Aurora.
>
>
> Bugs: AURORA-1237
> https://issues.apache.org/jira/browse/AURORA-1237
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adds a flag to enable the new behavior. If enabled, also sets
> ownership of the sandbox directory appropriately.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/executor/bin/thermos_executor_main.py f4f5cd77b6444c225ec960c7e2cf5349a80bd344
> src/main/python/apache/aurora/executor/common/sandbox.py 4780232318ffdf8c6bbbe78bee518886cffd580a
> src/main/python/apache/aurora/executor/thermos_task_runner.py 3896e3841562600379705dbf78a6f62728246348
>
> Diff: https://reviews.apache.org/r/43027/diff/
>
>
> Testing
> -------
>
> TBD
>
>
> Thanks,
>
> Benjamin Staffin
>
>