Announce no metadata on pull-through links

CHANGES/1453.bugfix

@@ -0,0 +1 @@
+Stopped announcing PEP 658 metadata on pull-through links, whose ?redirect= query made pip ask for the metadata at the package's own path: once the package was cached, pip got the package in place of its metadata and failed its hash check.

pulp_python/app/pypi/views.py

@@ -81,12 +81,18 @@ def _get_repo_version(path):
         return None
 
 
+# Part of the ETag of the simple pages. Change it whenever the pages render differently for
+# the same repository version, so a client revalidating a page cached before the change gets
+# the new page instead of a 304.
+SIMPLE_PAGE_REVISION = "pull-through-links-without-metadata"
+
+
 def _etag_func(request, path, **kwargs):
     """Compute unquoted ETag for the condition decorator. Returns None if no repo."""
     repo_ver = _get_repo_version(path)
     if repo_ver is None:
         return None
-    raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}"
+    raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}:{SIMPLE_PAGE_REVISION}"
     return hashlib.sha256(raw.encode()).hexdigest()[:16]
 
 
@@ -378,7 +384,12 @@ def parse_package(release_package):
                 "url": d_url,
                 "sha256": release_package.digests.get("sha256", ""),
                 "requires_python": release_package.requires_python,
-                "metadata_sha256": (release_package.metadata_digests or {}).get("sha256"),
+                # No PEP 658 metadata for a file served through this redirect link: pip asks for
+                # it by appending ".metadata" to the whole URL, which leaves the path naming the
+                # file itself. Once the file is in the repository, a client holding this link is
+                # served the file by its path and gets it in place of the metadata. Installers
+                # fall back to the metadata inside the file they download anyway.
+                "metadata_sha256": None,
                 "size": release_package.size,
                 "upload_time": release_package.upload_time,
                 "version": release_package.version,

pulp_python/tests/functional/api/test_full_mirror.py

@@ -208,36 +208,43 @@ def test_pull_through_filtering_bad_names(python_remote_factory, python_distribu
 @pytest.mark.parallel
 def test_pull_through_metadata(python_remote_factory, python_distribution_factory):
     """
-    Tests that metadata is correctly served when using pull-through.
+    Tests that a pull-through link announces no metadata, but the metadata is still served.
 
-    So when requesting the metadata url according to PEP 658 you should just need to add .metadata
-    to the end of the url path. Since pull-through includes a redirect query parameter we need to
-    test adding .metadata to the end of the url path vs adding it to the end of redirect query.
+    pip asks for PEP 658 metadata by appending ".metadata" to the end of the url, which puts it
+    into the redirect query parameter and leaves the path naming the package. Once the package
+    is in a repository, that request is answered with the package by its path, so the link must
+    not announce metadata. A client holding an older page still gets the metadata either way.
     """
     remote = python_remote_factory(includes=["pytz"])
     distro = python_distribution_factory(remote=remote.pulp_href)
 
     url = f"{distro.base_url}simple/pytz/"
     project_page = ProjectPage.from_response(requests.get(url), "pytz")
+    upstream_page = ProjectPage.from_response(
+        requests.get(urljoin(PYPI_URL, "simple/pytz/")), "pytz"
+    )
     filename1 = "pytz-2023.2-py2.py3-none-any.whl"
     filename2 = "pytz-2023.3-py2.py3-none-any.whl"
     package1 = next(p for p in project_page.packages if p.filename == filename1)
     package2 = next(p for p in project_page.packages if p.filename == filename2)
-    assert package1.has_metadata
-    assert package2.has_metadata
+    assert "?redirect=" in package1.url
+    assert not package1.has_metadata
+    assert not package2.has_metadata
+    upstream1 = next(p for p in upstream_page.packages if p.filename == filename1)
+    upstream2 = next(p for p in upstream_page.packages if p.filename == filename2)
 
-    # The correct way to get the metadata url: add to path (uv does this)
+    # Adding .metadata to the url path (uv does this)
     parts1 = urlsplit(package1.url)
     url1 = urlunsplit((parts1[0], parts1[1], parts1[2] + ".metadata", parts1[3], parts1[4]))
     r = requests.get(url1)
     assert r.status_code == 200
-    assert sha256(r.content).hexdigest() == package1.metadata_digests["sha256"]
+    assert sha256(r.content).hexdigest() == upstream1.metadata_digests["sha256"]
 
-    # The incorrect way to get the metadata url: add to end of string (pip does this)
+    # Adding .metadata to the end of the url (pip does this)
     url2 = package2.url + ".metadata"
     r = requests.get(url2)
     assert r.status_code == 200
-    assert sha256(r.content).hexdigest() == package2.metadata_digests["sha256"]
+    assert sha256(r.content).hexdigest() == upstream2.metadata_digests["sha256"]
 
 
 @pytest.mark.parallel
@@ -247,7 +254,7 @@ def test_pull_through_metadata_with_repo(
     python_distribution_factory,
     pulpcore_bindings,
 ):
-    """Tests that metadata is correctly saved when using pull-through with a repository."""
+    """Tests that metadata is correctly saved and announced when using pull-through with a repo."""
     remote = python_remote_factory(url=PYPI_URL, includes=["pip"])
     repo = python_repo_factory()
     distro = python_distribution_factory(repository=repo.pulp_href, remote=remote.pulp_href)
@@ -256,32 +263,36 @@ def test_pull_through_metadata_with_repo(
     project_page = ProjectPage.from_response(requests.get(pip_url), "pip")
     filename = "pip-26.0.1-py3-none-any.whl"
     package = next(p for p in project_page.packages if p.filename == filename)
-    assert package.has_metadata
     assert "?redirect=" in package.url
+    assert not package.has_metadata
+    upstream_page = ProjectPage.from_response(requests.get(urljoin(PYPI_URL, "simple/pip/")), "pip")
+    upstream = next(p for p in upstream_page.packages if p.filename == filename)
+    metadata_sha256 = upstream.metadata_digests["sha256"]
 
     # Retrieve the metadata and assert the content was not saved to the repository
     parts = urlsplit(package.url)
     url = urlunsplit((parts[0], parts[1], parts[2] + ".metadata", parts[3], parts[4]))
     r = requests.get(url)
     assert r.status_code == 200
-    assert sha256(r.content).hexdigest() == package.metadata_digests["sha256"]
+    assert sha256(r.content).hexdigest() == metadata_sha256
     project_page = ProjectPage.from_response(requests.get(pip_url), "pip")
     package = next(p for p in project_page.packages if p.filename == filename)
-    assert package.has_metadata
     assert "?redirect=" in package.url
+    assert not package.has_metadata
 
     # Now retrieve the package and assert the content was saved with metadata
     r = requests.get(package.url)
     assert r.status_code == 200
     pa = pulpcore_bindings.ArtifactsApi.list(sha256=package.digests["sha256"])
     assert pa.count == 1
-    ma = pulpcore_bindings.ArtifactsApi.list(sha256=package.metadata_digests["sha256"])
+    ma = pulpcore_bindings.ArtifactsApi.list(sha256=metadata_sha256)
     assert ma.count == 1
 
-    # Check the simple page is updated to point to the local repository
+    # Check the simple page is updated to point to the local repository, with the metadata
     project_page = ProjectPage.from_response(requests.get(pip_url), "pip")
     package = next(p for p in project_page.packages if p.filename == filename)
     assert "?redirect=" not in package.url
+    assert package.metadata_digests["sha256"] == metadata_sha256
     r = requests.get(package.metadata_url)
     assert r.status_code == 200
-    assert sha256(r.content).hexdigest() == package.metadata_digests["sha256"]
+    assert sha256(r.content).hexdigest() == metadata_sha256