Bug 1391476 - Automatically set cache/volume permissions in run-task; r?dustin
run-task's --chown and --chown-recursive are only used on volumes and
caches - the only locations that aren't controlled by the Docker image
itself and thus whose permissions could be "undefined."
Previous commits have taught run-task about the locations of all caches
and volumes. Therefore, we no longer need to manually define paths to
chown. Instead, we can chown as a side-effect of the path being a
cache or a volume.
So, this commit changes run-task to chown caches and volumes
automatically. Since we no longer have a use for --chown and
--chown-recursive, those arguments are removed.
There /could/ be some paths that are caches or volumes but aren't
getting defined as such in Taskgraph. I consider this a bug in
Taskgraph and the recourse is to properly define a path as a cache or
a volume there.
MozReview-Commit-ID: 1yqrhjil6gy
--- a/taskcluster/docker/image_builder/build-image.sh
+++ b/taskcluster/docker/image_builder/build-image.sh
@@ -21,17 +21,16 @@ test -n "$IMAGE_NAME" || raise_error "IM
# Create artifact folder
mkdir -p /home/worker/workspace/artifacts
# Construct a CONTEXT_FILE
CONTEXT_FILE=/home/worker/workspace/context.tar
# Run ./mach taskcluster-build-image with --context-only to build context
run-task \
- --chown-recursive "/home/worker/workspace" \
--vcs-checkout "/home/worker/checkouts/gecko" \
-- \
/home/worker/checkouts/gecko/mach taskcluster-build-image \
--context-only "$CONTEXT_FILE" \
"$IMAGE_NAME"
test -f "$CONTEXT_FILE" || raise_error "Context file wasn't created"
# Post context tar-ball to docker daemon
--- a/taskcluster/docker/recipes/run-task
+++ b/taskcluster/docker/recipes/run-task
@@ -212,23 +212,16 @@ def main(args):
task_args = args[i + 1:]
except ValueError:
our_args = args
task_args = []
parser = argparse.ArgumentParser()
parser.add_argument('--user', default='worker', help='user to run as')
parser.add_argument('--group', default='worker', help='group to run as')
- # We allow paths to be chowned by the --user:--group before permissions are
- # dropped. This is often necessary for caches/volumes, since they default
- # to root:root ownership.
- parser.add_argument('--chown', action='append',
- help='Directory to chown to --user:--group')
- parser.add_argument('--chown-recursive', action='append',
- help='Directory to recursively chown to --user:--group')
parser.add_argument('--vcs-checkout',
help='Directory where Gecko checkout should be created')
parser.add_argument('--comm-checkout',
help='Directory where Comm checkout should be created')
parser.add_argument('--tools-checkout',
help='Directory where build/tools checkout should be created')
parser.add_argument('--fetch-hgfingerprint', action='store_true',
help='Fetch the latest hgfingerprint from the secrets store, '
@@ -236,20 +229,16 @@ def main(args):
args = parser.parse_args(our_args)
# expand ~ in some paths
if args.vcs_checkout:
args.vcs_checkout = os.path.expanduser(args.vcs_checkout)
if args.tools_checkout:
args.tools_checkout = os.path.expanduser(args.tools_checkout)
- if args.chown:
- args.chown = [os.path.expanduser(p) for p in args.chown]
- if args.chown_recursive:
- args.chown_recursive = [os.path.expanduser(p) for p in args.chown_recursive]
if 'HG_STORE_PATH' in os.environ:
os.environ['HG_STORE_PATH'] = os.path.expanduser(os.environ['HG_STORE_PATH'])
if running_as_root:
try:
user = pwd.getpwnam(args.user)
except KeyError:
print('could not find user %s; specify --user to a known user' %
@@ -315,16 +304,18 @@ def main(args):
b'%s\n' % (cache, b' '.join(sorted(our_requirements))))
with open(requires_path, 'wb') as fh:
fh.write(b'\n'.join(sorted(our_requirements)))
# And make it read-only as a precaution against deletion.
os.chmod(requires_path, stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)
+ set_dir_permissions(cache, uid, gid)
+
# The cache has content and we have a requirements file. Validate
# requirements alignment.
elif os.path.exists(requires_path):
with open(requires_path, 'rb') as fh:
wanted_requirements = set(fh.read().splitlines())
print_line(b'cache', b'cache %s exists; requirements: %s\n' % (
cache, b' '.join(sorted(wanted_requirements))))
@@ -334,16 +325,18 @@ def main(args):
print('requirements for populated cache %s differ from '
'this task' % cache)
print('cache requirements: %s' % ' '.join(sorted(
wanted_requirements)))
print('our requirements: %s' % ' '.join(sorted(
our_requirements)))
return 1
+ chown_recursive(cache, user.pw_name, group.gr_name, uid, gid)
+
# The cache has content and no requirements file. This shouldn't
# 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
@@ -379,35 +372,16 @@ def main(args):
# The volume is almost certainly owned by root:root. Chown it so it
# is writable.
if running_as_root:
print_line(b'volume', b'changing ownership of 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
-
- print_line(b'chown', b'changing ownership of %s to %s:%s\n' % (
- path, user.pw_name, group.gr_name))
- set_dir_permissions(path, uid, gid)
-
- for path in args.chown_recursive or []:
- if not running_as_root:
- print_line(b'set_dir_permissions', b'--chown-recursive not allowed when not running as root')
- return 1
-
- chown_recursive(path, user.pw_name, group.gr_name, uid, gid)
-
def prepare_checkout_dir(checkout):
if not checkout:
return
# Ensure the directory for the source checkout exists.
try:
os.makedirs(os.path.dirname(checkout))
except OSError as e:
--- a/taskcluster/taskgraph/transforms/job/hazard.py
+++ b/taskcluster/taskgraph/transforms/job/hazard.py
@@ -61,14 +61,12 @@ def docker_worker_hazard(config, job, ta
env['MOZCONFIG'] = run['mozconfig']
# build-haz-linux.sh needs this otherwise it assumes the checkout is in
# the workspace.
env['GECKO_DIR'] = '/home/worker/checkouts/gecko'
worker['command'] = [
'/home/worker/bin/run-task',
- '--chown-recursive', '/home/worker/tooltool-cache',
- '--chown-recursive', '/home/worker/workspace',
'--vcs-checkout', '/home/worker/checkouts/gecko',
'--',
'/bin/bash', '-c', run['command']
]
--- a/taskcluster/taskgraph/transforms/job/mozharness.py
+++ b/taskcluster/taskgraph/transforms/job/mozharness.py
@@ -164,19 +164,16 @@ def mozharness_on_docker_worker_setup(co
# Retry if mozharness returns TBPL_RETRY
worker['retry-exit-status'] = 4
docker_worker_setup_secrets(config, job, taskdesc)
command = [
'/home/worker/bin/run-task',
- # Various caches/volumes are default owned by root:root.
- '--chown-recursive', '/home/worker/workspace',
- '--chown-recursive', '/home/worker/tooltool-cache',
'--vcs-checkout', '/home/worker/workspace/build/src',
'--tools-checkout', '/home/worker/workspace/build/tools',
'--',
'/home/worker/workspace/build/src/{}'.format(
run.get('job-script', 'taskcluster/scripts/builder/build-linux.sh')
),
]
--- a/taskcluster/taskgraph/transforms/job/mozharness_test.py
+++ b/taskcluster/taskgraph/transforms/job/mozharness_test.py
@@ -152,18 +152,16 @@ def mozharness_test_on_docker(config, jo
docker_worker_add_tooltool(config, job, taskdesc, internal=True)
if test['reboot']:
raise Exception('reboot: {} not supported on generic-worker'.format(test['reboot']))
# assemble the command line
command = [
'/home/worker/bin/run-task',
- # The workspace cache/volume is default owned by root:root.
- '--chown', '/home/worker/workspace',
]
# Support vcs checkouts regardless of whether the task runs from
# source or not in case it is needed on an interactive loaner.
support_vcs_checkout(config, job, taskdesc)
# If we have a source checkout, run mozharness from it instead of
# downloading a zip file with the same content.
--- a/taskcluster/taskgraph/transforms/job/spidermonkey.py
+++ b/taskcluster/taskgraph/transforms/job/spidermonkey.py
@@ -60,18 +60,16 @@ def docker_worker_spidermonkey(config, j
script = "build-sm.sh"
if run['using'] == 'spidermonkey-package':
script = "build-sm-package.sh"
elif run['using'] == 'spidermonkey-mozjs-crate':
script = "build-sm-mozjs-crate.sh"
worker['command'] = [
'/home/worker/bin/run-task',
- '--chown-recursive', '/home/worker/workspace',
- '--chown-recursive', '/home/worker/tooltool-cache',
'--vcs-checkout', '/home/worker/workspace/build/src',
'--',
'/bin/bash',
'-c',
'cd /home/worker && workspace/build/src/taskcluster/scripts/builder/%s' % script
]
--- a/taskcluster/taskgraph/transforms/job/toolchain.py
+++ b/taskcluster/taskgraph/transforms/job/toolchain.py
@@ -113,19 +113,16 @@ def docker_worker_toolchain(config, job,
})
if run['tooltool-downloads']:
internal = run['tooltool-downloads'] == 'internal'
docker_worker_add_tooltool(config, job, taskdesc, internal=internal)
worker['command'] = [
'/home/worker/bin/run-task',
- # Various caches/volumes are default owned by root:root.
- '--chown-recursive', '/home/worker/workspace',
- '--chown-recursive', '/home/worker/tooltool-cache',
'--vcs-checkout=/home/worker/workspace/build/src',
'--',
'bash',
'-c',
'cd /home/worker && '
'./workspace/build/src/taskcluster/scripts/misc/{}'.format(
run['script'])
]