aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 34337: Add Docker Parameters
Date Wed, 08 Jul 2015 21:38:40 GMT

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


Only nits remaining, and one request for test coverage.

One final disclaimer on the security issue this creates - IIUC, arbitrary user-specified volume
mounts opens up your cluster to privilege escalation.
See this discussion for some detail: https://github.com/docker/docker/issues/3124, specifically
this comment:
```
 thaJeztah commented on May 23

@JWGmeligMeyling files and folders created in the volume will have the same uid:gid (numeric)
as the user creating them in the container. If you add a user inside the container having
the same uid:gid as outside the container and run your contsiner as that user, that should
be possible
```

More direct coverage of the risk:
https://fosterelli.co/privilege-escalation-via-docker.html
http://reventlov.com/advisories/using-the-docker-command-to-root-the-host


I'm happy to be proven wrong on this suspicion, but please confirm for yourself that this
is safe to do.


src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java (line 66)
<https://reviews.apache.org/r/34337/#comment144179>

    Matching the terminology above, how about s/enable/allow/?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 186)
<https://reviews.apache.org/r/34337/#comment144183>

    I believe this throws an NPE when parameters is not set.  Can you confirm/deny in a unit
test case?



src/main/python/apache/aurora/config/schema/base.py (line 97)
<https://reviews.apache.org/r/34337/#comment144186>

    I believe this should be
    ```
    parameters = Default(List(Parameter), [])
    ```
    
    to avoid requiring the argument.



src/main/python/apache/aurora/config/thrift.py (lines 133 - 137)
<https://reviews.apache.org/r/34337/#comment144188>

    Please add a test case for this in
    `src/test/python/apache/aurora/config/test_thrift.py`


- Bill Farner


On July 5, 2015, 11:58 p.m., Mauricio Garavaglia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34337/
> -----------------------------------------------------------
> 
> (Updated July 5, 2015, 11:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Support Arbitrary Docker Parameters in DockerContainer
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift d740a90 
>   docs/configuration-reference.md dafd306 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java be79e70

>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java c0d165a 
>   src/main/python/apache/aurora/config/schema/base.py d1f1e4f 
>   src/main/python/apache/aurora/config/thrift.py 88dd1c7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java c0cadfb

> 
> Diff: https://reviews.apache.org/r/34337/diff/
> 
> 
> Testing
> -------
> 
> Used Docker as the container of a Job. Included volumes and label parameters which are
correctly picked up by mesos when starting the task. The docker container gets the specified
label and bind mounts the volumes correctly. I've been running multiple PostgreSQL databases
docker containers for several weeks deploying them as aurora jobs.
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message