From 5a902df9497937eab9600cbde46e4bbff0cb773a Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 3 Oct 2024 16:17:43 -0400 Subject: [PATCH] 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; +}