From 4eae4c1d6300f74b5dc7cf35c3e7c5b853fbaadf Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 7 Nov 2018 17:49:12 -0500 Subject: [PATCH] [stable-2.6] Prevent duplicate entries in rhsm_repository module (#48107) * Complie regular expressions for better performance * Skip on empty lines This fixes a bug where the previous repo would be inserted in the result twice since an empty line did not match any of the conditions that would exit the loop iteration. (cherry picked from commit 1e3b704ff12bd992136ec1648994ecefc7652438) Co-authored-by: Sam Doran --- ...rhsm_repository-loop-fix-improvements.yaml | 3 ++ .../modules/packaging/os/rhsm_repository.py | 53 ++++++++++--------- 2 files changed, 30 insertions(+), 26 deletions(-) create mode 100644 changelogs/rhsm_repository-loop-fix-improvements.yaml diff --git a/changelogs/rhsm_repository-loop-fix-improvements.yaml b/changelogs/rhsm_repository-loop-fix-improvements.yaml new file mode 100644 index 0000000000..3d26191396 --- /dev/null +++ b/changelogs/rhsm_repository-loop-fix-improvements.yaml @@ -0,0 +1,3 @@ +bugfixes: + - rhsm_repository - compile regular expressions to improve performance when looping over available repositories + - rhsm_repository - prevent duplicate repository entries from being entered in the final command diff --git a/lib/ansible/modules/packaging/os/rhsm_repository.py b/lib/ansible/modules/packaging/os/rhsm_repository.py index e276125436..42bce6f038 100644 --- a/lib/ansible/modules/packaging/os/rhsm_repository.py +++ b/lib/ansible/modules/packaging/os/rhsm_repository.py @@ -115,10 +115,10 @@ def get_repository_list(module, list_parameter): '+----------------------------------------------------------+', ' Available Repositories in /etc/yum.repos.d/redhat.repo' ] - repo_id_re_str = r'Repo ID: (.*)' - repo_name_re_str = r'Repo Name: (.*)' - repo_url_re_str = r'Repo URL: (.*)' - repo_enabled_re_str = r'Enabled: (.*)' + repo_id_re = re.compile(r'Repo ID:\s+(.*)') + repo_name_re = re.compile(r'Repo Name:\s+(.*)') + repo_url_re = re.compile(r'Repo URL:\s+(.*)') + repo_enabled_re = re.compile(r'Enabled:\s+(.*)') repo_id = '' repo_name = '' @@ -126,37 +126,38 @@ def get_repository_list(module, list_parameter): repo_enabled = '' repo_result = [] - - for line in out.split('\n'): - if line in skip_lines: + for line in out.splitlines(): + if line == '' or line in skip_lines: continue - repo_id_re = re.match(repo_id_re_str, line) - if repo_id_re: - repo_id = repo_id_re.group(1) + repo_id_match = repo_id_re.match(line) + if repo_id_match: + repo_id = repo_id_match.group(1) continue - repo_name_re = re.match(repo_name_re_str, line) - if repo_name_re: - repo_name = repo_name_re.group(1) + repo_name_match = repo_name_re.match(line) + if repo_name_match: + repo_name = repo_name_match.group(1) continue - repo_url_re = re.match(repo_url_re_str, line) - if repo_url_re: - repo_url = repo_url_re.group(1) + repo_url_match = repo_url_re.match(line) + if repo_url_match: + repo_url = repo_url_match.group(1) continue - repo_enabled_re = re.match(repo_enabled_re_str, line) - if repo_enabled_re: - repo_enabled = repo_enabled_re.group(1) + repo_enabled_match = repo_enabled_re.match(line) + if repo_enabled_match: + repo_enabled = repo_enabled_match.group(1) + + repo = { + "id": repo_id, + "name": repo_name, + "url": repo_url, + "enabled": True if repo_enabled == '1' else False + } - repo = { - "id": repo_id, - "name": repo_name, - "url": repo_url, - "enabled": True if repo_enabled == '1' else False - } repo_result.append(repo) + return repo_result @@ -206,7 +207,7 @@ def repository_modify(module, state, name): if not module.check_mode: rc, out, err = run_subscription_manager(module, rhsm_arguments) - results = out.split('\n') + results = out.splitlines() module.exit_json(results=results, changed=changed, repositories=updated_repo_list, diff=diff)