Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Containerfile as default if none provided #1804

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions pulp_container/app/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -798,16 +798,33 @@ 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:
raise serializers.ValidationError(
_("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"]
Expand All @@ -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:
Expand All @@ -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(
Expand All @@ -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

Expand Down
47 changes: 41 additions & 6 deletions pulp_container/tests/functional/api/test_build_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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