Repository: aurora Updated Branches: refs/heads/master bd11b1c51 -> 80cf585a0
Allow E_NAME_IN_USE in useradd/groupadd. Bugs closed: AURORA-1761 Reviewed at https://reviews.apache.org/r/51564/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/80cf585a Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/80cf585a Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/80cf585a Branch: refs/heads/master Commit: 80cf585a00003388bc6501e40d2db05eca19d3cb Parents: bd11b1c Author: Zhitao Li <[email protected]> Authored: Fri Sep 2 12:51:46 2016 -0600 Committer: John Sirois <[email protected]> Committed: Fri Sep 2 12:51:46 2016 -0600 ---------------------------------------------------------------------- .../apache/aurora/executor/common/sandbox.py | 41 +++++- .../aurora/executor/common/test_sandbox.py | 124 +++++++++++++++++-- 2 files changed, 154 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/80cf585a/src/main/python/apache/aurora/executor/common/sandbox.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/common/sandbox.py b/src/main/python/apache/aurora/executor/common/sandbox.py index a172691..4a0f3b5 100644 --- a/src/main/python/apache/aurora/executor/common/sandbox.py +++ b/src/main/python/apache/aurora/executor/common/sandbox.py @@ -190,6 +190,10 @@ class FileSystemImageSandbox(DirectorySandbox): # returncode from a `useradd` or `groupadd` call indicating that the uid/gid already exists. _USER_OR_GROUP_ID_EXISTS = 4 + # returncode from a `useradd` or `groupadd` call indicating that the user/group name + # already exists. + _USER_OR_GROUP_NAME_EXISTS = 9 + def __init__(self, root, **kwargs): self._task_fs_root = os.path.join( os.environ[DefaultSandboxProvider.MESOS_DIRECTORY_ENV_VARIABLE], @@ -205,6 +209,36 @@ class FileSystemImageSandbox(DirectorySandbox): super(FileSystemImageSandbox, self).__init__(root, **kwargs) + def _verify_group_match_in_taskfs(self, group_id, group_name): + try: + result = subprocess.check_output( + ['chroot', self._task_fs_root, 'getent', 'group', group_name]) + except subprocess.CalledProcessError as e: + raise self.CreationError( + 'Error when getting group id for name %s in task image: %s' % ( + group_name, e)) + splitted = result.split(':') + if (len(splitted) < 3 or splitted[0] != '%s' % group_name or + splitted[2] != '%s' % group_id): + raise self.CreationError( + 'Group id result %s from image does not match name %s and id %s' % ( + result, group_name, group_id)) + + def _verify_user_match_in_taskfs(self, user_id, user_name, group_id, group_name): + try: + result = subprocess.check_output( + ['chroot', self._task_fs_root, 'id', '%s' % user_name]) + except subprocess.CalledProcessError as e: + raise self.CreationError( + 'Error when getting user id for name %s in task image: %s' % ( + user_name, e)) + + expected_prefix = "uid=%s(%s) gid=%s(%s) groups=" % (user_id, user_name, group_id, group_name) + if not result.startswith(expected_prefix): + raise self.CreationError( + 'User group result %s from task image does not start with expected prefix %s' % ( + result, expected_prefix)) + def _create_user_and_group_in_taskfs(self): if self._user: pwent, grent = self.get_user_and_group() @@ -214,7 +248,8 @@ class FileSystemImageSandbox(DirectorySandbox): ['groupadd', '-R', self._task_fs_root, '-g', '%s' % grent.gr_gid, grent.gr_name]) except subprocess.CalledProcessError as e: # If the failure was due to the group existing, we're ok to continue, otherwise bail out. - if e.returncode == self._USER_OR_GROUP_ID_EXISTS: + if e.returncode in [self._USER_OR_GROUP_ID_EXISTS, self._USER_OR_GROUP_NAME_EXISTS]: + self._verify_group_match_in_taskfs(grent.gr_gid, grent.gr_name) log.info( 'Group %s(%s) already exists in the task''s filesystem, no need to create.' % ( grent.gr_name, grent.gr_gid)) @@ -232,7 +267,9 @@ class FileSystemImageSandbox(DirectorySandbox): pwent.pw_name]) except subprocess.CalledProcessError as e: # If the failure was due to the user existing, we're ok to continue, otherwise bail out. - if e.returncode == self._USER_OR_GROUP_ID_EXISTS: + if e.returncode in [self._USER_OR_GROUP_ID_EXISTS, self._USER_OR_GROUP_NAME_EXISTS]: + self._verify_user_match_in_taskfs( + pwent.pw_uid, pwent.pw_name, pwent.pw_gid, grent.gr_name) log.info( 'User %s (%s) already exists in the task''s filesystem, no need to create.' % ( self._user, pwent.pw_uid)) http://git-wip-us.apache.org/repos/asf/aurora/blob/80cf585a/src/test/python/apache/aurora/executor/common/test_sandbox.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/common/test_sandbox.py b/src/test/python/apache/aurora/executor/common/test_sandbox.py index 57ab39e..41ee884 100644 --- a/src/test/python/apache/aurora/executor/common/test_sandbox.py +++ b/src/test/python/apache/aurora/executor/common/test_sandbox.py @@ -141,7 +141,89 @@ def test_destroy_ioerror(): ds.destroy() -def assert_create_user_and_group(mock_check_call, gid_exists, uid_exists): +MOCK_MESOS_DIRECTORY = '/some/path' + + [email protected]('subprocess.check_output') [email protected](os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) +def test_verify_group_match(mock_check_output): + with temporary_dir() as d: + sandbox = FileSystemImageSandbox( + os.path.join(d, 'sandbox'), + user='someuser', + sandbox_mount_point='/some/path') + + mock_check_output.return_value = 'test-group:x:2:' + + # valid case + sandbox._verify_group_match_in_taskfs(2, 'test-group') + mock_check_output.assert_called_with( + ['chroot', sandbox._task_fs_root, 'getent', 'group', 'test-group']) + + # invalid group id + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_group_match_in_taskfs(3, 'test-group') + + # invalid group name + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_group_match_in_taskfs(2, 'invalid-group') + + # exception case + exception = subprocess.CalledProcessError( + returncode=1, + cmd='some command', + output=None) + mock_check_output.side_effect = exception + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_group_match_in_taskfs(2, 'test-group') + + [email protected]('subprocess.check_output') [email protected](os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) +def test_verify_user_match(mock_check_output): + with temporary_dir() as d: + sandbox = FileSystemImageSandbox( + os.path.join(d, 'sandbox'), + user='someuser', + sandbox_mount_point='/some/path') + + mock_check_output.return_value = 'uid=1(test-user) gid=2(test-group) groups=2(test-group)' + # valid case + sandbox._verify_user_match_in_taskfs(1, 'test-user', 2, 'test-group') + mock_check_output.assert_called_with(['chroot', sandbox._task_fs_root, 'id', 'test-user']) + + # invalid user id + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_user_match_in_taskfs(0, 'test-user', 2, 'test-group') + + # invalid user name + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_user_match_in_taskfs(1, 'invalid-user', 2, 'test-group') + + # invalid group id + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_user_match_in_taskfs(1, 'test-user', 0, 'test-group') + + # invalid group name + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_user_match_in_taskfs(1, 'test-user', 2, 'invalid-group') + + # exception case + exception = subprocess.CalledProcessError( + returncode=1, + cmd='some command', + output=None) + mock_check_output.side_effect = exception + with pytest.raises(FileSystemImageSandbox.CreationError): + sandbox._verify_user_match_in_taskfs(1, "test-user", 2, "test-group") + + +def assert_create_user_and_group(mock_check_call, + mock_verify, + gid_exists, + uid_exists, + group_name_exists, + user_name_exists): mock_pwent = pwd.struct_passwd(( 'someuser', # login name 'hunter2', # password @@ -157,14 +239,20 @@ def assert_create_user_and_group(mock_check_call, gid_exists, uid_exists): 835, # gid ['someuser'])) # members + returncode = 0 + if gid_exists or uid_exists: + returncode = FileSystemImageSandbox._USER_OR_GROUP_ID_EXISTS + elif group_name_exists or user_name_exists: + returncode = FileSystemImageSandbox._USER_OR_GROUP_NAME_EXISTS + exception = subprocess.CalledProcessError( - returncode=FileSystemImageSandbox._USER_OR_GROUP_ID_EXISTS, + returncode=returncode, cmd='some command', output=None) mock_check_call.side_effect = [ - None if gid_exists else exception, - None if uid_exists else exception] + exception if gid_exists or group_name_exists else None, + exception if uid_exists or user_name_exists else None] with temporary_dir() as d: with mock.patch.object( @@ -179,21 +267,39 @@ def assert_create_user_and_group(mock_check_call, gid_exists, uid_exists): sandbox._create_user_and_group_in_taskfs() assert len(mock_check_call.mock_calls) == 2 + assert len(mock_verify.mock_calls) == 1 -MOCK_MESOS_DIRECTORY = '/some/path' [email protected]('apache.aurora.executor.common.sandbox.' + + 'FileSystemImageSandbox._verify_user_match_in_taskfs') [email protected]('subprocess.check_call') [email protected](os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) +def test_uid_exists(mock_check_call, mock_verify): + assert_create_user_and_group(mock_check_call, mock_verify, False, True, False, False) + + [email protected]('apache.aurora.executor.common.sandbox.' + + 'FileSystemImageSandbox._verify_group_match_in_taskfs') [email protected]('subprocess.check_call') [email protected](os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) +def test_gid_exists(mock_check_call, mock_verify): + assert_create_user_and_group(mock_check_call, mock_verify, True, False, False, False) [email protected]('apache.aurora.executor.common.sandbox.' + + 'FileSystemImageSandbox._verify_user_match_in_taskfs') @mock.patch('subprocess.check_call') @mock.patch.dict(os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) -def test_uid_exists(mock_check_call): - assert_create_user_and_group(mock_check_call, False, True) +def test_user_name_exists(mock_check_call, mock_verify): + assert_create_user_and_group(mock_check_call, mock_verify, False, False, False, True) [email protected]('apache.aurora.executor.common.sandbox.' + + 'FileSystemImageSandbox._verify_group_match_in_taskfs') @mock.patch('subprocess.check_call') @mock.patch.dict(os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY}) -def test_gid_exists(mock_check_call): - assert_create_user_and_group(mock_check_call, True, False) +def test_group_name_exists(mock_check_call, mock_verify): + assert_create_user_and_group(mock_check_call, mock_verify, True, False, True, False) @mock.patch('subprocess.check_call')
