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.

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

The win_copy module in 2.4.0.0 fails if src contains a ":" character #31336

Closed
kinware opened this issue Oct 5, 2017 · 6 comments · Fixed by #31392
Closed

The win_copy module in 2.4.0.0 fails if src contains a ":" character #31336

kinware opened this issue Oct 5, 2017 · 6 comments · Fixed by #31392
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Milestone

Comments

@kinware
Copy link

kinware commented Oct 5, 2017

ISSUE TYPE
  • Bug Report
COMPONENT NAME

win_copy

ANSIBLE VERSION
ansible 2.4.0.0
  config file = /opt/ansible24/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/ansible24/venv/lib/python2.7/site-packages/ansible
  executable location = /opt/ansible24/bin/ansible
  python version = 2.7.9 (default, May  7 2017, 21:48:55) [GCC]
CONFIGURATION

None

OS / ENVIRONMENT

Linux control machine managing Windows hosts.

SUMMARY

We store managed Windows files on the Linux control machine using paths that contain c:, d: etc which allows simple copying. This works fine with ansible < 2.4.0.0 but stops working in 2.4.0.0. Details below.

STEPS TO REPRODUCE
mkdir -p /tmp/c:/temp
echo 42 > /tmp/c:/temp/test.txt
ansible target -m win_copy -a 'src=/tmp/c:/temp/test.txt dest=c:/temp/test.txt'
EXPECTED RESULTS

Copy should work, just like it did in ansible before 2.4.0.0.

ACTUAL RESULTS

Copy fails with an error "Get-AnsibleParam: Parameter 'src' has an invalid path '/tmp/c:/temp/test.txt' specified.".

target | FAILED! => {
    "changed": false, 
    "checksum": "34973274ccef6ab4dfaaf86599792fa9c3fe4689", 
    "dest": "c:/temp/test.txt", 
    "failed": true, 
    "msg": "Get-AnsibleParam: Parameter 'src' has an invalid path '/tmp/c:/temp/test.txt' specified.", 
    "operation": "file_copy", 
    "original_basename": "test.txt", 
    "size": 3, 
    "src": "/tmp/c:/temp/test.txt"
}
ANALYSIS

Copying files using win_copy fails if the source path (on the control host running Linux) contains a ":" character. Disabling the $src parameter validation in win_copy.ps1 (i.e removing -type 'path' from the Get-AnsibleParam call) allows the copy to work. This is our temporary workaround, but this will probably cause a number of other problems.

The root cause seems to lie in the Get-AnsibleParam function in ansible/module_utils/powershell/Ansible.ModuleUtils.Legacy.psm1. The check "Test-Path -IsValid" is where we believe it fails. For example, running "Test-Path -IsValid /tmp/c:/test.txt" in powershell returns $False.

The win_copy module has been greatly improved since 2.3.0.0. I am guessing the Test-Path -IsValid code was never reached in the previous versions, maybe because $src was left empty since it is not really needed on the remote end for a normal copy? I tried "transplanting" the 2.3.0.0 win_copy.ps1 module into 2.4.0.0, but got the same error, which is why I think there has been a change of how the parameters are supplied during the invocation of the win_copy module.

I am not quite sure of the best way to fix this in the code. We need to somehow make sure that win_copy.ps1 never tries to validate a path that actually references something on the control machine.

@ansibot
Copy link
Contributor

ansibot commented Oct 5, 2017

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community labels Oct 5, 2017
@nitzmahone nitzmahone added this to the 2.4.0 milestone Oct 6, 2017
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Oct 6, 2017
@nitzmahone
Copy link
Member

Yeah, it's an oversight that source path validation is occurring on the module side- that should only happen with remote_src (when the path is obviously a Windows path).

@jborean93
Copy link
Contributor

@kinware are you able to try out #31392, it works when I've manually tested it and once merged we can get it included in the upcoming 2.4.1 release.

@kinware
Copy link
Author

kinware commented Oct 6, 2017

@jborean93 Thanks for the quick fix! I applied the "query_args.pop('src', None)" fix to win_copy.py - and our deployments started working again. Awesome!

Leaving issue open until #31392 has been merged.

@jborean93
Copy link
Contributor

Hey @kinware, I am going to close the issue as the changes have been merged into devel and the stable-2.4 branch, this is be available in the 2.4.1 release.

@kinware
Copy link
Author

kinware commented Oct 6, 2017

Great! Again, thanks for stellar help!

@ansibot ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 7, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants