From 6b6ba4bdbd4b4c52a46bf3d0bcdbaca6b47534d1 Mon Sep 17 00:00:00 2001 From: Georg Date: Wed, 28 Jun 2023 16:39:30 +0200 Subject: [PATCH] Zypper pkgrepo alreadyconfigured (#585) * Fix zypper repository reconfiguration See https://github.com/saltstack/salt/issues/63402 for issue details. Signed-off-by: Georg Pfuetzenreuter * Functional pkgrepo tests for SUSE Signed-off-by: Georg Pfuetzenreuter * Change pkgrepo state to use f-strings Follow new styling rules. Signed-off-by: Georg Pfuetzenreuter --------- Signed-off-by: Georg Pfuetzenreuter Signed-off-by: Georg Pfuetzenreuter --- changelog/63402.fixed.md | 1 + salt/states/pkgrepo.py | 27 ++- .../functional/states/pkgrepo/test_suse.py | 219 ++++++++++++++++++ 3 files changed, 235 insertions(+), 12 deletions(-) create mode 100644 changelog/63402.fixed.md create mode 100644 tests/pytests/functional/states/pkgrepo/test_suse.py diff --git a/changelog/63402.fixed.md b/changelog/63402.fixed.md new file mode 100644 index 0000000000..c38715738a --- /dev/null +++ b/changelog/63402.fixed.md @@ -0,0 +1 @@ +Repaired zypper repositories being reconfigured without changes diff --git a/salt/states/pkgrepo.py b/salt/states/pkgrepo.py index c2d23f95bb..f041644287 100644 --- a/salt/states/pkgrepo.py +++ b/salt/states/pkgrepo.py @@ -464,7 +464,7 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): pre = __salt__["pkg.get_repo"](repo=repo, **kwargs) except CommandExecutionError as exc: ret["result"] = False - ret["comment"] = "Failed to examine repo '{}': {}".format(name, exc) + ret["comment"] = f"Failed to examine repo '{name}': {exc}" return ret # This is because of how apt-sources works. This pushes distro logic @@ -500,7 +500,10 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): else: break else: - break + if kwarg in ("comps", "key_url"): + break + else: + continue elif kwarg in ("comps", "key_url"): if sorted(sanitizedkwargs[kwarg]) != sorted(pre[kwarg]): break @@ -546,7 +549,7 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): break else: ret["result"] = True - ret["comment"] = "Package repo '{}' already configured".format(name) + ret["comment"] = f"Package repo '{name}' already configured" return ret if __opts__["test"]: @@ -581,7 +584,7 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): # This is another way to pass information back from the mod_repo # function. ret["result"] = False - ret["comment"] = "Failed to configure repo '{}': {}".format(name, exc) + ret["comment"] = f"Failed to configure repo '{name}': {exc}" return ret try: @@ -597,10 +600,10 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): ret["changes"] = {"repo": repo} ret["result"] = True - ret["comment"] = "Configured package repo '{}'".format(name) + ret["comment"] = f"Configured package repo '{name}'" except Exception as exc: # pylint: disable=broad-except ret["result"] = False - ret["comment"] = "Failed to confirm config of repo '{}': {}".format(name, exc) + ret["comment"] = f"Failed to confirm config of repo '{name}': {exc}" # Clear cache of available packages, if present, since changes to the # repositories may change the packages that are available. @@ -700,11 +703,11 @@ def absent(name, **kwargs): repo = __salt__["pkg.get_repo"](stripname, **kwargs) except CommandExecutionError as exc: ret["result"] = False - ret["comment"] = "Failed to configure repo '{}': {}".format(name, exc) + ret["comment"] = f"Failed to configure repo '{name}': {exc}" return ret if not repo: - ret["comment"] = "Package repo {} is absent".format(name) + ret["comment"] = f"Package repo {name} is absent" ret["result"] = True return ret @@ -727,7 +730,7 @@ def absent(name, **kwargs): repos = __salt__["pkg.list_repos"]() if stripname not in repos: ret["changes"]["repo"] = name - ret["comment"] = "Removed repo {}".format(name) + ret["comment"] = f"Removed repo {name}" if not remove_key: ret["result"] = True @@ -736,14 +739,14 @@ def absent(name, **kwargs): removed_keyid = __salt__["pkg.del_repo_key"](stripname, **kwargs) except (CommandExecutionError, SaltInvocationError) as exc: ret["result"] = False - ret["comment"] += ", but failed to remove key: {}".format(exc) + ret["comment"] += f", but failed to remove key: {exc}" else: ret["result"] = True ret["changes"]["keyid"] = removed_keyid - ret["comment"] += ", and keyid {}".format(removed_keyid) + ret["comment"] += f", and keyid {removed_keyid}" else: ret["result"] = False - ret["comment"] = "Failed to remove repo {}".format(name) + ret["comment"] = f"Failed to remove repo {name}" return ret diff --git a/tests/pytests/functional/states/pkgrepo/test_suse.py b/tests/pytests/functional/states/pkgrepo/test_suse.py new file mode 100644 index 0000000000..19ba928ce6 --- /dev/null +++ b/tests/pytests/functional/states/pkgrepo/test_suse.py @@ -0,0 +1,219 @@ +import pytest + +pytestmark = [ + pytest.mark.destructive_test, + pytest.mark.skip_if_not_root, +] + + +@pytest.fixture +def pkgrepo(states, grains): + if grains["os_family"] != "Suse": + raise pytest.skip.Exception( + "Test is only applicable to SUSE based operating systems", + _use_item_location=True, + ) + return states.pkgrepo + + +@pytest.fixture +def suse_state_tree(grains, pkgrepo, state_tree): + managed_sls_contents = """ + salttest: + pkgrepo.managed: + - enabled: 1 + - gpgcheck: 1 + - comments: + - '# Salt Test' + - refresh: 1 + {% if grains['osmajorrelease'] == 15 %} + - baseurl: https://download.opensuse.org/repositories/openSUSE:/Backports:/SLE-15-SP4/standard/ + - humanname: openSUSE Backports for SLE 15 SP4 + - gpgkey: https://download.opensuse.org/repositories/openSUSE:/Backports:/SLE-15-SP4/standard/repodata/repomd.xml.key + {% elif grains['osfullname'] == 'openSUSE Tumbleweed' %} + - baseurl: http://download.opensuse.org/tumbleweed/repo/oss/ + - humanname: openSUSE Tumbleweed OSS + - gpgkey: https://download.opensuse.org/tumbleweed/repo/oss/repodata/repomd.xml.key + {% endif %} + """ + + absent_sls_contents = """ + salttest: + pkgrepo: + - absent + """ + + modified_sls_contents = """ + salttest: + pkgrepo.managed: + - enabled: 1 + - gpgcheck: 1 + - comments: + - '# Salt Test (modified)' + - refresh: 1 + {% if grains['osmajorrelease'] == 15 %} + - baseurl: https://download.opensuse.org/repositories/openSUSE:/Backports:/SLE-15-SP4/standard/ + - humanname: Salt modified Backports + - gpgkey: https://download.opensuse.org/repositories/openSUSE:/Backports:/SLE-15-SP4/standard/repodata/repomd.xml.key + {% elif grains['osfullname'] == 'openSUSE Tumbleweed' %} + - baseurl: http://download.opensuse.org/tumbleweed/repo/oss/ + - humanname: Salt modified OSS + - gpgkey: https://download.opensuse.org/tumbleweed/repo/oss/repodata/repomd.xml.key + {% endif %} + """ + + managed_state_file = pytest.helpers.temp_file( + "pkgrepo/managed.sls", managed_sls_contents, state_tree + ) + absent_state_file = pytest.helpers.temp_file( + "pkgrepo/absent.sls", absent_sls_contents, state_tree + ) + modified_state_file = pytest.helpers.temp_file( + "pkgrepo/modified.sls", modified_sls_contents, state_tree + ) + + try: + with managed_state_file, absent_state_file, modified_state_file: + yield + finally: + pass + + +@pytest.mark.requires_salt_states("pkgrepo.managed", "pkgrepo.absent") +def test_pkgrepo_managed_absent(grains, modules, subtests, suse_state_tree): + """ + Test adding and removing a repository + """ + add_repo_test_passed = False + + def _run(name, test=False): + return modules.state.sls( + mods=name, + test=test, + ) + + with subtests.test("Add repository"): + ret = _run("pkgrepo.managed") + assert ret.failed is False + for state in ret: + assert state.result is True + add_repo_test_passed = True + + if add_repo_test_passed is False: + pytest.skip("Adding the repository failed, skipping removal tests.") + + with subtests.test("Remove repository, test"): + ret = _run("pkgrepo.absent", test=True) + assert ret.failed is False + for state in ret: + assert state.changes == {} + assert state.comment.startswith("Package repo 'salttest' will be removed.") + assert state.result is None + + with subtests.test("Remove repository"): + ret = _run("pkgrepo.absent") + assert ret.failed is False + for state in ret: + assert state.result is True + + with subtests.test("Remove repository again, test"): + ret = _run("pkgrepo.absent", test=True) + assert ret.failed is False + for state in ret: + assert state.changes == {} + assert state.comment == "Package repo salttest is absent" + assert state.result is True + + with subtests.test("Remove repository again"): + ret = _run("pkgrepo.absent") + assert ret.failed is False + for state in ret: + assert state.changes == {} + assert state.comment == "Package repo salttest is absent" + assert state.result is True + + +@pytest.mark.requires_salt_states("pkgrepo.managed") +def test_pkgrepo_managed_modify(grains, modules, subtests, suse_state_tree): + """ + Test adding and modifying a repository + """ + add_repo_test_passed = False + + def _run(name, test=False): + return modules.state.sls( + mods=name, + test=test, + ) + + with subtests.test("Add repository, test"): + ret = _run("pkgrepo.managed", test=True) + assert ret.failed is False + for state in ret: + assert state.changes == {"repo": "salttest"} + assert state.comment.startswith( + "Package repo 'salttest' would be configured." + ) + assert state.result is None + + with subtests.test("Add repository"): + ret = _run("pkgrepo.managed") + assert ret.failed is False + for state in ret: + assert state.changes == {"repo": "salttest"} + assert state.comment == "Configured package repo 'salttest'" + assert state.result is True + add_repo_test_passed = True + + if add_repo_test_passed is False: + pytest.skip("Adding the repository failed, skipping modification tests.") + + with subtests.test("Add repository again, test"): + ret = _run("pkgrepo.managed", test=True) + assert ret.failed is False + for state in ret: + assert state.changes == {} + assert state.comment == "Package repo 'salttest' already configured" + assert state.result is True + + with subtests.test("Add repository again"): + ret = _run("pkgrepo.managed") + assert ret.failed is False + for state in ret: + assert state.result is True + assert state.changes == {} + assert state.comment == "Package repo 'salttest' already configured" + + with subtests.test("Modify repository, test"): + ret = _run("pkgrepo.modified", test=True) + assert ret.failed is False + for state in ret: + assert state.changes == { + "comments": {"new": ["# Salt Test (modified)"], "old": None}, + "refresh": {"new": 1, "old": None}, + "gpgkey": { + "new": "https://download.opensuse.org/repositories/openSUSE:/Backports:/SLE-15-SP4/standard/repodata/repomd.xml.key", + "old": None, + }, + "name": { + "new": "Salt modified Backports", + "old": "openSUSE Backports for SLE 15 SP4", + }, + } + assert state.comment.startswith( + "Package repo 'salttest' would be configured." + ) + assert state.result is None + + with subtests.test("Modify repository"): + ret = _run("pkgrepo.modified") + assert ret.failed is False + for state in ret: + assert state.result is True + assert state.changes == { + "name": { + "new": "Salt modified Backports", + "old": "openSUSE Backports for SLE 15 SP4", + } + } + assert state.comment == "Configured package repo 'salttest'" -- 2.41.0