salt/zypper-pkgrepo-alreadyconfigured-585.patch

From 6b6ba4bdbd4b4c52a46bf3d0bcdbaca6b47534d1 Mon Sep 17 00:00:00 2001
From: Georg <georg@lysergic.dev>
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 <georg.pfuetzenreuter@suse.com>

* Functional pkgrepo tests for SUSE

Signed-off-by: Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com>

* Change pkgrepo state to use f-strings

Follow new styling rules.

Signed-off-by: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>

---------

Signed-off-by: Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com>
Signed-off-by: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>
---
 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