Bug 1302821 - Ensure owner has full privileges on directories; r?dustin draft
authorGregory Szorc <gps@mozilla.com>
Wed, 14 Sep 2016 12:26:15 -0700
changeset 413749 a31d0c7f19367de32e54033cf8a6a09901c4ba75
parent 413613 8a494adbc5cced90a4edf0c98cffde906bf7f3ae
child 531282 43cff14c63fbdf1f6b102f62f3386beeede158fb
push id29490
push usergszorc@mozilla.com
push dateWed, 14 Sep 2016 19:26:29 +0000
reviewersdustin
bugs1302821
milestone51.0a1
Bug 1302821 - Ensure owner has full privileges on directories; r?dustin Previously, when recursively changing ownership on directories we would only change the owner. We saw some permission denied failures in automation where the new owner couldn't modify files or directories. This *might* be due to the owner write bits not always being set. Or it could be something else (such as a filesystem bug - *cough* AUFS *cough*). This commit changes our recursive chown implementation to ensure owner read, write, and execute bits are set on directories. Because we're now always calling stat(), the code for calling chown() is made conditional because we have the stat information and can avoid the extra system call if it would be a no-op. MozReview-Commit-ID: JT9q3QR4Sit
testing/docker/recipes/run-task
--- a/testing/docker/recipes/run-task
+++ b/testing/docker/recipes/run-task
@@ -18,16 +18,17 @@ from __future__ import absolute_import, 
 import argparse
 import datetime
 import errno
 import grp
 import json
 import os
 import pwd
 import re
+import stat
 import subprocess
 import sys
 import urllib2
 
 
 FINGERPRINT_URL = 'http://taskcluster/secrets/v1/secret/project/taskcluster/gecko/hgfingerprint'
 
 
@@ -182,30 +183,43 @@ def main(args):
         return 1
 
     uid = user.pw_uid
     gid = group.gr_gid
 
     # Find all groups to which this user is a member.
     gids = [g.gr_gid for g in grp.getgrall() if args.group in g.gr_mem]
 
+    wanted_dir_mode = stat.S_IXUSR | stat.S_IRUSR | stat.S_IWUSR
+    def set_dir_permissions(path, uid, gid):
+        st = os.lstat(path)
+
+        if st.st_uid != uid or st.st_gid != gid:
+            os.chown(path, uid, gid)
+
+        # Also make sure dirs are writable in case we need to delete
+        # them.
+        if st.st_mode & wanted_dir_mode != wanted_dir_mode:
+            os.chmod(path, st.st_mode | wanted_dir_mode)
+
     # 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 []:
         print_line(b'chown', b'changing ownership of %s to %s:%s\n' % (
                    path, user.pw_name, group.gr_name))
-        os.chown(path, uid, gid)
+        set_dir_permissions(path, uid, gid)
 
     for path in args.chown_recursive or []:
         print_line(b'chown', b'recursively changing ownership of %s to %s:%s\n' %
                    (path, user.pw_name, group.gr_name))
         for root, dirs, files in os.walk(path):
             for d in dirs:
-                os.chown(os.path.join(root, d), uid, gid)
+                set_dir_permissions(os.path.join(root, d), uid, gid)
+
             for f in files:
                 os.chown(os.path.join(root, f), uid, gid)
 
     checkout = args.vcs_checkout
     if checkout:
         # Ensure the directory for the source checkout exists.
         try:
             os.makedirs(os.path.dirname(checkout))