From cf7506ab84aef4254b5f24079064e677965a3bb4 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Wed, 10 Dec 2025 10:34:07 +0100 Subject: [PATCH] chore: refactor slightly to bring into similar patterns to the scaffolder action. Signed-off-by: benjdlambert --- .changeset/nine-symbols-shout.md | 2 +- ...reateRegisterCatalogEntitiesAction.test.ts | 35 +-- .../createRegisterCatalogEntitiesAction.ts | 90 +++--- ...ateUnregisterCatalogEntitiesAction.test.ts | 285 +++++++++++++++--- .../createUnregisterCatalogEntitiesAction.ts | 88 ++++-- 5 files changed, 339 insertions(+), 161 deletions(-) diff --git a/.changeset/nine-symbols-shout.md b/.changeset/nine-symbols-shout.md index 87fff3091b..21a7838a1f 100644 --- a/.changeset/nine-symbols-shout.md +++ b/.changeset/nine-symbols-shout.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': minor --- -Add MCP actions to 1) register existing components from a code respository, 2) unregister components with the location ID return from a prior register existing components invocation. +Added `ActionsRegistry` actions for `register-entity` and `unregister-entity` diff --git a/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.test.ts b/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.test.ts index a85456ff42..31a89a31f3 100644 --- a/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.test.ts +++ b/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.test.ts @@ -40,15 +40,15 @@ describe('createRegisterCatalogEntitiesAction', () => { }); const result = await mockActionsRegistry.invoke({ - id: 'test:register-catalog-entities', + id: 'test:register-entity', input: { - locationURL: + locationUrl: 'https://github.com/example/repo/blob/main/catalog-info.yaml', }, }); expect(result.output).toEqual({ - locationID: mockLocationId, + locationId: mockLocationId, }); expect(mockCatalog.addLocation).toHaveBeenCalledWith( { @@ -61,7 +61,7 @@ describe('createRegisterCatalogEntitiesAction', () => { ); }); - it('should throw an error if locationURL is not provided', async () => { + it('should throw an error if locationUrl is not a valid URL', async () => { const mockActionsRegistry = actionsRegistryServiceMock(); const mockCatalog = catalogServiceMock(); @@ -72,27 +72,10 @@ describe('createRegisterCatalogEntitiesAction', () => { await expect( mockActionsRegistry.invoke({ - id: 'test:register-catalog-entities', - input: { locationURL: '' }, + id: 'test:register-entity', + input: { locationUrl: 'not-a-valid-url' }, }), - ).rejects.toThrow('a location URL must be specified'); - }); - - it('should throw an error if locationURL is not a valid URL', async () => { - const mockActionsRegistry = actionsRegistryServiceMock(); - const mockCatalog = catalogServiceMock(); - - createRegisterCatalogEntitiesAction({ - catalog: mockCatalog, - actionsRegistry: mockActionsRegistry, - }); - - await expect( - mockActionsRegistry.invoke({ - id: 'test:register-catalog-entities', - input: { locationURL: 'not-a-valid-url' }, - }), - ).rejects.toThrow('locationURL "not-a-valid-url" an invalid URL'); + ).rejects.toThrow('not-a-valid-url is an invalid URL'); }); it('should throw a ForwardedError if catalog.addLocation throws an error', async () => { @@ -111,9 +94,9 @@ describe('createRegisterCatalogEntitiesAction', () => { await expect( mockActionsRegistry.invoke({ - id: 'test:register-catalog-entities', + id: 'test:register-entity', input: { - locationURL: + locationUrl: 'https://github.com/example/repo/blob/main/catalog-info.yaml', }, }), diff --git a/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.ts b/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.ts index e2a8fb2984..c521672ae1 100644 --- a/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.ts +++ b/plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.ts @@ -14,8 +14,18 @@ * limitations under the License. */ import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; +import { InputError } from '@backstage/errors'; import { CatalogService } from '@backstage/plugin-catalog-node'; -import { InputError, ForwardedError } from '@backstage/errors'; + +const isValidUrl = (url: string) => { + try { + // eslint-disable-next-line no-new + new URL(url); + return true; + } catch (error) { + return false; + } +}; export const createRegisterCatalogEntitiesAction = ({ catalog, @@ -25,85 +35,55 @@ export const createRegisterCatalogEntitiesAction = ({ actionsRegistry: ActionsRegistryService; }) => { actionsRegistry.register({ - name: 'register-catalog-entities', - title: 'Register Catalog Entities', + name: 'register-entity', + title: 'Register entity in the Catalog', attributes: { destructive: false, readOnly: false, - idempotent: true, + idempotent: false, }, - description: `Register entities defined remotely (create new Locations). + description: `Registers one or more entities in the Backstage catalog by creating a Location entity that points to a remote catalog-info.yaml file. -This tool is analogous to the "register existing components" function you see in the Backstage dashboard, -where you supply a URL link that allows for the downloading of a 'catalog-info.yaml' file that -defines the various Entities (Components, Systems, Resources, APIs, Users, and Groups) you want to -create or import into the Backstage catalog. +This action is similar to the "Register existing component" flow in the Backstage UI, where you provide a URL to a catalog-info.yaml file describing catalog entities such as Components, Systems, Resources, APIs, Users, and Groups. The action will create a new Location entity that references the provided file; all entities defined within that file will be imported into the catalog. -The action within the catalog results in the creation of a Location entity that is the parent of owner -of all the additional entities defined in the 'catalog-info.yaml' file. - -The dynamically generated, random UUID created to go along with the newly created Location entity is returned -to the caller. That UUID can be later used if you want to unregister or delete the Location entity and all -of its children entities. - -/eExample invocation and the output from the invocation: - # Register a Location from a GitHub URL - register-catalog-entities locationURL: https://github.com/redhat-ai-dev/model-catalog-example/blob/main/developer-model-service/catalog-info.yaml - Output: { - "locationID": "aaaa-bbbb-cccc-dddd" - } +A unique identifier (locationId) will be returned for the newly created Location. You can use this identifier in the future to unregister or delete the Location and all entities it owns. `, schema: { input: z => z.object({ - locationURL: z + locationUrl: z .string() .describe( - `URL reference to the catalog-info.yaml file that describes what entities to import.`, + `URL reference to the catalog-info.yaml file that describes the entity to register. For example: https://github.com/backstage/demo/blob/master/catalog-info.yaml`, ), }), output: z => z.object({ - locationID: z + locationId: z .string() - .optional() - .describe('The location ID generated by the registration'), + .describe('The ID of the entity that was registered'), }), }, action: async ({ input, credentials }) => { - if (!input.locationURL || input.locationURL === '') { - throw new InputError('a location URL must be specified'); + if (!isValidUrl(input.locationUrl)) { + throw new InputError(`${input.locationUrl} is an invalid URL`); } - // Validate that the locationURL is a valid URL - try { - // eslint-disable-next-line no-new - new URL(input.locationURL); - } catch { - throw new InputError( - `locationURL "${input.locationURL}" an invalid URL`, - ); - } - try { - const locReq = { + const result = await catalog.addLocation( + { type: 'url', - target: input.locationURL, - }; - const result = await catalog.addLocation(locReq, { + target: input.locationUrl, + }, + { credentials, - }); + }, + ); - return { - output: { - locationID: result.location.id, - }, - }; - } catch (error) { - throw new ForwardedError( - `catalog import of "${input.locationURL} failed"`, - error, - ); - } + return { + output: { + locationId: result.location.id, + }, + }; }, }); }; diff --git a/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.test.ts b/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.test.ts index 225497c315..492a427630 100644 --- a/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.test.ts +++ b/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.test.ts @@ -16,70 +16,259 @@ import { createUnregisterCatalogEntitiesAction } from './createUnregisterCatalogEntitiesAction'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha'; -import { ForwardedError } from '@backstage/errors'; describe('createUnregisterCatalogEntitiesAction', () => { - it('should successfully unregister a catalog location with a valid locationId', async () => { - const mockActionsRegistry = actionsRegistryServiceMock(); - const mockCatalog = catalogServiceMock(); + describe('with locationId', () => { + it('should successfully unregister a catalog location with a valid locationId', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); - mockCatalog.removeLocationById = jest.fn().mockResolvedValue(undefined); + mockCatalog.removeLocationById = jest.fn().mockResolvedValue(undefined); - createUnregisterCatalogEntitiesAction({ - catalog: mockCatalog, - actionsRegistry: mockActionsRegistry, + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + const result = await mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { type: { locationId: 'test-location-id-1234' } }, + }); + + expect(result.output).toEqual({}); + expect(mockCatalog.removeLocationById).toHaveBeenCalledWith( + 'test-location-id-1234', + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); }); - const result = await mockActionsRegistry.invoke({ - id: 'test:unregister-catalog-entities', - input: { locationId: 'test-location-id-1234' }, - }); + it('should throw an error if catalog.removeLocationById throws an error', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); - expect(result.output).toEqual({}); - expect(mockCatalog.removeLocationById).toHaveBeenCalledWith( - 'test-location-id-1234', - expect.objectContaining({ - credentials: expect.any(Object), - }), - ); + const errorMessage = 'Failed to remove location'; + mockCatalog.removeLocationById = jest + .fn() + .mockRejectedValue(new Error(errorMessage)); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + await expect( + mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { type: { locationId: 'test-location-id-1234' } }, + }), + ).rejects.toThrow(errorMessage); + }); }); - it('should throw an error if locationId is not provided', async () => { - const mockActionsRegistry = actionsRegistryServiceMock(); - const mockCatalog = catalogServiceMock(); + describe('with locationUrl', () => { + it('should successfully unregister a catalog location with a valid locationUrl', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); - createUnregisterCatalogEntitiesAction({ - catalog: mockCatalog, - actionsRegistry: mockActionsRegistry, + const locationUrl = + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml'; + + mockCatalog.getLocations = jest.fn().mockResolvedValue({ + items: [ + { id: 'location-id-1', target: locationUrl }, + { id: 'location-id-2', target: 'https://other-url.com/catalog.yaml' }, + ], + }); + mockCatalog.removeLocationById = jest.fn().mockResolvedValue(undefined); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + const result = await mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { type: { locationUrl } }, + }); + + expect(result.output).toEqual({}); + expect(mockCatalog.getLocations).toHaveBeenCalledWith( + {}, + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); + expect(mockCatalog.removeLocationById).toHaveBeenCalledTimes(1); + expect(mockCatalog.removeLocationById).toHaveBeenCalledWith( + 'location-id-1', + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); }); - await expect( - mockActionsRegistry.invoke({ - id: 'test:unregister-catalog-entities', - input: { locationId: '' }, - }), - ).rejects.toThrow('a locationID must be specified'); - }); + it('should match locationUrl case-insensitively', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); - it('should throw a ForwardedError if catalog.removeLocationById throws an error', async () => { - const mockActionsRegistry = actionsRegistryServiceMock(); - const mockCatalog = catalogServiceMock(); + mockCatalog.getLocations = jest.fn().mockResolvedValue({ + items: [ + { + id: 'location-id-1', + target: + 'https://github.com/Backstage/Demo/blob/master/catalog-info.yaml', + }, + ], + }); + mockCatalog.removeLocationById = jest.fn().mockResolvedValue(undefined); - const errorMessage = 'Failed to remove location'; - mockCatalog.removeLocationById = jest - .fn() - .mockRejectedValue(new Error(errorMessage)); + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); - createUnregisterCatalogEntitiesAction({ - catalog: mockCatalog, - actionsRegistry: mockActionsRegistry, + const result = await mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { + type: { + locationUrl: + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml', + }, + }, + }); + + expect(result.output).toEqual({}); + expect(mockCatalog.removeLocationById).toHaveBeenCalledTimes(1); + expect(mockCatalog.removeLocationById).toHaveBeenCalledWith( + 'location-id-1', + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); }); - await expect( - mockActionsRegistry.invoke({ - id: 'test:unregister-catalog-entities', - input: { locationId: 'test-location-id-1234' }, - }), - ).rejects.toThrow(ForwardedError); + it('should unregister multiple locations with the same URL', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); + + const locationUrl = + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml'; + + mockCatalog.getLocations = jest.fn().mockResolvedValue({ + items: [ + { id: 'location-id-1', target: locationUrl }, + { id: 'location-id-2', target: locationUrl }, + ], + }); + mockCatalog.removeLocationById = jest.fn().mockResolvedValue(undefined); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + const result = await mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { type: { locationUrl } }, + }); + + expect(result.output).toEqual({}); + expect(mockCatalog.removeLocationById).toHaveBeenCalledTimes(2); + expect(mockCatalog.removeLocationById).toHaveBeenNthCalledWith( + 1, + 'location-id-1', + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); + expect(mockCatalog.removeLocationById).toHaveBeenNthCalledWith( + 2, + 'location-id-2', + expect.objectContaining({ + credentials: expect.any(Object), + }), + ); + }); + + it('should throw NotFoundError if no location matches the URL', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); + + mockCatalog.getLocations = jest.fn().mockResolvedValue({ + items: [ + { id: 'location-id-1', target: 'https://other-url.com/catalog.yaml' }, + ], + }); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + await expect( + mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { + type: { + locationUrl: + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml', + }, + }, + }), + ).rejects.toThrow(/NotFoundError/); + }); + + it('should throw NotFoundError with descriptive message when location not found', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); + + const locationUrl = + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml'; + + mockCatalog.getLocations = jest.fn().mockResolvedValue({ + items: [], + }); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + await expect( + mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { type: { locationUrl } }, + }), + ).rejects.toThrow(`Location with URL ${locationUrl} not found`); + }); + + it('should throw an error if catalog.getLocations throws an error', async () => { + const mockActionsRegistry = actionsRegistryServiceMock(); + const mockCatalog = catalogServiceMock(); + + const errorMessage = 'Failed to get locations'; + mockCatalog.getLocations = jest + .fn() + .mockRejectedValue(new Error(errorMessage)); + + createUnregisterCatalogEntitiesAction({ + catalog: mockCatalog, + actionsRegistry: mockActionsRegistry, + }); + + await expect( + mockActionsRegistry.invoke({ + id: 'test:unregister-entity', + input: { + type: { + locationUrl: + 'https://github.com/backstage/demo/blob/master/catalog-info.yaml', + }, + }, + }), + ).rejects.toThrow(errorMessage); + }); }); }); diff --git a/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.ts b/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.ts index a7ed9b6357..831cc7410a 100644 --- a/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.ts +++ b/plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.ts @@ -14,8 +14,8 @@ * limitations under the License. */ import { ActionsRegistryService } from '@backstage/backend-plugin-api/alpha'; +import { NotFoundError } from '@backstage/errors'; import { CatalogService } from '@backstage/plugin-catalog-node'; -import { ForwardedError, InputError } from '@backstage/errors'; export const createUnregisterCatalogEntitiesAction = ({ catalog, @@ -25,51 +25,77 @@ export const createUnregisterCatalogEntitiesAction = ({ actionsRegistry: ActionsRegistryService; }) => { actionsRegistry.register({ - name: 'unregister-catalog-entities', - title: 'Unregister Catalog Entities', + name: 'unregister-entity', + title: 'Unregister entity from the Catalog', attributes: { destructive: true, readOnly: false, idempotent: true, }, - description: `Unregister a Location and the entities it ownws. + description: `Unregisters a Location entity and all entities it owns from the Backstage catalog. -This tool is analogous to the "unregister location" function you see in the Backstage dashboard, -where you supply the UUID for a Location entity link that allows for the removal of the Location and -the various Entities (Components, Systems, Resources, APIs, Users, and Groups) that were also created -when the Location was imported. +This action is similar to the "Unregister location" function in the Backstage UI, where you provide the unique identifier (locationId) of a Location entity. The action will remove the specified Location from the catalog as well as all entities that were created when the Location was imported. -Example invocation and the output from the invocation: - # Register a Location from a GitHub URL - unregister-catalog-entities locationId: aaa-bbb-ccc-ddd - Output: {} +Once completed, all entities associated with the Location will be deleted from the catalog. `, schema: { input: z => - z.object({ - locationId: z - .string() - .describe( - `Location ID returned from the 'register-catalog-entities' call.`, - ), - }), + z + .object({ + type: z.union([ + z.object({ + locationId: z + .string() + .describe(`Location ID of the Entity to unregister`), + }), + z.object({ + locationUrl: z + .string() + .describe( + `URL of the catalog-info.yaml file to unregister for example: https://github.com/backstage/demo/blob/master/catalog-info.yaml`, + ), + }), + ]), + }) + .describe( + 'The type to the unregister-entity action. Either locationId or locationUrl must be provided.', + ), output: z => z.object({}), }, - action: async ({ input, credentials }) => { - if (!input.locationId || input.locationId === '') { - throw new InputError('a locationID must be specified'); - } - - try { - await catalog.removeLocationById(input.locationId, { + action: async ({ input: { type }, credentials }) => { + if ('locationId' in type) { + await catalog.removeLocationById(type.locationId, { credentials, }); - } catch (error) { - throw new ForwardedError( - `catalog removal of "${input.locationId} failed"`, - error, - ); + } else { + const locations = await catalog + .getLocations( + {}, + { + credentials, + }, + ) + .then(response => + response.items.filter( + location => + location.target.toLowerCase() === + type.locationUrl.toLowerCase(), + ), + ); + + if (locations.length === 0) { + throw new NotFoundError( + `Location with URL ${type.locationUrl} not found`, + ); + } + + for (const location of locations) { + await catalog.removeLocationById(location.id, { + credentials, + }); + } } + return { output: {} }; }, });