fix(catalog-backend): fix irreversible database migration down functions

Five migration down functions were broken, causing the SQL report to flag
them as not reversible. All issues were pre-existing bugs that were never
caught because the down migrations had never been exercised under the
reversibility checker.

- 20241003170511: add .notNullable() to both up and down when altering
  locations.target, preventing the NOT NULL constraint from being silently
  dropped by knex's .alter() when widening varchar(255) to text.
- 20220116144621: implement a real down function that recreates the three
  dropped legacy tables (entities, entities_search, entities_relations)
  with correct columns and indices.
- 20210302150147: drop dependent tables in the correct order to avoid a
  FK constraint violation, and fix a typo (references → refresh_state_references).
- 20201005122705: drop full_name from entities (not entities_search), and
  restore entities_unique_name with the correct column order (kind, name, namespace).
- 20200702153613: use table.integer('generation') instead of table.string()
  in the down function to restore the correct column type.

Signed-off-by: Fredrik Adelöw <freben@spotify.com>
Made-with: Cursor

.changeset/fix-alter-target-nullability.md

@@ -0,0 +1,11 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Fixed several database migration `down` functions that were not properly reversible, causing the SQL report to show warnings:
+
+- `20241003170511_alter_target_in_locations.js`: both `up` and `down` now include `.notNullable()` when altering the `locations.target` column, preventing the `NOT NULL` constraint from being accidentally dropped when widening the column type from `varchar(255)` to `text`.
+- `20220116144621_remove_legacy.js`: the `down` function now properly recreates the three dropped legacy tables (`entities`, `entities_search`, `entities_relations`) with correct columns and indices.
+- `20210302150147_refresh_state.js`: the `down` function now drops dependent tables in the correct order (avoiding a FK constraint violation) and fixes a typo where the table was referred to as `references` instead of `refresh_state_references`.
+- `20201005122705_add_entity_full_name.js`: the `down` function now drops the `full_name` column from `entities` (not `entities_search`), and restores the `entities_unique_name` index with the correct column order `(kind, name, namespace)`.
+- `20200702153613_entities.js`: the `down` function now uses `table.integer('generation')` instead of `table.string('generation')`, restoring the correct column type.

plugins/catalog-backend/migrations/20200702153613_entities.js

@@ -169,7 +169,7 @@ exports.down = async function down(knex) {
           'An opaque string that changes for each update operation to any part of the entity, including metadata.',
         );
       table
-        .string('generation')
+        .integer('generation')
         .notNullable()
         .unsigned()
         .comment(

plugins/catalog-backend/migrations/20201005122705_add_entity_full_name.js

@@ -52,12 +52,12 @@ exports.down = async function down(knex) {
   await knex.schema.alterTable('entities', table => {
     // https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta
     table.dropUnique([], 'entities_unique_full_name');
-    table.unique(['kind', 'namespace', 'name'], {
+    table.unique(['kind', 'name', 'namespace'], {
       indexName: 'entities_unique_name',
     });
   });
 
-  await knex.schema.alterTable('entities_search', table => {
+  await knex.schema.alterTable('entities', table => {
     table.dropColumn('full_name');
   });
 };

plugins/catalog-backend/migrations/20210302150147_refresh_state.js

@@ -175,33 +175,10 @@ exports.up = async function up(knex) {
  * @param {import('knex').Knex} knex
  */
 exports.down = async function down(knex) {
-  await knex.schema.alterTable('refresh_state_references', table => {
-    table.dropIndex([], 'refresh_state_references_source_key_idx');
-    table.dropIndex([], 'refresh_state_references_source_entity_ref_idx');
-    table.dropIndex([], 'refresh_state_references_target_entity_ref_idx');
-  });
-  await knex.schema.alterTable('refresh_state', table => {
-    table.dropUnique([], 'refresh_state_entity_ref_uniq');
-    table.dropIndex([], 'refresh_state_entity_id_idx');
-    table.dropIndex([], 'refresh_state_entity_ref_idx');
-    table.dropIndex([], 'refresh_state_next_update_at_idx');
-  });
-  await knex.schema.alterTable('final_entities', table => {
-    table.dropIndex([], 'final_entities_entity_id_idx');
-  });
-  await knex.schema.alterTable('relations', table => {
-    table.index('source_entity_ref', 'relations_source_entity_ref_idx');
-    table.index('originating_entity_id', 'relations_source_entity_id_idx');
-  });
-  await knex.schema.alterTable('search', table => {
-    table.dropIndex([], 'search_entity_id_idx');
-    table.dropIndex([], 'search_key_idx');
-    table.dropIndex([], 'search_value_idx');
-  });
-
+  // Drop tables that have FK constraints referencing refresh_state first.
+  await knex.schema.dropTable('refresh_state_references');
   await knex.schema.dropTable('search');
   await knex.schema.dropTable('final_entities');
   await knex.schema.dropTable('relations');
-  await knex.schema.dropTable('references');
   await knex.schema.dropTable('refresh_state');
 };

plugins/catalog-backend/migrations/20220116144621_remove_legacy.js

@@ -25,4 +25,86 @@ exports.up = async function up(knex) {
   await knex.schema.dropTable('entities');
 };
 
-exports.down = async function down() {};
+exports.down = async function down(knex) {
+  await knex.schema.createTable('entities', table => {
+    table.comment('All entities currently stored in the catalog');
+    table.uuid('id').primary().comment('Auto-generated ID of the entity');
+    table
+      .uuid('location_id')
+      .references('id')
+      .inTable('locations')
+      .nullable()
+      .comment('The location that originated the entity');
+    table
+      .string('etag')
+      .notNullable()
+      .comment(
+        'An opaque string that changes for each update operation to any part of the entity, including metadata.',
+      );
+    table
+      .integer('generation')
+      .notNullable()
+      .unsigned()
+      .comment(
+        'A positive nonzero number that indicates the current generation of data for this entity; the value is incremented each time the spec changes.',
+      );
+    table
+      .string('full_name')
+      .notNullable()
+      .comment('The full name of the entity');
+    table
+      .text('data')
+      .notNullable()
+      .comment('The entire JSON data blob of the entity');
+    table.unique(['full_name'], { indexName: 'entities_unique_full_name' });
+    table.index('location_id', 'entity_location_id_idx');
+  });
+
+  await knex.schema.createTable('entities_search', table => {
+    table.comment(
+      'Flattened key-values from the entities, used for quick filtering',
+    );
+    table
+      .uuid('entity_id')
+      .references('id')
+      .inTable('entities')
+      .onDelete('CASCADE')
+      .comment('The entity that matches this key/value');
+    table
+      .string('key')
+      .notNullable()
+      .comment('A key that occurs in the entity');
+    table
+      .string('value')
+      .nullable()
+      .comment('The corresponding value to match on');
+    table.index(['key'], 'entities_search_key');
+    table.index(['value'], 'entities_search_value');
+    table.index(['entity_id'], 'entity_id_idx');
+  });
+
+  await knex.schema.createTable('entities_relations', table => {
+    table.comment('All relations between entities in the catalog');
+    table
+      .uuid('originating_entity_id')
+      .references('id')
+      .inTable('entities')
+      .onDelete('CASCADE')
+      .notNullable()
+      .comment('The entity that provided the relation');
+    table
+      .string('source_full_name')
+      .notNullable()
+      .comment('The full name of the source entity of the relation');
+    table
+      .string('type')
+      .notNullable()
+      .comment('The type of the relation between the entities');
+    table
+      .string('target_full_name')
+      .notNullable()
+      .comment('The full name of the target entity of the relation');
+    table.index('source_full_name', 'source_full_name_idx');
+    table.index('originating_entity_id', 'originating_entity_id_idx');
+  });
+};

plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js

@@ -22,7 +22,7 @@
  */
 exports.up = async function up(knex) {
   await knex.schema.alterTable('locations', table => {
-    table.text('target').alter();
+    table.text('target').notNullable().alter();
   });
 };
 
@@ -42,6 +42,6 @@ exports.down = async function down(knex) {
   }
 
   await knex.schema.alterTable('locations', table => {
-    table.string('target').alter();
+    table.string('target').notNullable().alter();
   });
 };

plugins/catalog-backend/report.sql.md

@@ -2,9 +2,6 @@
 
 > Do not edit this file. It is a report generated by `yarn build:api-reports`
 
-> [!WARNING]
-> Failed to migrate down from '20241003170511_alter_target_in_locations.js'
-
 ## Sequences
 
 - `location_update_log_id_seq` (bigint)
@@ -47,7 +44,7 @@
 | --------------------- | ------------------- | -------- | ---------- | ------- |
 | `id`                  | `uuid`              | false    | -          | -       |
 | `location_entity_ref` | `character varying` | false    | 255        | -       |
-| `target`              | `text`              | true     | -          | -       |
+| `target`              | `text`              | false    | -          | -       |
 | `type`                | `character varying` | false    | 255        | -       |
 
 ### Indices