Merge pull request #30611 from isbm/isbm-zypper-latest

Bugfix: Zypper `pkg.latest` crash fix

salt/modules/zypper.py

@@ -597,6 +597,7 @@ def install(name=None,
             pkgs=None,
             sources=None,
             downloadonly=None,
+            version=None,
             **kwargs):
     '''
     Install the passed package(s), add refresh=True to run 'zypper refresh'
@@ -665,23 +666,20 @@ def install(name=None,
                        'new': '<new-version>'}}
     '''
     try:
-        pkg_params, pkg_type = __salt__['pkg_resource.parse_targets'](
+        pkg_params, pkg_type = __salt__['pkg_resource.parse_targets'](name, pkgs, sources, **kwargs)
-            name, pkgs, sources, **kwargs
-        )
     except MinionError as exc:
         raise CommandExecutionError(exc)
 
     if pkg_params is None or len(pkg_params) == 0:
         return {}
 
-    version_num = kwargs.get('version')
+    version_num = version
     if version_num:
         if pkgs is None and sources is None:
             # Allow "version" to work for single package target
             pkg_params = {name: version_num}
         else:
-            log.warning('\'version\' parameter will be ignored for multiple '
+            log.warning("'version' parameter will be ignored for multiple package targets")
-                        'package targets')
 
     if pkg_type == 'repository':
         targets = []
@@ -690,18 +688,13 @@ def install(name=None,
             if version_num is None:
                 targets.append(param)
             else:
-                match = re.match('^([<>])?(=)?([^<>=]+)$', version_num)
+                match = re.match(r'^([<>])?(=)?([^<>=]+)$', version_num)
                 if match:
                     gt_lt, equal, verstr = match.groups()
-                    prefix = gt_lt or ''
+                    targets.append('{0}{1}{2}'.format(param, ((gt_lt or '') + (equal or '')) or '=', verstr))
-                    prefix += equal or ''
-                    # If no prefix characters were supplied, use '='
-                    prefix = prefix or '='
-                    targets.append('{0}{1}{2}'.format(param, prefix, verstr))
                     log.debug(targets)
                 else:
-                    msg = ('Invalid version string {0!r} for package '
+                    msg = ('Invalid version string {0!r} for package {1!r}'.format(version_num, name))
-                           '{1!r}'.format(version_num, name))
                     problems.append(msg)
         if problems:
             for problem in problems:
@@ -730,17 +723,12 @@ def install(name=None,
     while targets:
         cmd = cmd_install + targets[:500]
         targets = targets[500:]
-
+        call = __salt__['cmd.run_all'](cmd, output_loglevel='trace', python_shell=False)
-        out = __salt__['cmd.run'](
+        if call['retcode'] != 0:
-            cmd,
+            raise CommandExecutionError(call['stderr'])  # Fixme: This needs a proper report mechanism.
-            output_loglevel='trace',
+        else:
-            python_shell=False
+            for line in call['stdout'].splitlines():
-        )
+                match = re.match(r"^The selected package '([^']+)'.+has lower version", line)
-        for line in out.splitlines():
-            match = re.match(
-                "^The selected package '([^']+)'.+has lower version",
-                line
-            )
                 if match:
                     downgrades.append(match.group(1))
 
@@ -751,6 +739,7 @@ def install(name=None,
         __salt__['cmd.run'](cmd, output_loglevel='trace', python_shell=False)
     __context__.pop('pkg.list_pkgs', None)
     new = list_pkgs()
+
     return salt.utils.compare_dicts(old, new)
 
 

salt/states/pkg.py

@@ -1373,8 +1373,7 @@ def latest(
     '''
     rtag = __gen_rtag()
     refresh = bool(
-        salt.utils.is_true(refresh)
+        salt.utils.is_true(refresh) or (os.path.isfile(rtag) and refresh is not False)
-        or (os.path.isfile(rtag) and refresh is not False)
     )
 
     if kwargs.get('sources'):
@@ -1391,6 +1390,14 @@ def latest(
                     'result': False,
                     'comment': 'Invalidly formatted "pkgs" parameter. See '
                                'minion log.'}
+    else:
+        if isinstance(pkgs, list) and len(pkgs) == 0:
+            return {
+                'name': name,
+                'changes': {},
+                'result': True,
+                'comment': 'No packages to install provided'
+            }
         else:
             desired_pkgs = [name]
 
@@ -1431,33 +1438,29 @@ def latest(
                 log.error(msg)
                 problems.append(msg)
             else:
-                if salt.utils.compare_versions(ver1=cur[pkg],
+                if salt.utils.compare_versions(ver1=cur[pkg], oper='!=', ver2=avail[pkg], cmp_func=cmp_func):
-                    oper='!=',
-                    ver2=avail[pkg],
-                    cmp_func=cmp_func):
                     targets[pkg] = avail[pkg]
                 else:
                     if not cur[pkg] or __salt__['portage_config.is_changed_uses'](pkg):
                         targets[pkg] = avail[pkg]
     else:
         for pkg in desired_pkgs:
-            if not avail[pkg]:
+            if pkg not in avail:
-                if not cur[pkg]:
+                if not cur.get(pkg):
                     msg = 'No information found for \'{0}\'.'.format(pkg)
                     log.error(msg)
                     problems.append(msg)
-            elif not cur[pkg] \
+            elif not cur.get(pkg) \
-                    or salt.utils.compare_versions(ver1=cur[pkg],
+                    or salt.utils.compare_versions(ver1=cur[pkg], oper='<', ver2=avail[pkg], cmp_func=cmp_func):
-                                                   oper='<',
-                                                   ver2=avail[pkg],
-                                                   cmp_func=cmp_func):
                 targets[pkg] = avail[pkg]
 
     if problems:
-        return {'name': name,
+        return {
+            'name': name,
             'changes': {},
             'result': False,
-                'comment': ' '.join(problems)}
+            'comment': ' '.join(problems)
+        }
 
     if targets:
         # Find up-to-date packages
@@ -1471,9 +1474,7 @@ def latest(
 
         if __opts__['test']:
             to_be_upgraded = ', '.join(sorted(targets))
-            comment = 'The following packages are set to be ' \
+            comment = ['The following packages are set to be installed/upgraded: {0}'.format(to_be_upgraded)]
-                      'installed/upgraded: ' \
-                      '{0}'.format(to_be_upgraded)
             if up_to_date:
                 up_to_date_nb = len(up_to_date)
                 if up_to_date_nb <= 10:
@@ -1482,19 +1483,16 @@ def latest(
                         '{0} ({1})'.format(name, cur[name])
                         for name in up_to_date_sorted
                     )
-                    comment += (
+                    comment.append('The following packages are already up-to-date: {0}'.format(up_to_date_details))
-                        ' The following packages are already '
-                        'up-to-date: {0}'
-                    ).format(up_to_date_details)
                 else:
-                    comment += ' {0} packages are already up-to-date'.format(
+                    comment.append('{0} packages are already up-to-date'.format(up_to_date_nb))
-                        up_to_date_nb
-                    )
 
-            return {'name': name,
+            return {
+                'name': name,
                 'changes': {},
                 'result': None,
-                    'comment': comment}
+                'comment': ' '.join(comment)
+            }
 
         # Build updated list of pkgs to exclude non-targeted ones
         targeted_pkgs = list(targets.keys()) if pkgs else None