Zypper repository rewrite (#1990)

* Remove support for ancient zypper versions

Even SLES11 has zypper 1.x.

* zypper_repository: don't silently ignore repo changes

So far when a repo URL changes this got silently ignored (leading to
incorrect package installations) due to this code:

    elif 'already exists. Please use another alias' in stderr:
        changed = False

Removing this reveals that we correctly detect that a repo definition
has changes (via repo_subset) but don't indicate this as change but as a
nonexistent repo. This makes us currenlty bail out silently in the above
statement.

To fix this distinguish between non existent and modified repos and
remove the repo first in case of modifications (since there is no force
option in zypper to overwrite it and 'zypper mr' uses different
arguments).

To do this we have to identify a repo by name, alias or url.

* Don't fail on empty values

This unbreaks deleting repositories

* refactor zypper_repository module

* add properties enabled and priority
* allow changing of one property and correctly report changed
* allow overwrite of multiple repositories by alias and URL
* cleanup of unused code and more structuring

* respect enabled option

* make zypper_repository conform to python2.4

* allow repo deletion only by alias

* check for non-existant url field and use alias instead

* remove empty notes and aliases

* add version_added for priority and overwrite_multiple

* add version requirement on zypper and distribution

* zypper 1.0 is enough and exists

* make suse versions note, not requirement

based on comment by @alxgu

lib/ansible/modules/extras/packaging/os/zypper_repository.py

@@ -58,16 +58,28 @@ options:
         required: false
         default: "no"
         choices: [ "yes", "no" ]
-        aliases: []
     refresh:
         description:
             - Enable autorefresh of the repository.
         required: false
         default: "yes"
         choices: [ "yes", "no" ]
-        aliases: []
+    priority:
-notes: []
+        description:
-requirements: [ zypper ]
+            - Set priority of repository. Packages will always be installed
+              from the repository with the smallest priority number.
+        required: false
+        version_added: "2.1"
+    overwrite_multiple:
+        description:
+            - Overwrite multiple repository entries, if repositories with both name and
+              URL already exist.
+        required: false
+        default: "no"
+        choices: [ "yes", "no" ]
+        version_added: "2.1"
+requirements: 
+    - "zypper >= 1.0  # included in openSuSE >= 11.1 or SuSE Linux Enterprise Server/Desktop >= 11.0"
 '''
 
 EXAMPLES = '''
@@ -83,18 +95,10 @@ EXAMPLES = '''
 
 REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck']
 
-def zypper_version(module):
-    """Return (rc, message) tuple"""
-    cmd = ['/usr/bin/zypper', '-V']
-    rc, stdout, stderr = module.run_command(cmd, check_rc=False)
-    if rc == 0:
-        return rc, stdout
-    else:
-        return rc, stderr
-
 def _parse_repos(module):
-    """parses the output of zypper -x lr and returns a parse repo dictionary"""
+    """parses the output of zypper -x lr and return a parse repo dictionary"""
     cmd = ['/usr/bin/zypper', '-x', 'lr']
+
     from xml.dom.minidom import parseString as parseXML
     rc, stdout, stderr = module.run_command(cmd, check_rc=False)
     if rc == 0:
@@ -120,81 +124,81 @@ def _parse_repos(module):
             d['stdout'] = stdout
         module.fail_json(msg='Failed to execute "%s"' % " ".join(cmd), **d)
 
-def _parse_repos_old(module):
+def _repo_changes(realrepo, repocmp):
-    """parses the output of zypper sl and returns a parse repo dictionary"""
+    for k in repocmp:
-    cmd = ['/usr/bin/zypper', 'sl']
+        if repocmp[k] and k not in realrepo:
-    repos = []
-    rc, stdout, stderr = module.run_command(cmd, check_rc=True)
-    for line in stdout.split('\n'):
-        matched = re.search(r'\d+\s+\|\s+(?P<enabled>\w+)\s+\|\s+(?P<autorefresh>\w+)\s+\|\s+(?P<type>\w+)\s+\|\s+(?P<name>\w+)\s+\|\s+(?P<url>.*)', line)
-        if matched == None:
-            continue
-
-        m = matched.groupdict()
-        m['alias']= m['name']
-        m['priority'] = 100
-        m['gpgcheck'] = 1
-        repos.append(m)
-
-    return repos
-
-def repo_exists(module, old_zypper, **kwargs):
-
-    def repo_subset(realrepo, repocmp):
-        for k in repocmp:
-            if k not in realrepo:
-                return False
-
-        for k, v in realrepo.items():
-            if k in repocmp:
-                if v.rstrip("/") != repocmp[k].rstrip("/"):
-                    return False
-        return True
-
-    if old_zypper:
-        repos = _parse_repos_old(module)
-    else:
-        repos = _parse_repos(module)
-
-    for repo in repos:
-        if repo_subset(repo, kwargs):
             return True
+
+    for k, v in realrepo.items():
+        if k in repocmp and repocmp[k]:
+            valold = str(repocmp[k] or "")
+            valnew = v or ""
+            if k == "url":
+                valold, valnew = valold.rstrip("/"), valnew.rstrip("/")
+            if valold != valnew:
+                return True
     return False
 
+def repo_exists(module, repodata, overwrite_multiple):
+    existing_repos = _parse_repos(module)
 
-def add_repo(module, repo, alias, description, disable_gpg_check, old_zypper, refresh):
+    # look for repos that have matching alias or url to the one searched
-    if old_zypper:
+    repos = []
-        cmd = ['/usr/bin/zypper', 'sa']
+    for kw in ['alias', 'url']:
+        name = repodata[kw]
+        for oldr in existing_repos:
+            if repodata[kw] == oldr[kw] and oldr not in repos:
+                repos.append(oldr)
+
+    if len(repos) == 0:
+        # Repo does not exist yet
+        return (False, False, None)
+    elif len(repos) == 1:
+        # Found an existing repo, look for changes
+        has_changes = _repo_changes(repos[0], repodata)
+        return (True, has_changes, repos)
+    elif len(repos) == 2 and overwrite_multiple:
+        # Found two repos and want to overwrite_multiple
+        return (True, True, repos)
     else:
-        cmd = ['/usr/bin/zypper', 'ar', '--check']
+        # either more than 2 repos (shouldn't happen)
+        # or overwrite_multiple is not active
+        module.fail_json(msg='More than one repo matched "%s": "%s"' % (name, repos))
 
-    if repo.startswith("file:/") and old_zypper:
+def modify_repo(module, repodata, old_repos):
-        cmd.extend(['-t', 'Plaindir'])
+    repo = repodata['url']
-    else:
+    cmd = ['/usr/bin/zypper', 'ar', '--check']
-        cmd.extend(['-t', 'plaindir'])
+    if repodata['name']:
+        cmd.extend(['--name', repodata['name']])
+
+    if repodata['priority']:
+        cmd.extend(['--priority', str(repodata['priority'])])
 
-    if description:
+    if repodata['enabled'] == '0':
-        cmd.extend(['--name', description])
+        cmd.append('--disable')
 
-    if disable_gpg_check and not old_zypper:
+    if repodata['gpgcheck'] == '1':
+        cmd.append('--gpgcheck')
+    else:
         cmd.append('--no-gpgcheck')
 
-    if refresh:
+    if repodata['autorefresh'] == '1':
         cmd.append('--refresh')
 
     cmd.append(repo)
 
     if not repo.endswith('.repo'):
-        cmd.append(alias)
+        cmd.append(repodata['alias'])
+
+    if old_repos is not None:
+        for oldrepo in old_repos:
+            remove_repo(module, oldrepo['url'])
 
     rc, stdout, stderr = module.run_command(cmd, check_rc=False)
     changed = rc == 0
     if rc == 0:
         changed = True
-    elif 'already exists. Please use another alias' in stderr:
-        changed = False
     else:
-        #module.fail_json(msg=stderr if stderr else stdout)
         if stderr:
             module.fail_json(msg=stderr)
         else:
@@ -203,16 +207,8 @@ def add_repo(module, repo, alias, description, disable_gpg_check, old_zypper, re
     return changed
 
 
-def remove_repo(module, repo, alias, old_zypper):
+def remove_repo(module, repo):
-
+    cmd = ['/usr/bin/zypper', 'rr', repo]
-    if old_zypper:
-        cmd = ['/usr/bin/zypper', 'sd']
-    else:
-        cmd = ['/usr/bin/zypper', 'rr']
-    if alias:
-        cmd.append(alias)
-    else:
-        cmd.append(repo)
 
     rc, stdout, stderr = module.run_command(cmd, check_rc=True)
     changed = rc == 0
@@ -237,59 +233,68 @@ def main():
             description=dict(required=False),
             disable_gpg_check = dict(required=False, default='no', type='bool'),
             refresh = dict(required=False, default='yes', type='bool'),
+            priority = dict(required=False, type='int'),
+            enabled = dict(required=False, default='yes', type='bool'),
+            overwrite_multiple = dict(required=False, default='no', type='bool'),
         ),
         supports_check_mode=False,
     )
 
     repo = module.params['repo']
+    alias = module.params['name']
     state = module.params['state']
-    name = module.params['name']
+    overwrite_multiple = module.params['overwrite_multiple']
-    description = module.params['description']
+
-    disable_gpg_check = module.params['disable_gpg_check']
+    repodata = {
-    refresh = module.params['refresh']
+        'url': repo,
+        'alias': alias,
+        'name': module.params['description'],
+        'priority': module.params['priority'],
+    }
+    # rewrite bools in the language that zypper lr -x provides for easier comparison
+    if module.params['enabled']:
+        repodata['enabled'] = '1'
+    else:
+        repodata['enabled'] = '0'
+    if module.params['disable_gpg_check']:
+        repodata['gpgcheck'] = '0'
+    else:
+        repodata['gpgcheck'] = '1'
+    if module.params['refresh']:
+        repodata['autorefresh'] = '1'
+    else:
+        repodata['autorefresh'] = '0'
 
     def exit_unchanged():
-        module.exit_json(changed=False, repo=repo, state=state, name=name)
+        module.exit_json(changed=False, repodata=repodata, state=state)
-
-    rc, out = zypper_version(module)
-    match = re.match(r'zypper\s+(\d+)\.(\d+)\.(\d+)', out)
-    if not match or  int(match.group(1)) > 0:
-        old_zypper = False
-    else:
-        old_zypper = True
 
     # Check run-time module parameters
     if state == 'present' and not repo:
         module.fail_json(msg='Module option state=present requires repo')
-    if state == 'absent' and not repo and not name:
+    if state == 'absent' and not repo and not alias:
         module.fail_json(msg='Alias or repo parameter required when state=absent')
 
     if repo and repo.endswith('.repo'):
-        if name:
+        if alias:
-            module.fail_json(msg='Incompatible option: \'name\'. Do not use name when adding repo files')
+            module.fail_json(msg='Incompatible option: \'name\'. Do not use name when adding .repo files')
     else:
-        if not name and state == "present":
+        if not alias and state == "present":
-            module.fail_json(msg='Name required when adding non-repo files:')
+            module.fail_json(msg='Name required when adding non-repo files.')
 
-    if repo and repo.endswith('.repo'):
+    exists, mod, old_repos = repo_exists(module, repodata, overwrite_multiple)
-        exists = repo_exists(module, old_zypper, url=repo, alias=name)
-    elif repo:
-        exists = repo_exists(module, old_zypper, url=repo)
-    else:
-        exists = repo_exists(module, old_zypper, alias=name)
 
     if state == 'present':
-        if exists:
+        if exists and not mod:
             exit_unchanged()
-
+        changed = modify_repo(module, repodata, old_repos)
-        changed = add_repo(module, repo, name, description, disable_gpg_check, old_zypper, refresh)
     elif state == 'absent':
         if not exists:
             exit_unchanged()
+        if not repo:
+            repo=alias
+        changed = remove_repo(module, repo)
 
-        changed = remove_repo(module, repo, name, old_zypper)
+    module.exit_json(changed=changed, repodata=repodata, state=state)
-
-    module.exit_json(changed=changed, repo=repo, state=state)
 
 # import module snippets
 from ansible.module_utils.basic import *