-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issues with quoting #36
base: master
Are you sure you want to change the base?
Conversation
Fixes #34. There are some cases where quote stripping (needed when ansible would otherwise try to run sudo in a jail) would remove too little or too many quotes. This attempts to rectify this using shlex-based parsing (mostly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this change with a relatively complex playbook and I got this error message:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'Requested entry (plugin_type: connection plugin: sshjail setting: retries ) was not defined in configuration.'
This wasn't caused by this change though, but by 7e14429.
When renaming "reconnection_retries" back to "retries" in sshjail.py, I can execute my playbooks successfully, so I think the parser change works just fine now.
Still requesting change, so maybe the retries issue could also be resolved in here (+ address my nitpicking about trailing whitespace).
while words[0] == executable or words[0] == 'sudo': | ||
cmd = words[-1] | ||
words = shlex.split(cmd) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has trailing whitespaces
okay, so this change was due to ansible/ansible@a2239d8, which landed first in 2.11.3 I upgraded my local installation from 2.11.2 to 2.11.3 now and added an ansible minimum version requirement to my playbooks. I also opened #37 to add a check to the module, so that there is a structured error message for others running into this. |
FWIW I'm having to apply this patch for Ansible 2.12. It looks like the fix was only applied on the 2.11 branch? |
There is some confusion - the patch to sshjail (#34 or #36) is required in all cases, but while testing #36 I noticed that sshjail didn't work at all on certain versions of ansible (2.11.0, 2.11.1, and 2.11.2) due to a bug in those versions. |
Fixes #34. There are some cases where quote stripping (needed when
ansible would otherwise try to run sudo in a jail) would remove too
little or too many quotes. This attempts to rectify this using
shlex-based parsing (mostly).