mirror of
https://github.com/ansible-collections/community.general.git
synced 2025-04-28 13:21:25 -07:00
fix Mac chown/chmod -R issue, add error checks
The changes to chown/chmod were broken on Mac (-R was being appended to the end of the command- OSX requires it before the file list). A number of base action remote setup commands were also blindly proceeding without checking for success. Added error raises for unrecoverable failure cases.
This commit is contained in:
parent
5fdac707fd
commit
05af5c88ea
2 changed files with 29 additions and 13 deletions
|
@ -308,11 +308,15 @@ class ActionBase(with_metaclass(ABCMeta, object)):
|
||||||
# to work!
|
# to work!
|
||||||
if remote_user == 'root':
|
if remote_user == 'root':
|
||||||
# SSh'ing as root, therefore we can chown
|
# SSh'ing as root, therefore we can chown
|
||||||
self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive)
|
res = self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive)
|
||||||
|
if res['rc'] != 0:
|
||||||
|
raise AnsibleError('Failed to set owner on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
|
||||||
if execute:
|
if execute:
|
||||||
# root can read things that don't have read bit but can't
|
# root can read things that don't have read bit but can't
|
||||||
# execute them.
|
# execute them.
|
||||||
self._remote_chmod('u+x', remote_path, recursive=recursive)
|
res = self._remote_chmod('u+x', remote_path, recursive=recursive)
|
||||||
|
if res['rc'] != 0:
|
||||||
|
raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
|
||||||
else:
|
else:
|
||||||
if execute:
|
if execute:
|
||||||
mode = 'rx'
|
mode = 'rx'
|
||||||
|
@ -325,14 +329,18 @@ class ActionBase(with_metaclass(ABCMeta, object)):
|
||||||
# fs acls failed -- do things this insecure way only
|
# fs acls failed -- do things this insecure way only
|
||||||
# if the user opted in in the config file
|
# if the user opted in in the config file
|
||||||
self._display.warning('Using world-readable permissions for temporary files Ansible needs to create when becoming an unprivileged user which may be insecure. For information on securing this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
|
self._display.warning('Using world-readable permissions for temporary files Ansible needs to create when becoming an unprivileged user which may be insecure. For information on securing this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
|
||||||
self._remote_chmod('a+%s' % mode, remote_path, recursive=recursive)
|
res = self._remote_chmod('a+%s' % mode, remote_path, recursive=recursive)
|
||||||
|
if res['rc'] != 0:
|
||||||
|
raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
|
||||||
else:
|
else:
|
||||||
raise AnsibleError('Failed to set permissions on the temporary files Ansible needs to create when becoming an unprivileged user. For information on working around this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
|
raise AnsibleError('Failed to set permissions on the temporary files Ansible needs to create when becoming an unprivileged user. For information on working around this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
|
||||||
elif execute:
|
elif execute:
|
||||||
# Can't depend on the file being transferred with execute
|
# Can't depend on the file being transferred with execute
|
||||||
# permissions. Only need user perms because no become was
|
# permissions. Only need user perms because no become was
|
||||||
# used here
|
# used here
|
||||||
self._remote_chmod('u+x', remote_path, recursive=recursive)
|
res = self._remote_chmod('u+x', remote_path, recursive=recursive)
|
||||||
|
if res['rc'] != 0:
|
||||||
|
raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
|
||||||
|
|
||||||
return remote_path
|
return remote_path
|
||||||
|
|
||||||
|
@ -569,7 +577,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
|
||||||
# not sudoing to root, so maybe can't delete files as that other user
|
# not sudoing to root, so maybe can't delete files as that other user
|
||||||
# have to clean up temp files as original user in a second step
|
# have to clean up temp files as original user in a second step
|
||||||
cmd2 = self._connection._shell.remove(tmp, recurse=True)
|
cmd2 = self._connection._shell.remove(tmp, recurse=True)
|
||||||
self._low_level_execute_command(cmd2, sudoable=False)
|
res2 = self._low_level_execute_command(cmd2, sudoable=False)
|
||||||
|
if res2['rc'] != 0:
|
||||||
|
display.warning('Error deleting remote temporary files (rc: {0}, stderr: {1})'.format(res2['rc'], res2['stderr']))
|
||||||
|
|
||||||
try:
|
try:
|
||||||
data = json.loads(self._filter_leading_non_json_lines(res.get('stdout', u'')))
|
data = json.loads(self._filter_leading_non_json_lines(res.get('stdout', u'')))
|
||||||
|
|
|
@ -54,23 +54,29 @@ class ShellBase(object):
|
||||||
|
|
||||||
def chmod(self, mode, path, recursive=True):
|
def chmod(self, mode, path, recursive=True):
|
||||||
path = pipes.quote(path)
|
path = pipes.quote(path)
|
||||||
cmd = ['chmod', mode, path]
|
cmd = ['chmod']
|
||||||
|
|
||||||
if recursive:
|
if recursive:
|
||||||
cmd.append('-R')
|
cmd.append('-R') # many chmods require -R before file list
|
||||||
|
|
||||||
|
cmd.extend([mode, path])
|
||||||
|
|
||||||
return ' '.join(cmd)
|
return ' '.join(cmd)
|
||||||
|
|
||||||
def chown(self, path, user, group=None, recursive=True):
|
def chown(self, path, user, group=None, recursive=True):
|
||||||
path = pipes.quote(path)
|
path = pipes.quote(path)
|
||||||
user = pipes.quote(user)
|
user = pipes.quote(user)
|
||||||
|
|
||||||
if group is None:
|
cmd = ['chown']
|
||||||
cmd = ['chown', user, path]
|
|
||||||
else:
|
|
||||||
group = pipes.quote(group)
|
|
||||||
cmd = ['chown', '%s:%s' % (user, group), path]
|
|
||||||
|
|
||||||
if recursive:
|
if recursive:
|
||||||
cmd.append('-R')
|
cmd.append('-R') # many chowns require -R before file list
|
||||||
|
|
||||||
|
if group is None:
|
||||||
|
cmd.extend([user, path])
|
||||||
|
else:
|
||||||
|
group = pipes.quote(group)
|
||||||
|
cmd.extend(['%s:%s' % (user, group), path])
|
||||||
|
|
||||||
return ' '.join(cmd)
|
return ' '.join(cmd)
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue