Bug 1391476 - Tell run-task about volumes so it can sanitize them; r?dustin draft
authorGregory Szorc <gps@mozilla.com>
Tue, 22 Aug 2017 16:48:54 -0700
changeset 650873 c7df9aec9234933051d83b94840c8ba259e72fb7
parent 650872 a3010c14c6d45c6bd3765fd574c3900593f160b1
child 650874 4a7e2e26087f5279ab47868164708f2f60649d1c
push id75523
push usergszorc@mozilla.com
push dateWed, 23 Aug 2017 00:42:07 +0000
reviewersdustin
bugs1391476
milestone57.0a1
Bug 1391476 - Tell run-task about volumes so it can sanitize them; r?dustin We recently introduced support for telling run-task about caches so it could sanitize them automatically. We also recently taught docker-worker and docker-engine how to declare volumes. Building on that work, we now pass a list of paths corresponding to Docker volumes to run-task. run-task now verifies volumes behave as expected. Unless the volume paths correspond to caches, run-task verifies they are empty and chowns them to an appropriate owner. MozReview-Commit-ID: 5lm2uIitrS3
taskcluster/docker/recipes/run-task
taskcluster/taskgraph/transforms/task.py
--- a/taskcluster/docker/recipes/run-task
+++ b/taskcluster/docker/recipes/run-task
@@ -329,16 +329,42 @@ def main(args):
         # happen because run-task should be the first thing that touches a
         # cache.
         else:
             print('cache %s is not empty and is missing a .cacherequires '
                   'file; the cache names for this task are likely '
                   'mis-configured')
             return 1
 
+    if 'TASKCLUSTER_VOLUMES' in os.environ:
+        volumes = os.environ['TASKCLUSTER_VOLUMES'].split(';')
+        del os.environ['TASKCLUSTER_VOLUMES']
+    else:
+        volumes = []
+
+    # Sanitize volumes.
+    for volume in volumes:
+        # If a volume is a cache, it was dealt with above. Otherwise, the
+        # worker is supposed to give us an empty filesystem. It will likely
+        # be owned by root:root. So we may need to normalize permissions.
+        if volume in caches:
+            print_line(b'volume', b'volume %s is a cache\n' % volume)
+            continue
+
+        if os.listdir(volume):
+            print('error: volume %s is not empty; non-cache volumes are '
+                  'supposed to be empty; you likely found a TaskCluster '
+                  'worker bug' % volume)
+            return 1
+
+        if running_as_root:
+            print_line(b'volume', b'changing ownership of non-cache volume %s '
+                                  b'to %d:%d\n' % (volume, uid, gid))
+            set_dir_permissions(volume, uid, gid)
+
     # Change ownership of requested paths.
     # FUTURE: parse argument values for user/group if we don't want to
     # use --user/--group.
     for path in args.chown or []:
         if not running_as_root:
             print_line(b'set_dir_permissions', b'--chown not allowed when not running as root')
             return 1
 
--- a/taskcluster/taskgraph/transforms/task.py
+++ b/taskcluster/taskgraph/transforms/task.py
@@ -686,29 +686,29 @@ def build_docker_worker_payload(config, 
         for artifact in worker['artifacts']:
             artifacts[artifact['name']] = {
                 'path': artifact['path'],
                 'type': artifact['type'],
                 'expires': task_def['expires'],  # always expire with the task
             }
         payload['artifacts'] = artifacts
 
+    run_task = payload.get('command', [''])[0].endswith('run-task')
+
     if 'caches' in worker:
         caches = {}
 
         # run-task knows how to validate caches.
         #
         # To help ensure new run-task features and bug fixes don't interfere
         # with existing caches, we seed the hash of run-task into cache names.
         # So, any time run-task changes, we should get a fresh set of caches.
         # This means run-task can make changes to cache interaction at any time
         # without regards for backwards or future compatibility.
 
-        run_task = payload.get('command', [''])[0].endswith('run-task')
-
         if run_task:
             suffix = '-%s' % _run_task_suffix()
         else:
             suffix = ''
 
         skip_untrusted = config.params['project'] == 'try' or level == 1
 
         for cache in worker['caches']:
@@ -723,16 +723,21 @@ def build_docker_worker_payload(config, 
 
         # Assertion: only run-task is interested in this.
         if run_task:
             payload['env']['TASKCLUSTER_CACHES'] = ';'.join(sorted(
                 caches.values()))
 
         payload['cache'] = caches
 
+    # And send down volumes information to run-task as well.
+    if run_task and worker.get('volumes'):
+        payload['env']['TASKCLUSTER_VOLUMES'] = ';'.join(
+            sorted(worker['volumes']))
+
     if features:
         payload['features'] = features
     if capabilities:
         payload['capabilities'] = capabilities
 
     # coalesce / superseding
     if 'coalesce-name' in task and level > 1:
         key = COALESCE_KEY.format(