diff --git a/.changeset/angry-jobs-ring.md b/.changeset/angry-jobs-ring.md new file mode 100644 index 0000000000..a216af1683 --- /dev/null +++ b/.changeset/angry-jobs-ring.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-client': patch +--- + +Updated API docs diff --git a/.changeset/few-oranges-complain.md b/.changeset/few-oranges-complain.md new file mode 100644 index 0000000000..70eb921a92 --- /dev/null +++ b/.changeset/few-oranges-complain.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Fix handling of `optional` property in `catalog:register` scaffolder action diff --git a/packages/catalog-client/src/types/api.ts b/packages/catalog-client/src/types/api.ts index ee6579efca..5285bda2f7 100644 --- a/packages/catalog-client/src/types/api.ts +++ b/packages/catalog-client/src/types/api.ts @@ -353,6 +353,10 @@ export type Location = { export type AddLocationRequest = { type?: string; target: string; + /** + * If set to true, the location will not be added, but the response will + * contain the entities that match the given location. + */ dryRun?: boolean; }; @@ -363,8 +367,13 @@ export type AddLocationRequest = { */ export type AddLocationResponse = { location: Location; + /** + * The entities matching this location. Will only be filled in dryRun mode + */ entities: Entity[]; - // Only set in dryRun mode. + /** + * True, if the location exists. Will only be filled in dryRun mode + */ exists?: boolean; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts index c718f3906e..bae9f04575 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts @@ -265,9 +265,7 @@ describe('catalog:register', () => { it('should ignore failures when dry running the location in the catalog if `optional` is set', async () => { addLocation - .mockResolvedValueOnce({ - entities: [], - }) + .mockRejectedValueOnce(new Error('Not found')) .mockRejectedValueOnce(new Error('Not found')); await action.handler({ ...mockContext, @@ -300,4 +298,54 @@ describe('catalog:register', () => { 'http://foo/var', ); }); + + it('should fetch entities when adding location in the catalog fails and `optional` is set', async () => { + addLocation + .mockRejectedValueOnce(new Error('Already registered')) + .mockResolvedValueOnce({ + entities: [ + { + metadata: { + namespace: 'default', + name: 'test', + }, + kind: 'Component', + } as Entity, + ], + }); + await action.handler({ + ...mockContext, + input: { + catalogInfoUrl: 'http://foo/var', + optional: true, + }, + }); + + expect(addLocation).toHaveBeenNthCalledWith( + 1, + { + type: 'url', + target: 'http://foo/var', + }, + {}, + ); + expect(addLocation).toHaveBeenNthCalledWith( + 2, + { + dryRun: true, + type: 'url', + target: 'http://foo/var', + }, + {}, + ); + + expect(mockContext.output).toHaveBeenCalledWith( + 'catalogInfoUrl', + 'http://foo/var', + ); + expect(mockContext.output).toHaveBeenCalledWith( + 'entityRef', + 'component:default/test', + ); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts index e866eb12ba..3e35e16508 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts @@ -17,7 +17,7 @@ import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { CatalogApi } from '@backstage/catalog-client'; -import { stringifyEntityRef } from '@backstage/catalog-model'; +import { stringifyEntityRef, Entity } from '@backstage/catalog-model'; import { createTemplateAction } from '@backstage/plugin-scaffolder-node'; import yaml from 'yaml'; @@ -144,17 +144,26 @@ export function createCatalogRegisterAction(options: { ctx.logger.info(`Registering ${catalogInfoUrl} in the catalog`); - await catalogClient.addLocation( - { - type: 'url', - target: catalogInfoUrl, - }, - ctx.secrets?.backstageToken - ? { token: ctx.secrets.backstageToken } - : {}, - ); + try { + // 1st try to register the location, this will throw an error if the location already exists (see catch) + await catalogClient.addLocation( + { + type: 'url', + target: catalogInfoUrl, + }, + ctx.secrets?.backstageToken + ? { token: ctx.secrets.backstageToken } + : {}, + ); + } catch (e) { + if (!input.optional) { + // if optional is false or unset, it is not allowed to register the same location twice, we rethrow the error + throw e; + } + } try { + // 2nd retry the registration as a dry run, this will not throw an error if the location already exists const result = await catalogClient.addLocation( { dryRun: true, @@ -166,18 +175,18 @@ export function createCatalogRegisterAction(options: { : {}, ); - if (result.entities.length > 0) { + if (result.entities.length) { const { entities } = result; - let entity: any; + let entity: Entity | undefined; // prioritise 'Component' type as it is the most central kind of entity entity = entities.find( - (e: any) => + e => !e.metadata.name.startsWith('generated-') && e.kind === 'Component', ); if (!entity) { entity = entities.find( - (e: any) => !e.metadata.name.startsWith('generated-'), + e => !e.metadata.name.startsWith('generated-'), ); } if (!entity) {