From fc6d6059a906e2a92334d80be831a66b0a8588c0 Mon Sep 17 00:00:00 2001 From: git-hyagi <45576767+git-hyagi@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:54:03 -0300 Subject: [PATCH] Add repository_version param as a building context closes: #479 --- CHANGES/479.feature | 2 + pulp_container/app/serializers.py | 33 +---- pulp_container/app/tasks/builder.py | 36 +++--- pulp_container/app/viewsets.py | 6 +- .../tests/functional/api/test_build_images.py | 116 ++++++++++++++---- staging_docs/admin/guides/build-image.md | 25 ++-- 6 files changed, 134 insertions(+), 84 deletions(-) create mode 100644 CHANGES/479.feature diff --git a/CHANGES/479.feature b/CHANGES/479.feature new file mode 100644 index 000000000..9db7ce01f --- /dev/null +++ b/CHANGES/479.feature @@ -0,0 +1,2 @@ +Replaced `artifacts` by `repository_version` (i.e., a file plugin repository version HREF) +as the parameter to provide the build context for a Containerfile. diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index 3cac522bd..c2d831600 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -1,5 +1,4 @@ from gettext import gettext as _ -import os import re from django.core.validators import URLValidator @@ -750,11 +749,10 @@ class OCIBuildImageSerializer(ValidateFieldsMixin, serializers.Serializer): tag = serializers.CharField( required=False, default="latest", help_text="A tag name for the new image being built." ) - artifacts = serializers.JSONField( + repository_version = RepositoryVersionRelatedField( required=False, - help_text="A JSON string where each key is an artifact href and the value is it's " - "relative path (name) inside the /pulp_working_directory of the build container " - "executing the Containerfile.", + help_text=_("RepositoryVersion to be used as the build context for container images."), + allow_null=True, ) def __init__(self, *args, **kwargs): @@ -778,28 +776,7 @@ def validate(self, data): raise serializers.ValidationError( _("'containerfile' or 'containerfile_artifact' must " "be specified.") ) - artifacts = {} - if "artifacts" in data: - for url, relative_path in data["artifacts"].items(): - if os.path.isabs(relative_path): - raise serializers.ValidationError( - _("Relative path cannot start with '/'. " "{0}").format(relative_path) - ) - artifactfield = RelatedField( - view_name="artifacts-detail", - queryset=Artifact.objects.all(), - source="*", - initial=url, - ) - try: - artifact = artifactfield.run_validation(data=url) - artifact.touch() - artifacts[str(artifact.pk)] = relative_path - except serializers.ValidationError as e: - # Append the URL of missing Artifact to the error message - e.detail[0] = "%s %s" % (e.detail[0], url) - raise e - data["artifacts"] = artifacts + return data class Meta: @@ -808,7 +785,7 @@ class Meta: "containerfile", "repository", "tag", - "artifacts", + "repository_version", ) diff --git a/pulp_container/app/tasks/builder.py b/pulp_container/app/tasks/builder.py index 58daa3796..4dbf541c8 100644 --- a/pulp_container/app/tasks/builder.py +++ b/pulp_container/app/tasks/builder.py @@ -14,7 +14,7 @@ ) from pulp_container.constants import MEDIA_TYPE from pulp_container.app.utils import calculate_digest -from pulpcore.plugin.models import Artifact, ContentArtifact, Content +from pulpcore.plugin.models import Artifact, ContentArtifact, Content, RepositoryVersion def get_or_create_blob(layer_json, manifest, path): @@ -96,7 +96,7 @@ def add_image_from_directory_to_repository(path, repository, tag): def build_image_from_containerfile( - containerfile_pk=None, artifacts=None, repository_pk=None, tag=None + containerfile_pk=None, repo_version=None, repository_pk=None, tag=None ): """ Builds an OCI container image from a Containerfile. @@ -106,11 +106,10 @@ def build_image_from_containerfile( Args: containerfile_pk (str): The pk of an Artifact that contains the Containerfile - artifacts (dict): A dictionary where each key is an artifact PK and the value is it's - relative path (name) inside the /pulp_working_directory of the build - container executing the Containerfile. repository_pk (str): The pk of a Repository to add the OCI container image tag (str): Tag name for the new image in the repository + repo_version: The pk of a RepositoryVersion with the artifacts used in the build context + of the Containerfile. Returns: A class:`pulpcore.plugin.models.RepositoryVersion` that contains the new OCI container @@ -124,16 +123,16 @@ def build_image_from_containerfile( working_directory = os.path.abspath(working_directory) context_path = os.path.join(working_directory, "context") os.makedirs(context_path, exist_ok=True) - for key, val in artifacts.items(): - artifact = Artifact.objects.get(pk=key) - dest_path = os.path.join(context_path, val) - dirs = os.path.split(dest_path)[0] - if dirs: - os.makedirs(dirs, exist_ok=True) - with open(dest_path, "wb") as dest: - shutil.copyfileobj(artifact.file, dest) - containerfile_path = os.path.join(working_directory, "Containerfile") + if repo_version: + file_repository = RepositoryVersion.objects.get(pk=repo_version) + content_artifacts = ContentArtifact.objects.filter( + content__in=file_repository.content + ).order_by("-content__pulp_created") + for content in content_artifacts.select_related("artifact").iterator(): + _copy_file_from_artifact(context_path, content.relative_path, content.artifact.file) + + containerfile_path = os.path.join(working_directory, "Containerfile") with open(containerfile_path, "wb") as dest: shutil.copyfileobj(containerfile.file, dest) @@ -166,3 +165,12 @@ def build_image_from_containerfile( repository_version = add_image_from_directory_to_repository(image_dir, repository, tag) return repository_version + + +def _copy_file_from_artifact(context_path, relative_path, artifact): + dest_path = os.path.join(context_path, relative_path) + dirs = os.path.dirname(dest_path) + if dirs: + os.makedirs(dirs, exist_ok=True) + with open(dest_path, "wb") as dest: + shutil.copyfileobj(artifact.file, dest) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index e42be79f9..91d862346 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -696,6 +696,7 @@ class ContainerRepositoryViewSet( "condition": [ "has_model_or_obj_perms:container.build_image_containerrepository", "has_model_or_obj_perms:container.view_containerrepository", + "has_repo_or_repo_ver_param_model_or_obj_perms:file.view_filerepository", ], }, { @@ -946,8 +947,7 @@ def build_image(self, request, pk): containerfile.touch() tag = serializer.validated_data["tag"] - artifacts = serializer.validated_data["artifacts"] - Artifact.objects.filter(pk__in=artifacts.keys()).touch() + repo_version = serializer.validated_data.get("repository_version", None) result = dispatch( tasks.build_image_from_containerfile, @@ -956,7 +956,7 @@ def build_image(self, request, pk): "containerfile_pk": str(containerfile.pk), "tag": tag, "repository_pk": str(repository.pk), - "artifacts": artifacts, + "repo_version": repo_version.pk, }, ) return OperationPostponedResponse(result, request) diff --git a/pulp_container/tests/functional/api/test_build_images.py b/pulp_container/tests/functional/api/test_build_images.py index 5569dfa31..c8ad239a9 100644 --- a/pulp_container/tests/functional/api/test_build_images.py +++ b/pulp_container/tests/functional/api/test_build_images.py @@ -2,16 +2,10 @@ from tempfile import NamedTemporaryFile -from pulp_smash.pulp3.utils import ( - gen_distribution, - gen_repo, -) +from pulp_smash.pulp3.utils import gen_distribution from pulp_smash.pulp3.bindings import monitor_task -from pulpcore.client.pulp_container import ( - ContainerContainerDistribution, - ContainerContainerRepository, -) +from pulpcore.client.pulp_container import ApiException, ContainerContainerDistribution @pytest.fixture @@ -19,7 +13,7 @@ def containerfile_name(): """A fixture for a basic container file used for building images.""" with NamedTemporaryFile() as containerfile: containerfile.write( - b"""FROM busybox:latest + b"""FROM quay.io/quay/busybox:latest # Copy a file using COPY statement. Use the relative path specified in the 'artifacts' parameter. COPY foo/bar/example.txt /tmp/inside-image.txt # Print the content of the file when the container starts @@ -29,35 +23,103 @@ def containerfile_name(): yield containerfile.name +@pytest.fixture +def populated_file_repo( + file_bindings, + file_repo, + tmp_path_factory, +): + filename = tmp_path_factory.mktemp("fixtures") / "example.txt" + filename.write_bytes(b"test content") + upload_task = file_bindings.ContentFilesApi.create( + relative_path="foo/bar/example.txt", file=filename, repository=file_repo.pulp_href + ).task + monitor_task(upload_task) + + return file_repo + + +@pytest.fixture +def build_image(container_repository_api): + def _build_image(repository, containerfile, repo_version=None): + build_response = container_repository_api.build_image( + container_container_repository_href=repository, + containerfile=containerfile, + repository_version=repo_version, + ) + monitor_task(build_response.task) + + return _build_image + + def test_build_image( - pulpcore_bindings, - container_repository_api, + build_image, + containerfile_name, container_distribution_api, + container_repo, + populated_file_repo, + delete_orphans_pre, gen_object_with_cleanup, - containerfile_name, local_registry, ): - """Test if a user can build an OCI image.""" - with NamedTemporaryFile() as text_file: - text_file.write(b"some text") - text_file.flush() - artifact = gen_object_with_cleanup(pulpcore_bindings.ArtifactsApi, text_file.name) - - repository = gen_object_with_cleanup( - container_repository_api, ContainerContainerRepository(**gen_repo()) - ) - - artifacts = '{{"{}": "foo/bar/example.txt"}}'.format(artifact.pulp_href) - build_response = container_repository_api.build_image( - repository.pulp_href, containerfile=containerfile_name, artifacts=artifacts + """Test build an OCI image from a file repository_version.""" + build_image( + container_repo.pulp_href, + containerfile_name, + repo_version=f"{populated_file_repo.pulp_href}versions/1/", ) - monitor_task(build_response.task) distribution = gen_object_with_cleanup( container_distribution_api, - ContainerContainerDistribution(**gen_distribution(repository=repository.pulp_href)), + ContainerContainerDistribution(**gen_distribution(repository=container_repo.pulp_href)), ) local_registry.pull(distribution.base_path) image = local_registry.inspect(distribution.base_path) assert image[0]["Config"]["Cmd"] == ["cat", "/tmp/inside-image.txt"] + + +def test_build_image_from_repo_version_with_anon_user( + build_image, + containerfile_name, + container_repo, + populated_file_repo, + delete_orphans_pre, + gen_user, +): + """Test if a user without permission to file repo can build an OCI image.""" + user_helpless = gen_user( + model_roles=[ + "container.containerdistribution_collaborator", + "container.containerrepository_content_manager", + ] + ) + with user_helpless, pytest.raises(ApiException): + build_image( + container_repo.pulp_href, + containerfile_name, + repo_version=f"{populated_file_repo.pulp_href}versions/1/", + ) + + +def test_build_image_from_repo_version_with_creator_user( + build_image, + containerfile_name, + container_repo, + populated_file_repo, + delete_orphans_pre, + gen_user, +): + """Test if a user (with the expected permissions) can build an OCI image.""" + user = gen_user( + object_roles=[ + ("container.containerrepository_content_manager", container_repo.pulp_href), + ("file.filerepository_viewer", populated_file_repo.pulp_href), + ], + ) + with user: + build_image( + container_repo.pulp_href, + containerfile_name, + repo_version=f"{populated_file_repo.pulp_href}versions/1/", + ) diff --git a/staging_docs/admin/guides/build-image.md b/staging_docs/admin/guides/build-image.md index b71f69e9c..bdc8ac9cf 100644 --- a/staging_docs/admin/guides/build-image.md +++ b/staging_docs/admin/guides/build-image.md @@ -7,23 +7,24 @@ Users can add new images to a container repository by uploading a Containerfile. The syntax for Containerfile is the same as for a Dockerfile. The same REST API endpoint also accepts a JSON -string that maps artifacts in Pulp to a filename. Any artifacts passed in are available inside the +string that maps artifacts in Pulp to a filename. Any file passed in are available inside the build container at `/pulp_working_directory`. -## Create a Repository +## Create a Container Repository ```bash -REPO_HREF=$(pulp container repository create --name building | jq -r '.pulp_href') +CONTAINER_REPO=$(pulp container repository create --name building | jq -r '.pulp_href') ``` -## Create an Artifact +## Create a File Repository and populate it ```bash +FILE_REPO=$(pulp file repository create --name bar --autopublish | jq -r '.pulp_href') + echo 'Hello world!' > example.txt -ARTIFACT_HREF=$(http --form POST http://localhost/pulp/api/v3/artifacts/ \ - file@./example.txt \ - | jq -r '.pulp_href') +pulp file content upload --relative-path foo/bar/example.txt \ +--file ./example.txt --repository bar ``` ## Create a Containerfile @@ -41,12 +42,12 @@ CMD ["cat", "/inside-image.txt"]' >> Containerfile ## Build an OCI image ```bash -TASK_HREF=$(http --form POST :$REPO_HREF'build_image/' containerfile@./Containerfile \ -artifacts="{\"$ARTIFACT_HREF\": \"foo/bar/example.txt\"}" | jq -r '.task') +TASK_HREF=$(http --form POST :$CONTAINER_REPO'build_image/' "containerfile@./Containerfile" \ +repository_version=${FILE_REPO}versions/1/ | jq -r '.task') ``` + !!! warning - Non-staff users, lacking read access to the `artifacts` endpoint, may encounter restricted - functionality as they are prohibited from listing artifacts uploaded to Pulp and utilizing - them within the build process. + File repositories synced with on-demand remotes will not automatically pull the missing artifacts. + Trying to build using a file that is not yet pulled will fail.