Correctly handle .repo files in zypper_repository module

Before the changes, removing a repository required a repo url. This shouldn't be required since zypper allows removing a repo based on its alias (mapped to name in this module).

The name variable was always required, which is misleading since repofiles provide their own alias. So a runtime check was added to avoid this confusion.

Additionaly, running this module on .repo files weren't idempotent. e.g

Before:
$ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo name=foo"
{"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": true}
$ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo name=foo"
{"msg": "Repository named 'devel_languages_python' already exists. Please use another alias.\n", "failed": true}

After:
$ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo"
{"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": true}
$ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo"
{"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": false}

Signed-off-by: Hector Acosta <hector.acosta@gmail.com>

packaging/zypper_repository

@@ -29,15 +29,15 @@ description:
     - Add or remove Zypper repositories on SUSE and openSUSE
 options:
     name:
-        required: true
+        required: false
         default: none
         description:
-            - A name for the repository.
+            - A name for the repository. Not required when adding repofiles.
     repo:
-        required: true
+        required: false
         default: none
         description:
-            - URI of the repository or .repo file.
+            - URI of the repository or .repo file. Required when state=present.
     state:
         required: false
         choices: [ "absent", "present" ]
@@ -68,15 +68,53 @@ EXAMPLES = '''
 
 # Remove NVIDIA repository
 - zypper_repository: name=nvidia-repo repo='ftp://download.nvidia.com/opensuse/12.2' state=absent
+
+# Add python development repository
+- zypper_repository: repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo
 '''
+from xml.dom.minidom import parseString as parseXML
+
+REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck']
 
 
-def repo_exists(module, repo):
+def _parse_repos(module):
-    """Return (rc, stdout, stderr, found) tuple"""
+    """parses the output of zypper -x lr and returns a parse repo dictionary"""
-    cmd = ['/usr/bin/zypper', 'lr', '--uri']
+    cmd = ['/usr/bin/zypper', '-x', 'lr']
+    repos = []
 
-    rc, stdout, stderr = module.run_command(cmd, check_rc=False)
+    rc, stdout, stderr = module.run_command(cmd, check_rc=True)
-    return (rc, stdout, stderr, repo in stdout)
+    dom = parseXML(stdout)
+    repo_list = dom.getElementsByTagName('repo')
+    for repo in repo_list:
+        opts = {}
+        for o in REPO_OPTS:
+            opts[o] = repo.getAttribute(o)
+        opts['url'] = repo.getElementsByTagName('url')[0].firstChild.data
+        # A repo can be uniquely identified by an alias + url
+        repos.append(opts)
+
+    return repos
+
+
+def repo_exists(module, **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 != repocmp[k]:
+                    return False
+        return True
+
+    repos = _parse_repos(module)
+
+    for repo in repos:
+        if repo_subset(repo, kwargs):
+            return True
+    return False
 
 
 def add_repo(module, repo, alias, description, disable_gpg_check):
@@ -95,15 +133,27 @@ def add_repo(module, repo, alias, description, disable_gpg_check):
 
     rc, stdout, stderr = module.run_command(cmd, check_rc=False)
     changed = rc == 0
-    return (rc, stdout, stderr, changed)
+    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)
+
+    return changed
 
 
-def remove_repo(module, repo):
+def remove_repo(module, repo, alias):
-    cmd = ['/usr/bin/zypper', 'rr', repo]
 
-    rc, stdout, stderr = module.run_command(cmd, check_rc=False)
+    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
-    return (rc, stdout, stderr, changed)
+    return changed
 
 
 def fail_if_rc_is_null(module, rc, stdout, stderr):
@@ -114,8 +164,8 @@ def fail_if_rc_is_null(module, rc, stdout, stderr):
 def main():
     module = AnsibleModule(
         argument_spec=dict(
-            name=dict(required=True),
+            name=dict(required=False),
-            repo=dict(required=True),
+            repo=dict(required=False),
             state=dict(choices=['present', 'absent'], default='present'),
             description=dict(required=False),
             disable_gpg_check = dict(required=False, default='no', type='bool'),
@@ -132,22 +182,34 @@ def main():
     def exit_unchanged():
         module.exit_json(changed=False, repo=repo, state=state, name=name)
 
-    rc, stdout, stderr, exists = repo_exists(module, repo)
+    # Check run-time module parameters
-    fail_if_rc_is_null(module, rc, stdout, stderr)
+    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:
+        module.fail_json(msg='Alias or repo parameter required when state=absent')
+
+    if repo and repo.endswith('.repo'):
+        if name:
+            module.fail_json(msg='Incompatible option: \'name\'. Do not use name when adding repo files')
+    else:
+        if not name:
+            module.fail_json(msg='Name required when adding non-repo files:')
+
+    if repo and repo.endswith('.repo'):
+        exists = repo_exists(module, url=repo, alias=name)
+    else:
+        exists = repo_exists(module, alias=name)
 
     if state == 'present':
         if exists:
             exit_unchanged()
 
-        result = add_repo(module, repo, name, description, disable_gpg_check)
+        changed = add_repo(module, repo, name, description, disable_gpg_check)
     elif state == 'absent':
         if not exists:
             exit_unchanged()
 
-        result = remove_repo(module, repo)
+        changed = remove_repo(module, repo, name)
-
-    rc, stdout, stderr, changed = result
-    fail_if_rc_is_null(module, rc, stdout, stderr)
 
     module.exit_json(changed=changed, repo=repo, state=state)