From a2c93f5e9991d07cf7a470d2a245aaea3ff27fbc Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 8 Oct 2021 07:56:34 +0200 Subject: [PATCH] zypper_repository: Improve .repo file idempotency (#3474) (#3528) * If repo option points to .repo file, download for later parsing * Parse downloaded .repo file content (ini format) * Validate downloaded file, map values to repodata, workaround to ignore old .repo related code * Integration Test adjusted to install python package 'requests' first * Revert "Integration Test adjusted to install python package 'requests' first" This reverts commit 0d18352c2238d098831ba6d59b66e731fa8f0cd9. Not allowed to introduce new dependencies at this point, module_utils usage required * Remove python 'requests' dependency, using 'fetch_url' and 'to_text' from 'ansible.module_utils' instead * Prefer alias (name) if given instead repo (url) * If gpgkey was given in .repo file ensure key get automatically imported * ConfigParser Import made Python2 compatible * New .repo code moved below existing run-time parameters checks to keep previous logic * Obsolete workaround removed * two pylint/pep8 errors fixed * name added to autorefresh assert * Missing assert for 'Delete test repo' added * name added to priority option assert * name added to check repo is updated by url assert * name added to check repo is updated by name assert * name added to check add a repo by releasever assert * name added to check remove added repo assert * name added to check add a repo by basearch assert * name added to check remove added repo #2 assert * Bugfix to avoid 'KeyError' Exception in if statements * Refactoring of configparser related code, usage of module_utils, py2 compatibility * Removal of some leftover from earlier testing * Integration tests for add/remove repositories by url to .repo file added * Additional name added to list of test repos that has to be removed * Test added to verify cleanup of local .repo file after removal via zypper * Changelog fragment related to PR #3474 added * yamllint error resolved * Refactoring to reduce indentation and removal of else statements * Integration tests added for loading .repo file from local path * Test .repo file added * Dependency to setup_remote_tmp_dir added * New entry added to 'remove repositories added during test' * Support for .repo file from local path * Changelog: Ref to https://github.com/ansible-collections/community.general/issues/3466 added Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein (cherry picked from commit f0fcb221cd3d279f45bb1b463eb86ed9041324f1) Co-authored-by: Dominik Wombacher --- ...pository_improve_repo_file_idempotency.yml | 7 ++ .../modules/packaging/os/zypper_repository.py | 60 +++++++++- .../files/systemsmanagement_Uyuni_Utils.repo | 7 ++ .../targets/zypper_repository/meta/main.yml | 2 + .../targets/zypper_repository/tasks/test.yml | 2 + .../tasks/zypper_repository.yml | 106 ++++++++++++++++-- 6 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/3474-zypper_repository_improve_repo_file_idempotency.yml create mode 100644 tests/integration/targets/zypper_repository/files/systemsmanagement_Uyuni_Utils.repo create mode 100644 tests/integration/targets/zypper_repository/meta/main.yml diff --git a/changelogs/fragments/3474-zypper_repository_improve_repo_file_idempotency.yml b/changelogs/fragments/3474-zypper_repository_improve_repo_file_idempotency.yml new file mode 100644 index 0000000000..4f3b56329c --- /dev/null +++ b/changelogs/fragments/3474-zypper_repository_improve_repo_file_idempotency.yml @@ -0,0 +1,7 @@ +bugfixes: + - zypper_repository - when an URL to a .repo file was provided in option + ``repo=`` and ``state=present`` only the first run was successful, + future runs failed due to missing checks prior starting zypper. + Usage of ``state=absent`` in combination with a .repo file was not + working either (https://github.com/ansible-collections/community.general/issues/1791, + https://github.com/ansible-collections/community.general/issues/3466). diff --git a/plugins/modules/packaging/os/zypper_repository.py b/plugins/modules/packaging/os/zypper_repository.py index 38aeab618e..a29650a17e 100644 --- a/plugins/modules/packaging/os/zypper_repository.py +++ b/plugins/modules/packaging/os/zypper_repository.py @@ -137,6 +137,10 @@ from distutils.version import LooseVersion from ansible.module_utils.basic import AnsibleModule, missing_required_lib +from ansible.module_utils.urls import fetch_url +from ansible.module_utils.common.text.converters import to_text +from ansible.module_utils.six.moves import configparser, StringIO +from io import open REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck'] @@ -382,12 +386,62 @@ def main(): if not alias and state == "present": module.fail_json(msg='Name required when adding non-repo files.') + # Download / Open and parse .repo file to ensure idempotency + if repo and repo.endswith('.repo'): + if repo.startswith(('http://', 'https://')): + response, info = fetch_url(module=module, url=repo, force=True) + if not response or info['status'] != 200: + module.fail_json(msg='Error downloading .repo file from provided URL') + repofile_text = to_text(response.read(), errors='surrogate_or_strict') + else: + try: + with open(repo, encoding='utf-8') as file: + repofile_text = file.read() + except IOError: + module.fail_json(msg='Error opening .repo file from provided path') + + repofile = configparser.ConfigParser() + try: + repofile.readfp(StringIO(repofile_text)) + except configparser.Error: + module.fail_json(msg='Invalid format, .repo file could not be parsed') + + # No support for .repo file with zero or more than one repository + if len(repofile.sections()) != 1: + err = "Invalid format, .repo file contains %s repositories, expected 1" % len(repofile.sections()) + module.fail_json(msg=err) + + section = repofile.sections()[0] + repofile_items = dict(repofile.items(section)) + # Only proceed if at least baseurl is available + if 'baseurl' not in repofile_items: + module.fail_json(msg='No baseurl found in .repo file') + + # Set alias (name) and url based on values from .repo file + alias = section + repodata['alias'] = section + repodata['url'] = repofile_items['baseurl'] + + # If gpgkey is part of the .repo file, auto import key + if 'gpgkey' in repofile_items: + auto_import_keys = True + + # Map additional values, if available + if 'name' in repofile_items: + repodata['name'] = repofile_items['name'] + if 'enabled' in repofile_items: + repodata['enabled'] = repofile_items['enabled'] + if 'autorefresh' in repofile_items: + repodata['autorefresh'] = repofile_items['autorefresh'] + if 'gpgcheck' in repofile_items: + repodata['gpgcheck'] = repofile_items['gpgcheck'] + exists, mod, old_repos = repo_exists(module, repodata, overwrite_multiple) - if repo: - shortname = repo - else: + if alias: shortname = alias + else: + shortname = repo if state == 'present': if exists and not mod: diff --git a/tests/integration/targets/zypper_repository/files/systemsmanagement_Uyuni_Utils.repo b/tests/integration/targets/zypper_repository/files/systemsmanagement_Uyuni_Utils.repo new file mode 100644 index 0000000000..1df76802a7 --- /dev/null +++ b/tests/integration/targets/zypper_repository/files/systemsmanagement_Uyuni_Utils.repo @@ -0,0 +1,7 @@ +[systemsmanagement_Uyuni_Utils] +name=Several utilities to develop, build or release Uyuni (openSUSE_Leap_15.3) +type=rpm-md +baseurl=https://download.opensuse.org/repositories/systemsmanagement:/Uyuni:/Utils/openSUSE_Leap_15.3/ +gpgcheck=1 +gpgkey=https://download.opensuse.org/repositories/systemsmanagement:/Uyuni:/Utils/openSUSE_Leap_15.3/repodata/repomd.xml.key +enabled=1 diff --git a/tests/integration/targets/zypper_repository/meta/main.yml b/tests/integration/targets/zypper_repository/meta/main.yml new file mode 100644 index 0000000000..1810d4bec9 --- /dev/null +++ b/tests/integration/targets/zypper_repository/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - setup_remote_tmp_dir diff --git a/tests/integration/targets/zypper_repository/tasks/test.yml b/tests/integration/targets/zypper_repository/tasks/test.yml index e2b2f8473b..1033ee1e7d 100644 --- a/tests/integration/targets/zypper_repository/tasks/test.yml +++ b/tests/integration/targets/zypper_repository/tasks/test.yml @@ -19,6 +19,8 @@ - testrefresh - testprio - Apache_PHP_Modules + - systemsmanagement_Uyuni_Stable + - systemsmanagement_Uyuni_Utils - name: collect repo configuration after test shell: "grep . /etc/zypp/repos.d/*" diff --git a/tests/integration/targets/zypper_repository/tasks/zypper_repository.yml b/tests/integration/targets/zypper_repository/tasks/zypper_repository.yml index 4490ddca7d..dbd9bb0064 100644 --- a/tests/integration/targets/zypper_repository/tasks/zypper_repository.yml +++ b/tests/integration/targets/zypper_repository/tasks/zypper_repository.yml @@ -4,6 +4,11 @@ state: absent register: zypper_result +- name: verify no change on test repo deletion + assert: + that: + - "not zypper_result.changed" + - name: Add test repo community.general.zypper_repository: name: test @@ -51,7 +56,8 @@ command: zypper -x lr testrefresh register: zypper_result -- assert: +- name: verify autorefresh option set properly + assert: that: - '"autorefresh=\"0\"" in zypper_result.stdout' @@ -66,7 +72,8 @@ command: zypper -x lr testprio register: zypper_result -- assert: +- name: verify priority option set properly + assert: that: - '"priority=\"55\"" in zypper_result.stdout' @@ -88,7 +95,8 @@ command: zypper lr chrome2 register: zypper_result2 -- assert: +- name: ensure same url cause update of existing repo even if name differ + assert: that: - "zypper_result1.rc != 0" - "'not found' in zypper_result1.stderr" @@ -108,7 +116,8 @@ command: zypper lr samename register: zypper_result -- assert: +- name: ensure url get updated on repo with same name + assert: that: - "'/science/' not in zypper_result.stdout" - "'/devel:/languages:/ruby/' in zypper_result.stdout" @@ -140,7 +149,8 @@ state: present register: add_repo_again -- assert: +- name: no update in case of $releasever usage in url + assert: that: - add_repo is changed - add_repo_again is not changed @@ -151,10 +161,21 @@ state: absent register: remove_repo -- assert: +- name: verify repo was removed + assert: that: - remove_repo is changed +- name: get list of files in /etc/zypp/repos.d/ + command: ls /etc/zypp/repos.d/ + changed_when: false + register: releaseverrepo_etc_zypp_reposd + +- name: verify removal of file releaseverrepo.repo in /etc/zypp/repos.d/ + assert: + that: + - "'releaseverrepo' not in releaseverrepo_etc_zypp_reposd.stdout" + - name: add a repo by basearch community.general.zypper_repository: name: basearchrepo @@ -169,7 +190,8 @@ state: present register: add_repo_again -- assert: +- name: no update in case of $basearch usage in url + assert: that: - add_repo is changed - add_repo_again is not changed @@ -180,6 +202,74 @@ state: absent register: remove_repo -- assert: +- name: verify repo was removed + assert: that: - remove_repo is changed + +- name: add new repository via url to .repo file + community.general.zypper_repository: + repo: http://download.opensuse.org/repositories/systemsmanagement:/Uyuni:/Stable/openSUSE_Leap_{{ ansible_distribution_version }}/systemsmanagement:Uyuni:Stable.repo + state: present + register: added_by_repo_file + +- name: get repository details from zypper + command: zypper lr systemsmanagement_Uyuni_Stable + register: get_repository_details_from_zypper + +- name: verify adding via .repo file was successful + assert: + that: + - "added_by_repo_file is changed" + - "get_repository_details_from_zypper.rc == 0" + - "'/systemsmanagement:/Uyuni:/Stable/' in get_repository_details_from_zypper.stdout" + +- name: add same repository via url to .repo file again to verify idempotency + community.general.zypper_repository: + repo: http://download.opensuse.org/repositories/systemsmanagement:/Uyuni:/Stable/openSUSE_Leap_{{ ansible_distribution_version }}/systemsmanagement:Uyuni:Stable.repo + state: present + register: added_again_by_repo_file + +- name: verify nothing was changed adding a repo with the same .repo file + assert: + that: + - added_again_by_repo_file is not changed + +- name: remove repository via url to .repo file + community.general.zypper_repository: + repo: http://download.opensuse.org/repositories/systemsmanagement:/Uyuni:/Stable/openSUSE_Leap_{{ ansible_distribution_version }}/systemsmanagement:Uyuni:Stable.repo + state: absent + register: removed_by_repo_file + +- name: get list of files in /etc/zypp/repos.d/ + command: ls /etc/zypp/repos.d/ + changed_when: false + register: etc_zypp_reposd + +- name: verify removal via .repo file was successful, including cleanup of local .repo file in /etc/zypp/repos.d/ + assert: + that: + - "removed_by_repo_file" + - "'/systemsmanagement:/Uyuni:/Stable/' not in etc_zypp_reposd.stdout" + +- name: Copy test .repo file + copy: + src: 'files/systemsmanagement_Uyuni_Utils.repo' + dest: '{{ remote_tmp_dir }}' + +- name: add new repository via local path to .repo file + community.general.zypper_repository: + repo: "{{ remote_tmp_dir }}/systemsmanagement_Uyuni_Utils.repo" + state: present + register: added_by_repo_local_file + +- name: get repository details for systemsmanagement_Uyuni_Utils from zypper + command: zypper lr systemsmanagement_Uyuni_Utils + register: get_repository_details_from_zypper_for_systemsmanagement_Uyuni_Utils + +- name: verify adding repository via local .repo file was successful + assert: + that: + - "added_by_repo_local_file is changed" + - "get_repository_details_from_zypper_for_systemsmanagement_Uyuni_Utils.rc == 0" + - "'/systemsmanagement:/Uyuni:/Utils/' in get_repository_details_from_zypper_for_systemsmanagement_Uyuni_Utils.stdout"