Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pmrun.py: Recreate the older behavior where the entire success command was quoted #66929

Merged
merged 4 commits into from Feb 7, 2020
Merged

pmrun.py: Recreate the older behavior where the entire success command was quoted #66929

merged 4 commits into from Feb 7, 2020

Conversation

lotaris-xx
Copy link
Contributor

@lotaris-xx lotaris-xx commented Jan 30, 2020 •
edited

SUMMARY

Adds a set of quotes around the entire success command. This was the behaviour back in the 2.7.x days. Have a client using pmrun that is blocked from upgrading to current ansible by this.

The original change that removed the "extra" quotes caused a regression. I don't see how adding these back in would break anything for the few pmrun users out there.

I'm setting this as a bugfix as it is a regression for an actual user of this plugin, but if that isn't appropriate please let me know.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

pmrun.py

ADDITIONAL INFORMATION

Before Change:

SSH: EXEC sshpass -d9 ssh -vvv -C -o ControlMaster=auto -o ControlPersist=60s -o 'User="svcansible"' -o ConnectTimeout=10 -o ControlPath=/root/.ansible/cp/0e4ca94f55 -tt host.example.com '/bin/sh -c '"'"'pmrun su - root -c /bin/sh -c '"'"'"'"'"'"'"'"'echo BECOME-SUCCESS-shuzxrrmarpilrelhdqileldxrzurzoi ; /usr/bin/python /export/home/svcansible/.ansible/tmp/ansible-tmp-1580344076.66-265998120001699/AnsiballZ_ping.py'"'"'"'"'"'"'"'"' && sleep 0'"'"''

After Change:

SSH: EXEC sshpass -d9 ssh -vvv -C -o ControlMaster=auto -o ControlPersist=60s -o 'User="svcansible"' -o ConnectTimeout=10 -o ControlPath=/home/allensmi/.ansible/cp/0e4ca94f55 -tt host.example.com '/bin/sh -c '"'"'pmrun su - root -c  '"'"'"'"'"'"'"'"'/bin/sh -c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'echo BECOME-SUCCESS-wgsjpfenagisqjuuerrlvkhktnbtllmt ; /usr/bin/python /export/home/svcansible/.ansible/tmp/ansible-tmp-1580369890.14-92289213217361/AnsiballZ_ping.py'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"' && sleep 0'"'"''

2.7.x behaviour:

SSH: EXEC sshpass -d7 ssh -vvv -C -o ControlMaster=auto -o ControlPersist=60s -o User=svcansible -o ConnectTimeout=10 -o ControlPath=/root/.ansible/cp/0e4ca94f55 -tt host.example.com '/bin/sh -c '"'"'pmrun su - root -c '"'"'"'"'"'"'"'"'/bin/sh -c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'echo BECOME-SUCCESS-rwtwnphglvcmsupmuvemzcxcnzrohpfr; /usr/bin/python /export/home/svcansible/.ansible/tmp/ansible-tmp-1580343509.65-25583205775894/AnsiballZ_ping.py'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"' && sleep 0'"'"''

@lotaris-xx lotaris-xx changed the title Recreate the older behavior where the entire success command was quoted pmrun.py: Recreate the older behavior where the entire success command was quoted Jan 30, 2020
@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. needs_triage Needs a first human triage before being processed. small_patch support:community This issue/PR relates to code supported by the Ansible community. labels Jan 30, 2020
@lotaris-xx lotaris-xx closed this Jan 30, 2020
@lotaris-xx lotaris-xx reopened this Jan 30, 2020
@lotaris-xx
Copy link
Contributor Author

Switching to shlex_quote the output is now identical to what was produced under 2.7.x

@lotaris-xx lotaris-xx requested a review from sivel January 30, 2020 08:44
samdoran
samdoran reviewed Feb 6, 2020
View reviewed changes
Copy link
Contributor

@samdoran samdoran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good. Just needs a changelog fragment. See this fragment as an example.

@ansibot
Copy link
Contributor

ansibot commented Feb 7, 2020

@lotaris-xx this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot ansibot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed community_review In order to be merged, this PR must follow the community review workflow. labels Feb 7, 2020
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Feb 7, 2020
@lotaris-xx
Copy link
Contributor Author

Change looks good. Just needs a changelog fragment. See this fragment as an example.

Added.

Thanks,
-Allen

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Feb 7, 2020
@lotaris-xx lotaris-xx closed this Feb 7, 2020
@lotaris-xx lotaris-xx reopened this Feb 7, 2020
@lotaris-xx
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 7, 2020
@samdoran samdoran merged commit fd8eb77 into ansible:devel Feb 7, 2020
@samdoran
Copy link
Contributor

samdoran commented Feb 7, 2020

Please create a backport PR for this to be included in previous versions.

@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Feb 10, 2020
@lotaris-xx lotaris-xx mentioned this pull request Feb 10, 2020
Merged
nitzmahone pushed a commit that referenced this pull request Feb 10, 2020
@lotaris-xx
Backport of pmrun.py: Recreate the older behavior where the entire su…
7ace25d 
…ccess command was quoted (#67241)

* pmrun - quote success command (#66929)

* Recreate the older behavior where the entire success command was quoted

* Use shlex_quote for a correct fix of this

* Add changelog fragment

(cherry picked from commit fd8eb77)

* Backport of 66929-pmrun-quote-entire-success-command-string

* Update changelog fragment

* Delete 66929-pmrun-quote-entire-success-command-string.yml

Unclear from docs, but this is the devel fragment so removing.

* Update changelog
@ansible ansible locked and limited conversation to collaborators Mar 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samdoran samdoran samdoran approved these changes

@sivel sivel Awaiting requested review from sivel

Assignees
No one assigned
Labels
affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lotaris-xx @ansibot @samdoran @sivel