diff --git a/CHANGES/+rel_path_migration.bugfix b/CHANGES/+rel_path_migration.bugfix new file mode 100644 index 00000000000..f10c71a54be --- /dev/null +++ b/CHANGES/+rel_path_migration.bugfix @@ -0,0 +1 @@ +Fixed the migration to allow `ContentArtifact.relative_path` be `None`. 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 22fcf266537..bb5c9c17eb4 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,39 +1,76 @@ # Generated by Django 5.2.15 on 2026-08-10 10:26 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 +# Obervations made while fixing this: +# - "ALTER TABLE ... VALIDATE CONSTRAINT" is idempotent. +# Even though postgres tracks whether a constraint was created / is still "NOT VALID". +# - "NOT VALID" cannot be used when changing the type of a column to a domain that adds a check. +# - Neither "ADD CONSTRAINT" nor "VALIDATE CONSTRAINT" have "IF (NOT) EXISTS" clauses. +# One can fake "ADD CONSTRAINT IF NOT EXISTS" by a clever combination of create-drop-rename. + + class Migration(migrations.Migration): dependencies = [ ('core', '0155_create_rel_path_domains'), ] + # This is needed to allow the concurrent reindex in the end. + atomic = False + operations = [ migrations.AlterField( model_name='contentartifact', name='relative_path', - field=pulpcore.app.models.fields.RelativePathField(), + field=django.db.models.fields.TextField(null=True, default=None), ), - migrations.AlterField( - model_name='distribution', - name='base_path', - field=pulpcore.app.models.fields.RelativePathField(), + migrations.RunSQL( + sql=""" + ALTER TABLE "core_contentartifact" ADD CONSTRAINT "temp_relative_path_check" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + """, + reverse_sql=""" + ALTER TABLE "core_contentartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + UPDATE "core_contentartifact" SET "relative_path" = '' WHERE "relative_path" is NULL; + """, + elidable=True, ), - migrations.AlterField( - model_name='publishedartifact', - name='relative_path', - field=pulpcore.app.models.fields.RelativePathField(), + + migrations.RunSQL( + sql=""" + ALTER TABLE "core_distribution" ADD CONSTRAINT "temp_relative_path_check" CHECK ('/' || "base_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + """, + reverse_sql=""" + ALTER TABLE "core_distribution" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + elidable=True, ), - migrations.AlterField( - model_name='publishedmetadata', - name='relative_path', - field=pulpcore.app.models.fields.RelativePathField(), + + migrations.RunSQL( + sql=""" + ALTER TABLE "core_publishedartifact" ADD CONSTRAINT "temp_relative_path_check" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + """, + reverse_sql=""" + ALTER TABLE "core_publishedartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, ), - migrations.AddIndex( + + migrations.RunSQL( + sql=""" + ALTER TABLE "core_publishedmetadata" ADD CONSTRAINT "temp_relative_path_check" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + """, + reverse_sql=""" + ALTER TABLE "core_publishedmetadata" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + ), + + django.contrib.postgres.operations.AddIndexConcurrently( + # Moving this to another migration would confuse the timelines again. 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'), ), diff --git a/pulpcore/app/migrations/0157_distribution_base_path_constraint.py b/pulpcore/app/migrations/0157_distribution_base_path_constraint.py index 9fe24229546..66be1709834 100644 --- a/pulpcore/app/migrations/0157_distribution_base_path_constraint.py +++ b/pulpcore/app/migrations/0157_distribution_base_path_constraint.py @@ -1,5 +1,6 @@ # Generated by Django 5.2.17 on 2026-08-11 10:34 +import pulpcore.app.models.fields from django.db import migrations @@ -95,10 +96,43 @@ class Migration(migrations.Migration): + # This migration received the aftermath for the distribution change, because + # that needs to happen before the trigger can be installed. + + atomic = False + dependencies = [ ('core', '0156_alter_contentartifact_relative_path_and_more'), ] operations = [ + migrations.RunSQL( + sql=""" + -- This is a weird way to fake "ADD CONSTRAINT IF NOT EXISTS". + -- Needed because neither ADD nor VALIDATE CONSTRAINT have "IF (NOT) EXISTS". + ALTER TABLE "core_distribution" ADD CONSTRAINT "temp_relative_path_check_2" CHECK ('/' || "base_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + ALTER TABLE "core_distribution" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + ALTER TABLE "core_distribution" RENAME CONSTRAINT "temp_relative_path_check_2" TO "temp_relative_path_check"; + -- + + ALTER TABLE "core_distribution" VALIDATE CONSTRAINT "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + migrations.AlterField( + model_name='distribution', + name='base_path', + field=pulpcore.app.models.fields.RelativePathField(), + ), + migrations.RunSQL( + sql=""" + ALTER TABLE "core_distribution" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), migrations.RunSQL(sql=ADD_TRIGGER, reverse_sql=REMOVE_TRIGGER, elidable=False), ] diff --git a/pulpcore/app/migrations/0159_IDEMPOTENT_AFTERMATH_OF_0156.py b/pulpcore/app/migrations/0159_IDEMPOTENT_AFTERMATH_OF_0156.py new file mode 100644 index 00000000000..82e9c7d4825 --- /dev/null +++ b/pulpcore/app/migrations/0159_IDEMPOTENT_AFTERMATH_OF_0156.py @@ -0,0 +1,22 @@ +# Generated by Django 5.2.15 on 2026-09-03 15:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0158_domain_default_content_guard'), + ] + + operations = [ + migrations.RunSQL( + # This operation does nothing in the corrected timeline. + # But if you are upgrading from 3.117.0 you'll need this. + # It is idempotent. + # This needs to be plain SQL, since Django would see no change and not do it. + sql="ALTER TABLE core_contentartifact ALTER COLUMN relative_path DROP NOT NULL;", + reverse_sql="", + elidable=True, + ), + ] diff --git a/pulpcore/app/migrations/0160_AFTERMATH_OF_0156_content_artifact.py b/pulpcore/app/migrations/0160_AFTERMATH_OF_0156_content_artifact.py new file mode 100644 index 00000000000..868ca2b1b61 --- /dev/null +++ b/pulpcore/app/migrations/0160_AFTERMATH_OF_0156_content_artifact.py @@ -0,0 +1,50 @@ +# Generated by Django 5.2.15 on 2026-09-04 08:26 + +import pulpcore.app.models.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + # This migration may run for a long time, but it does not perform in a transaction. + # And it therefore needs to take extra care about partially validated check constraints. + # This migration should not result in any database change if the 3.117.0 timeline was taken. + + atomic = False + + dependencies = [ + ('core', '0159_IDEMPOTENT_AFTERMATH_OF_0156'), + ] + + operations = [ + migrations.RunSQL( + sql=""" + -- This is a weird way to fake "ADD CONSTRAINT IF NOT EXISTS". + -- Needed because neither ADD nor VALIDATE CONSTRAINT have "IF (NOT) EXISTS". + ALTER TABLE "core_contentartifact" ADD CONSTRAINT "temp_relative_path_check_2" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + ALTER TABLE "core_contentartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + ALTER TABLE "core_contentartifact" RENAME CONSTRAINT "temp_relative_path_check_2" TO "temp_relative_path_check"; + -- + + UPDATE "core_contentartifact" SET relative_path = NULL WHERE "relative_path" = ''; + ALTER TABLE "core_contentartifact" VALIDATE CONSTRAINT "temp_relative_path_check"; + """, + reverse_sql=""" + UPDATE "core_contentartifact" SET "relative_path" = '' WHERE "relative_path" is NULL; + """, + elidable=True, + ), + migrations.AlterField( + model_name='contentartifact', + name='relative_path', + field=pulpcore.app.models.fields.RelativePathField(null=True, default=None), + ), + migrations.RunSQL( + sql=""" + ALTER TABLE "core_contentartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + ] diff --git a/pulpcore/app/migrations/0161_AFTERMATH_OF_0156_distribution.py b/pulpcore/app/migrations/0161_AFTERMATH_OF_0156_distribution.py new file mode 100644 index 00000000000..8c038daaafc --- /dev/null +++ b/pulpcore/app/migrations/0161_AFTERMATH_OF_0156_distribution.py @@ -0,0 +1,20 @@ +# Generated by Django 5.2.15 on 2026-09-04 08:27 + +from django.db import migrations + + +class Migration(migrations.Migration): + + atomic = False + + dependencies = [ + ('core', '0160_AFTERMATH_OF_0156_content_artifact'), + ] + + operations = [ + # Intentionally empty. + # The operations one would expect here needed to be added before the triggers were + # installed. Leaving this here as an empty migration for this message. + # Also this might be a good place if we needed to inject some instructions into weird + # migration timelines later. + ] diff --git a/pulpcore/app/migrations/0162_AFTERMATH_OF_0156_published_artifact.py b/pulpcore/app/migrations/0162_AFTERMATH_OF_0156_published_artifact.py new file mode 100644 index 00000000000..cb0f204af24 --- /dev/null +++ b/pulpcore/app/migrations/0162_AFTERMATH_OF_0156_published_artifact.py @@ -0,0 +1,48 @@ +# Generated by Django 5.2.15 on 2026-09-04 08:27 + +import pulpcore.app.models.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + # This migration may run for a long time, but it does not perform in a transaction. + # And it therefore needs to take extra care about partially validated check constraints. + # This migration should not result in any database change if the 3.117.0 timeline was taken. + + atomic = False + + dependencies = [ + ('core', '0161_AFTERMATH_OF_0156_distribution'), + ] + + operations = [ + migrations.RunSQL( + sql=""" + -- This is a weird way to fake "ADD CONSTRAINT IF NOT EXISTS". + -- Needed because neither ADD nor VALIDATE CONSTRAINT have "IF (NOT) EXISTS". + ALTER TABLE "core_publishedartifact" ADD CONSTRAINT "temp_relative_path_check_2" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + ALTER TABLE "core_publishedartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + ALTER TABLE "core_publishedartifact" RENAME CONSTRAINT "temp_relative_path_check_2" TO "temp_relative_path_check"; + -- + + ALTER TABLE "core_publishedartifact" VALIDATE CONSTRAINT "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + migrations.AlterField( + model_name='publishedartifact', + name='relative_path', + field=pulpcore.app.models.fields.RelativePathField(), + ), + migrations.RunSQL( + sql=""" + ALTER TABLE "core_publishedartifact" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + ] diff --git a/pulpcore/app/migrations/0163_AFTERMATH_OF_0156_published_metadata.py b/pulpcore/app/migrations/0163_AFTERMATH_OF_0156_published_metadata.py new file mode 100644 index 00000000000..639c5afc688 --- /dev/null +++ b/pulpcore/app/migrations/0163_AFTERMATH_OF_0156_published_metadata.py @@ -0,0 +1,48 @@ +# Generated by Django 5.2.15 on 2026-09-04 08:28 + +import pulpcore.app.models.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + # This migration may run for a long time, but it does not perform in a transaction. + # And it therefore needs to take extra care about partially validated check constraints. + # This migration should not result in any database change if the 3.117.0 timeline was taken. + + atomic = False + + dependencies = [ + ('core', '0162_AFTERMATH_OF_0156_published_artifact'), + ] + + operations = [ + migrations.RunSQL( + sql=""" + -- This is a weird way to fake "ADD CONSTRAINT IF NOT EXISTS". + -- Needed because neither ADD nor VALIDATE CONSTRAINT have "IF (NOT) EXISTS". + ALTER TABLE "core_publishedmetadata" ADD CONSTRAINT "temp_relative_path_check_2" CHECK ('/' || "relative_path" || '/' !~ '[\n\r\s\t\?#]|(/\.{0,2}/)') NOT VALID; + ALTER TABLE "core_publishedmetadata" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + ALTER TABLE "core_publishedmetadata" RENAME CONSTRAINT "temp_relative_path_check_2" TO "temp_relative_path_check"; + -- + + ALTER TABLE "core_publishedmetadata" VALIDATE CONSTRAINT "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + migrations.AlterField( + model_name='publishedmetadata', + name='relative_path', + field=pulpcore.app.models.fields.RelativePathField(), + ), + migrations.RunSQL( + sql=""" + ALTER TABLE "core_publishedmetadata" DROP CONSTRAINT IF EXISTS "temp_relative_path_check"; + """, + reverse_sql=""" + """, + elidable=True, + ), + ] diff --git a/pulpcore/app/models/content.py b/pulpcore/app/models/content.py index e62c3f85a6e..6da84d2efe8 100644 --- a/pulpcore/app/models/content.py +++ b/pulpcore/app/models/content.py @@ -658,7 +658,7 @@ class ContentArtifact(BaseModel, QueryMixin): Artifact, on_delete=models.PROTECT, null=True, related_name="content_memberships" ) content = models.ForeignKey(Content, on_delete=models.CASCADE) - relative_path = RelativePathField() + relative_path = RelativePathField(null=True, default=None) objects = BulkCreateManager()