diff --git a/CHANGES/8041.bugfix b/CHANGES/8041.bugfix new file mode 100644 index 00000000000..a7280c399cc --- /dev/null +++ b/CHANGES/8041.bugfix @@ -0,0 +1 @@ +Fixed ``ArtifactFileField.pre_save`` spuriously rejecting artifact uploads whose filenames start with ``artifact`` when ``MEDIA_ROOT`` is empty (S3/Azure object-storage backends). The ``startswith`` guard now short-circuits when ``MEDIA_ROOT`` is empty, preventing false detection of fresh uploads as already-stored artifacts. diff --git a/pulpcore/app/models/fields.py b/pulpcore/app/models/fields.py index f6dcf31535f..032945109a5 100644 --- a/pulpcore/app/models/fields.py +++ b/pulpcore/app/models/fields.py @@ -70,7 +70,12 @@ def pre_save(self, model_instance, add): artifact_storage_path, os.path.join(settings.MEDIA_ROOT, artifact_storage_path), ] - is_in_artifact_storage = file.name.startswith(os.path.join(settings.MEDIA_ROOT, "artifact")) + # Guard against empty MEDIA_ROOT (object-storage backends such as S3/Azure set it to ""), + # where os.path.join("", "artifact") == "artifact" and any filename starting with + # "artifact" would be falsely detected as already residing in artifact storage. + is_in_artifact_storage = bool(settings.MEDIA_ROOT) and file.name.startswith( + os.path.join(settings.MEDIA_ROOT, "artifact") + ) if not already_in_place and is_in_artifact_storage: raise ValueError( diff --git a/pulpcore/tests/functional/api/test_artifact_presave_gh8041.py b/pulpcore/tests/functional/api/test_artifact_presave_gh8041.py new file mode 100644 index 00000000000..c7f167842f8 --- /dev/null +++ b/pulpcore/tests/functional/api/test_artifact_presave_gh8041.py @@ -0,0 +1,40 @@ +""" +Regression test for https://github.com/pulp/pulpcore/issues/8041 + +``ArtifactFileField.pre_save`` used a raw ``startswith`` against ``settings.MEDIA_ROOT`` +to detect files already in artifact storage. When ``MEDIA_ROOT`` is ``""`` +(S3/Azure object-storage backends), ``os.path.join("", "artifact") == "artifact"``, +so any upload whose filename started with ``"artifact"`` was falsely detected as +already-stored and raised ``ValueError`` → HTTP 500. +""" + +import os + +import pytest + + +@pytest.mark.parametrize( + "filename", + [ + "artifact-foo-1.0-1.noarch.rpm", + "artifact-bar.tar.gz", + ], +) +def test_artifact_upload_artifact_prefix_filename_when_media_root_empty( + pulpcore_bindings, tmp_path, pulp_settings, filename +): + """Upload a file whose name starts with ``artifact`` — must succeed on object-storage backends. + + This test only applies when ``MEDIA_ROOT`` is empty (object-storage backends such + as S3 and Azure). On filesystem backends the bug does not occur, so the test is + skipped to avoid false positives. + """ + if pulp_settings.MEDIA_ROOT: + pytest.skip("Bug GH-8041 only affects backends where MEDIA_ROOT is empty (S3/Azure).") + + temp_file = tmp_path / filename + temp_file.write_bytes(os.urandom(32)) + + # Before the fix this raised HTTP 500 (ValueError in ArtifactFileField.pre_save). + artifact = pulpcore_bindings.ArtifactsApi.create(str(temp_file)) + assert artifact.pulp_href is not None