diff --git a/changelogs/fragments/macos-solaris-regression.yml b/changelogs/fragments/macos-solaris-regression.yml new file mode 100644 index 00000000000..a7c685bff6d --- /dev/null +++ b/changelogs/fragments/macos-solaris-regression.yml @@ -0,0 +1,4 @@ +bugfixes: + - >- + become - fix a regression on Solaris where chmod can return 5 which we + interpret as auth failure and stop trying become tmpdir permission fallbacks diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index ed3f3eff13b..ccb9002fd0e 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -18,7 +18,7 @@ import time from abc import ABCMeta, abstractmethod from ansible import constants as C -from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleActionSkip, AnsibleActionFail, AnsiblePluginRemovedError +from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleActionSkip, AnsibleActionFail, AnsiblePluginRemovedError, AnsibleAuthenticationFailure from ansible.executor.module_common import modify_module from ansible.executor.interpreter_discovery import discover_interpreter, InterpreterDiscoveryRequiredError from ansible.module_utils.common._collections_compat import Sequence @@ -624,9 +624,20 @@ class ActionBase(with_metaclass(ABCMeta, object)): # macOS chmod's +a flag takes its own argument. As a slight hack, we # pass that argument as the first element of remote_paths. So we end # up running `chmod +a [that argument] [file 1] [file 2] ...` - res = self._remote_chmod([chmod_acl_mode] + remote_paths, '+a') - if res['rc'] == 0: - return remote_paths + try: + res = self._remote_chmod([chmod_acl_mode] + remote_paths, '+a') + except AnsibleAuthenticationFailure as e: + # Solaris-based chmod will return 5 when it sees an invalid mode, + # and +a is invalid there. Because it returns 5, which is the same + # thing sshpass returns on auth failure, our sshpass code will + # assume that auth failed. If we don't handle that case here, none + # of the other logic below will get run. This is fairly hacky and a + # corner case, but probably one that shows up pretty often in + # Solaris-based environments (and possibly others). + pass + else: + if res['rc'] == 0: + return remote_paths # Step 3e: Common group # Otherwise, we're a normal user. We failed to chown the paths to the diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 86ef3d2e95f..b31048c1dc2 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -27,7 +27,7 @@ from ansible import constants as C from units.compat import unittest from units.compat.mock import patch, MagicMock, mock_open -from ansible.errors import AnsibleError +from ansible.errors import AnsibleError, AnsibleAuthenticationFailure from ansible.module_utils.six import text_type from ansible.module_utils.six.moves import shlex_quote, builtins from ansible.module_utils._text import to_bytes @@ -332,6 +332,9 @@ class TestActionBase(unittest.TestCase): remote_paths = ['/tmp/foo/bar.txt', '/tmp/baz.txt'] remote_user = 'remoteuser1' + # Used for skipping down to common group dir. + CHMOD_ACL_FLAGS = ('+a', 'A+user:remoteuser2:r:allow') + def runWithNoExpectation(execute=False): return action_base._fixup_perms2( remote_paths, @@ -455,12 +458,27 @@ class TestActionBase(unittest.TestCase): ['remoteuser2 allow read'] + remote_paths, '+a') + # This case can cause Solaris chmod to return 5 which the ssh plugin + # treats as failure. To prevent a regression and ensure we still try the + # rest of the cases below, we mock the thrown exception here. + # This function ensures that only the macOS case (+a) throws this. + def raise_if_plus_a(definitely_not_underscore, mode): + if mode == '+a': + raise AnsibleAuthenticationFailure() + return {'rc': 0, 'stdout': '', 'stderr': ''} + + action_base._remote_chmod.side_effect = raise_if_plus_a + assertSuccess() + # Step 3e: Common group + def rc_1_if_chmod_acl(definitely_not_underscore, mode): + rc = 0 + if mode in CHMOD_ACL_FLAGS: + rc = 1 + return {'rc': rc, 'stdout': '', 'stderr': ''} + action_base._remote_chmod = MagicMock() - action_base._remote_chmod.side_effect = \ - lambda x, y: \ - dict(rc=1, stdout='', stderr='') if y == '+a' \ - else dict(rc=0, stdout='', stderr='') + action_base._remote_chmod.side_effect = rc_1_if_chmod_acl get_shell_option = action_base.get_shell_option action_base.get_shell_option = MagicMock()