Merge "Allow setting copied files group more precisely"

docker/base/set_configs.py

@@ -99,19 +99,8 @@ class ConfigFile(object):
         self._set_permission(dest)
 
     def _set_permission(self, path):
-        user = pwd.getpwnam(self.owner)
+        handle_permissions({'owner': self.owner, 'path': path,
-        uid, gid = user.pw_uid, user.pw_gid
+                            'perm': self.perm})
-        LOG.info('Setting file %s owner to %s:%s', path,
-                 self.owner, self.owner)
-        os.chown(path, uid, gid)
-        # NOTE(Jeffrey4l): py3 need '0oXXX' format for octal literals, and py2
-        # support such format too.
-        perm = self.perm
-        if len(perm) == 4 and perm[1] != 'o':
-            perm = ''.join([perm[:1], 'o', perm[1:]])
-        perm = int(perm, base=0)
-        LOG.info('Setting file %s permission to %s', path, self.perm)
-        os.chmod(path, perm)
 
     def copy(self):
 
@@ -156,15 +145,16 @@ class ConfigFile(object):
                       self.perm, actual_perm)
             return False
         # check owner
+        desired_user, desired_group = user_group(self.owner)
         actual_user = pwd.getpwuid(file_stat.st_uid)
-        if actual_user.pw_name != self.owner:
+        if actual_user.pw_name != desired_user:
             LOG.error('Dest file does not have expected user: %s,'
-                      ' actual: %s ', self.owner, actual_user.pw_name)
+                      ' actual: %s ', desired_user, actual_user.pw_name)
             return False
         actual_group = grp.getgrgid(file_stat.st_gid)
-        if actual_group.gr_name != self.owner:
+        if actual_group.gr_name != desired_group:
             LOG.error('Dest file does not have expected group: %s,'
-                      ' actual: %s ', self.owner, actual_group.gr_name)
+                      ' actual: %s ', desired_group, actual_group.gr_name)
             return False
         return True
 
@@ -296,6 +286,16 @@ def copy_config(config):
         f.write(config['command'])
 
 
+def user_group(owner):
+    if ':' in owner:
+        user, group = owner.split(':', 1)
+        if not group:
+            group = user
+    else:
+        user, group = owner, owner
+    return user, group
+
+
 def handle_permissions(config):
     for permission in config.get('permissions', list()):
         path = permission.get('path')
@@ -303,15 +303,9 @@ def handle_permissions(config):
         recurse = permission.get('recurse', False)
         perm = permission.get('perm')
 
-        if ':' in owner:
+        desired_user, desired_group = user_group(owner)
-            user, group = owner.split(':', 1)
+        uid = pwd.getpwnam(desired_user).pw_uid
-            if not group:
+        gid = grp.getgrnam(desired_group).gr_gid
-                group = user
-        else:
-            user, group = owner, owner
-
-        uid = pwd.getpwnam(user).pw_uid
-        gid = grp.getgrnam(group).gr_gid
 
         def set_perms(path, uid, gid, perm):
             LOG.info('Setting permission for %s', path)
@@ -325,6 +319,8 @@ def handle_permissions(config):
                 LOG.exception('Set permission failed for %s', path)
 
             if perm:
+                # NOTE(Jeffrey4l): py3 need '0oXXX' format for octal literals,
+                # and py2 support such format too.
                 if len(perm) == 4 and perm[1] != 'o':
                     perm = ''.join([perm[:1], 'o', perm[1:]])
                 perm = int(perm, base=0)

releasenotes/notes/allow_setting_group_in_config_files-cef8580912854741.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Allow setting the copied files's group via the 'config_files' structure of
+    config.json, in the same fashion than the 'permissions' attribute does.

tests/test_set_config.py

@@ -10,7 +10,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import collections
 import copy
 import imp
 import json
@@ -117,23 +116,16 @@ class ConfigFileTest(base.BaseTestCase):
         mock_remove.assert_called_with(config_file.dest)
 
     @mock.patch('os.chmod')
-    @mock.patch('os.chown')
+    @mock.patch.object(set_configs, 'handle_permissions')
-    @mock.patch('pwd.getpwnam')
     def test_set_permission(self,
-                            mock_getpwnam,
+                            mock_handle_permissions,
-                            mock_chown,
                             mock_chmod):
 
-        User = collections.namedtuple('User', ['pw_uid', 'pw_gid'])
-        user = User(3333, 4444)
-        mock_getpwnam.return_value = user
-
         config_file = copy.deepcopy(FAKE_CONFIG_FILE)
         config_file._set_permission(config_file.dest)
-
+        mock_handle_permissions.assert_called_with({'owner': 'user1',
-        mock_getpwnam.assert_called_with(config_file.owner)
+                                                    'path': config_file.dest,
-        mock_chown.assert_called_with(config_file.dest, 3333, 4444)
+                                                    'perm': '0644'})
-        mock_chmod.assert_called_with(config_file.dest, 420)
 
     @mock.patch('glob.glob', return_value=[])
     def test_copy_no_source_not_optional(self,