Merge pull request #26748 from 04kash/long-url

Modify locations and refresh-keys Tables to Support Long URLs

.changeset/mighty-forks-exercise.md

@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Added migration `20241003170511_alter_target_in_locations.js` to change the target column in the `locations` table to TEXT type.
+Added a hash for the key column in the `refresh_keys` table.

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

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// @ts-check
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.up = async function up(knex) {
+  await knex.schema.alterTable('locations', table => {
+    table.text('target').alter();
+  });
+};
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.down = async function down(knex) {
+  const oversizedEntries = await knex('locations').where(
+    knex.raw('LENGTH(target) > 255'),
+  );
+
+  if (oversizedEntries.length > 0) {
+    throw new Error(
+      `Migration aborted: Found ${oversizedEntries.length} entries with 'target' exceeding 255 characters. Manual intervention required.`,
+    );
+  }
+
+  await knex.schema.alterTable('locations', table => {
+    table.string('target').alter();
+  });
+};

plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts

@@ -480,7 +480,7 @@ describe('DefaultProcessingDatabase', () => {
     );
 
     it.each(databases.eachSupportedId())(
-      'stores the refresh keys for the entity',
+      'stores the refresh keys for the entity where key length is 255 chars or less',
       async databaseId => {
         const mockLogger = {
           debug: jest.fn(),
@@ -535,6 +535,67 @@ describe('DefaultProcessingDatabase', () => {
         });
       },
     );
+
+    it.each(databases.eachSupportedId())(
+      'stores the refresh keys for the entity where key length is greater than 255 chars',
+      async databaseId => {
+        const mockLogger = {
+          debug: jest.fn(),
+          error: jest.fn(),
+          warn: jest.fn(),
+        };
+        const { knex, db } = await createDatabase(
+          databaseId,
+          mockLogger as unknown as Logger,
+        );
+        await insertRefreshStateRow(knex, {
+          entity_id: id,
+          entity_ref: 'location:default/fakelocation',
+          unprocessed_entity: '{}',
+          processed_entity: '{}',
+          errors: '[]',
+          next_update_at: '2021-04-01 13:37:00',
+          last_discovery_at: '2021-04-01 13:37:00',
+        });
+
+        const deferredEntities = [
+          {
+            entity: {
+              apiVersion: '1',
+              kind: 'Location',
+              metadata: {
+                name: 'next',
+              },
+            },
+            locationKey: 'mock',
+          },
+        ];
+
+        await db.transaction(tx =>
+          db.updateProcessedEntity(tx, {
+            id,
+            processedEntity,
+            resultHash: '',
+            relations: [],
+            deferredEntities,
+            refreshKeys: [
+              {
+                key: `url:https://example.com/foo-bar-test-group/very-long-group-name-that-exceeds-255-characters-just-to-test-the-limits-of-url-length-in-the-catalog-info-yaml-file-and-see-how-the-backstage-system-handles-it-making/test-this-alright-1/-/blob/main/catalog-info.yaml`,
+              },
+            ],
+          }),
+        );
+
+        const refreshKeys = await knex<DbRefreshKeysRow>('refresh_keys')
+          .where({ entity_id: id })
+          .select();
+
+        expect(refreshKeys[0]).toEqual({
+          entity_id: id,
+          key: `url:https://example.com/foo-bar-test-group/very-long-group-name-that-exceeds-255-characters-just-to-test-the-limits-of-url-length-in-the-catalog-info-yaml-file-and-see-how-the-back#sha256:edfb606500d184900e63891e5279d35bf0069ea251e90d15c0a430de6023d905`,
+        });
+      },
+    );
   });
 
   describe('updateEntityCache', () => {

plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts

@@ -41,7 +41,7 @@ import {
 import { checkLocationKeyConflict } from './operations/refreshState/checkLocationKeyConflict';
 import { insertUnprocessedEntity } from './operations/refreshState/insertUnprocessedEntity';
 import { updateUnprocessedEntity } from './operations/refreshState/updateUnprocessedEntity';
-import { generateStableHash } from './util';
+import { generateStableHash, generateTargetKey } from './util';
 import {
   EventBroker,
   EventParams,
@@ -158,7 +158,7 @@ export class DefaultProcessingDatabase implements ProcessingDatabase {
       'refresh_keys',
       refreshKeys.map(k => ({
         entity_id: id,
-        key: k.key,
+        key: generateTargetKey(k.key),
       })),
       BATCH_SIZE,
     );

plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts

@@ -16,6 +16,7 @@
 
 import { Knex } from 'knex';
 import { DbRefreshStateRow } from '../../tables';
+import { generateTargetKey } from '../../util';
 
 /**
  * Schedules a future refresh of entities, by so called "refresh keys" that may
@@ -30,10 +31,12 @@ export async function refreshByRefreshKeys(options: {
 }): Promise<void> {
   const { tx, keys } = options;
 
+  const hashedKeys = keys.map(k => generateTargetKey(k));
+
   await tx<DbRefreshStateRow>('refresh_state')
     .whereIn('entity_id', function selectEntityRefs(inner) {
       inner
-        .whereIn('key', keys)
+        .whereIn('key', hashedKeys)
         .select({
           entity_id: 'refresh_keys.entity_id',
         })

plugins/catalog-backend/src/database/util.ts

@@ -23,3 +23,11 @@ export function generateStableHash(entity: Entity) {
     .update(stableStringify({ ...entity }))
     .digest('hex');
 }
+
+export function generateTargetKey(target: string) {
+  return target.length > 255
+    ? `${target.slice(0, 180)}#sha256:${createHash('sha256')
+        .update(target)
+        .digest('hex')}`
+    : target;
+}

plugins/catalog-backend/src/tests/migrations.test.ts

@@ -308,4 +308,91 @@ describe('migrations', () => {
       await knex.destroy();
     },
   );
+
+  it.each(databases.eachSupportedId())(
+    '20241003170511_alter_target_in_locations.js, %p',
+    async databaseId => {
+      const knex = await databases.init(databaseId);
+
+      await migrateUntilBefore(
+        knex,
+        '20241003170511_alter_target_in_locations.js',
+      );
+
+      // Insert a simple URL before the migration
+      await knex
+        .insert({
+          id: '1f99f7f6-1d87-4e8c-994a-8a2ba1d542f6',
+          type: 'url',
+          target: 'https://example.com',
+        })
+        .into('locations');
+
+      await migrateUpOnce(knex);
+
+      // Verify that the target column is now 'text'
+      const columnInfo = await knex('locations').columnInfo();
+      expect(columnInfo.target.type).toBe('text');
+
+      // Insert a long URL (exceeding 255 characters)
+      await knex
+        .insert({
+          id: '1f99f6f7-1d87-4e8c-994a-2a8ba1d542f6',
+          type: 'url',
+          target:
+            'https://example.com/foo-bar-test-group/very-long-group-name-that-exceeds-255-characters-just-to-test-the-limits-of-url-length-in-the-catalog-info-yaml-file-and-see-how-the-backstage-system-handles-it-making/test-this-alright-1/-/blob/main/catalog-info.yaml',
+        })
+        .into('locations');
+
+      // Verify that both the simple and long URLs exist after the migration
+      await expect(knex('locations').where({ type: 'url' })).resolves.toEqual(
+        expect.arrayContaining([
+          {
+            id: '1f99f7f6-1d87-4e8c-994a-8a2ba1d542f6',
+            target: 'https://example.com',
+            type: 'url',
+          },
+          {
+            id: '1f99f6f7-1d87-4e8c-994a-2a8ba1d542f6',
+            target:
+              'https://example.com/foo-bar-test-group/very-long-group-name-that-exceeds-255-characters-just-to-test-the-limits-of-url-length-in-the-catalog-info-yaml-file-and-see-how-the-backstage-system-handles-it-making/test-this-alright-1/-/blob/main/catalog-info.yaml',
+            type: 'url',
+          },
+        ]),
+      );
+
+      await expect(migrateDownOnce(knex)).rejects.toThrow(
+        `Migration aborted: Found 1 entries with 'target' exceeding 255 characters. Manual intervention required.`,
+      );
+
+      // Now remove the long URL
+      await knex('locations')
+        .where({
+          id: '1f99f6f7-1d87-4e8c-994a-2a8ba1d542f6',
+        })
+        .del();
+
+      // Retry the migration down after removing the long URL
+      await migrateDownOnce(knex);
+
+      // Verify that the column type has reverted to varchar
+      const revertedColumnInfo = await knex('locations').columnInfo();
+      expect(revertedColumnInfo.target.type).toMatch(
+        /^(varchar|character varying)$/,
+      );
+
+      // Verify that the short URL still exists in the table
+      await expect(knex('locations').where({ type: 'url' })).resolves.toEqual(
+        expect.arrayContaining([
+          {
+            id: '1f99f7f6-1d87-4e8c-994a-8a2ba1d542f6',
+            target: 'https://example.com',
+            type: 'url',
+          },
+        ]),
+      );
+
+      await knex.destroy();
+    },
+  );
 });