diff --git a/CHANGES/7550.feature b/CHANGES/7550.feature new file mode 100644 index 0000000000..688e3f4c3e --- /dev/null +++ b/CHANGES/7550.feature @@ -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`. diff --git a/pulp_file/tests/functional/api/test_overwrite_content.py b/pulp_file/tests/functional/api/test_overwrite_content.py new file mode 100644 index 0000000000..7a850c3a76 --- /dev/null +++ b/pulp_file/tests/functional/api/test_overwrite_content.py @@ -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/") diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index a2350072ea..3aaabc525a 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -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 @@ -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)) + 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) + } + 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 diff --git a/pulpcore/app/serializers/content.py b/pulpcore/app/serializers/content.py index c011965ef0..252681b769 100644 --- a/pulpcore/app/serializers/content.py +++ b/pulpcore/app/serializers/content.py @@ -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", @@ -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) @@ -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) @@ -126,6 +143,7 @@ class Meta: model = models.Content fields = base.ModelSerializer.Meta.fields + ( "repository", + "overwrite", "pulp_labels", "vuln_report", ) diff --git a/pulpcore/app/serializers/repository.py b/pulpcore/app/serializers/repository.py index fbcf19b928..8730e60228 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -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 = {} @@ -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"] diff --git a/pulpcore/exceptions/__init__.py b/pulpcore/exceptions/__init__.py index 4c458e4393..1008eb1bc5 100644 --- a/pulpcore/exceptions/__init__.py +++ b/pulpcore/exceptions/__init__.py @@ -18,6 +18,7 @@ PublishError, ) from .validation import ( + ContentOverwriteError, DigestValidationError, InvalidSignatureError, SizeValidationError, diff --git a/pulpcore/exceptions/validation.py b/pulpcore/exceptions/validation.py index 2131e5fce5..09b2ab4db3 100644 --- a/pulpcore/exceptions/validation.py +++ b/pulpcore/exceptions/validation.py @@ -137,3 +137,26 @@ def __str__(self): "Found {n} duplicate contents in repository version" "(see the logs (cid={cid}) for details).".format(n=self.dup_count, cid=self.cid) ) + + +class ContentOverwriteError(PulpException): + """ + Raised when content would overwrite existing repository content and overwrite is disabled. + """ + + http_status_code = http.client.CONFLICT + error_code = "PLP0023" + + def __init__(self, pulp_type, conflict_map): + self.pulp_type = pulp_type + self.conflict_map = conflict_map + + def __str__(self): + pairs = ", ".join( + f"{incoming}->{existing}" for incoming, existing in self.conflict_map.items() + ) + return f"[{self.error_code}] " + _( + "Content overwrite rejected: new content of type '{pulp_type}' would overwrite " + "{n} existing content unit(s) in the repository based on repo_key_fields. " + "Conflicting content (incoming->existing): [{pairs}]" + ).format(pulp_type=self.pulp_type, n=len(self.conflict_map), pairs=pairs) diff --git a/pulpcore/plugin/actions.py b/pulpcore/plugin/actions.py index 7dc739d5e9..55060d907a 100644 --- a/pulpcore/plugin/actions.py +++ b/pulpcore/plugin/actions.py @@ -1,5 +1,7 @@ from drf_spectacular.utils import extend_schema +from rest_framework import status from rest_framework.decorators import action +from rest_framework.response import Response from pulpcore.app import tasks from pulpcore.app.models import RepositoryVersion @@ -8,6 +10,7 @@ AsyncOperationResponseSerializer, RepositoryAddRemoveContentSerializer, ) +from pulpcore.exceptions import ContentOverwriteError from pulpcore.tasking.tasks import dispatch __all__ = ["ModifyRepositoryActionMixin"] @@ -35,14 +38,33 @@ def modify(self, request, pk): else: base_version_pk = None + add_content_units = serializer.validated_data.get("add_content_units", []) + remove_content_units = serializer.validated_data.get("remove_content_units", []) + overwrite = serializer.validated_data.get("overwrite", True) + + if not overwrite and add_content_units: + version = ( + RepositoryVersion.objects.get(pk=base_version_pk) + if base_version_pk + else repository.latest_version() + ) + if version: + remove_pks = None if "*" in remove_content_units else remove_content_units + try: + repository.cast().check_content_overwrite( + version, add_content_units, remove_content_pks=remove_pks + ) + except ContentOverwriteError as e: + return Response({"detail": str(e)}, status=status.HTTP_409_CONFLICT) + task = dispatch( self.modify_task, exclusive_resources=[repository], kwargs={ "repository_pk": pk, "base_version_pk": base_version_pk, - "add_content_units": serializer.validated_data.get("add_content_units", []), - "remove_content_units": serializer.validated_data.get("remove_content_units", []), + "add_content_units": add_content_units, + "remove_content_units": remove_content_units, }, ) return OperationPostponedResponse(task, request) diff --git a/pulpcore/plugin/exceptions.py b/pulpcore/plugin/exceptions.py index 0406964ecd..1093b6c532 100644 --- a/pulpcore/plugin/exceptions.py +++ b/pulpcore/plugin/exceptions.py @@ -1,4 +1,5 @@ from pulpcore.exceptions import ( + ContentOverwriteError, ValidationError, DigestValidationError, InvalidSignatureError, @@ -15,6 +16,7 @@ ) __all__ = [ + "ContentOverwriteError", "ValidationError", "DigestValidationError", "InvalidSignatureError",