Allow setting copied files group more precisely

Previously with the config_files structure of config.json, the group
name was automatically set to the one of the user name. It is now
possible to set the group name in the same fashion than the
'permissions' structure with:

  owner: 'desired_owner:desired_group'

Closes-Bug: #1696095
Change-Id: Ibae9f74e2351c81a717294467aedc51ea773c41e

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,