diff --git a/.changeset/mighty-forks-exercise.md b/.changeset/mighty-forks-exercise.md new file mode 100644 index 0000000000..2ec5b90fa2 --- /dev/null +++ b/.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. diff --git a/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js b/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js new file mode 100644 index 0000000000..855e1a7f8d --- /dev/null +++ b/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 } + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('locations', table => { + table.text('target').alter(); + }); +}; + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +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(); + }); +}; diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index 9110dca640..37608b1493 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts +++ b/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('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', () => { diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index 56d143e374..1cc39b3f43 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts +++ b/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, ); diff --git a/plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts b/plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts index dc34c9cdf0..c84b6c8e13 100644 --- a/plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts +++ b/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 { const { tx, keys } = options; + const hashedKeys = keys.map(k => generateTargetKey(k)); + await tx('refresh_state') .whereIn('entity_id', function selectEntityRefs(inner) { inner - .whereIn('key', keys) + .whereIn('key', hashedKeys) .select({ entity_id: 'refresh_keys.entity_id', }) diff --git a/plugins/catalog-backend/src/database/util.ts b/plugins/catalog-backend/src/database/util.ts index 771fab7fba..4c03929fc5 100644 --- a/plugins/catalog-backend/src/database/util.ts +++ b/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; +} diff --git a/plugins/catalog-backend/src/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index d961427f5e..873746ac2b 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/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(); + }, + ); });