diff --git a/CHANGES/1453.bugfix b/CHANGES/1453.bugfix new file mode 100644 index 00000000..6a6cc926 --- /dev/null +++ b/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. diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 8d580a2f..c2a45e12 100644 --- a/pulp_python/app/pypi/views.py +++ b/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, diff --git a/pulp_python/tests/functional/api/test_full_mirror.py b/pulp_python/tests/functional/api/test_full_mirror.py index e89cdac2..d1a7f1e7 100644 --- a/pulp_python/tests/functional/api/test_full_mirror.py +++ b/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