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

3 files changed, 31 insertions, 39 deletions

diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py
index 4598bedcc..59dc2f38b 100644
--- a/docker/base/set_configs.py
+++ b/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)
-        uid, gid = user.pw_uid, user.pw_gid
-        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)
+        handle_permissions({'owner': self.owner, 'path': path,
+                            'perm': self.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:
-            user, group = owner.split(':', 1)
-            if not group:
-                group = user
-        else:
-            user, group = owner, owner
-
-        uid = pwd.getpwnam(user).pw_uid
-        gid = grp.getgrnam(group).gr_gid
+        desired_user, desired_group = user_group(owner)
+        uid = pwd.getpwnam(desired_user).pw_uid
+        gid = grp.getgrnam(desired_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)
diff --git a/releasenotes/notes/allow_setting_group_in_config_files-cef8580912854741.yaml b/releasenotes/notes/allow_setting_group_in_config_files-cef8580912854741.yaml
new file mode 100644
index 000000000..95997d2ce
--- /dev/null
+++ b/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.
diff --git a/tests/test_set_config.py b/tests/test_set_config.py
index e67db9bc2..fa8e45301 100644
--- a/tests/test_set_config.py
+++ b/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('pwd.getpwnam')
+    @mock.patch.object(set_configs, 'handle_permissions')
     def test_set_permission(self,
-                            mock_getpwnam,
-                            mock_chown,
+                            mock_handle_permissions,
                             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_getpwnam.assert_called_with(config_file.owner)
-        mock_chown.assert_called_with(config_file.dest, 3333, 4444)
-        mock_chmod.assert_called_with(config_file.dest, 420)
+        mock_handle_permissions.assert_called_with({'owner': 'user1',
+                                                    'path': config_file.dest,
+                                                    'perm': '0644'})
 
     @mock.patch('glob.glob', return_value=[])
     def test_copy_no_source_not_optional(self,