diff --git a/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.test.ts b/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.test.ts index ef21648619..4241599351 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.test.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.test.ts @@ -236,5 +236,57 @@ describe('buildEntitySearch', () => { }, ]); }); + + it('rejects duplicate keys', () => { + expect(() => + buildEntitySearch('eid', { + relations: [], + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'n', + Namespace: 'ns', + }, + }), + ).toThrow( + `Entity has duplicate keys that vary only in casing, 'metadata.namespace'`, + ); + + expect(() => + buildEntitySearch('eid', { + relations: [ + { + type: 'dup', + targetRef: 'k:ns/a', + }, + { + type: 'DUP', + targetRef: 'k:ns/b', + }, + ], + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + }), + ).toThrow( + `Entity has duplicate keys that vary only in casing, 'relations.dup'`, + ); + + expect(() => + buildEntitySearch('eid', { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + spec: { + owner: 'o', + OWNER: 'o', + lifecycle: 'production', + lifeCycle: 'production', + }, + }), + ).toThrow( + `Entity has duplicate keys that vary only in casing, 'spec.owner', 'spec.lifecycle'`, + ); + }); }); }); diff --git a/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.ts b/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.ts index 0ea54e29e9..53e8d4c1cd 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/buildEntitySearch.ts @@ -15,6 +15,7 @@ */ import { DEFAULT_NAMESPACE, Entity } from '@backstage/catalog-model'; +import { InputError } from '@backstage/errors'; import { DbSearchRow } from '../../tables'; // These are excluded in the generic loop, either because they do not make sense @@ -199,5 +200,23 @@ export function buildEntitySearch( }); } + // This validates that there are no keys that vary only in casing, such + // as `spec.foo` and `spec.Foo`. + const keys = new Set(raw.map(r => r.key)); + const lowerKeys = new Set(raw.map(r => r.key.toLocaleLowerCase('en-US'))); + if (keys.size !== lowerKeys.size) { + const difference = []; + for (const key of keys) { + const lower = key.toLocaleLowerCase('en-US'); + if (!lowerKeys.delete(lower)) { + difference.push(lower); + } + } + const badKeys = `'${difference.join("', '")}'`; + throw new InputError( + `Entity has duplicate keys that vary only in casing, ${badKeys}`, + ); + } + return mapToRows(raw, entityId); } diff --git a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts index 353e849176..2ed65d3903 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts @@ -209,6 +209,11 @@ export async function performStitching(options: { entity.metadata.etag = hash; } + // This may throw if the entity is invalid, so we call it before + // the final_entities write, even though we may end up not needing + // to write the search index. + const searchEntries = buildEntitySearch(entityId, entity); + const amountOfRowsChanged = await knex('final_entities') .update({ final_entity: JSON.stringify(entity), @@ -223,7 +228,6 @@ export async function performStitching(options: { return 'abandoned'; } - const searchEntries = buildEntitySearch(entityId, entity); await knex.transaction(async trx => { await trx('search').where({ entity_id: entityId }).delete(); await trx.batchInsert('search', searchEntries, BATCH_SIZE);