From 748a056b0964e2abf93ad66951957c834a32b81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Tue, 6 Jun 2017 10:58:32 +0200 Subject: [PATCH] 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 | 48 +++++++++---------- ...roup_in_config_files-cef8580912854741.yaml | 4 ++ tests/test_set_config.py | 18 ++----- 3 files changed, 31 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/allow_setting_group_in_config_files-cef8580912854741.yaml diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py index 4598bedcc7..59dc2f38bf 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 0000000000..95997d2ce0 --- /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 e67db9bc24..fa8e45301d 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,