Skip to content
Open
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
3 changes: 3 additions & 0 deletions CHANGES/7550.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added an `overwrite` boolean parameter to the repository content modify and content upload
endpoints. When set to `false`, the operation will return a 409 Conflict error if the content
being added would overwrite existing content based on `repo_key_fields`. Defaults to `true`.
224 changes: 224 additions & 0 deletions pulp_file/tests/functional/api/test_overwrite_content.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
"""Tests for the repository content overwrite protection feature."""

import uuid

import pytest

from pulpcore.client.pulp_file.exceptions import ApiException
from pulpcore.tests.functional.utils import PulpTaskError


@pytest.mark.parallel
def test_modify_overwrite_false_rejects_overwrite(
file_bindings,
file_repo,
file_content_unit_with_name_factory,
monitor_task,
):
"""Adding content with a conflicting relative_path and overwrite=False returns a 409."""
shared_path = str(uuid.uuid4())

# Create two content units that share the same relative_path but have different artifacts
content_a = file_content_unit_with_name_factory(shared_path)
content_b = file_content_unit_with_name_factory(shared_path)
assert content_a.pulp_href != content_b.pulp_href

# Add the first content unit (overwrite is irrelevant for an empty repo)
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_a.pulp_href]},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# Attempt to add the second content unit with overwrite=False — expect HTTP 409
with pytest.raises(ApiException) as exc_info:
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_b.pulp_href], "overwrite": False},
)
assert exc_info.value.status == 409
assert "PLP0023" in exc_info.value.body

# Repo version should not have advanced
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")


@pytest.mark.parallel
def test_modify_overwrite_false_allows_non_conflicting(
file_bindings,
file_repo,
file_content_unit_with_name_factory,
monitor_task,
):
"""Adding content without a conflicting relative_path and overwrite=False succeeds."""
content_a = file_content_unit_with_name_factory(str(uuid.uuid4()))
content_b = file_content_unit_with_name_factory(str(uuid.uuid4()))

# Add the first content unit
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_a.pulp_href], "overwrite": False},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# Add non-conflicting content with overwrite=False — should succeed
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_b.pulp_href], "overwrite": False},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/2/")


@pytest.mark.parallel
def test_modify_default_allows_overwrite(
file_bindings,
file_repo,
file_content_unit_with_name_factory,
monitor_task,
):
"""Default modify (overwrite=True) still allows overwriting content."""
shared_path = str(uuid.uuid4())
content_a = file_content_unit_with_name_factory(shared_path)
content_b = file_content_unit_with_name_factory(shared_path)

monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_a.pulp_href]},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# With default overwrite=True, adding conflicting content succeeds
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_b.pulp_href]},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/2/")


@pytest.mark.parallel
def test_modify_overwrite_false_allows_replace_when_removing_conflict(
file_bindings,
file_repo,
file_content_unit_with_name_factory,
monitor_task,
):
"""Replacing content with overwrite=False succeeds when the conflicting unit is removed."""
shared_path = str(uuid.uuid4())
content_a = file_content_unit_with_name_factory(shared_path)
content_b = file_content_unit_with_name_factory(shared_path)

# Seed the repo with content_a
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{"add_content_units": [content_a.pulp_href]},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# Remove content_a and add content_b in the same call with overwrite=False — should succeed
monitor_task(
file_bindings.RepositoriesFileApi.modify(
file_repo.pulp_href,
{
"remove_content_units": [content_a.pulp_href],
"add_content_units": [content_b.pulp_href],
"overwrite": False,
},
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/2/")


@pytest.mark.parallel
def test_content_upload_overwrite_false_rejects_overwrite(
file_bindings,
file_repo,
random_artifact_factory,
monitor_task,
):
"""Uploading content with overwrite=False and a conflicting relative_path is rejected."""
shared_path = str(uuid.uuid4())

# Upload first content unit into the repo
artifact_a = random_artifact_factory()
monitor_task(
file_bindings.ContentFilesApi.create(
relative_path=shared_path,
artifact=artifact_a.pulp_href,
repository=file_repo.pulp_href,
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# Attempt to upload a second content unit with the same relative_path and overwrite=False
artifact_b = random_artifact_factory()
with pytest.raises(PulpTaskError) as exc_info:
monitor_task(
file_bindings.ContentFilesApi.create(
relative_path=shared_path,
artifact=artifact_b.pulp_href,
repository=file_repo.pulp_href,
overwrite=False,
).task
)
assert exc_info.value.task.error["description"]
assert "PLP0023" in exc_info.value.task.error["description"]

# Repo version should not have advanced
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")


@pytest.mark.parallel
def test_content_upload_overwrite_false_allows_non_conflicting(
file_bindings,
file_repo,
random_artifact_factory,
monitor_task,
):
"""Uploading content with overwrite=False and no conflict succeeds."""
# Upload first content unit into the repo with overwrite=False
artifact_a = random_artifact_factory()
monitor_task(
file_bindings.ContentFilesApi.create(
relative_path=str(uuid.uuid4()),
artifact=artifact_a.pulp_href,
repository=file_repo.pulp_href,
overwrite=False,
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/1/")

# Upload a second content unit with a different relative_path
artifact_b = random_artifact_factory()
monitor_task(
file_bindings.ContentFilesApi.create(
relative_path=str(uuid.uuid4()),
artifact=artifact_b.pulp_href,
repository=file_repo.pulp_href,
overwrite=False,
).task
)
repo = file_bindings.RepositoriesFileApi.read(file_repo.pulp_href)
assert repo.latest_version_href.endswith("/versions/2/")
69 changes: 68 additions & 1 deletion pulpcore/app/models/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
)
from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE
from pulpcore.download.factory import DownloaderFactory
from pulpcore.exceptions import ResourceImmutableError
from pulpcore.exceptions import ContentOverwriteError, ResourceImmutableError

from pulpcore.cache import Cache

Expand Down Expand Up @@ -242,6 +242,73 @@ def finalize_new_version(self, new_version):
"""
pass

def check_content_overwrite(self, version, add_content_pks, remove_content_pks=None):
"""
Check that content being added would not overwrite existing repository content.

Uses repo_key_fields to determine if any content to be added conflicts with content
already present in the version. Raises ContentOverwriteError (HTTP 409) if a conflict
is detected.

Plugin writers may override this method to customize overwrite-check behavior.

Args:
version (pulpcore.app.models.RepositoryVersion): The version to check against.
add_content_pks (list): List of primary keys for Content to be added.
remove_content_pks (list): Optional list of primary keys for Content being removed.
These are excluded from the conflict check.
"""
existing_content = version.content
if remove_content_pks:
existing_content = existing_content.exclude(pk__in=remove_content_pks)
if not existing_content.exists():
return

add_content = Content.objects.filter(pk__in=add_content_pks)

for type_obj in self.CONTENT_TYPES:
repo_key_fields = type_obj.repo_key_fields
if not repo_key_fields:
continue

pulp_type = type_obj.get_pulp_type()
incoming_qs = type_obj.objects.filter(pk__in=add_content.filter(pulp_type=pulp_type))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we have this pattern in our repo-version utils file, but I wonder if it is more performant than a normal type_obj.objects.filter(pk__in=add_content). Theoretically it should filter down the number of content to check to only those of the correct type, but does it actually increase the speed?

if not incoming_qs.exists():
continue

for batch in batch_qs(incoming_qs.values(*repo_key_fields)):
find_dup_qs = Q()
for content_dict in batch:
find_dup_qs |= Q(**content_dict)

existing_dups = (
type_obj.objects.filter(pk__in=existing_content)
.filter(find_dup_qs)
.values("pk", *repo_key_fields)
)
if not existing_dups.exists():
continue

existing_by_key = {
tuple(row[f] for f in repo_key_fields): row["pk"] for row in existing_dups
}
incoming_by_key = {
tuple(row[f] for f in repo_key_fields): row["pk"]
for row in incoming_qs.filter(find_dup_qs).values("pk", *repo_key_fields)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this query if we add pk to the batch_qs(incoming_qs.values(*repo_key_fields) at the top, then create this dictionary during the assembling of the find_dup_qs.

}
conflict_map = {
str(incoming_by_key[key]): str(existing_by_key[key])
for key in incoming_by_key
if key in existing_by_key
}
if conflict_map:
_logger.info(
"Content overwrite conflict: type=%s, incoming->existing=%s",
pulp_type,
conflict_map,
)
raise ContentOverwriteError(pulp_type, conflict_map)

def latest_version(self):
"""
Get the latest RepositoryVersion on a repository
Expand Down
20 changes: 19 additions & 1 deletion pulpcore/app/serializers/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ class NoArtifactContentSerializer(base.ModelSerializer):
view_name_pattern=r"repositories(-.*/.*)-detail",
queryset=models.Repository.objects.all(),
)
overwrite = serializers.BooleanField(
required=False,
default=True,
write_only=True,
help_text=_(
"When set to true, existing content in the repository that conflicts based on "
"repo_key_fields will be silently overwritten. When set to false, the upload will "
"return a 409 Conflict error if content would be overwritten. Only used when "
"'repository' is specified. Defaults to true."
),
)
vuln_report = RelatedField(
read_only=True,
view_name="vuln_report-detail",
Expand Down Expand Up @@ -77,6 +88,7 @@ def create(self, validated_data):
validated_data (dict): Data to save to the database
"""
repository = validated_data.pop("repository", None)
overwrite = validated_data.pop("overwrite", True)
artifacts = self.get_artifacts(validated_data)

content = self.retrieve(validated_data)
Expand Down Expand Up @@ -113,9 +125,14 @@ def create(self, validated_data):
)

if repository:
repository.cast()
repository = repository.cast()
content_to_add = self.Meta.model.objects.filter(pk=content.pk)

if not overwrite:
version = repository.latest_version()
if version:
repository.check_content_overwrite(version, [content.pk])

# create new repo version with uploaded package
with repository.new_version() as new_version:
new_version.add_content(content_to_add)
Expand All @@ -126,6 +143,7 @@ class Meta:
model = models.Content
fields = base.ModelSerializer.Meta.fields + (
"repository",
"overwrite",
"pulp_labels",
"vuln_report",
)
Expand Down
11 changes: 10 additions & 1 deletion pulpcore/app/serializers/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,15 @@ class RepositoryAddRemoveContentSerializer(ModelSerializer, NestedHyperlinkedMod
"for the new repository version"
),
)
overwrite = serializers.BooleanField(
required=False,
default=True,
help_text=_(
"When set to true, existing content in the repository that conflicts based on "
"repo_key_fields will be silently overwritten. When set to false, the update will "
"return a 409 Conflict error if content would be overwritten. Defaults to true."
),
)

def validate_add_content_units(self, value):
add_content_units = {}
Expand Down Expand Up @@ -366,4 +375,4 @@ def validate_remove_content_units(self, value):

class Meta:
model = models.RepositoryVersion
fields = ["add_content_units", "remove_content_units", "base_version"]
fields = ["add_content_units", "remove_content_units", "base_version", "overwrite"]
1 change: 1 addition & 0 deletions pulpcore/exceptions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
PublishError,
)
from .validation import (
ContentOverwriteError,
DigestValidationError,
InvalidSignatureError,
SizeValidationError,
Expand Down
Loading
Loading