From 588b53e4ba66169e69376ce04fe15de4476276bc Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:21:38 -0400 Subject: [PATCH 1/6] fix(migrations): normalize invalid base_path values before domain type cast (issue #8067) Migration 0156 fails with a CheckViolation when any existing row in core_distribution has a base_path value that doesn't satisfy the relative_path domain constraint introduced in 0155. Add a RunPython step before the AlterField operation that: - Finds all rows whose base_path values would fail the constraint - Normalizes them (strips whitespace, leading/trailing slashes, double slashes, and bare dot/dotdot components) - Raises a RuntimeError with the offending rows listed if any path cannot be automatically fixed, allowing administrators to correct the data manually before retrying the migration Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ..._contentartifact_relative_path_and_more.py | 88 +++++++++++++++++-- 1 file changed, 83 insertions(+), 5 deletions(-) diff --git a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py index ce1d7bc03c..392e903305 100644 --- a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py +++ b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py @@ -1,28 +1,106 @@ # Generated by Django 5.2.15 on 2026-08-10 10:26 +import re + import django.contrib.postgres.indexes import django.contrib.postgres.operations import django.db.models.expressions import pulpcore.app.models.fields from django.db import migrations +# PostgreSQL regex matching paths that violate the relative_path domain constraint. +# Mirrors: '/' || VALUE || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)' +_INVALID_PATH_RE = r"[\n\r\s\t\?#]|(/\.{0,2}/)" + + +def _normalize_path(value): + """Return a normalized version of value that satisfies the relative_path constraint.""" + # Strip query string and URL fragment + value = re.sub(r"[?#].*$", "", value) + # Remove all whitespace and control characters + value = re.sub(r"[\n\r\t\s]+", "", value) + # Strip leading and trailing slashes + value = value.strip("/") + # Collapse runs of slashes + value = re.sub(r"/+", "/", value) + # Drop bare "." and ".." path components produced by the above steps + while True: + cleaned = re.sub(r"(?:^|/)\.\.?(?:/|$)", "/", value).strip("/") + cleaned = re.sub(r"/+", "/", cleaned) + if cleaned == value: + break + value = cleaned + return value + + +def fix_base_path_violations(apps, schema_editor): + """ + Normalize core_distribution.base_path values that would violate the + relative_path domain check constraint introduced in migration 0155. + + The constraint rejects paths where '/' || VALUE || '/' matches + '[\n\r\s\t\?#]|(/\.{0,2}/)' — i.e. paths containing whitespace, '?', '#', + double-slashes, or dot/dotdot segments. This function detects such rows, + normalizes them where possible, and raises RuntimeError listing any that + cannot be automatically fixed. + """ + from django.db import connection + + unfixable = [] + + with connection.cursor() as cursor: + cursor.execute( + "SELECT pulp_id, base_path FROM core_distribution" + " WHERE '/' || base_path || '/' ~ %s", + [_INVALID_PATH_RE], + ) + rows = cursor.fetchall() + + for pk, value in rows: + normalized = _normalize_path(value) + test = f"/{normalized}/" + if not normalized or re.search(r"[\n\r\s\t?#]|(/\.{0,2}/)", test): + unfixable.append(f" core_distribution.base_path (pk={pk!r}): {value!r}") + else: + with connection.cursor() as cursor: + cursor.execute( + "UPDATE core_distribution SET base_path = %s WHERE pulp_id = %s", + [normalized, pk], + ) + + if unfixable: + raise RuntimeError( + "The following core_distribution rows have base_path values that violate " + "the 'relative_path' domain constraint and could not be automatically " + "normalized. Fix or delete these records before running migrations:\n" + + "\n".join(unfixable) + ) + class Migration(migrations.Migration): atomic = False dependencies = [ - ('core', '0155_create_rel_path_domains'), + ("core", "0155_create_rel_path_domains"), ] operations = [ + migrations.RunPython( + fix_base_path_violations, + migrations.RunPython.noop, + ), migrations.AlterField( - model_name='distribution', - name='base_path', + model_name="distribution", + name="base_path", field=pulpcore.app.models.fields.RelativePathField(), ), django.contrib.postgres.operations.AddIndexConcurrently( - model_name='distribution', - index=django.contrib.postgres.indexes.SpGistIndex(django.contrib.postgres.indexes.OpClass("base_path", name='text_ops'), include=('pulp_domain',), name='core_distribution_base_path_text'), + model_name="distribution", + index=django.contrib.postgres.indexes.SpGistIndex( + django.contrib.postgres.indexes.OpClass("base_path", name="text_ops"), + include=("pulp_domain",), + name="core_distribution_base_path_text", + ), ), ] From c367a30820f3ef515896b6e5470bd48cade3d680 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:29:11 -0400 Subject: [PATCH 2/6] fix(migrations): add tests and remove unused import for migration 0156 fix - Remove unused 'import django.db.models.expressions' (ruff I001) - Add unit tests for _normalize_path and fix_base_path_violations: * Parametrized tests covering trailing/leading slashes, whitespace, query strings, fragments, dot/dotdot components, and clean paths * Mock-based tests for the database update path and unfixable path error handling Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ..._contentartifact_relative_path_and_more.py | 1 - .../tests/unit/models/test_0156_migration.py | 114 ++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 pulpcore/tests/unit/models/test_0156_migration.py diff --git a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py index 392e903305..44015a99cf 100644 --- a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py +++ b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py @@ -4,7 +4,6 @@ import django.contrib.postgres.indexes import django.contrib.postgres.operations -import django.db.models.expressions import pulpcore.app.models.fields from django.db import migrations diff --git a/pulpcore/tests/unit/models/test_0156_migration.py b/pulpcore/tests/unit/models/test_0156_migration.py new file mode 100644 index 0000000000..212c44e420 --- /dev/null +++ b/pulpcore/tests/unit/models/test_0156_migration.py @@ -0,0 +1,114 @@ +"""Unit tests for migration 0156's normalize helper and migration function.""" + +import importlib +from unittest.mock import MagicMock, call, patch + +import pytest + +_migration = importlib.import_module( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more" +) +_normalize_path = _migration._normalize_path +fix_base_path_violations = _migration.fix_base_path_violations + + +# --------------------------------------------------------------------------- +# _normalize_path — pure Python, no database required +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "value, expected", + [ + # trailing slash is stripped + ("fedora/", "fedora"), + # leading slash is stripped + ("/fedora", "fedora"), + # double slash is collapsed + ("fedora//el9", "fedora/el9"), + # newline is removed + ("fedora\nel9", "fedorael9"), + # space is removed + ("fedora el9", "fedorael9"), + # query string is stripped + ("fedora?foo=1", "fedora"), + # fragment is stripped + ("fedora#anchor", "fedora"), + # dot component is removed + ("fedora/./el9", "fedora/el9"), + # dotdot component is collapsed + ("fedora/../el9", "el9"), + # clean path is returned unchanged + ("fedora/el9/x86_64", "fedora/el9/x86_64"), + ], +) +def test_normalize_path(value, expected): + assert _normalize_path(value) == expected + + +# --------------------------------------------------------------------------- +# fix_base_path_violations — mock the DB cursor to avoid constraint conflicts +# --------------------------------------------------------------------------- + + +def _make_cursor(rows): + """Return a context-manager mock cursor that yields *rows* on fetchall().""" + cursor = MagicMock() + cursor.__enter__ = lambda s: s + cursor.__exit__ = MagicMock(return_value=False) + cursor.fetchall.return_value = rows + return cursor + + +def test_fix_base_path_violations_normalizes_row(monkeypatch): + """A row with a trailing-slash base_path is UPDATE-d to the normalized value.""" + pk = "some-uuid" + bad_path = "trailing/" + good_path = "trailing" + + select_cursor = _make_cursor([(pk, bad_path)]) + update_cursor = _make_cursor([]) + + cursors = iter([select_cursor, update_cursor]) + connection_mock = MagicMock() + connection_mock.cursor.side_effect = lambda: next(cursors) + + with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + fix_base_path_violations(None, None) + + update_cursor.execute.assert_called_once_with( + "UPDATE core_distribution SET base_path = %s WHERE pulp_id = %s", + [good_path, pk], + ) + + +def test_fix_base_path_violations_skips_clean_rows(monkeypatch): + """A row with a valid base_path is NOT UPDATE-d.""" + pk = "some-uuid" + clean_path = "fedora/el9" + + # SELECT returns no rows (clean_path doesn't match the invalid regex) + select_cursor = _make_cursor([]) + connection_mock = MagicMock() + connection_mock.cursor.return_value = select_cursor + + with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + fix_base_path_violations(None, None) + + # Only one cursor call (the SELECT), no UPDATE + assert connection_mock.cursor.call_count == 1 + + +def test_fix_base_path_violations_raises_for_unfixable_path(): + """A row whose path normalizes to empty string raises RuntimeError.""" + pk = "some-uuid" + # A bare "." normalizes to "" which is unfixable + unfixable_path = "." + + select_cursor = _make_cursor([(pk, unfixable_path)]) + connection_mock = MagicMock() + connection_mock.cursor.return_value = select_cursor + + with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + with pytest.raises(RuntimeError, match="could not be automatically normalized"): + fix_base_path_violations(None, None) From 8e25b7c77f6028626f2fd953c3465bc7c8e85f10 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:41:24 -0400 Subject: [PATCH 3/6] fix(migrations): move pulpcore import to second-party group per ruff isort config ruff isort treats pulpcore as 'second-party' with its own import group after third-party packages (django.*). Separate it from the Django imports with a blank line to satisfy the I001 rule. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../0156_alter_contentartifact_relative_path_and_more.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py index 44015a99cf..6caedc481f 100644 --- a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py +++ b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py @@ -4,9 +4,10 @@ import django.contrib.postgres.indexes import django.contrib.postgres.operations -import pulpcore.app.models.fields from django.db import migrations +import pulpcore.app.models.fields + # PostgreSQL regex matching paths that violate the relative_path domain constraint. # Mirrors: '/' || VALUE || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)' _INVALID_PATH_RE = r"[\n\r\s\t\?#]|(/\.{0,2}/)" From fd32e2aed40e25515df087557c6783e77e08e6e1 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:43:33 -0400 Subject: [PATCH 4/6] fix(tests): remove unused imports and variable in test_0156_migration - Remove unused 'call' import (ruff F401) - Remove unused 'pk'/'clean_path' variables from skips-clean-rows test (ruff F841) Co-Authored-By: Claude Sonnet 4.6 (1M context) --- pulpcore/tests/unit/models/test_0156_migration.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pulpcore/tests/unit/models/test_0156_migration.py b/pulpcore/tests/unit/models/test_0156_migration.py index 212c44e420..cc24a0f735 100644 --- a/pulpcore/tests/unit/models/test_0156_migration.py +++ b/pulpcore/tests/unit/models/test_0156_migration.py @@ -1,7 +1,7 @@ """Unit tests for migration 0156's normalize helper and migration function.""" import importlib -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import pytest @@ -84,10 +84,7 @@ def test_fix_base_path_violations_normalizes_row(monkeypatch): def test_fix_base_path_violations_skips_clean_rows(monkeypatch): """A row with a valid base_path is NOT UPDATE-d.""" - pk = "some-uuid" - clean_path = "fedora/el9" - - # SELECT returns no rows (clean_path doesn't match the invalid regex) + # SELECT returns no rows (valid paths don't match the invalid regex) select_cursor = _make_cursor([]) connection_mock = MagicMock() connection_mock.cursor.return_value = select_cursor From 1dd61f21fe58cd5dc86592f12706e0c0ced43042 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:44:27 -0400 Subject: [PATCH 5/6] fix(migrations): apply ruff format fixes to migration 0156 - Join SQL string onto single line (ruff format prefers no implicit concatenation) - Remove blank line after class declaration (ruff format) Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../0156_alter_contentartifact_relative_path_and_more.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py index 6caedc481f..da88264954 100644 --- a/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py +++ b/pulpcore/app/migrations/0156_alter_contentartifact_relative_path_and_more.py @@ -50,8 +50,7 @@ def fix_base_path_violations(apps, schema_editor): with connection.cursor() as cursor: cursor.execute( - "SELECT pulp_id, base_path FROM core_distribution" - " WHERE '/' || base_path || '/' ~ %s", + "SELECT pulp_id, base_path FROM core_distribution WHERE '/' || base_path || '/' ~ %s", [_INVALID_PATH_RE], ) rows = cursor.fetchall() @@ -78,7 +77,6 @@ def fix_base_path_violations(apps, schema_editor): class Migration(migrations.Migration): - atomic = False dependencies = [ From 9223259c4979ac19bb1af45b134225265f969f13 Mon Sep 17 00:00:00 2001 From: Dennis Kliban Date: Fri, 4 Sep 2026 21:45:26 -0400 Subject: [PATCH 6/6] fix(style): wrap long patch() calls to satisfy ruff line-length limit ruff format wraps lines >88 chars; the patch() calls in tests had very long module paths that needed to be reformatted. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- pulpcore/tests/unit/models/test_0156_migration.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pulpcore/tests/unit/models/test_0156_migration.py b/pulpcore/tests/unit/models/test_0156_migration.py index cc24a0f735..69037df5a8 100644 --- a/pulpcore/tests/unit/models/test_0156_migration.py +++ b/pulpcore/tests/unit/models/test_0156_migration.py @@ -73,7 +73,10 @@ def test_fix_base_path_violations_normalizes_row(monkeypatch): connection_mock = MagicMock() connection_mock.cursor.side_effect = lambda: next(cursors) - with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): fix_base_path_violations(None, None) update_cursor.execute.assert_called_once_with( @@ -89,7 +92,10 @@ def test_fix_base_path_violations_skips_clean_rows(monkeypatch): connection_mock = MagicMock() connection_mock.cursor.return_value = select_cursor - with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): fix_base_path_violations(None, None) # Only one cursor call (the SELECT), no UPDATE @@ -106,6 +112,9 @@ def test_fix_base_path_violations_raises_for_unfixable_path(): connection_mock = MagicMock() connection_mock.cursor.return_value = select_cursor - with patch("pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", connection_mock): + with patch( + "pulpcore.app.migrations.0156_alter_contentartifact_relative_path_and_more.connection", + connection_mock, + ): with pytest.raises(RuntimeError, match="could not be automatically normalized"): fix_base_path_violations(None, None)