From d0fafa7a5cedf973dbb1a97702a130b8814d068b Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 20 Jan 2025 20:41:18 -0500 Subject: [PATCH] Fix 500 error when using flatpak with reclaimed content fixes: #1887 --- CHANGES/1887.bugfix | 1 + pulp_container/app/registry_api.py | 32 ++++- .../tests/functional/api/test_flatpak.py | 116 +++++++++++++----- pulp_container/tests/functional/conftest.py | 11 ++ 4 files changed, 125 insertions(+), 35 deletions(-) create mode 100644 CHANGES/1887.bugfix diff --git a/CHANGES/1887.bugfix b/CHANGES/1887.bugfix new file mode 100644 index 000000000..f6e8ad8a4 --- /dev/null +++ b/CHANGES/1887.bugfix @@ -0,0 +1 @@ +Fixed flatpak index returning 500 when Manifest content was on_demand or had been reclaimed. diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index f262fd58c..f3960b967 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -580,6 +580,32 @@ def recurse_through_manifest_lists(self, tag, manifest, oss, architectures, mani tag, mlm.manifest_list, oss, architectures, manifests ) + def get_manifest_config(self, manifest): + # Special handling for the manifest's config options not being fully stored on the model yet + # See migrations 38 & 43 + config = { + "labels": manifest.labels, + "architecture": manifest.architecture, + "os": manifest.os, + } + if any(not config[value] for value in ("labels", "architecture", "os")): + # Try to read the original config blob, could be missing if user did a reclaim :( + if manifest.config_blob: + try: + config_artifact = manifest.config_blob._artifacts.get() + except Artifact.DoesNotExist: + log.warning(f"Manifest {manifest.pk}'s config blob was not found.") + else: + with storage.open(config_artifact.file.name) as file: + raw_data = file.read() + config_data = json.loads(raw_data) + config["labels"] = config_data.get("config", {}).get("Labels") + config["os"] = config_data["os"] + config["architecture"] = config_data["architecture"] + else: + log.warning(f"Manifest {manifest.pk} has no config blob.") + return config + def get(self, request): req_repositories = None req_tags = None @@ -636,10 +662,8 @@ def get(self, request): tag.name, tag.tagged_manifest, req_oss, req_architectures, manifests ) for manifest, tagged in manifests.items(): - with storage.open(manifest.config_blob._artifacts.get().file.name) as file: - raw_data = file.read() - config_data = json.loads(raw_data) - labels = config_data.get("config", {}).get("Labels") + config_data = self.get_manifest_config(manifest) + labels = config_data["labels"] if not labels: continue if any(label not in labels.keys() for label in req_label_exists): diff --git a/pulp_container/tests/functional/api/test_flatpak.py b/pulp_container/tests/functional/api/test_flatpak.py index 54cc29341..4c181a502 100644 --- a/pulp_container/tests/functional/api/test_flatpak.py +++ b/pulp_container/tests/functional/api/test_flatpak.py @@ -3,39 +3,10 @@ import pytest import subprocess -from django.conf import settings - from pulp_container.tests.functional.constants import REGISTRY_V2 -def test_flatpak_install( - add_to_cleanup, - registry_client, - local_registry, - container_namespace_api, - container_push_repository_api, - container_tag_api, - container_manifest_api, -): - if not settings.FLATPAK_INDEX: - pytest.skip("This test requires FLATPAK_INDEX to be enabled") - - image_path1 = f"{REGISTRY_V2}/pulp/oci-net.fishsoup.busyboxplatform:latest" - registry_client.pull(image_path1) - local_registry.tag_and_push(image_path1, "pulptest/oci-net.fishsoup.busyboxplatform:latest") - image_path2 = f"{REGISTRY_V2}/pulp/oci-net.fishsoup.hello:latest" - registry_client.pull(image_path2) - local_registry.tag_and_push(image_path2, "pulptest/oci-net.fishsoup.hello:latest") - namespace = container_namespace_api.list(name="pulptest").results[0] - add_to_cleanup(container_namespace_api, namespace.pulp_href) - - repo = container_push_repository_api.list(name="pulptest/oci-net.fishsoup.hello").results[0] - tag = container_tag_api.list(repository_version=repo.latest_version_href).results[0] - manifest = container_manifest_api.read(tag.tagged_manifest) - - assert manifest.is_flatpak - assert not manifest.is_bootable - +def run_flatpak_commands(pulp_settings): # Install flatpak: subprocess.check_call( [ @@ -43,7 +14,7 @@ def test_flatpak_install( "--user", "remote-add", "pulptest", - "oci+" + settings.CONTENT_ORIGIN, + "oci+" + pulp_settings.CONTENT_ORIGIN, ] ) # See @@ -81,3 +52,86 @@ def test_flatpak_install( ] ) subprocess.run(["flatpak", "--user", "remote-delete", "pulptest"]) + + +def test_flatpak_install( + add_to_cleanup, + registry_client, + local_registry, + container_namespace_api, + container_push_repository_api, + container_tag_api, + container_manifest_api, + pulp_settings, +): + if not pulp_settings.FLATPAK_INDEX: + pytest.skip("This test requires FLATPAK_INDEX to be enabled") + + image_path1 = f"{REGISTRY_V2}/pulp/oci-net.fishsoup.busyboxplatform:latest" + registry_client.pull(image_path1) + local_registry.tag_and_push(image_path1, "pulptest/oci-net.fishsoup.busyboxplatform:latest") + image_path2 = f"{REGISTRY_V2}/pulp/oci-net.fishsoup.hello:latest" + registry_client.pull(image_path2) + local_registry.tag_and_push(image_path2, "pulptest/oci-net.fishsoup.hello:latest") + namespace = container_namespace_api.list(name="pulptest").results[0] + add_to_cleanup(container_namespace_api, namespace.pulp_href) + + repo = container_push_repository_api.list(name="pulptest/oci-net.fishsoup.hello").results[0] + tag = container_tag_api.list(repository_version=repo.latest_version_href).results[0] + manifest = container_manifest_api.read(tag.tagged_manifest) + + assert manifest.is_flatpak + assert not manifest.is_bootable + + run_flatpak_commands(pulp_settings) + + +def test_flatpak_on_demand( + container_tag_api, + container_manifest_api, + container_repository_factory, + container_remote_factory, + container_sync, + container_distribution_factory, + container_namespace_api, + pulpcore_bindings, + monitor_task, + add_to_cleanup, + pulp_settings, +): + if not pulp_settings.FLATPAK_INDEX: + pytest.skip("This test requires FLATPAK_INDEX to be enabled") + + # Set up repositories with immediate sync + remote1 = container_remote_factory( + upstream_name="pulp/oci-net.fishsoup.busyboxplatform", include_tags=["latest"] + ) + remote2 = container_remote_factory( + upstream_name="pulp/oci-net.fishsoup.hello", include_tags=["latest"] + ) + repo1 = container_repository_factory(remote=remote1.pulp_href) + repo2 = container_repository_factory(remote=remote2.pulp_href) + container_sync(repo1) + container_sync(repo2) + container_distribution_factory( + base_path="pulptest/oci-net.fishsoup.busyboxplatform", repository=repo1.pulp_href + ) + container_distribution_factory( + base_path="pulptest/oci-net.fishsoup.hello", repository=repo2.pulp_href + ) + namespace = container_namespace_api.list(name="pulptest").results[0] + add_to_cleanup(container_namespace_api, namespace.pulp_href) + + # Assert the repos were set up correctly + tag = container_tag_api.list(repository_version=f"{repo2.versions_href}1/").results[0] + manifest = container_manifest_api.read(tag.tagged_manifest) + assert manifest.is_flatpak + assert not manifest.is_bootable + + # reclaim disk space to turn the manifests + config-blogs into on-demand + reclaim_response = pulpcore_bindings.RepositoriesReclaimSpaceApi.reclaim( + {"repo_hrefs": [repo1.pulp_href, repo2.pulp_href]} + ) + monitor_task(reclaim_response.task) + + run_flatpak_commands(pulp_settings) diff --git a/pulp_container/tests/functional/conftest.py b/pulp_container/tests/functional/conftest.py index 9e926b688..a342d3749 100644 --- a/pulp_container/tests/functional/conftest.py +++ b/pulp_container/tests/functional/conftest.py @@ -434,6 +434,17 @@ def _sync(repo, remote=None): return _sync +@pytest.fixture +def container_distribution_factory(container_distribution_api, gen_object_with_cleanup): + def _container_distribution_factory(**kwargs): + distro = {"name": str(uuid4()), "base_path": str(uuid4())} + if kwargs: + distro.update(kwargs) + return gen_object_with_cleanup(container_distribution_api, distro) + + return _container_distribution_factory + + @pytest.fixture def pull_through_distribution( gen_object_with_cleanup,