From b266b1915ba30d2e258723845e9193cac592d344 Mon Sep 17 00:00:00 2001 From: git-hyagi <45576767+git-hyagi@users.noreply.github.com> Date: Mon, 7 Oct 2024 20:13:24 -0300 Subject: [PATCH] Add the `type` field to the Manifest model closes: #1751 --- CHANGES/1751.feature | 1 + .../commands/container-handle-image-data.py | 24 ++++-- .../0042_add_manifest_type_field.py | 18 +++++ pulp_container/app/models.py | 76 ++++++++++++++++--- pulp_container/app/registry_api.py | 2 +- pulp_container/app/serializers.py | 6 ++ pulp_container/app/tasks/builder.py | 1 + pulp_container/app/tasks/sync_stages.py | 2 + pulp_container/constants.py | 13 ++++ .../tests/functional/api/test_build_images.py | 5 ++ .../functional/api/test_pull_through_cache.py | 8 ++ .../tests/functional/api/test_push_content.py | 10 ++- .../tests/functional/api/test_sync.py | 26 ++++++- pulp_container/tests/functional/conftest.py | 13 ++++ 14 files changed, 183 insertions(+), 22 deletions(-) create mode 100644 CHANGES/1751.feature create mode 100644 pulp_container/app/migrations/0042_add_manifest_type_field.py diff --git a/CHANGES/1751.feature b/CHANGES/1751.feature new file mode 100644 index 000000000..da6ef7d42 --- /dev/null +++ b/CHANGES/1751.feature @@ -0,0 +1 @@ +Added the `type` field to help differentiate Manifests. diff --git a/pulp_container/app/management/commands/container-handle-image-data.py b/pulp_container/app/management/commands/container-handle-image-data.py index a8635f1e7..53c118933 100644 --- a/pulp_container/app/management/commands/container-handle-image-data.py +++ b/pulp_container/app/management/commands/container-handle-image-data.py @@ -39,10 +39,14 @@ class Command(BaseCommand): def handle(self, *args, **options): manifests_updated_count = 0 - manifests_v1 = Manifest.objects.filter(data__isnull=True, media_type=MEDIA_TYPE.MANIFEST_V1) + manifests_v1 = Manifest.objects.filter( + Q(media_type=MEDIA_TYPE.MANIFEST_V1), Q(data__isnull=True) | Q(type__isnull=True) + ) manifests_updated_count += self.update_manifests(manifests_v1) - manifests_v2 = Manifest.objects.filter(Q(data__isnull=True) | Q(annotations={}, labels={})) + manifests_v2 = Manifest.objects.filter( + Q(data__isnull=True) | Q(annotations={}, labels={}) | Q(type__isnull=True) + ) manifests_v2 = manifests_v2.exclude( media_type__in=[MEDIA_TYPE.MANIFEST_LIST, MEDIA_TYPE.INDEX_OCI, MEDIA_TYPE.MANIFEST_V1] ) @@ -68,6 +72,15 @@ def handle(self, *args, **options): def update_manifests(self, manifests_qs): manifests_updated_count = 0 manifests_to_update = [] + fields_to_update = [ + "annotations", + "labels", + "is_bootable", + "is_flatpak", + "data", + "type", + ] + for manifest in manifests_qs.iterator(): # suppress non-existing/already migrated artifacts and corrupted JSON files with suppress(ObjectDoesNotExist, JSONDecodeError): @@ -76,7 +89,6 @@ def update_manifests(self, manifests_qs): manifests_to_update.append(manifest) if len(manifests_to_update) > 1000: - fields_to_update = ["annotations", "labels", "is_bootable", "is_flatpak", "data"] manifests_qs.model.objects.bulk_update( manifests_to_update, fields_to_update, @@ -85,7 +97,6 @@ def update_manifests(self, manifests_qs): manifests_to_update.clear() if manifests_to_update: - fields_to_update = ["annotations", "labels", "is_bootable", "is_flatpak", "data"] manifests_qs.model.objects.bulk_update( manifests_to_update, fields_to_update, @@ -100,11 +111,12 @@ def init_manifest(self, manifest): manifest_data, raw_bytes_data = get_content_data(manifest_artifact) manifest.data = raw_bytes_data.decode("utf-8") - if not (manifest.annotations or manifest.labels): + if not (manifest.annotations or manifest.labels or manifest.type): manifest.init_metadata(manifest_data) manifest._artifacts.clear() - return True + elif not manifest.type: + return manifest.init_image_nature() return False diff --git a/pulp_container/app/migrations/0042_add_manifest_type_field.py b/pulp_container/app/migrations/0042_add_manifest_type_field.py new file mode 100644 index 000000000..72cb2d1b7 --- /dev/null +++ b/pulp_container/app/migrations/0042_add_manifest_type_field.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.16 on 2024-10-09 12:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('container', '0041_add_pull_through_pull_permissions'), + ] + + operations = [ + migrations.AddField( + model_name='manifest', + name='type', + field=models.CharField(null=True), + ), + ] diff --git a/pulp_container/app/models.py b/pulp_container/app/models.py index b96701c92..f9fcd400e 100644 --- a/pulp_container/app/models.py +++ b/pulp_container/app/models.py @@ -32,7 +32,7 @@ from . import downloaders from pulp_container.app.utils import get_content_data -from pulp_container.constants import MEDIA_TYPE, SIGNATURE_TYPE +from pulp_container.constants import MANIFEST_TYPE, MEDIA_TYPE, SIGNATURE_TYPE logger = getLogger(__name__) @@ -72,6 +72,7 @@ class Manifest(Content): digest (models.TextField): The manifest digest. schema_version (models.IntegerField): The manifest schema version. media_type (models.TextField): The manifest media type. + type (models.TextField): The manifest's type (flatpak, bootable, signature, etc.). data (models.TextField): The manifest's data in text format. annotations (models.JSONField): Metadata stored inside the image manifest. labels (models.JSONField): Metadata stored inside the image configuration. @@ -99,6 +100,7 @@ class Manifest(Content): digest = models.TextField(db_index=True) schema_version = models.IntegerField() media_type = models.TextField(choices=MANIFEST_CHOICES) + type = models.CharField(null=True) data = models.TextField(null=True) annotations = models.JSONField(default=dict) @@ -154,6 +156,11 @@ def init_image_nature(self): return self.init_manifest_nature() def init_manifest_list_nature(self): + updated_type = False + if not self.type: + self.type = self.manifest_list_type() + updated_type = True + for manifest in self.listed_manifests.all(): # it suffices just to have a single manifest of a specific nature; # there is no case where the manifest is both bootable and flatpak-based @@ -164,17 +171,41 @@ def init_manifest_list_nature(self): self.is_flatpak = True return True - return False + return updated_type def init_manifest_nature(self): - if self.is_bootable_image(): - self.is_bootable = True - return True - elif self.is_flatpak_image(): - self.is_flatpak = True - return True - else: - return False + known_types = self.known_types() + for manifest_type in known_types.values(): + func = manifest_type["check_function"] + if func(): + self.type = manifest_type["type"] + + # set custom attributes (is_flatpak, is_bootable) to keep compatibility + if manifest_type.get("custom", None): + custom_key_name = list(manifest_type["custom"])[0] + custom_key_value = manifest_type["custom"][custom_key_name] + setattr(self, custom_key_name, custom_key_value) + + return True + + return False + + def known_types(self): + return { + "bootable": { + "check_function": self.is_bootable_image, + "type": MANIFEST_TYPE.BOOTABLE, + "custom": {"is_bootable": True}, + }, + "flatpak": { + "check_function": self.is_flatpak_image, + "type": MANIFEST_TYPE.FLATPAK, + "custom": {"is_flatpak": True}, + }, + "helm": {"check_function": self.is_helm_image, "type": MANIFEST_TYPE.HELM}, + "sign": {"check_function": self.is_cosign, "type": MANIFEST_TYPE.SIGNATURE}, + "image": {"check_function": self.is_manifest_image, "type": MANIFEST_TYPE.IMAGE}, + } def is_bootable_image(self): if ( @@ -188,6 +219,31 @@ def is_bootable_image(self): def is_flatpak_image(self): return True if self.labels.get("org.flatpak.ref") else False + def is_helm_image(self): + json_manifest = json.loads(self.data) + # schema1 does not have config, just return since it is deprecated + if not json_manifest.get("config", None): + return False + return json_manifest.get("config").get("mediaType") == MEDIA_TYPE.HELM + + def is_manifest_image(self): + return self.media_type in (MEDIA_TYPE.MANIFEST_OCI, MEDIA_TYPE.MANIFEST_V2) + + def is_cosign(self): + json_manifest = json.loads(self.data) + # schema1 has fsLayers instead of layers, just return since it is deprecated + if not json_manifest.get("layers", None): + return False + return any( + layers.get("mediaType", None) == MEDIA_TYPE.COSIGN for layers in json_manifest["layers"] + ) + + def manifest_list_type(self): + if self.media_type == MEDIA_TYPE.MANIFEST_LIST: + return MANIFEST_TYPE.MANIFEST_LIST + if self.media_type == MEDIA_TYPE.INDEX_OCI: + return MANIFEST_TYPE.OCI_INDEX + class Meta: default_related_name = "%(app_label)s_%(model_name)s" unique_together = ("digest",) diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index 83c95e78e..d652e330b 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -1238,7 +1238,7 @@ def put(self, request, path, pk=None): # once relations for listed manifests are established, it is # possible to initialize the nature of the manifest list if manifest.init_manifest_list_nature(): - manifest.save(update_fields=["is_bootable", "is_flatpak"]) + manifest.save(update_fields=["is_bootable", "is_flatpak", "type"]) found_blobs = models.Blob.objects.filter( digest__in=found_manifests.values_list("blobs__digest"), diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index 1287e49fe..3d8666c84 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -64,6 +64,11 @@ class ManifestSerializer(NoArtifactContentSerializer): digest = serializers.CharField(help_text="sha256 of the Manifest file") schema_version = serializers.IntegerField(help_text="Manifest schema version") media_type = serializers.CharField(help_text="Manifest media type of the file") + type = serializers.CharField( + help_text="Manifest type (flatpak, bootable, signature, etc.).", + required=False, + default=None, + ) listed_manifests = DetailRelatedField( many=True, help_text="Manifests that are referenced by this Manifest List", @@ -116,6 +121,7 @@ class Meta: "labels", "is_bootable", "is_flatpak", + "type", ) model = models.Manifest diff --git a/pulp_container/app/tasks/builder.py b/pulp_container/app/tasks/builder.py index 80d87b339..0739242aa 100644 --- a/pulp_container/app/tasks/builder.py +++ b/pulp_container/app/tasks/builder.py @@ -83,6 +83,7 @@ def add_image_from_directory_to_repository(path, repository, tag): with repository.new_version() as new_repo_version: manifest_json = json.loads(manifest_text_data) + manifest.init_metadata(manifest_json) config_blob = get_or_create_blob(manifest_json["config"], manifest, path) manifest.config_blob = config_blob diff --git a/pulp_container/app/tasks/sync_stages.py b/pulp_container/app/tasks/sync_stages.py index 15ade2596..57f607599 100644 --- a/pulp_container/app/tasks/sync_stages.py +++ b/pulp_container/app/tasks/sync_stages.py @@ -366,6 +366,7 @@ def create_manifest_list(self, manifest_list_data, raw_text_data, media_type, di data=raw_text_data, ) + manifest_list.type = manifest_list.manifest_list_type() manifest_list_dc = DeclarativeContent(content=manifest_list) manifest_list_dc.extra_data["listed_manifests"] = [] return manifest_list_dc @@ -392,6 +393,7 @@ def create_manifest(self, manifest_data, raw_text_data, media_type, digest=None) annotations=manifest_data.get("annotations", {}), ) + manifest.init_manifest_nature() manifest_dc = DeclarativeContent(content=manifest) return manifest_dc diff --git a/pulp_container/constants.py b/pulp_container/constants.py index 8d6463481..78426de96 100644 --- a/pulp_container/constants.py +++ b/pulp_container/constants.py @@ -19,6 +19,8 @@ FOREIGN_BLOB_OCI_TAR_GZIP="application/vnd.oci.image.layer.nondistributable.v1.tar+gzip", FOREIGN_BLOB_OCI_TAR_ZSTD="application/vnd.oci.image.layer.nondistributable.v1.tar+zstd", OCI_EMPTY_JSON="application/vnd.oci.empty.v1+json", + HELM="application/vnd.cncf.helm.config.v1+json", + COSIGN="application/vnd.dev.cosign.simplesigning.v1+json", ) V2_ACCEPT_HEADERS = { @@ -71,3 +73,14 @@ SIGNATURE_PAYLOAD_MAX_SIZE = 4 * MEGABYTE SIGNATURE_API_EXTENSION_VERSION = 2 + +MANIFEST_TYPE = SimpleNamespace( + IMAGE="image", + BOOTABLE="bootable", + FLATPAK="flatpak", + HELM="helm", + OCI_INDEX="oci-index", + MANIFEST_LIST="manifestlist", + SIGNATURE="signature", + UNKNOWN="unknown", +) diff --git a/pulp_container/tests/functional/api/test_build_images.py b/pulp_container/tests/functional/api/test_build_images.py index 00574123d..3980246b0 100644 --- a/pulp_container/tests/functional/api/test_build_images.py +++ b/pulp_container/tests/functional/api/test_build_images.py @@ -7,6 +7,7 @@ from pulp_smash.pulp3.bindings import monitor_task from pulpcore.client.pulp_container import ApiException, ContainerContainerDistribution +from pulp_container.constants import MANIFEST_TYPE @pytest.fixture @@ -62,6 +63,7 @@ def _build_image(repository, containerfile=None, containerfile_name=None, build_ def test_build_image_with_uploaded_containerfile( build_image, + check_manifest_fields, containerfile_name, container_distribution_api, container_repo, @@ -85,6 +87,9 @@ def test_build_image_with_uploaded_containerfile( local_registry.pull(distribution.base_path) image = local_registry.inspect(distribution.base_path) assert image[0]["Config"]["Cmd"] == ["cat", "/tmp/inside-image.txt"] + assert check_manifest_fields( + manifest_filters={"digest": image[0]["Digest"]}, fields={"type": MANIFEST_TYPE.IMAGE} + ) def test_build_image_from_repo_version_with_anon_user( diff --git a/pulp_container/tests/functional/api/test_pull_through_cache.py b/pulp_container/tests/functional/api/test_pull_through_cache.py index f4f6ac01d..3eb170e50 100644 --- a/pulp_container/tests/functional/api/test_pull_through_cache.py +++ b/pulp_container/tests/functional/api/test_pull_through_cache.py @@ -7,6 +7,7 @@ from subprocess import CalledProcessError from uuid import uuid4 +from pulp_container.constants import MANIFEST_TYPE from pulp_container.tests.functional.constants import ( REGISTRY_V2, PULP_HELLO_WORLD_REPO, @@ -38,6 +39,7 @@ def _add_pull_through_entities_to_cleanup(path): def pull_and_verify( anonymous_user, add_pull_through_entities_to_cleanup, + check_manifest_fields, container_pull_through_distribution_api, container_distribution_api, container_repository_api, @@ -59,6 +61,12 @@ def _pull_and_verify(images, pull_through_distribution): local_registry.pull(local_image_path) local_image = local_registry.inspect(local_image_path) + # 1.1. check pulp manifest model fields + assert check_manifest_fields( + manifest_filters={"digest": local_image[0]["Digest"]}, + fields={"type": MANIFEST_TYPE.IMAGE}, + ) + path, tag = local_image_path.split(":") tags_to_verify.append(tag) diff --git a/pulp_container/tests/functional/api/test_push_content.py b/pulp_container/tests/functional/api/test_push_content.py index baf659f62..a08f908dd 100644 --- a/pulp_container/tests/functional/api/test_push_content.py +++ b/pulp_container/tests/functional/api/test_push_content.py @@ -17,7 +17,7 @@ PulpTestCase, ) -from pulp_container.constants import MEDIA_TYPE +from pulp_container.constants import MEDIA_TYPE, MANIFEST_TYPE from pulp_container.tests.functional.api import rbac_base from pulp_container.tests.functional.constants import REGISTRY_V2_REPO_PULP @@ -39,6 +39,7 @@ def test_push_using_registry_client_admin( add_to_cleanup, registry_client, local_registry, + check_manifest_fields, container_namespace_api, ): """Test push with official registry client and logged in as admin.""" @@ -48,6 +49,13 @@ def test_push_using_registry_client_admin( registry_client.pull(image_path) local_registry.tag_and_push(image_path, local_url) local_registry.pull(local_url) + + # check pulp manifest model fields + local_image = local_registry.inspect(local_url) + assert check_manifest_fields( + manifest_filters={"digest": local_image[0]["Digest"]}, fields={"type": MANIFEST_TYPE.IMAGE} + ) + # ensure that same content can be pushed twice without permission errors local_registry.tag_and_push(image_path, local_url) diff --git a/pulp_container/tests/functional/api/test_sync.py b/pulp_container/tests/functional/api/test_sync.py index bfb088fdd..d2d534fe7 100644 --- a/pulp_container/tests/functional/api/test_sync.py +++ b/pulp_container/tests/functional/api/test_sync.py @@ -3,9 +3,11 @@ import pytest from pulpcore.tests.functional.utils import PulpTaskError -from pulp_container.tests.functional.constants import PULP_FIXTURE_1, PULP_LABELED_FIXTURE - +from pulp_container.constants import MEDIA_TYPE, MANIFEST_TYPE from pulp_container.tests.functional.constants import ( + PULP_FIXTURE_1, + PULP_LABELED_FIXTURE, + PULP_HELLO_WORLD_LINUX_AMD64_DIGEST, REGISTRY_V2_FEED_URL, ) @@ -39,13 +41,29 @@ def _synced_container_repository_factory( @pytest.mark.parallel -def test_basic_sync(container_repo, container_remote, container_repository_api, container_sync): - container_sync(container_repo, container_remote) +def test_basic_sync( + check_manifest_fields, + container_repo, + container_remote, + container_repository_api, + container_sync, +): + repo_version = container_sync(container_repo, container_remote).created_resources[0] repository = container_repository_api.read(container_repo.pulp_href) assert "versions/1/" in repository.latest_version_href latest_version_href = repository.latest_version_href + + assert check_manifest_fields( + manifest_filters={ + "repository_version": repo_version, + "media_type": [MEDIA_TYPE.MANIFEST_V2], + "digest": PULP_HELLO_WORLD_LINUX_AMD64_DIGEST, + }, + fields={"type": MANIFEST_TYPE.IMAGE}, + ) + container_sync( container_repo, container_remote ) # We expect that this second sync doesn't create a new repo version diff --git a/pulp_container/tests/functional/conftest.py b/pulp_container/tests/functional/conftest.py index 4152eedeb..bcae14793 100644 --- a/pulp_container/tests/functional/conftest.py +++ b/pulp_container/tests/functional/conftest.py @@ -461,3 +461,16 @@ def _pull_through_distribution(includes=None, excludes=None, private=False): return distribution return _pull_through_distribution + + +@pytest.fixture +def check_manifest_fields(container_manifest_api): + def _check_manifest_fields(**kwargs): + manifest = container_manifest_api.list(**kwargs["manifest_filters"]) + manifest = manifest.to_dict()["results"][0] + for key in kwargs["fields"]: + if manifest[key] != kwargs["fields"][key]: + return False + return True + + return _check_manifest_fields