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

win_command no longer works for executables in non-standard paths #403

Closed
nre-ableton opened this issue Aug 24, 2022 · 7 comments · Fixed by #404
Closed

win_command no longer works for executables in non-standard paths #403

nre-ableton opened this issue Aug 24, 2022 · 7 comments · Fixed by #404

Comments

@nre-ableton
Copy link

SUMMARY

The changes made in 83cd290 (released in version 1.11.0) have broken the win_command module so that it doesn't work with executables in non-standard paths, even if these paths are part of the global Windows PATH.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

win_command

ANSIBLE VERSION
ansible [core 2.13.1]
  config file = /home/nre/Code/<redacted>/ansible.cfg
  configured module search path = ['/home/nre/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/nre/.cache/virtualenv/<redacted>/lib/python3.10/site-packages/ansible
  ansible collection location = /home/nre/Code/<redacted>/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/nre/.cache/virtualenv/<redacted>/bin/ansible
  python version = 3.10.4 (main, Jun 29 2022, 12:14:53) [GCC 11.2.0]
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
# /home/nre/Code/<redacted>/.ansible/collections/ansible_collections
Collection      Version
--------------- -------
ansible.windows 1.11.0 
CONFIGURATION
ANSIBLE_NOCOWS(/home/nre/Code/<redacted>/ansible.cfg) = True
COLLECTIONS_PATHS(/home/nre/Code/<redacted>/ansible.cfg) = ['/home/nre/Code/<redacted>/.ansible/collections', '/usr/share/ansible/collections']
DEFAULT_FORKS(/home/nre/Code/<redacted>/ansible.cfg) = 50
DEFAULT_HOST_LIST(/home/nre/Code/<redacted>/ansible.cfg) = ['/home/nre/Code/<redacted>/ansible/playbooks/inventory']
DEFAULT_ROLES_PATH(/home/nre/Code/<redacted>/ansible.cfg) = ['/home/nre/Code/<redacted>/ansible/roles', '/home/nre/Code/<redacted>/.ansible/external_roles']
DEFAULT_VAULT_PASSWORD_FILE(/home/nre/Code/<redacted>/ansible.cfg) = /home/nre/Code/<redacted>/ansible/playbooks/vault/vault-password
HOST_KEY_CHECKING(/home/nre/Code/<redacted>/ansible.cfg) = False
HOST_PATTERN_MISMATCH(/home/nre/Code/<redacted>/ansible.cfg) = error
INVENTORY_ENABLED(/home/nre/Code/<redacted>/ansible.cfg) = ['yaml']
RETRY_FILES_ENABLED(/home/nre/Code/<redacted>/ansible.cfg) = False
OS / ENVIRONMENT

Pop!_OS 22.04 LTS

STEPS TO REPRODUCE
---
- name: Run test task
  hosts: example_win_host

  tasks:
    - name: Test git version
      win_command: "git --version"
EXPECTED RESULTS

To run the command successfully.

ACTUAL RESULTS

The command fails with an error:

fatal: [example_win_host]: FAILED! => {"changed": false, "cmd": "git --version", "msg": "Failed to run: 'git --version': Exception calling \"CreateProcess\" with \"7\" argument(s): \"CreateProcessW() failed (The filename, directory name, or volume label syntax is incorrect, Win32ErrorCode 123 - 0x0000007B)\"", "rc": 123}

As a workaround, this can be fixed with:

- name: Test git version
  win_command: "git --version"
  environment:
    PATH: "C:\\Program Files\\git\\cmd"

However, this is impractical to do with large playbooks with many instances of win_command that invoke various commands. Also, the git installer adds this path to the global Windows PATH, which is presumably how such win_command tasks worked on versions previous to 1.11.0.

In 83cd290, win_command also was changed to support cmd and argv options. Unfortunately, they also do not work. When using the cmd syntax, the error is the same as above. When using argv, there is a different error:

- name: Test git version
  win_command:
    argv:
      - "git"
      - "--version"

...which results in:

fatal: [example_win_host]: FAILED! => {"changed": false, "cmd": "\"C:\\Program Files\\Git\\cmd\\git.exe\" \"C:\\Program Files\\Git\\mingw64\\bin\\git.exe\" --version", "msg": "Failed to run: '\"C:\\Program Files\\Git\\cmd\\git.exe\" \"C:\\Program Files\\Git\\mingw64\\bin\\git.exe\" --version': Cannot process argument transformation on parameter 'FilePath'. Cannot convert value to type System.String.", "rc": 2}

This might be caused by a separate bug in Resolve-ExecutablePath, which returns all paths concatenated if an executable was found in more than one location. This similar to the behavior of where, but this module should act more like which and just return the first result only.

FYI @jborean93

@jborean93
Copy link
Collaborator

Looks like you are correct and the problem is Resolve-ExecutablePath is returning multiple locations rather than the first found. Will have a fix soon.

@jborean93
Copy link
Collaborator

#404 should fix this problem for you.

@nre-ableton
Copy link
Author

@jborean93 Great, thanks. However I don't think that PR fixes this issue, I believe it will still be broken for the raw arguments and cmd cases. I'll try out your branch tomorrow and confirm, though.

@jborean93
Copy link
Collaborator

jborean93 commented Aug 24, 2022

It would be great if you could test it out but it should work. It did locally and I added a test for _raw_params/cmd and argv scenarios https://github.com/ansible-collections/ansible.windows/pull/404/files#diff-b9f342319c5346aa898adb4d2f57c14beb52059a97b8ea66a0e39f5ca82fbe90R365-R369.

The code for _raw_params/cmd split the first element in the value and try and resolve that in the PATH entries. The argv argument essentially allows you to skip that test and be more explicit as to what the exe is without having to quote it.

@nre-ableton
Copy link
Author

@jborean93 Yes, you are correct. I tested all three argument forms of win_command with your branch, and I'm happy to report that they all work. 🎉 Looking forward to #404 being merged and a new release being made!

@jborean93
Copy link
Collaborator

Thanks for confirming, hoping to push out a new release in the coming days.

@nre-ableton
Copy link
Author

Great, thanks so much for the quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants