diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index 6a767d857..4b8e46ce6 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -798,9 +798,12 @@ def validate(self, data): """Validates that all the fields make sense.""" data = super().validate(data) - if bool(data.get("containerfile", None)) == bool(data.get("containerfile_name", None)): + # only one of containerfile or containerfile_name should be provided, none of them is ok + # in case we have a build_context because Pulp will try to find a file called Containerfile + # in the build_context provided + if data.get("containerfile", None) and data.get("containerfile_name", None): raise serializers.ValidationError( - _("Exactly one of 'containerfile' or 'containerfile_name' must be specified.") + _("Only one of 'containerfile' or 'containerfile_name' must be specified.") ) if "containerfile_name" in data and "build_context" not in data: @@ -808,6 +811,20 @@ def validate(self, data): _("A 'build_context' must be specified when 'containerfile_name' is present.") ) + # with none of build_context nor containerfile_name nor containerfile + # there is not enough information to build + if not ( + data.get("containerfile", None) + or data.get("containerfile_name", None) + or data.get("build_context", None) + ): + raise serializers.ValidationError( + _( + "At least one of 'build_context' or 'containerfile' or 'containerfile_name' " + "must be provided" + ) + ) + # TODO: this can be removed after https://github.com/pulp/pulpcore/issues/5786 if data.get("build_context", None): data["repository_version"] = data["build_context"] @@ -821,6 +838,12 @@ def deferred_files_validation(self, data): """ if build_context := data.get("build_context", None): + data["build_context_pk"] = build_context.repository.pk + + # if a containerfile was passed with the build_context we can skip the following checks + if data.get("containerfile", None): + return data + # check if the on_demand_artifacts exist for on_demand_artifact in build_context.on_demand_artifacts.iterator(): if not on_demand_artifact.content_artifact.artifact: @@ -831,7 +854,7 @@ def deferred_files_validation(self, data): ) ) - # check if the containerfile_name exists in the build_context (File Repository) + # check if the provided containerfile_name exists in the build_context (File Repository) if ( data.get("containerfile_name", None) and not FileContent.objects.filter( @@ -847,7 +870,21 @@ def deferred_files_validation(self, data): ) ) - data["build_context_pk"] = build_context.repository.pk + # check if a Containerfile exists in the build_context when + # no containerfile_name is provided + if ( + not data.get("containerfile_name", None) + and not FileContent.objects.filter( + repositories__in=[build_context.repository.pk], + relative_path="Containerfile", + ).exists() + ): + raise serializers.ValidationError( + _("Could not find the Containerfile in the build_context provided") + ) + + if not data.get("containerfile_name", None): + data["containerfile_name"] = "Containerfile" return data diff --git a/pulp_container/tests/functional/api/test_build_images.py b/pulp_container/tests/functional/api/test_build_images.py index 3980246b0..7eb350cae 100644 --- a/pulp_container/tests/functional/api/test_build_images.py +++ b/pulp_container/tests/functional/api/test_build_images.py @@ -140,15 +140,31 @@ def test_build_image_from_repo_version_with_creator_user( def test_build_image_without_containerfile( build_image, + check_manifest_fields, + container_distribution_api, container_repo, + delete_orphans_pre, + gen_object_with_cleanup, + local_registry, populated_file_repo, ): - """Test build an OCI image without a containerfile""" - with pytest.raises(ApiException): - build_image( - repository=container_repo.pulp_href, - build_context=f"{populated_file_repo.pulp_href}versions/2/", - ) + """Test build an OCI image without explicitly passing a Containerfile""" + build_image( + repository=container_repo.pulp_href, + build_context=f"{populated_file_repo.pulp_href}versions/2/", + ) + + distribution = gen_object_with_cleanup( + container_distribution_api, + 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"] + assert check_manifest_fields( + manifest_filters={"digest": image[0]["Digest"]}, fields={"type": MANIFEST_TYPE.IMAGE} + ) def test_build_image_without_expected_files( @@ -237,3 +253,22 @@ def containerfile_without_context_files(): local_registry.pull(distribution.base_path) image = local_registry.inspect(distribution.base_path) assert image[0]["Config"]["Cmd"] == ["ls", "/"] + + +def test_with_containerfilename_and_containerfile( + build_image, + containerfile_name, + container_repo, + delete_orphans_pre, + populated_file_repo, +): + """Test build an OCI image with a containerfile and containerfile_name""" + with pytest.raises(ApiException) as e: + build_image( + repository=container_repo.pulp_href, + containerfile_name="Non_existing_file", + containerfile=containerfile_name, + build_context=f"{populated_file_repo.pulp_href}versions/2/", + ) + assert e.value.status == 400 + assert "Only one of 'containerfile' or 'containerfile_name' must be specified." in e.value.body