From 5132d28818568a0c45132f9f49b49cba138a6b08 Mon Sep 17 00:00:00 2001 From: Beth Griggs Date: Fri, 21 Jun 2024 18:35:30 +0100 Subject: [PATCH] fix(plugin-org): implement batching in useGetEntities to handle large headers The useGetEntities hook can result in requests to /api/catalog/entities where the headers exceed the default maximum Node.js header size of 16KB. This commit implements batching in the useGetEntities hook to handle cases where this is likely to occur. Refs: https://github.com/backstage/backstage/issues/22139 Signed-off-by: Beth Griggs --- .changeset/fast-pens-smell.md | 5 + .../OwnershipCard/useGetEntities.test.ts | 104 ++++++++++++++++++ .../Cards/OwnershipCard/useGetEntities.ts | 50 +++++---- 3 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 .changeset/fast-pens-smell.md diff --git a/.changeset/fast-pens-smell.md b/.changeset/fast-pens-smell.md new file mode 100644 index 0000000000..6d909a397c --- /dev/null +++ b/.changeset/fast-pens-smell.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +The `useGetEntities` hook could result in requests to `/api/catalog/entities` where the headers exceed the default maximum Node.js header size of 16KB. The hook logic has been adjusted to batch the requests. diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts index eac5a171ba..2c223c3f4f 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts @@ -231,6 +231,110 @@ describe('useGetEntities', () => { ); }); }); + + describe('useGetEntities with 500+ relations', () => { + const manyGroups = Array.from({ length: 555 }, (_, i) => + createGroupEntityFromName(`group${i + 1}`), + ); + + const givenUserWithManyGroups = { + kind: 'User', + metadata: { + name: givenUser, + }, + spec: { + memberOf: manyGroups.map( + group => `group:default/${group.metadata.name}`, + ), + }, + } as Partial as Entity; + + beforeEach(() => { + getEntityRelationsMock.mockImplementation(entity => + entity?.kind === 'User' + ? manyGroups.map(group => createGroupRefFromName(group.metadata.name)) + : [], + ); + (catalogApiMock.getEntities as jest.Mock).mockClear(); + }); + + it('should handle 500+ relations without exceeding URL length limits', async () => { + const { result } = renderHook( + ({ entity }) => useGetEntities(entity, 'aggregated'), + { + initialProps: { entity: givenUserWithManyGroups }, + }, + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + + const callArgs = (catalogApiMock.getEntities as jest.Mock).mock + .calls[0][0]; + + expect( + callArgs.filter[0]['relations.ownedBy'].length, + ).toBeLessThanOrEqual(100); + + const owners = callArgs.filter[0]['relations.ownedBy']; + + expect(Array.isArray(owners)).toBeTruthy(); + expect(owners.length).toBeLessThanOrEqual(100); + }); + }); + + describe('useGetEntities exceeding default 16384 bytes header size', () => { + const largeNumberOfGroups = Array.from({ length: 600 }, (_, i) => + createGroupEntityFromName(`very-long-group-name-${i + 1}`), + ); + + const givenUserWithLargeNumberOfGroups = { + kind: 'User', + metadata: { + name: givenUser, + }, + spec: { + memberOf: largeNumberOfGroups.map( + group => `group:default/${group.metadata.name}`, + ), + }, + } as Partial as Entity; + + beforeEach(() => { + getEntityRelationsMock.mockImplementation(entity => + entity?.kind === 'User' + ? largeNumberOfGroups.map(group => + createGroupRefFromName(group.metadata.name), + ) + : [], + ); + (catalogApiMock.getEntities as jest.Mock).mockClear(); + }); + + it('should batch the request to avoid exceeding header size limits', async () => { + const { result } = renderHook( + ({ entity }) => useGetEntities(entity, 'aggregated'), + { + initialProps: { entity: givenUserWithLargeNumberOfGroups }, + }, + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + const callArgs = (catalogApiMock.getEntities as jest.Mock).mock + .calls[0][0]; + + const url = new URL( + `http://localhost/api/catalog/entities?${new URLSearchParams( + callArgs as any, + )}`, + ); + const headerSize = url.href.length; + expect(headerSize).toBeLessThanOrEqual(16384); + + const owners = callArgs.filter[0]['relations.ownedBy']; + expect(Array.isArray(owners)).toBeTruthy(); + expect(owners.length).toBeLessThanOrEqual(100); + }); + }); }); function createGroupEntityFromName(name: string): Entity { diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts index 5a708bcfee..d919e1d7c5 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts @@ -143,26 +143,38 @@ const getOwners = async ( return [stringifyEntityRef(entity)]; }; -const getOwnedEntitiesByOwners = ( +const batchGetOwnedEntitiesByOwners = async ( owners: string[], kinds: string[], catalogApi: CatalogApi, -) => - catalogApi.getEntities({ - filter: [ - { - kind: kinds, - 'relations.ownedBy': owners, - }, - ], - fields: [ - 'kind', - 'metadata.name', - 'metadata.namespace', - 'spec.type', - 'relations', - ], - }); + batchSize: number = 100, +) => { + const batches = []; + + for (let i = 0; i < owners.length; i += batchSize) { + const batch = owners.slice(i, i + batchSize); + batches.push( + catalogApi.getEntities({ + filter: [ + { + kind: kinds, + 'relations.ownedBy': batch, + }, + ], + fields: [ + 'kind', + 'metadata.name', + 'metadata.namespace', + 'spec.type', + 'relations', + ], + }), + ); + } + + const results = await Promise.all(batches); + return results.flatMap(result => result.items); +}; export function useGetEntities( entity: Entity, @@ -191,13 +203,13 @@ export function useGetEntities( } = useAsync(async () => { const owners = await getOwners(entity, relations, catalogApi); - const ownedEntitiesList = await getOwnedEntitiesByOwners( + const ownedEntitiesList = await batchGetOwnedEntitiesByOwners( owners, kinds, catalogApi, ); - const counts = ownedEntitiesList.items.reduce( + const counts = ownedEntitiesList.reduce( (acc: EntityTypeProps[], ownedEntity) => { const match = acc.find( x => x.kind === ownedEntity.kind && x.type === ownedEntity.spec?.type,