From 83a883091061919426b5e792aa2e5d1045eaeb91 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 17 Sep 2024 12:55:07 -0400 Subject: [PATCH 01/15] allow longer URLs to be registered in software-catalog Signed-off-by: Kashish Mittal --- .changeset/mighty-forks-exercise.md | 6 ++++++ plugins/catalog-backend/migrations/20200511113813_init.js | 2 +- .../migrations/20220616202842_refresh_keys.js | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 .changeset/mighty-forks-exercise.md diff --git a/.changeset/mighty-forks-exercise.md b/.changeset/mighty-forks-exercise.md new file mode 100644 index 0000000000..af842ecba9 --- /dev/null +++ b/.changeset/mighty-forks-exercise.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Change `locations` table's `target` column to text +Change `refresh-keys` table's `key` column to text diff --git a/plugins/catalog-backend/migrations/20200511113813_init.js b/plugins/catalog-backend/migrations/20200511113813_init.js index 710ff537e0..5b7e3b8dec 100644 --- a/plugins/catalog-backend/migrations/20200511113813_init.js +++ b/plugins/catalog-backend/migrations/20200511113813_init.js @@ -36,7 +36,7 @@ exports.up = async function up(knex) { .comment('Auto-generated ID of the location'); table.string('type').notNullable().comment('The type of location'); table - .string('target') + .text('target') .notNullable() .comment('The actual target of the location'); }) diff --git a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js index a7ec819489..ca0a477622 100644 --- a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js +++ b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js @@ -32,7 +32,7 @@ exports.up = async function up(knex) { .onDelete('CASCADE') .comment('A reference to the entity that the refresh key is tied to'); table - .string('key') + .text('key') .notNullable() .comment( 'A reference to a key which should be used to trigger a refresh on this entity', From e6323845e8b38201f7f703aec155b7672ee771f6 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 17 Sep 2024 16:53:03 -0400 Subject: [PATCH 02/15] remove index on key in refresh_keys Signed-off-by: Kashish Mittal --- .../catalog-backend/migrations/20220616202842_refresh_keys.js | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js index ca0a477622..800ce4d0e4 100644 --- a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js +++ b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js @@ -38,7 +38,6 @@ exports.up = async function up(knex) { 'A reference to a key which should be used to trigger a refresh on this entity', ); table.index('entity_id', 'refresh_keys_entity_id_idx'); - table.index('key', 'refresh_keys_key_idx'); }); }; From 3181b0f3d1e48db57bce4a7a9427e7298f2dc277 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Mon, 23 Sep 2024 11:35:07 -0400 Subject: [PATCH 03/15] hash keys in refresh_keys Signed-off-by: Kashish Mittal --- .../migrations/20220616202842_refresh_keys.js | 3 ++- .../src/database/DefaultProcessingDatabase.ts | 3 ++- .../database/operations/provider/refreshByRefreshKeys.ts | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js index 800ce4d0e4..a7ec819489 100644 --- a/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js +++ b/plugins/catalog-backend/migrations/20220616202842_refresh_keys.js @@ -32,12 +32,13 @@ exports.up = async function up(knex) { .onDelete('CASCADE') .comment('A reference to the entity that the refresh key is tied to'); table - .text('key') + .string('key') .notNullable() .comment( 'A reference to a key which should be used to trigger a refresh on this entity', ); table.index('entity_id', 'refresh_keys_entity_id_idx'); + table.index('key', 'refresh_keys_key_idx'); }); }; diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index 56d143e374..8bf93e5c91 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts @@ -51,6 +51,7 @@ import { DateTime } from 'luxon'; import { CATALOG_CONFLICTS_TOPIC } from '../constants'; import { CatalogConflictEventPayload } from '../catalog/types'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { createHash } from 'crypto'; // The number of items that are sent per batch to the database layer, when // doing .batchInsert calls to knex. This needs to be low enough to not cause @@ -158,7 +159,7 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { 'refresh_keys', refreshKeys.map(k => ({ entity_id: id, - key: k.key, + key: createHash('sha256').update(k.key).digest('hex'), })), 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..ac36550864 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 { createHash } from 'crypto'; /** * Schedules a future refresh of entities, by so called "refresh keys" that may @@ -30,10 +31,14 @@ export async function refreshByRefreshKeys(options: { }): Promise { const { tx, keys } = options; + const hashedKeys = keys.map(k => + createHash('sha256').update(k).digest('hex'), + ); + await tx('refresh_state') .whereIn('entity_id', function selectEntityRefs(inner) { inner - .whereIn('key', keys) + .whereIn('key', hashedKeys) .select({ entity_id: 'refresh_keys.entity_id', }) From 99dff4238fa9a6e9725e006bb830b96415a70f91 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Mon, 23 Sep 2024 11:38:09 -0400 Subject: [PATCH 04/15] update changeset Signed-off-by: Kashish Mittal --- .changeset/mighty-forks-exercise.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/mighty-forks-exercise.md b/.changeset/mighty-forks-exercise.md index af842ecba9..1c50aa3285 100644 --- a/.changeset/mighty-forks-exercise.md +++ b/.changeset/mighty-forks-exercise.md @@ -2,5 +2,5 @@ '@backstage/plugin-catalog-backend': patch --- -Change `locations` table's `target` column to text -Change `refresh-keys` table's `key` column to text +Changed the target column in the locations table to TEXT type. +Added a hash for the key column in the refresh_keys table. From 6593b127c2a03e3e17d14fe7caa37718ce4a16a0 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Mon, 23 Sep 2024 12:31:09 -0400 Subject: [PATCH 05/15] update tests Signed-off-by: Kashish Mittal --- .../src/database/DefaultProcessingDatabase.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index 9110dca640..e287d8cd18 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts @@ -531,7 +531,7 @@ describe('DefaultProcessingDatabase', () => { expect(refreshKeys[0]).toEqual({ entity_id: id, - key: 'protocol:foo-bar.com', + key: '1fae60d52c9630ddcacb375c1789ed33055ebe6565da6b3446369fb9a1044b47', }); }, ); From d3e45d6d6bc265f233ec181a9ed0f1817d2c3868 Mon Sep 17 00:00:00 2001 From: Kashish Mittal <113269381+04kash@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:07:24 -0400 Subject: [PATCH 06/15] Update .changeset/mighty-forks-exercise.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Signed-off-by: Kashish Mittal <113269381+04kash@users.noreply.github.com> Signed-off-by: Kashish Mittal --- .changeset/mighty-forks-exercise.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/mighty-forks-exercise.md b/.changeset/mighty-forks-exercise.md index 1c50aa3285..0743da73c6 100644 --- a/.changeset/mighty-forks-exercise.md +++ b/.changeset/mighty-forks-exercise.md @@ -3,4 +3,4 @@ --- Changed the target column in the locations table to TEXT type. -Added a hash for the key column in the refresh_keys table. +Added a hash for the key column in the `refresh_keys` table. From 120f74bc919f081a8ed832df571d3a51f85a69c6 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 3 Oct 2024 16:10:54 -0400 Subject: [PATCH 07/15] add a migration to catalog-backend to alter target column of locations table to type text Signed-off-by: Kashish Mittal --- .../migrations/20200511113813_init.js | 2 +- ...0241003170511_alter_target_in_locations.js | 37 +++++++++++++++++++ .../src/tests/migrations.test.ts | 22 +++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js diff --git a/plugins/catalog-backend/migrations/20200511113813_init.js b/plugins/catalog-backend/migrations/20200511113813_init.js index 5b7e3b8dec..710ff537e0 100644 --- a/plugins/catalog-backend/migrations/20200511113813_init.js +++ b/plugins/catalog-backend/migrations/20200511113813_init.js @@ -36,7 +36,7 @@ exports.up = async function up(knex) { .comment('Auto-generated ID of the location'); table.string('type').notNullable().comment('The type of location'); table - .text('target') + .string('target') .notNullable() .comment('The actual target of the location'); }) 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..d909186a73 --- /dev/null +++ b/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js @@ -0,0 +1,37 @@ +/* + * 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) { + await knex.schema.alterTable('locations', table => { + table.string('target').alter(); + }); +}; diff --git a/plugins/catalog-backend/src/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index d961427f5e..ee0ed8eb83 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/plugins/catalog-backend/src/tests/migrations.test.ts @@ -308,4 +308,26 @@ 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', + ); + + await migrateUpOnce(knex); + const columnInfo = await knex('locations').columnInfo(); + expect(columnInfo.target.type).toBe('text'); + + await migrateDownOnce(knex); + const revertedColumnInfo = await knex('locations').columnInfo(); + expect(revertedColumnInfo.target.type).toBe('varchar'); + + await knex.destroy(); + }, + ); }); From 5a902df9497937eab9600cbde46e4bbff0cb773a Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 3 Oct 2024 16:17:43 -0400 Subject: [PATCH 08/15] Add utility to conditionally hash URLs exceeding length limit for better performance and debuggability Signed-off-by: Kashish Mittal --- .../DefaultProcessingDatabase.test.ts | 67 ++++++++++++++++++- .../src/database/DefaultProcessingDatabase.ts | 5 +- .../provider/refreshByRefreshKeys.ts | 6 +- plugins/catalog-backend/src/database/util.ts | 8 +++ 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index e287d8cd18..27031f7626 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(), @@ -531,7 +531,70 @@ describe('DefaultProcessingDatabase', () => { expect(refreshKeys[0]).toEqual({ entity_id: id, - key: '1fae60d52c9630ddcacb375c1789ed33055ebe6565da6b3446369fb9a1044b47', + key: 'protocol:foo-bar.com', + }); + }, + ); + + 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(); + + console.log(refreshKeys[0].key); + + 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-#sha256:edfb606500d184900e63891e5279d35bf0069ea251e90d15c0a430de6023d905`, }); }, ); diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index 8bf93e5c91..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, @@ -51,7 +51,6 @@ import { DateTime } from 'luxon'; import { CATALOG_CONFLICTS_TOPIC } from '../constants'; import { CatalogConflictEventPayload } from '../catalog/types'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { createHash } from 'crypto'; // The number of items that are sent per batch to the database layer, when // doing .batchInsert calls to knex. This needs to be low enough to not cause @@ -159,7 +158,7 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { 'refresh_keys', refreshKeys.map(k => ({ entity_id: id, - key: createHash('sha256').update(k.key).digest('hex'), + 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 ac36550864..c84b6c8e13 100644 --- a/plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts +++ b/plugins/catalog-backend/src/database/operations/provider/refreshByRefreshKeys.ts @@ -16,7 +16,7 @@ import { Knex } from 'knex'; import { DbRefreshStateRow } from '../../tables'; -import { createHash } from 'crypto'; +import { generateTargetKey } from '../../util'; /** * Schedules a future refresh of entities, by so called "refresh keys" that may @@ -31,9 +31,7 @@ export async function refreshByRefreshKeys(options: { }): Promise { const { tx, keys } = options; - const hashedKeys = keys.map(k => - createHash('sha256').update(k).digest('hex'), - ); + const hashedKeys = keys.map(k => generateTargetKey(k)); await tx('refresh_state') .whereIn('entity_id', function selectEntityRefs(inner) { diff --git a/plugins/catalog-backend/src/database/util.ts b/plugins/catalog-backend/src/database/util.ts index 771fab7fba..48158166c6 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, 150)}#sha256:${createHash('sha256') + .update(target) + .digest('hex')}` + : target; +} From d8a27e058d42a00cda9d1446431b4bdb3eb601d3 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 3 Oct 2024 16:21:55 -0400 Subject: [PATCH 09/15] remove console.log statments Signed-off-by: Kashish Mittal --- .../src/database/DefaultProcessingDatabase.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index 27031f7626..26ad1c84d0 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts @@ -590,8 +590,6 @@ describe('DefaultProcessingDatabase', () => { .where({ entity_id: id }) .select(); - console.log(refreshKeys[0].key); - 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-#sha256:edfb606500d184900e63891e5279d35bf0069ea251e90d15c0a430de6023d905`, From 2fa071307f9d476a9c29e794eaefb20c57c86d72 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 3 Oct 2024 16:36:34 -0400 Subject: [PATCH 10/15] update tests Signed-off-by: Kashish Mittal --- plugins/catalog-backend/src/tests/migrations.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index ee0ed8eb83..9b9d2f55bf 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/plugins/catalog-backend/src/tests/migrations.test.ts @@ -325,7 +325,9 @@ describe('migrations', () => { await migrateDownOnce(knex); const revertedColumnInfo = await knex('locations').columnInfo(); - expect(revertedColumnInfo.target.type).toBe('varchar'); + expect(revertedColumnInfo.target.type).toMatch( + /^(varchar|character varying)$/, + ); await knex.destroy(); }, From a558eb4ea1222d96a78a5507eb0803b76ab05d4c Mon Sep 17 00:00:00 2001 From: Kashish Mittal <113269381+04kash@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:45:42 -0400 Subject: [PATCH 11/15] Update key hash in generateTargetKey Co-authored-by: Patrik Oldsberg Signed-off-by: Kashish Mittal <113269381+04kash@users.noreply.github.com> Signed-off-by: Kashish Mittal --- plugins/catalog-backend/src/database/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/util.ts b/plugins/catalog-backend/src/database/util.ts index 48158166c6..4c03929fc5 100644 --- a/plugins/catalog-backend/src/database/util.ts +++ b/plugins/catalog-backend/src/database/util.ts @@ -26,7 +26,7 @@ export function generateStableHash(entity: Entity) { export function generateTargetKey(target: string) { return target.length > 255 - ? `${target.slice(0, 150)}#sha256:${createHash('sha256') + ? `${target.slice(0, 180)}#sha256:${createHash('sha256') .update(target) .digest('hex')}` : target; From ada90f414d6390884937250f3f7755e8940a5f53 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Fri, 4 Oct 2024 11:28:16 -0400 Subject: [PATCH 12/15] update tests Signed-off-by: Kashish Mittal --- .../src/database/DefaultProcessingDatabase.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index 26ad1c84d0..37608b1493 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts @@ -592,7 +592,7 @@ describe('DefaultProcessingDatabase', () => { 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-#sha256:edfb606500d184900e63891e5279d35bf0069ea251e90d15c0a430de6023d905`, + 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`, }); }, ); From b999701b08b0cb43e42bedf31bbbdcf8a2739688 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Fri, 4 Oct 2024 12:11:19 -0400 Subject: [PATCH 13/15] update migration tests Signed-off-by: Kashish Mittal --- .../src/tests/migrations.test.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/plugins/catalog-backend/src/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index 9b9d2f55bf..370241dda1 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/plugins/catalog-backend/src/tests/migrations.test.ts @@ -319,16 +319,73 @@ describe('migrations', () => { '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')).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 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 both the simple and long URLs exist after the rollback + await expect(knex('locations')).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 knex.destroy(); }, ); From a75c1b59a0e4006e7a76d24b94fd18465d8870f6 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 8 Oct 2024 10:25:24 -0400 Subject: [PATCH 14/15] update tests Signed-off-by: Kashish Mittal --- ...0241003170511_alter_target_in_locations.js | 10 ++++++++ .../src/tests/migrations.test.ts | 24 ++++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js b/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js index d909186a73..855e1a7f8d 100644 --- a/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js +++ b/plugins/catalog-backend/migrations/20241003170511_alter_target_in_locations.js @@ -31,6 +31,16 @@ exports.up = async function up(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/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index 370241dda1..873746ac2b 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/plugins/catalog-backend/src/tests/migrations.test.ts @@ -345,7 +345,7 @@ describe('migrations', () => { .into('locations'); // Verify that both the simple and long URLs exist after the migration - await expect(knex('locations')).resolves.toEqual( + await expect(knex('locations').where({ type: 'url' })).resolves.toEqual( expect.arrayContaining([ { id: '1f99f7f6-1d87-4e8c-994a-8a2ba1d542f6', @@ -361,6 +361,18 @@ describe('migrations', () => { ]), ); + 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 @@ -369,20 +381,14 @@ describe('migrations', () => { /^(varchar|character varying)$/, ); - // Verify that both the simple and long URLs exist after the rollback - await expect(knex('locations')).resolves.toEqual( + // 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', }, - { - 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', - }, ]), ); From 6a054790d9eddad842574a1c948f0fe09202f225 Mon Sep 17 00:00:00 2001 From: Kashish Mittal <113269381+04kash@users.noreply.github.com> Date: Tue, 8 Oct 2024 18:24:38 -0400 Subject: [PATCH 15/15] Update mighty-forks-exercise.md Signed-off-by: Kashish Mittal --- .changeset/mighty-forks-exercise.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/mighty-forks-exercise.md b/.changeset/mighty-forks-exercise.md index 0743da73c6..2ec5b90fa2 100644 --- a/.changeset/mighty-forks-exercise.md +++ b/.changeset/mighty-forks-exercise.md @@ -2,5 +2,5 @@ '@backstage/plugin-catalog-backend': patch --- -Changed the target column in the locations table to TEXT type. +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.