Windows: state pkg.installed ignores extra_install_flags

[dafyddj]

Description of Issue/Question

When using the state pkg.installed the argument extra_install_flags is ignored.
AFAICT, the issue was introduced in 2016.11.8 and exists in all later versions (but worked as expected in 2016.11.7).

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

PS C:\Python27\Scripts> ..\python.exe salt-call --local -l debug state.single pkg.installed git "extra_install_flags=/COMPONENTS=assoc,assoc_sh"

The command executed is seen below, and doesn't contain the extra flag specified above:

[INFO    ] Executing command '"C:\Windows\system32\cmd.exe" /s /c ""c:\salt\var\cache\salt\minion\extrn_files\base\githu
b.com\git-for-windows\git\releases\download\v2.19.1.windows.1\Git-2.19.1-64-bit.exe" /VERYSILENT /NORESTART /SP- /NOCANC
EL"' in directory 'c:\salt\var\cache\salt\minion\extrn_files\base\github.com\git-for-windows\git\releases\download\v2.19
.1.windows.1'
[DEBUG   ] Using existing pkg metadata db for saltenv 'base' (age is 0:00:45.436922)
[DEBUG   ] Could not LazyLoad pkg.hold: 'pkg.hold' is not available.
[DEBUG   ] Using existing pkg metadata db for saltenv 'base' (age is 0:00:45.467922)
[INFO    ] Made the following changes:
'git' changed from 'absent' to '2.19.1'

[DEBUG   ] Refreshing modules...
[INFO    ] Loading fresh modules for state activity
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[INFO    ] Completed state [git] at time 13:03:06.233000 (duration_in_ms=94406.0)
[DEBUG   ] LazyLoaded state.check_result
[DEBUG   ] LazyLoaded highstate.output
[DEBUG   ] LazyLoaded nested.output
local:
----------
          ID: git
    Function: pkg.installed
      Result: True
     Comment: The following packages were installed/updated: git
     Started: 13:01:31.827000
    Duration: 94406.0 ms
     Changes:
              ----------
              git:
                  ----------
                  new:
                      2.19.1
                  old:

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  94.406 s

As a result, the Windows File Explorer right-button context menu contains Git related entries which shouldn't be there.

Versions Report

PS C:\Python27\Scripts> ..\python.exe salt-call --versions-report
Salt Version:
           Salt: 2018.11.0-234-ge57a110

Dependency Versions:
           cffi: 1.5.0
       cherrypy: 5.0.1
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
         Jinja2: 2.8
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.7
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (v2.7.12:d33e0cf91556, Jun 27 2016, 15:24:40) [MSC v.1500 64 bit (AMD64)]
   python-gnupg: 0.3.8
         PyYAML: 3.11
          PyZMQ: 15.2.0
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.3
            ZMQ: 4.1.2

System Versions:
           dist:
         locale: cp1252
        machine: AMD64
        release: 8.1
         system: Windows
        version: 8.1 6.3.9600  Multiprocessor Free

[damon-atkins]

Please try salt-call -l debug pkg.install git "extra_install_flags=/COMPONENTS=assoc,assoc_sh"

[dafyddj]

Yes, that works.

PS C:\Python27> .\python.exe .\Scripts\salt-call -l debug pkg.install git "extra_install_flags=/COMPONENTS=assoc,assoc_sh"
...
...
[INFO    ] Executing command '"C:\Windows\system32\cmd.exe" /s /c ""c:\salt\var\cache\salt\minion\extrn_files\base\githu
b.com\git-for-windows\git\releases\download\v2.19.1.windows.1\Git-2.19.1-64-bit.exe" /VERYSILENT /NORESTART /SP- /NOCANC
EL /COMPONENTS=assoc,assoc_sh"' in directory 'c:\salt\var\cache\salt\minion\extrn_files\base\github.com\git-for-windows\
git\releases\download\v2.19.1.windows.1'
[DEBUG   ] Using existing pkg metadata db for saltenv 'base' (age is 1:13:02.299922)
[DEBUG   ] LazyLoaded nested.output
local:
    ----------
    git:
        ----------
        new:
            2.19.1
        old:

[damon-atkins]

So the bug is in states/pkg.py

[damon-atkins]

Sorry just remember you can not call kwargs in a simple way for a state.

Important bit in yaml

    - kwargs:
        extra_install_flags: '/COMPONENTS=assoc,assoc_sh'

[Ch3LL]

yeah i dont see how that command wouldn't work, as the state pkg module just passes **kwargs.

[dafyddj]

Still not working.

PS C:\Python27> cat C:\srv\salt\test.sls
git:
  pkg.installed:
    - kwargs:
       - extra_install_flags: /COMPONENTS=assoc,assoc_sh

PS C:\Python27> .\python.exe .\Scripts\salt-call -l debug state.apply test
...
...
[INFO    ] Executing command '"C:\Windows\system32\cmd.exe" /s /c ""c:\salt\var\cache\salt\minion\extrn_files\base\githu
b.com\git-for-windows\git\releases\download\v2.19.1.windows.1\Git-2.19.1-64-bit.exe" /VERYSILENT /NORESTART /SP- /NOCANC
EL"' in directory 'c:\salt\var\cache\salt\minion\extrn_files\base\github.com\git-for-windows\git\releases\download\v2.19
.1.windows.1'

[dafyddj]

I put the blame here:

salt/salt/states/pkg.py

Lines 1524 to 1532 in 29a4d48

	# If just a name (and optionally a version) is passed, just pack them into
	# the pkgs argument.
	if name and not any((pkgs, sources)):
	if version:
	pkgs = [{name: version}]
	version = None
	else:
	pkgs = [name]

and here:

salt/salt/modules/win_pkg.py

Lines 1183 to 1192 in 2301443

	if not pkgs and len(pkg_params) == 1:
	# Only use the 'version' param if a single item was passed to the 'name'
	# parameter
	pkg_params = {
	name: {
	'version': kwargs.get('version'),
	'extra_install_flags': kwargs.get('extra_install_flags')
	}
	}

[Ch3LL]

this :

git:
  pkg.installed:
    - kwargs:
       - extra_install_flags: /COMPONENTS=assoc,assoc_sh

should be this:

    - kwargs:
        extra_install_flags: '/COMPONENTS=assoc,assoc_sh'

as @damon-atkins pointed out above. note removing that extra - by extra_install_flags

[dafyddj]

Ok, I should have said I tried various different constructions of kwargs: and none worked.
I should also point out that this problem occurs with the state pkg.latest too, for the same reason.

I've pointed out the bug above, which is the test in line 1183 of win_pkg.py.

[Ch3LL]

ahh thanks for pointing that out again @dafyddj

ping @saltstack/team-windows anyone want to tackle this one?

[plinkable]

I've just hit this issue again, and I can't spot a work-around. I'm using 2018.3.4 (py3) on Server 2016

[dafyddj]

I had meant to come back to this. I patched my win_pkg.py from 2018.3.4 with the following to resolve the issue:
(I'm not a Python dev so use at your own risk!)

*** Downloads/win_pkg.py	2019-05-09 17:16:23.000000000 +0100
--- salt/_modules/win_pkg.py	2019-05-09 01:05:03.000000000 +0100
***************
*** 1166,1176 ****
      # "sources" argument
      pkg_params = __salt__['pkg_resource.parse_targets'](name, pkgs, **kwargs)[0]
  
-     if len(pkg_params) > 1:
-         if kwargs.get('extra_install_flags') is not None:
-             log.warning('\'extra_install_flags\' argument will be ignored for '
-                         'multiple package targets')
- 
      # Windows expects an Options dictionary containing 'version'
      for pkg in pkg_params:
          pkg_params[pkg] = {'version': pkg_params[pkg]}
--- 1166,1171 ----
***************
*** 1185,1194 ****
          pkg_params = {
              name: {
                  'version': kwargs.get('version'),
-                 'extra_install_flags': kwargs.get('extra_install_flags')
              }
          }
  
      # Get a list of currently installed software for comparison at the end
      old = list_pkgs(saltenv=saltenv, refresh=refresh, versions_as_list=True)
  
--- 1180,1198 ----
          pkg_params = {
              name: {
                  'version': kwargs.get('version'),
              }
          }
  
+     if kwargs.get('extra_install_flags') is not None:
+         if len(pkg_params) > 1:
+             log.warning('\'extra_install_flags\' argument will be ignored for '
+                         'multiple package targets')
+         else:
+             for pkg in pkg_params:
+                 pkg_params[pkg]['extra_install_flags'] = \
+                         kwargs.get('extra_install_flags')
+ 
+ 
      # Get a list of currently installed software for comparison at the end
      old = list_pkgs(saltenv=saltenv, refresh=refresh, versions_as_list=True)

[cmcmarrow]

I have been able to replicate the problem and will now start working on a proper solution.