Handle the situation when apt repo lines have or do not have trailing slashes properly

changelog/60907.fixed

@@ -0,0 +1 @@
+Handle the situation when apt repo lines have or do not have trailing slashes properly.

changelog/61483.fixed

@@ -0,0 +1 @@
+When running with test=True and there are no changes, don't show that there are changes.

salt/modules/aptpkg.py

@@ -2850,26 +2850,26 @@ def mod_repo(repo, saltenv="base", aptkey=True, **kwargs):
     kw_type = kwargs.get("type")
     kw_dist = kwargs.get("dist")
 
-    for source in repos:
+    for apt_source in repos:
         # This series of checks will identify the starting source line
         # and the resulting source line.  The idea here is to ensure
         # we are not returning bogus data because the source line
         # has already been modified on a previous run.
         repo_matches = (
-            source.type == repo_type
-            and source.uri.rstrip("/") == repo_uri.rstrip("/")
-            and source.dist == repo_dist
+            apt_source.type == repo_type
+            and apt_source.uri.rstrip("/") == repo_uri.rstrip("/")
+            and apt_source.dist == repo_dist
         )
-        kw_matches = source.dist == kw_dist and source.type == kw_type
+        kw_matches = apt_source.dist == kw_dist and apt_source.type == kw_type
 
         if repo_matches or kw_matches:
             for comp in full_comp_list:
-                if comp in getattr(source, "comps", []):
-                    mod_source = source
-            if not source.comps:
-                mod_source = source
-            if kwargs["architectures"] != source.architectures:
-                mod_source = source
+                if comp in getattr(apt_source, "comps", []):
+                    mod_source = apt_source
+            if not apt_source.comps:
+                mod_source = apt_source
+            if kwargs["architectures"] != apt_source.architectures:
+                mod_source = apt_source
             if mod_source:
                 break
 
@@ -2887,6 +2887,11 @@ def mod_repo(repo, saltenv="base", aptkey=True, **kwargs):
     for key in kwargs:
         if key in _MODIFY_OK and hasattr(mod_source, key):
             setattr(mod_source, key, kwargs[key])
+
+    if mod_source.uri != repo_uri:
+        mod_source.uri = repo_uri
+        mod_source.line = mod_source.repo_line()
+
     sources.save()
     # on changes, explicitly refresh
     if refresh:
@@ -2998,23 +3003,28 @@ def expand_repo_def(**kwargs):
     else:
         signedby = _get_opts(repo)["signedby"].get("value", "")
 
-    source_entry = source_list.add(
+    _source_entry = source_list.add(
         type=source_entry.type,
         uri=source_entry.uri,
         dist=source_entry.dist,
         orig_comps=getattr(source_entry, "comps", []),
         architectures=getattr(source_entry, "architectures", []),
         **kwargs,
     )
-
-    sanitized["file"] = source_entry.file
-    sanitized["comps"] = getattr(source_entry, "comps", [])
-    sanitized["disabled"] = source_entry.disabled
-    sanitized["dist"] = source_entry.dist
-    sanitized["type"] = source_entry.type
-    sanitized["uri"] = source_entry.uri
-    sanitized["line"] = source_entry.line.strip()
-    sanitized["architectures"] = getattr(source_entry, "architectures", [])
+    if hasattr(_source_entry, "set_enabled"):
+        _source_entry.set_enabled(not source_entry.disabled)
+    else:
+        _source_entry.disabled = source_entry.disabled
+        _source_entry.line = _source_entry.repo_line()
+
+    sanitized["file"] = _source_entry.file
+    sanitized["comps"] = getattr(_source_entry, "comps", [])
+    sanitized["disabled"] = _source_entry.disabled
+    sanitized["dist"] = _source_entry.dist
+    sanitized["type"] = _source_entry.type
+    sanitized["uri"] = _source_entry.uri
+    sanitized["line"] = _source_entry.line.strip()
+    sanitized["architectures"] = getattr(_source_entry, "architectures", [])
     sanitized["signedby"] = signedby
     if HAS_APT and signedby:
         # python3-apt does not supported the signed-by opt currently.

salt/states/pkgrepo.py

@@ -466,9 +466,6 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs):
             else salt.utils.data.is_true(enabled)
         )
 
-    if __grains__["os_family"] == "Debian":
-        repo = salt.utils.pkg.deb.strip_uri(repo)
-
     for kwarg in _STATE_INTERNAL_KEYWORDS:
         kwargs.pop(kwarg, None)
 
@@ -510,22 +507,23 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs):
                 if sorted(sanitizedkwargs[kwarg]) != sorted(pre[kwarg]):
                     break
             elif kwarg == "line" and __grains__["os_family"] == "Debian":
-                # split the line and sort everything after the URL
-                sanitizedsplit = sanitizedkwargs[kwarg].split()
-                sanitizedsplit[3:] = sorted(sanitizedsplit[3:])
-                reposplit, _, pre_comments = (
-                    x.strip() for x in pre[kwarg].partition("#")
-                )
-                reposplit = reposplit.split()
-                reposplit[3:] = sorted(reposplit[3:])
-                if sanitizedsplit != reposplit:
-                    break
-                if "comments" in kwargs:
-                    post_comments = salt.utils.pkg.deb.combine_comments(
-                        kwargs["comments"]
+                if not sanitizedkwargs["disabled"]:
+                    # split the line and sort everything after the URL
+                    sanitizedsplit = sanitizedkwargs[kwarg].split()
+                    sanitizedsplit[3:] = sorted(sanitizedsplit[3:])
+                    reposplit, _, pre_comments = (
+                        x.strip() for x in pre[kwarg].partition("#")
                     )
-                    if pre_comments != post_comments:
+                    reposplit = reposplit.split()
+                    reposplit[3:] = sorted(reposplit[3:])
+                    if sanitizedsplit != reposplit:
                         break
+                    if "comments" in kwargs:
+                        post_comments = salt.utils.pkg.deb.combine_comments(
+                            kwargs["comments"]
+                        )
+                        if pre_comments != post_comments:
+                            break
             elif kwarg == "comments" and __grains__["os_family"] == "RedHat":
                 precomments = salt.utils.pkg.rpm.combine_comments(pre[kwarg])
                 kwargcomments = salt.utils.pkg.rpm.combine_comments(

tests/pytests/functional/modules/test_pkg.py

@@ -9,6 +9,7 @@
 import salt.utils.path
 import salt.utils.pkg
 import salt.utils.platform
+from saltfactories.utils.functional import Loaders
 
 log = logging.getLogger(__name__)
 
@@ -319,15 +320,16 @@ def test_hold_unhold(grains, modules, states, test_pkg):
 @pytest.mark.requires_salt_modules("pkg.refresh_db")
 @pytest.mark.slow_test
 @pytest.mark.requires_network
-def test_refresh_db(grains, modules, tmp_path, minion_opts):
+def test_refresh_db(grains, tmp_path, minion_opts):
     """
     test refreshing the package database
     """
     rtag = salt.utils.pkg.rtag(minion_opts)
     salt.utils.pkg.write_rtag(minion_opts)
     assert os.path.isfile(rtag) is True
 
-    ret = modules.pkg.refresh_db()
+    loader = Loaders(minion_opts)
+    ret = loader.modules.pkg.refresh_db()
     if not isinstance(ret, dict):
         pytest.skip("Upstream repo did not return coherent results: {}".format(ret))
 

tests/pytests/functional/states/pkgrepo/test_debian.py

@@ -491,11 +491,37 @@ def test_repo_present_absent_trailing_slash_uri(pkgrepo, trailing_slash_repo_fil
     """
     test adding a repo with a trailing slash in the uri
     """
+    # with the trailing slash
     repo_content = "deb http://www.deb-multimedia.org/ stable main"
     # initial creation
     ret = pkgrepo.managed(
         name=repo_content, file=trailing_slash_repo_file, refresh=False, clean_file=True
     )
+    with salt.utils.files.fopen(trailing_slash_repo_file, "r") as fp:
+        file_content = fp.read()
+    assert file_content.strip() == "deb http://www.deb-multimedia.org/ stable main"
+    assert ret.changes
+    # no changes
+    ret = pkgrepo.managed(
+        name=repo_content, file=trailing_slash_repo_file, refresh=False
+    )
+    assert not ret.changes
+    # absent
+    ret = pkgrepo.absent(name=repo_content)
+    assert ret.result
+
+
+@pytest.mark.requires_salt_states("pkgrepo.managed", "pkgrepo.absent")
+def test_repo_present_absent_no_trailing_slash_uri(pkgrepo, trailing_slash_repo_file):
+    """
+    test adding a repo with a trailing slash in the uri
+    """
+    # without the trailing slash
+    repo_content = "deb http://www.deb-multimedia.org stable main"
+    # initial creation
+    ret = pkgrepo.managed(
+        name=repo_content, file=trailing_slash_repo_file, refresh=False, clean_file=True
+    )
     with salt.utils.files.fopen(trailing_slash_repo_file, "r") as fp:
         file_content = fp.read()
     assert file_content.strip() == "deb http://www.deb-multimedia.org stable main"