diff --git a/CHANGES/+head-based-manifest-version-check.feature b/CHANGES/+head-based-manifest-version-check.feature new file mode 100644 index 000000000..41a1080ee --- /dev/null +++ b/CHANGES/+head-based-manifest-version-check.feature @@ -0,0 +1,4 @@ +Pull-through manifest resolution now performs a HEAD-based version check and serves locally +stored manifests without downloading the manifest body. Per Docker Hub's pull definition, version +checks do not count toward rate limits, substantially reducing 429 errors for repeat tag +resolutions. diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index 616522d93..c44f5f10c 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -1478,14 +1478,43 @@ def fetch_manifest(self, remote, pk): Fetch a manifest from the upstream remote. Returns the local manifest, if it exists in Pulp, and the full response from upstream. Raises response errors if manifest is not found or the download fails. + + A HEAD request is issued first to check the manifest's digest against what is already + stored in Pulp. Per Docker Hub's pull-rate accounting, a HEAD ("version check") does not + count as a pull, unlike a GET. If the manifest is already stored locally, the HEAD + response is returned as-is and the counted GET is skipped entirely. """ relative_url = "/v2/{name}/manifests/{pk}".format( name=remote.namespaced_upstream_name, pk=pk ) tag_url = urljoin(remote.url, relative_url) + + head_response = self._fetch_manifest_response(remote, tag_url, pk, http_method="head") + digest = head_response.headers.get("docker-content-digest") + if digest: + manifest = models.Manifest.objects.filter( + digest=digest, pulp_domain=get_domain() + ).first() + if manifest is not None: + return manifest, head_response + + # The manifest is not stored locally yet, or the upstream did not report a digest on the + # HEAD response; fall back to a full GET, identical to the previous behavior. + response = self._fetch_manifest_response(remote, tag_url, pk, http_method="get") + digest = response.headers.get("docker-content-digest") + return models.Manifest.objects.filter( + digest=digest, pulp_domain=get_domain() + ).first(), response + + def _fetch_manifest_response(self, remote, tag_url, pk, http_method): + """ + Issue a HEAD or GET request for a manifest and map response errors consistently. + """ downloader = remote.get_downloader(url=tag_url) try: - response = downloader.fetch(extra_data={"headers": V2_ACCEPT_HEADERS}) + return downloader.fetch( + extra_data={"headers": V2_ACCEPT_HEADERS, "http_method": http_method} + ) except ClientResponseError as response_error: if response_error.status == 429: # the client could request the manifest outside the docker hub pull limit; @@ -1498,11 +1527,6 @@ def fetch_manifest(self, remote, pk): except (ClientConnectionError, TimeoutException): # The remote server is not available at the moment raise GatewayTimeout() - else: - digest = response.headers.get("docker-content-digest") - return models.Manifest.objects.filter( - digest=digest, pulp_domain=get_domain() - ).first(), response def put(self, request, path, pk=None): """ diff --git a/pulp_container/tests/unit/test_registry_api.py b/pulp_container/tests/unit/test_registry_api.py new file mode 100644 index 000000000..849c516b4 --- /dev/null +++ b/pulp_container/tests/unit/test_registry_api.py @@ -0,0 +1,137 @@ +"""Unit tests for HEAD-based manifest version checking in pull-through caching.""" + +import unittest +from unittest.mock import MagicMock, patch + +from aiohttp.client_exceptions import ClientConnectionError, ClientResponseError +from rest_framework.exceptions import Throttled + +from pulp_container.app.exceptions import BadGateway, GatewayTimeout, ManifestNotFound +from pulp_container.app.registry_api import Manifests +from pulpcore.plugin.exceptions import TimeoutException + + +def _mock_response(digest): + response = MagicMock() + response.headers = {"docker-content-digest": digest} if digest else {} + return response + + +class TestFetchManifest(unittest.TestCase): + """Exercise the HEAD-first manifest resolution flow.""" + + def setUp(self): + self.view = Manifests() + self.remote = MagicMock() + self.remote.namespaced_upstream_name = "library/test" + self.remote.url = "https://registry.example/" + + def _set_downloaders(self, *downloaders): + self.remote.get_downloader = MagicMock(side_effect=list(downloaders)) + + @patch("pulp_container.app.registry_api.get_domain") + @patch("pulp_container.app.registry_api.models.Manifest.objects") + def test_local_hit_issues_only_head(self, mock_manifest_objects, mock_get_domain): + digest = "sha256:" + "a" * 64 + local_manifest = MagicMock() + mock_manifest_objects.filter.return_value.first.return_value = local_manifest + + head_downloader = MagicMock() + head_downloader.fetch.return_value = _mock_response(digest) + self._set_downloaders(head_downloader) + + manifest, response = self.view.fetch_manifest(self.remote, "latest") + + self.assertIs(manifest, local_manifest) + self.remote.get_downloader.assert_called_once() + head_downloader.fetch.assert_called_once() + _, kwargs = head_downloader.fetch.call_args + self.assertEqual(kwargs["extra_data"]["http_method"], "head") + + @patch("pulp_container.app.registry_api.get_domain") + @patch("pulp_container.app.registry_api.models.Manifest.objects") + def test_local_miss_falls_back_to_get(self, mock_manifest_objects, mock_get_domain): + digest = "sha256:" + "b" * 64 + mock_manifest_objects.filter.return_value.first.return_value = None + + head_downloader = MagicMock() + head_downloader.fetch.return_value = _mock_response(digest) + get_downloader = MagicMock() + get_downloader.fetch.return_value = _mock_response(digest) + self._set_downloaders(head_downloader, get_downloader) + + manifest, response = self.view.fetch_manifest(self.remote, "latest") + + self.assertIsNone(manifest) + self.assertIs(response, get_downloader.fetch.return_value) + self.assertEqual(self.remote.get_downloader.call_count, 2) + head_downloader.fetch.assert_called_once() + get_downloader.fetch.assert_called_once() + _, kwargs = get_downloader.fetch.call_args + self.assertEqual(kwargs["extra_data"]["http_method"], "get") + + @patch("pulp_container.app.registry_api.get_domain") + @patch("pulp_container.app.registry_api.models.Manifest.objects") + def test_missing_digest_header_falls_back_to_get(self, mock_manifest_objects, mock_get_domain): + mock_manifest_objects.filter.return_value.first.return_value = None + + head_downloader = MagicMock() + head_downloader.fetch.return_value = _mock_response(digest=None) + get_downloader = MagicMock() + get_downloader.fetch.return_value = _mock_response("sha256:" + "c" * 64) + self._set_downloaders(head_downloader, get_downloader) + + self.view.fetch_manifest(self.remote, "latest") + + self.assertEqual(self.remote.get_downloader.call_count, 2) + get_downloader.fetch.assert_called_once() + + def test_404_on_head_raises_manifest_not_found(self): + head_downloader = MagicMock() + head_downloader.fetch.side_effect = ClientResponseError( + request_info=MagicMock(), history=(), status=404 + ) + self._set_downloaders(head_downloader) + + with self.assertRaises(ManifestNotFound): + self.view.fetch_manifest(self.remote, "missing-tag") + + def test_429_on_head_raises_throttled(self): + head_downloader = MagicMock() + head_downloader.fetch.side_effect = ClientResponseError( + request_info=MagicMock(), history=(), status=429 + ) + self._set_downloaders(head_downloader) + + with self.assertRaises(Throttled): + self.view.fetch_manifest(self.remote, "latest") + + def test_other_status_on_head_raises_bad_gateway(self): + head_downloader = MagicMock() + head_downloader.fetch.side_effect = ClientResponseError( + request_info=MagicMock(), history=(), status=500, message="boom" + ) + self._set_downloaders(head_downloader) + + with self.assertRaises(BadGateway): + self.view.fetch_manifest(self.remote, "latest") + + def test_connection_error_on_head_raises_gateway_timeout(self): + head_downloader = MagicMock() + head_downloader.fetch.side_effect = ClientConnectionError() + self._set_downloaders(head_downloader) + + with self.assertRaises(GatewayTimeout): + self.view.fetch_manifest(self.remote, "latest") + + def test_timeout_on_head_raises_gateway_timeout(self): + head_downloader = MagicMock() + head_downloader.fetch.side_effect = TimeoutException() + self._set_downloaders(head_downloader) + + with self.assertRaises(GatewayTimeout): + self.view.fetch_manifest(self.remote, "latest") + + +if __name__ == "__main__": + unittest.main()