Fix a bug for url parsing when the CV name is "collections". (#1376)

fixes: #1388

Signed-off-by: James Tanner <tanner.jc@gmail.com>

CHANGES/1388.bugfix

@@ -0,0 +1 @@
+Fix 404 on collection detail routing for collections with a name of "collections".

pulp_ansible/app/urls.py

@@ -174,37 +174,40 @@
         name="collection-artifact-download",
     ),
     path(
-        "index/<str:namespace>/<str:name>/",
+        "all-collections/",
+        views_v3.UnpaginatedCollectionViewSet.as_view({"get": "list"}),
+        name="metadata-collection-list",
+    ),
+    path(
+        "all-versions/",
+        views_v3.UnpaginatedCollectionVersionViewSet.as_view({"get": "list"}),
+        name="metadata-collection-versions-list",
+    ),
+]
+
+v3_collection_detail_urls = [
+    path(
+        "",
         views_v3.CollectionViewSet.as_view(
             {"get": "retrieve", "patch": "update", "delete": "destroy"}
         ),
         name="collections-detail",
     ),
     path(
-        "index/<str:namespace>/<str:name>/versions/",
+        "versions/",
         views_v3.CollectionVersionViewSet.as_view({"get": "list"}),
         name="collection-versions-list",
     ),
     path(
-        "index/<str:namespace>/<str:name>/versions/<str:version>/",
+        "versions/<str:version>/",
         views_v3.CollectionVersionViewSet.as_view({"get": "retrieve", "delete": "destroy"}),
         name="collection-versions-detail",
     ),
     path(
-        "index/<str:namespace>/<str:name>/versions/<str:version>/docs-blob/",
+        "versions/<str:version>/docs-blob/",
         views_v3.CollectionVersionDocsViewSet.as_view({"get": "retrieve"}),
         name="collection-versions-detail-docs",
     ),
-    path(
-        "all-collections/",
-        views_v3.UnpaginatedCollectionViewSet.as_view({"get": "list"}),
-        name="metadata-collection-list",
-    ),
-    path(
-        "all-versions/",
-        views_v3.UnpaginatedCollectionVersionViewSet.as_view({"get": "list"}),
-        name="metadata-collection-versions-list",
-    ),
 ]
 
 namespace_urls = [
@@ -225,6 +228,10 @@
 v3_plugin_urls = [
     # path:var captures /, so it has to have something at the end to make it work
     # correctly.
+    path(
+        "content/<path:distro_base_path>/collections/index/<str:namespace>/<str:name>/",
+        include(v3_collection_detail_urls),
+    ),
     path("content/<path:distro_base_path>/collections/", include(v3_collection_urls)),
     path("content/<path:distro_base_path>/namespaces/", include(namespace_urls)),
     path(

pulp_ansible/tests/functional/api/collection/v3/test_collection_naming_edgecases.py

@@ -0,0 +1,174 @@
+import hashlib
+import json
+import shutil
+import subprocess
+import os
+
+import pytest
+
+from orionutils.generator import build_collection
+from orionutils.generator import randstr
+
+from pulpcore.client.pulp_ansible.exceptions import ApiException
+
+
+def busted_collection_build(basedir=None, namespace=None, name=None, version=None):
+    """Make artifacts with namespaces and names that wouldn't normally be allowed."""
+
+    def file_checksum(fp):
+        with open(fp, "rb") as f:
+            checksum = hashlib.sha256(f.read()).hexdigest()
+        return checksum
+
+    # make a content dir
+    content_dir = os.path.join(basedir, "content")
+    os.makedirs(content_dir)
+
+    # make various files
+    for dname in ["docs", "plugins", "roles"]:
+        os.makedirs(os.path.join(content_dir, dname))
+    with open(os.path.join(content_dir, "README.md"), "w") as f:
+        f.write("")
+    with open(os.path.join(content_dir, "plugins", "README.md"), "w") as f:
+        f.write("")
+
+    # make files.json
+    fdata = {"files": [], "format": 1}
+    for root, dirs, files in os.walk(content_dir):
+        for dirname in dirs:
+            dirpath = os.path.join(root, dirname)
+            relative_dir = dirpath.replace(content_dir + "/", "")
+            fdata["files"].append(
+                {
+                    "name": relative_dir,
+                    "ftype": "dir",
+                    "chksum_type": None,
+                    "chksum_sha256": None,
+                    "format": 1,
+                }
+            )
+        for filen in files:
+            filepath = os.path.join(root, filen)
+            relative_file = filepath.replace(content_dir + "/", "")
+            fdata["files"].append(
+                {
+                    "name": relative_file,
+                    "ftype": "file",
+                    "chksum_type": "sha256",
+                    "chksum_sha256": file_checksum(filepath),
+                    "format": 1,
+                }
+            )
+    with open(os.path.join(content_dir, "FILES.json"), "w") as f:
+        f.write(json.dumps(fdata, indent=2))
+
+    # make manifest.json
+    mdata = {
+        "format": 1,
+        "collection_info": {
+            "namespace": namespace,
+            "name": name,
+            "version": version,
+            "authors": ["your name <example@domain.com>"],
+            "readme": "README.md",
+            "tags": [],
+            "description": "your collection description",
+            "license": ["GPL-2.0-or-later"],
+            "license_file": None,
+            "dependencies": {},
+            "repository": "http://example.com/repository",
+            "homepage": "http://example.com",
+            "issues": "http://example.com/issue/tracker",
+        },
+        "file_manifest_file": {
+            "name": "FILES.json",
+            "ftype": "file",
+            "chksum_type": "sha256",
+            "chksum_sha256": file_checksum(os.path.join(content_dir, "FILES.json")),
+            "format": 1,
+        },
+    }
+    with open(os.path.join(content_dir, "MANIFEST.json"), "w") as f:
+        f.write(json.dumps(mdata, indent=2))
+
+    # make the tarball
+    cmd = "tar czvf artifact.tar.gz *"
+    subprocess.run(cmd, shell=True, cwd=content_dir)
+    return os.path.join(content_dir, "artifact.tar.gz")
+
+
+@pytest.mark.parallel
+def test_collection_named_collections(
+    tmp_path,
+    ansible_repo,
+    ansible_distribution_factory,
+    ansible_collection_version_api_client,
+    galaxy_v3_content_collection_index_api,
+    monitor_task,
+):
+    """A CV with a name of 'collections' could break url parsing."""
+    # https://issues.redhat.com/browse/AAH-2158
+    pulp_dist = ansible_distribution_factory(ansible_repo)
+
+    spec = {
+        "repository": "https://github.com/foo/bar",
+        "namespace": randstr(),
+        "name": "collections",
+        "version": "1.0.0",
+    }
+    artifact = build_collection(base="skeleton", config=spec)
+
+    # orionutils creates the tarball inside the site-packages path,
+    # so we'd like to get it out of there before doing anything further.
+    artifact_fn = os.path.join(tmp_path, os.path.basename(artifact.filename))
+    shutil.move(artifact.filename, artifact_fn)
+
+    body = {"file": artifact_fn}
+    body["repository"] = ansible_repo.pulp_href
+    response = ansible_collection_version_api_client.create(**body)
+    monitor_task(response.task)
+
+    # validate the collection shows up ...
+    resp = galaxy_v3_content_collection_index_api.list(pulp_dist.base_path)
+    assert resp.meta.count == 1
+    assert resp.data[0].namespace == spec["namespace"]
+    assert resp.data[0].name == spec["name"]
+
+    # validate we can get the direct path to the collection ...
+    resp = galaxy_v3_content_collection_index_api.read(
+        pulp_dist.base_path, namespace=spec["namespace"], name=spec["name"]
+    )
+    assert resp.namespace == spec["namespace"]
+    assert resp.name == spec["name"]
+
+
+@pytest.mark.parallel
+def test_collection_named_with_slashes(
+    tmp_path,
+    ansible_repo,
+    ansible_distribution_factory,
+    ansible_collection_version_api_client,
+):
+    """A CV with a slash in the name could break url parsing."""
+    # https://issues.redhat.com/browse/AAH-2158
+    ansible_distribution_factory(ansible_repo)
+
+    spec = {
+        "repository": "https://github.com/foo/bar",
+        "namespace": "collections/are/fun",
+        "name": "index/foo/myname",
+        "version": "1.0.0",
+    }
+
+    tarball = busted_collection_build(
+        basedir=tmp_path,
+        namespace=spec["namespace"],
+        name=spec["name"],
+        version=spec["version"],
+    )
+
+    body = {"file": tarball}
+    body["repository"] = ansible_repo.pulp_href
+
+    with pytest.raises(ApiException):
+        ansible_collection_version_api_client.create(**body)

pulp_ansible/tests/functional/conftest.py

@@ -25,6 +25,7 @@
     PulpAnsibleApiV3PluginAnsibleClientConfigurationApi,
     PulpAnsibleDefaultApiV3PluginAnsibleClientConfigurationApi,
     PulpAnsibleApiV3PluginAnsibleContentNamespacesApi,
+    PulpAnsibleDefaultApiV3PluginAnsibleContentCollectionsIndexApi,
 )
 
 
@@ -123,6 +124,12 @@ def galaxy_v3_namespaces_api_client(ansible_bindings_client):
     return PulpAnsibleApiV3NamespacesApi(ansible_bindings_client)
 
 
+@pytest.fixture
+def galaxy_v3_content_collection_index_api(ansible_bindings_client):
+    """Provides the Galaxy V3 Collections Index API client object."""
+    return PulpAnsibleDefaultApiV3PluginAnsibleContentCollectionsIndexApi(ansible_bindings_client)
+
+
 @pytest.fixture
 def galaxy_v3_plugin_namespaces_api_client(ansible_bindings_client):
     """Provides the Galaxy V3 Namespace API client object."""