diff --git a/CHANGES/412.bugfix b/CHANGES/412.bugfix new file mode 100644 index 00000000..d3e8e6da --- /dev/null +++ b/CHANGES/412.bugfix @@ -0,0 +1,2 @@ +Fixed pull-through metadata so that ``dist.tarball``, and therefore ``resolved`` in +``package-lock.json``, points at Pulp instead of the upstream registry. diff --git a/pulp_npm/app/models.py b/pulp_npm/app/models.py index 7b6bbc44..aa1afa2c 100644 --- a/pulp_npm/app/models.py +++ b/pulp_npm/app/models.py @@ -1,4 +1,7 @@ +import asyncio import json +import os +from contextlib import suppress from logging import getLogger import semver @@ -115,31 +118,37 @@ class Meta: ] def content_handler(self, path): - data = {} - - if not self.repository: + # A name+version path is a tarball request, handled by normal artifact lookup. + # An unparsable path (name is None) is not a packument request either. + name, version = extract_package_info(path) + if not name or version: return None - repository_version = self.repository_version - if not repository_version: - repository_version = self.repository.latest_version() + repository_version = None + if self.repository: + repository_version = self.repository_version or self.repository.latest_version() - content = repository_version.content - name, version = extract_package_info(path) - if name and version: - return None + packages = ( + Package.objects.filter(name=name, pk__in=repository_version.content) + if repository_version is not None + else Package.objects.none() + ) - packages = Package.objects.filter(name=name, pk__in=content) + if packages: + return self._packument_from_local_packages(name, packages) - if not packages: - return None + # A remote is attached directly (pull-through, no repository content yet): + # fetch it ourselves and rewrite tarball URLs, rather than returning None + # and letting pulpcore proxy that same remote with unmodified URLs. + if self.remote: + return self._packument_from_remote(name) - data["name"] = name - data["versions"] = {} - versions = [] + return None + def _tarball_url_prefix(self): + """Base URL under which this distribution serves package tarballs.""" if settings.DOMAIN_ENABLED: - prefix_url = "{}/".format( + return "{}/".format( urlpath_sanitize( settings.CONTENT_ORIGIN, settings.CONTENT_PATH_PREFIX, @@ -147,14 +156,18 @@ def content_handler(self, path): self.base_path, ) ) - else: - prefix_url = "{}/".format( - urlpath_sanitize( - settings.CONTENT_ORIGIN, - settings.CONTENT_PATH_PREFIX, - self.base_path, - ) + return "{}/".format( + urlpath_sanitize( + settings.CONTENT_ORIGIN, + settings.CONTENT_PATH_PREFIX, + self.base_path, ) + ) + + def _packument_from_local_packages(self, name, packages): + data = {"name": name, "versions": {}} + versions = [] + prefix_url = self._tarball_url_prefix() for package in packages: tarball_url = f"{prefix_url}{package.name}/-/{package.relative_path.split('/')[-1]}" @@ -177,5 +190,47 @@ def content_handler(self, path): ) data["dist-tags"] = {"latest": latest} - serialized_data = json.dumps(data) - return Response(body=serialized_data) + return Response(body=json.dumps(data), content_type="application/json") + + def _packument_from_remote(self, name): + """Fetch the upstream packument and rewrite dist.tarball to point at Pulp.""" + remote = self.remote.cast() + url = f"{remote.url.rstrip('/')}/{name}" + + async def download(): + # The downloader's aiohttp session must be created inside the loop that + # uses it, so it is built here rather than outside. The session belongs + # to the remote's factory, so close it to not leak sockets per request. + downloader = remote.get_downloader(url=url) + try: + return await downloader.run() + finally: + if session := getattr(downloader, "session", None): + await session.close() + + # content_handler runs via sync_to_async, i.e. in a worker thread with no + # event loop, so the downloader's own blocking fetch() cannot be used. + result = None + try: + result = asyncio.run(download()) + with open(result.path, encoding="utf-8") as fd: + data = json.load(fd) + except Exception: + logger.exception("Failed to read npm metadata for '%s' from '%s'", name, url) + return None + finally: + # The packument was downloaded to a temporary file; only the parsed JSON + # is needed, and packuments are mutable so they are not cached. + if result and result.path: + with suppress(OSError): + os.unlink(result.path) + + prefix_url = self._tarball_url_prefix() + for version_data in data.get("versions", {}).values(): + tarball = version_data.get("dist", {}).get("tarball") + if not tarball: + continue + filename = tarball.split("/")[-1] + version_data["dist"]["tarball"] = f"{prefix_url}{name}/-/{filename}" + + return Response(body=json.dumps(data), content_type="application/json") diff --git a/pulp_npm/tests/functional/api/test_dist_tags.py b/pulp_npm/tests/functional/api/test_dist_tags.py index 2559ca64..605ff1ca 100644 --- a/pulp_npm/tests/functional/api/test_dist_tags.py +++ b/pulp_npm/tests/functional/api/test_dist_tags.py @@ -1,92 +1,12 @@ """Tests that verify dist-tags.latest is resolved using semver, not lexicographic order.""" -import asyncio -import base64 -import io import json -import os -import tarfile import uuid from urllib.parse import urljoin -import aiohttp import pytest - -def _pulp_base_url(): - protocol = os.environ.get("API_PROTOCOL", "https") - host = os.environ.get("API_HOST", "pulp") - port = os.environ.get("API_PORT", "443") - return f"{protocol}://{host}:{port}" - - -def _pulp_auth(): - return aiohttp.BasicAuth( - os.environ.get("ADMIN_USERNAME", "admin"), - os.environ.get("ADMIN_PASSWORD", "password"), - ) - - -def _build_npm_tgz(name="test-pkg", version="1.0.0"): - package_json = json.dumps({"name": name, "version": version}).encode() - buf = io.BytesIO() - with tarfile.open(fileobj=buf, mode="w:gz") as tar: - info = tarfile.TarInfo(name="package/package.json") - info.size = len(package_json) - tar.addfile(info, io.BytesIO(package_json)) - buf.seek(0) - return buf.read() - - -def _build_publish_body(name, version, tgz_bytes): - base_name = name.split("/")[-1] if "/" in name else name - tarball_filename = f"{base_name}-{version}.tgz" - return { - "_id": name, - "name": name, - "dist-tags": {"latest": version}, - "versions": { - version: { - "name": name, - "version": version, - "dist": {"tarball": f"{name}/-/{tarball_filename}"}, - } - }, - "_attachments": { - tarball_filename: { - "content_type": "application/octet-stream", - "data": base64.b64encode(tgz_bytes).decode(), - "length": len(tgz_bytes), - } - }, - } - - -def _npm_publish_url(base_path, package_name, domain=None): - escaped = package_name.replace("/", "%2F") - if domain: - return f"{_pulp_base_url()}/npm/{domain}/{base_path}/{escaped}" - return f"{_pulp_base_url()}/npm/{base_path}/{escaped}" - - -def _run(coro): - return asyncio.run(coro) - - -async def _put_publish(url, body, auth=None): - async with aiohttp.ClientSession(auth=auth or _pulp_auth()) as session: - async with session.put(url, json=body, ssl=False) as resp: - text = await resp.text() - return resp.status, text - - -def _publish_versions(base_path, pkg_name, versions, domain=None): - for ver in versions: - tgz = _build_npm_tgz(name=pkg_name, version=ver) - body = _build_publish_body(pkg_name, ver, tgz) - url = _npm_publish_url(base_path, pkg_name, domain=domain) - status, text = _run(_put_publish(url, body)) - assert status == 201, f"Publish {ver} failed ({status}): {text}" +from pulp_npm.tests.functional.utils import publish_npm_versions @pytest.mark.parallel @@ -106,7 +26,7 @@ def test_dist_tags_latest_is_highest_semver( distro = npm_distribution_factory(repository=repo.pulp_href) pkg_name = f"semver-order-{uuid.uuid4().hex[:8]}" - _publish_versions(distro.base_path, pkg_name, ["1.0.0", "9.0.0", "10.0.0"], domain=domain) + publish_npm_versions(distro.base_path, pkg_name, ["1.0.0", "9.0.0", "10.0.0"], domain=domain) content_metadata = json.loads(http_get(urljoin(distro.base_url, pkg_name))) assert content_metadata["dist-tags"]["latest"] == "10.0.0" @@ -126,7 +46,7 @@ def test_dist_tags_latest_excludes_prerelease( distro = npm_distribution_factory(repository=repo.pulp_href) pkg_name = f"no-prerelease-{uuid.uuid4().hex[:8]}" - _publish_versions( + publish_npm_versions( distro.base_path, pkg_name, ["1.0.0", "2.0.0", "3.0.0-alpha.1"], domain=domain ) @@ -148,7 +68,7 @@ def test_dist_tags_latest_falls_back_to_prerelease( distro = npm_distribution_factory(repository=repo.pulp_href) pkg_name = f"only-pre-{uuid.uuid4().hex[:8]}" - _publish_versions(distro.base_path, pkg_name, ["1.0.0-beta.1"], domain=domain) + publish_npm_versions(distro.base_path, pkg_name, ["1.0.0-beta.1"], domain=domain) content_metadata = json.loads(http_get(urljoin(distro.base_url, pkg_name))) assert content_metadata["dist-tags"]["latest"] == "1.0.0-beta.1" diff --git a/pulp_npm/tests/functional/api/test_pull_through_caching.py b/pulp_npm/tests/functional/api/test_pull_through_caching.py index 682526ad..219fcf69 100644 --- a/pulp_npm/tests/functional/api/test_pull_through_caching.py +++ b/pulp_npm/tests/functional/api/test_pull_through_caching.py @@ -1,6 +1,12 @@ import json +import time +import uuid + +import pytest +from aiohttp.client_exceptions import ClientResponseError from pulp_npm.tests.functional.constants import NPM_FIXTURE_URL +from pulp_npm.tests.functional.utils import http_get_with_headers, publish_npm_versions def test_pull_through_install( @@ -16,7 +22,12 @@ def test_pull_through_install( latest_package_version = package_metadata["dist-tags"]["latest"] latest_package_metadata = package_metadata["versions"][latest_package_version] - package_filename = latest_package_metadata["dist"]["tarball"].removeprefix(NPM_FIXTURE_URL) + tarball_url = latest_package_metadata["dist"]["tarball"] + + # The tarball URL should resolve through Pulp, not the upstream remote. + assert tarball_url.startswith(distro.base_url) + assert not tarball_url.startswith(NPM_FIXTURE_URL) + package_filename = tarball_url.removeprefix(distro.base_url) package_download = http_get(f"{distro.base_url}{package_filename}") @@ -24,3 +35,157 @@ def test_pull_through_install( content = npm_bindings.ContentPackagesApi.list(name=PACKAGE) assert content.count == 1 + + +def test_pull_through_install_scoped_package( + npm_bindings, npm_remote_factory, npm_distribution_factory, http_get, delete_orphans_pre +): + """Test that a scoped package can be installed from a pull-through distro. + + The scope separator is a slash in both the packument path and the rewritten + tarball URL, so this exercises the parts of the path handling that a flat + package name cannot. + """ + remote = npm_remote_factory(url=NPM_FIXTURE_URL) + distro = npm_distribution_factory(remote=remote.pulp_href) + PACKAGE = "@babel/code-frame" + + package_metadata = json.loads(http_get(f"{distro.base_url}{PACKAGE}")) + assert package_metadata["name"] == PACKAGE + + latest_package_version = package_metadata["dist-tags"]["latest"] + latest_package_metadata = package_metadata["versions"][latest_package_version] + tarball_url = latest_package_metadata["dist"]["tarball"] + + assert tarball_url.startswith(distro.base_url) + assert not tarball_url.startswith(NPM_FIXTURE_URL) + + # The scope must survive the rewrite: "@babel/code-frame/-/code-frame-.tgz". + relative_path = tarball_url.removeprefix(distro.base_url) + assert relative_path == f"{PACKAGE}/-/code-frame-{latest_package_version}.tgz" + + package_download = http_get(tarball_url) + + assert len(package_download) > 100 + + content = npm_bindings.ContentPackagesApi.list(name=PACKAGE) + assert content.count == 1 + assert content.results[0].version == latest_package_version + + +@pytest.mark.parallel +@pytest.mark.parametrize("scope", ["", "@pulp-npm-test/"], ids=["unscoped", "scoped"]) +def test_packument_from_local_packages( + npm_repository_factory, npm_distribution_factory, pulp_settings, http_get, scope +): + """A distribution backed by a repository serves a packument built from its content.""" + domain = "default" if pulp_settings.DOMAIN_ENABLED else None + repo = npm_repository_factory() + distro = npm_distribution_factory(repository=repo.pulp_href) + + pkg_name = f"{scope}local-packument-{uuid.uuid4().hex[:8]}" + base_name = pkg_name.split("/")[-1] + publish_npm_versions(distro.base_path, pkg_name, ["1.0.0", "1.1.0"], domain=domain) + + body, headers = http_get_with_headers(f"{distro.base_url}{pkg_name}") + assert headers["Content-Type"].startswith("application/json") + + packument = json.loads(body) + assert packument["name"] == pkg_name + assert set(packument["versions"]) == {"1.0.0", "1.1.0"} + assert packument["dist-tags"]["latest"] == "1.1.0" + + for version, version_metadata in packument["versions"].items(): + assert version_metadata["name"] == pkg_name + assert version_metadata["_id"] == f"{pkg_name}@{version}" + tarball_url = version_metadata["dist"]["tarball"] + assert tarball_url == f"{distro.base_url}{pkg_name}/-/{base_name}-{version}.tgz" + assert len(http_get(tarball_url)) > 0 + + +def test_pull_through_with_repository_serves_cached_packument( + npm_bindings, + npm_remote_factory, + npm_repository_factory, + npm_distribution_factory, + http_get, + delete_orphans_pre, +): + """A distro with both a repository and a remote hands off to local content once cached. + + The first packument request has no local content and is served from the remote; + pulling a tarball through adds that package to the repository, after which the + packument is built from local content instead. + """ + remote = npm_remote_factory(url=NPM_FIXTURE_URL) + repo = npm_repository_factory() + distro = npm_distribution_factory(repository=repo.pulp_href, remote=remote.pulp_href) + PACKAGE = "commander" + + remote_packument = json.loads(http_get(f"{distro.base_url}{PACKAGE}")) + assert remote_packument["name"] == PACKAGE + # Every upstream version is listed, none of which is cached in the repository yet. + assert len(remote_packument["versions"]) > 1 + + version = remote_packument["dist-tags"]["latest"] + tarball_url = remote_packument["versions"][version]["dist"]["tarball"] + assert tarball_url.startswith(distro.base_url) + + assert len(http_get(tarball_url)) > 100 + + local_packument = _wait_for_cached_packument(http_get, distro, PACKAGE, version) + + assert local_packument["dist-tags"]["latest"] == version + assert local_packument["versions"][version]["_id"] == f"{PACKAGE}@{version}" + # The tarball URL keeps pointing at the same place once served from local content. + assert local_packument["versions"][version]["dist"]["tarball"] == tarball_url + + content = npm_bindings.ContentPackagesApi.list(name=PACKAGE) + assert content.count == 1 + + +def _wait_for_cached_packument(http_get, distro, package, version, timeout=60): + """Poll the packument until it is served from the repository's own content. + + Adding the pulled-through package to the repository is dispatched as a task, so + the handoff is not necessarily visible on the request right after the download. + """ + deadline = time.monotonic() + timeout + while True: + packument = json.loads(http_get(f"{distro.base_url}{package}")) + if list(packument.get("versions", {})) == [version]: + return packument + if time.monotonic() >= deadline: + raise AssertionError( + f"Packument for '{package}' was not served from local content within " + f"{timeout}s: {sorted(packument.get('versions', {}))}" + ) + time.sleep(1) + + +@pytest.mark.parallel +def test_pull_through_packument_missing_upstream( + npm_remote_factory, npm_distribution_factory, http_get +): + """A packument the remote does not have results in a 404, not a server error.""" + remote = npm_remote_factory(url=NPM_FIXTURE_URL) + distro = npm_distribution_factory(remote=remote.pulp_href) + + with pytest.raises(ClientResponseError) as exp: + http_get(f"{distro.base_url}pulp-npm-nonexistent-{uuid.uuid4().hex}") + + assert exp.value.status == 404 + + +@pytest.mark.parallel +def test_pull_through_packument_unreachable_remote( + npm_remote_factory, npm_distribution_factory, http_get +): + """An unreachable remote fails the request instead of raising out of the handler.""" + remote = npm_remote_factory(url="http://npm-unreachable-fixture/") + distro = npm_distribution_factory(remote=remote.pulp_href) + + with pytest.raises(ClientResponseError) as exp: + http_get(f"{distro.base_url}react") + + assert exp.value.status >= 400 diff --git a/pulp_npm/tests/functional/utils.py b/pulp_npm/tests/functional/utils.py index 9c7b342b..d6a6f51d 100644 --- a/pulp_npm/tests/functional/utils.py +++ b/pulp_npm/tests/functional/utils.py @@ -1,6 +1,15 @@ # coding=utf-8 """Utilities for tests for the npm plugin.""" +import asyncio +import base64 +import io +import json +import os +import tarfile + +import aiohttp + def gen_npm_content_attrs(artifact): """Generate a dict with content unit attributes. @@ -10,3 +19,92 @@ def gen_npm_content_attrs(artifact): """ # FIXME: Add content specific metadata here. return {"_artifact": artifact["pulp_href"]} + + +def pulp_base_url(): + """Base URL of the Pulp instance under test.""" + protocol = os.environ.get("API_PROTOCOL", "https") + host = os.environ.get("API_HOST", "pulp") + port = os.environ.get("API_PORT", "443") + return f"{protocol}://{host}:{port}" + + +def pulp_auth(): + """Admin credentials for the Pulp instance under test.""" + return aiohttp.BasicAuth( + os.environ.get("ADMIN_USERNAME", "admin"), + os.environ.get("ADMIN_PASSWORD", "password"), + ) + + +def build_npm_tgz(name="test-pkg", version="1.0.0"): + """Build a minimal npm tarball containing only ``package/package.json``.""" + package_json = json.dumps({"name": name, "version": version}).encode() + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + info = tarfile.TarInfo(name="package/package.json") + info.size = len(package_json) + tar.addfile(info, io.BytesIO(package_json)) + buf.seek(0) + return buf.read() + + +def build_publish_body(name, version, tgz_bytes): + """Build the body of an ``npm publish`` request for a single version.""" + base_name = name.split("/")[-1] if "/" in name else name + tarball_filename = f"{base_name}-{version}.tgz" + return { + "_id": name, + "name": name, + "dist-tags": {"latest": version}, + "versions": { + version: { + "name": name, + "version": version, + "dist": {"tarball": f"{name}/-/{tarball_filename}"}, + } + }, + "_attachments": { + tarball_filename: { + "content_type": "application/octet-stream", + "data": base64.b64encode(tgz_bytes).decode(), + "length": len(tgz_bytes), + } + }, + } + + +def npm_publish_url(base_path, package_name, domain=None): + """URL an ``npm publish`` request is sent to, with the scope separator escaped.""" + escaped = package_name.replace("/", "%2F") + if domain: + return f"{pulp_base_url()}/npm/{domain}/{base_path}/{escaped}" + return f"{pulp_base_url()}/npm/{base_path}/{escaped}" + + +async def _put_publish(url, body, auth=None): + async with aiohttp.ClientSession(auth=auth or pulp_auth()) as session: + async with session.put(url, json=body, ssl=False) as resp: + text = await resp.text() + return resp.status, text + + +def publish_npm_versions(base_path, pkg_name, versions, domain=None): + """Publish each of ``versions`` of ``pkg_name`` into the distribution's repository.""" + for ver in versions: + tgz = build_npm_tgz(name=pkg_name, version=ver) + body = build_publish_body(pkg_name, ver, tgz) + url = npm_publish_url(base_path, pkg_name, domain=domain) + status, text = asyncio.run(_put_publish(url, body)) + assert status == 201, f"Publish {ver} failed ({status}): {text}" + + +def http_get_with_headers(url): + """Like the ``http_get`` fixture, but returns ``(body, headers)``.""" + + async def _send_request(): + async with aiohttp.ClientSession(raise_for_status=True) as session: + async with session.get(url, ssl=False) as response: + return await response.content.read(), dict(response.headers) + + return asyncio.run(_send_request())