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_package: cannot use env-vars (%TEMP%) in path #12

Open
jborean93 opened this issue Mar 11, 2020 · 15 comments
Open

win_package: cannot use env-vars (%TEMP%) in path #12

jborean93 opened this issue Mar 11, 2020 · 15 comments

Comments

@jborean93
Copy link
Collaborator

From @olenm on Apr 17, 2018 22:12

ISSUE TYPE
  • Bug Report
COMPONENT NAME

win_package

ANSIBLE VERSION
ansible 2.4.3.0
  config file = /Users/HOME/pwd/ansible.cfg
  configured module search path = ['/Users/HOME/pwd/base/library']
  ansible python module location = /usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.6.4 (default, Jan  6 2018, 11:51:15) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]

and

ansible 2.5.0
  config file = /Users/HOME/pwd/ansible.cfg
  configured module search path = ['/Users/HOME/pwd/base/library']
  ansible python module location = /usr/local/Cellar/python3/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.6.4 (default, Jan  6 2018, 11:51:15) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]
CONFIGURATION
ANSIBLE_SSH_ARGS(/Users/HOME/pwd/ansible.cfg) = -F ssh.cfg
ANSIBLE_SSH_CONTROL_PATH(/Users/HOME/pwd/ansible.cfg) = ~/.ssh/mux-%r@%h:%p
DEFAULT_HASH_BEHAVIOUR(/Users/HOME/pwd/ansible.cfg) = merge
DEFAULT_HOST_LIST(/Users/HOME/pwd/ansible.cfg) = ['/Users/HOME/pwd/base/inventory']
DEFAULT_MODULE_PATH(/Users/HOME/pwd/ansible.cfg) = ['/Users/HOME/pwd/base/library']
DEFAULT_ROLES_PATH(/Users/HOME/pwd/ansible.cfg) = ['/Users/HOME/pwd/base/roles']
OS / ENVIRONMENT

running ansible-playbook from a mac, remote system is windows server 2016 core

SUMMARY

running win_package with env variables fails (tested with %TEMP%)

STEPS TO REPRODUCE
# vars
msi_path: "%TEMP%\\bla.msi"

# tasks
- win_package:
    path: "{{msi_path}}"
    state: present
EXPECTED RESULTS

expected it to dereference the env-variable and run without erroring

ACTUAL RESULTS
fatal: [18.208.8.19]: FAILED! => {
    "changed": false,
    "msg": "the file at the local path %TEMP%\\bla.msi cannot be reached",
    "reboot_required": false,
    "restart_required": false
}

Copied from original issue: ansible/ansible#38917

@jborean93
Copy link
Collaborator Author

From @jhawkesworth on Apr 18, 2018 08:49

I don't think it can work like this as the win_package path parameter can take a url or a windows path, so you could get undesirable results if you expanded environment variable names in urls.

If you need to use %TEMP% dir you can retrieve it from facts. Like this

$ cat test.yml
---

- hosts: windowsbox
  gather_facts: true
  vars:
    msi_path: '{{ansible_env.TEMP}}\bla.msi'
  tasks:
   - name: show path
     debug:
       var: msi_path


@jborean93
Copy link
Collaborator Author

From @jborean93 on Apr 18, 2018 09:09

@jhawkesworth it could still be doable, we would just need to verify if the value for path is an actual path and then expand if it is otherwise keep it the same as before.

@jborean93
Copy link
Collaborator Author

From @jhawkesworth on Apr 18, 2018 09:48

@jborean93 yep I think we talked about this before and were reluctant to have a 'path_or_url' type, so its still a 'str' in the module parameters.
I wonder if it would be better to move over time to having 2 mutually-exclusive path and url parameters, rather than trying to get path to handle both things. Less intuitive I suppose.

@jborean93
Copy link
Collaborator Author

From @olenm on Apr 19, 2018 17:49

I can argue either way to have it path and url or simply have path handle both url's and file-paths. Should be easy to see if a protocol is defined in the string of path and deduce if it's a URL - then parse it accordingly (including basic auth-fun), and if not use path and pull out %var% blobs. - And hope nobody uses %vars% in their URL paths.

Thanks! I will attempt the ansible_env.TEMP instead of the windows-env-var syntax; especially since powershell MS is making the move to powershell and using ${env:bla} (makes me wonder how long until that is supported in CMD and other windows-areas).

Edit: I would suggest maybe add to the win_package docs that %vars% are currently not supported? I was using %vars% elsewhere and assumed it should work here too.

@jborean93
Copy link
Collaborator Author

From @olenm on Apr 19, 2018 20:29

Well I tried {{ansible_env.TEMP}} and it failed on another part - actually it hung forever (maybe a different issue completely - likely due to "arguments": https://app.datadoghq.com/account/settings#agent/windows

I'll tinker with it later and may file another bug - currently 'fix' is to keep using win_msi for the time being.

@jborean93
Copy link
Collaborator Author

From @trondhindenes on Apr 22, 2018 08:29

Powershell doesn't have the concept of %VAR%, that's a cmd thing. IMHO this is not a bug.

@jborean93
Copy link
Collaborator Author

From @jborean93 on Apr 23, 2018 08:13

@trondhindenes, we already have this ability in some other modules to automatically expand environment variables enclosed in %. While I agree it isn't a PowerShell thing it's probably something we want to add here to expand path like values.

@jborean93
Copy link
Collaborator Author

From @jhawkesworth on Apr 24, 2018 07:02

Yeah, perhaps we didn't do a good thing allowing the DOS-style %VAR% syntax for expansion, but I guess it is understood by many.

I wonder if perhaps we should/could/already do allow ${env:var_name} syntax and perhaps move over to in future in examples, just because we shouldn't expect playbook readers to understand DOS-isms.

@olenm - feel free to create a documentation pull request regarding explaining that you can't use path expansion currently.

I guess if we could parse out the module parameter types this could be automatically added to the documentation pages and then it would be there in the module docs.

I wonder if having a path_or_url module parameter type might be useful elsewhere, or whether it would be better to add it just to this module (probably the latter until some other places where it is useful are discovered).

@jborean93
Copy link
Collaborator Author

From @jborean93 on Apr 24, 2018 07:15

@jhawkesworth the good thing about dos style expansion is that we can easily expand it safely with [System.Environment]::ExpandEnvironmentVariables whereas the latter would require us to do Invoke-Expression which is a bit more insecure. In the end this should be a simple fix, just run https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.Legacy.psm1#L186-L198 to determine whether the expanded value is a path. If it is use that value, if it isn't use the unexpected value.

The trouble we have is that win_package allows you to specify the path as a URL whereas we probably should have made that path only and get people to use win_get_url but hindsight is 20/20.

@jborean93
Copy link
Collaborator Author

From @jhawkesworth on Apr 24, 2018 16:13

@jborean93 good point, I was thinking more of playbook readability than safety-of-implementation.

@jborean93
Copy link
Collaborator Author

From @dagwieers on Apr 26, 2018 10:27

There is one problem in the case where the intent is to store a value with the %VAR% preserved. One such case is win_regedit where the automatic expansion is usually unwanted. So the main question for every supplied entry containing %VAR% is whether we are the "consumer" of that data, or we are only "processing" it for someone else to consume (as-is).

The behaviour may depend on this, but should also be clearly communicated to the user in the module documentation. Does the module allow for variable expansion or not (or conditionally).

@jborean93
Copy link
Collaborator Author

From @shorbachuk on Feb 03, 2019 18:21

Any update or workaround on this? I have a use case and I would like to do something like this:

- name: Install the Visual C thingy
  win_package:
    path: %windir%\syswow64\cscript.exe 
    product_id: '{CF2BEA3C-26EA-32F8-AA9B-331F7E34BA97}'
    chdir: C:\Temp\Install
    arguments: InstallWrapper.vbs

@jborean93
Copy link
Collaborator Author

From @shorbachuk on Feb 03, 2019 18:59

I would also add that I am fine with the ${env:var_name} syntax. I've tried the syntax below and it doesn't work:
- name: Install the Visual C thingy win_package: path: '${env:windir}\syswow64\cscript.exe' product_id: '{CF2BEA3C-26EA-32F8-AA9B-331F7E34BA97}' chdir: C:\Temp\Install arguments: InstallWrapper.vbs

@jborean93
Copy link
Collaborator Author

From @ShachafGoldstein on Jun 04, 2019 18:01

@jborean93 @dagwieers

Why not change it here to expand the vars? - It won't break other use cases that need unexpanded vars and solve it on a larger scale and future uses

https://github.com/ansible/ansible/blob/3fadf4a1cbfe64ad1827248c0e9b73a2b38093cf/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.CommandUtil.psm1#L46-L53

@tobiicerb
Copy link

Still an issue. By the way win_get_url handles it correctly.

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

No branches or pull requests

2 participants