chore: code review comments

Signed-off-by: benjdlambert <ben@blam.sh>
This commit is contained in:
benjdlambert
2025-05-27 14:19:41 +02:00
parent 2d2d97e12b
commit a9219496d5
11 changed files with 115 additions and 77 deletions
+1
View File
@@ -35,6 +35,7 @@ backend:
# auth:
# keys:
# - secret: ${BACKEND_SECRET}
auth:
# TODO: once plugins have been migrated we can remove this, but right now it
# is require for the backend-next to work in this repo
@@ -47,15 +47,14 @@ export class DefaultActionsService implements ActionsService {
return new DefaultActionsService(discovery, config, logger, auth);
}
async listActions({ credentials }: { credentials: BackstageCredentials }) {
async list({ credentials }: { credentials: BackstageCredentials }) {
const pluginSources =
this.config.getOptionalStringArray('backend.actions.pluginSources') ?? [];
const remoteActionsList = await Promise.all(
pluginSources.map(async source => {
const pluginBaseUrl = await this.discovery.getBaseUrl(source);
const response = await this.makeRequest({
url: `${pluginBaseUrl}/.backstage/actions/v1/actions`,
path: `/.backstage/actions/v1/actions`,
pluginId: source,
credentials,
});
@@ -76,17 +75,16 @@ export class DefaultActionsService implements ActionsService {
return { actions: remoteActionsList.flat() };
}
async invokeAction(opts: {
async invoke(opts: {
id: string;
input?: JsonObject;
credentials: BackstageCredentials;
}) {
const pluginId = this.pluginIdFromActionId(opts.id);
const baseUrl = await this.discovery.getBaseUrl(pluginId);
const response = await this.makeRequest({
url: `${baseUrl}/.backstage/actions/v1/actions/${opts.id}/invoke`,
path: `/.backstage/actions/v1/actions/${encodeURIComponent(
opts.id,
)}/invoke`,
pluginId,
credentials: opts.credentials,
options: {
@@ -107,18 +105,20 @@ export class DefaultActionsService implements ActionsService {
}
private async makeRequest(opts: {
url: string;
path: string;
pluginId: string;
options?: RequestInit;
credentials: BackstageCredentials;
}) {
const { url, credentials, options } = opts;
const { path, pluginId, credentials, options } = opts;
const baseUrl = await this.discovery.getBaseUrl(pluginId);
const { token } = await this.auth.getPluginRequestToken({
onBehalfOf: credentials,
targetPluginId: opts.pluginId,
});
return fetch(url, {
return fetch(`${baseUrl}${path}`, {
...options,
headers: {
...options?.headers,
@@ -66,6 +66,10 @@ describe('actionsServiceFactory', () => {
id: 'my-plugin:test',
name: 'testy',
title: 'Test',
schema: {
input: {},
output: {},
},
};
beforeEach(() => {
@@ -89,13 +93,13 @@ describe('actionsServiceFactory', () => {
);
});
describe('listActions', () => {
describe('list', () => {
it('should list all plugins in config to find actions and handle failures gracefully', async () => {
const subject = await ServiceFactoryTester.from(actionsServiceFactory, {
dependencies: defaultServices,
}).getSubject();
const { actions } = await subject.listActions({
const { actions } = await subject.list({
credentials: mockCredentials.service('user:default/mock'),
});
@@ -106,7 +110,7 @@ describe('actionsServiceFactory', () => {
});
});
describe('invokeAction', () => {
describe('invoke', () => {
it('should invoke the action and return the output', async () => {
server.use(
rest.post(
@@ -118,7 +122,7 @@ describe('actionsServiceFactory', () => {
dependencies: defaultServices,
}).getSubject();
const { output } = await subject.invokeAction({
const { output } = await subject.invoke({
id: 'my-plugin:test',
credentials: mockCredentials.service('user:default/mock'),
});
@@ -139,7 +143,7 @@ describe('actionsServiceFactory', () => {
}).getSubject();
await expect(
subject.invokeAction({
subject.invoke({
id: 'my-plugin:test',
credentials: mockCredentials.service('user:default/mock'),
}),
@@ -159,7 +163,7 @@ describe('actionsServiceFactory', () => {
}).getSubject();
await expect(
subject.invokeAction({
subject.invoke({
id: 'my-plugin:test',
credentials: mockCredentials.service('user:default/mock'),
}),
@@ -238,7 +242,7 @@ describe('actionsServiceFactory', () => {
innerRouter.post('/invoke', async (req, res) => {
const { id, input } = req.body;
const response = await actionsService.invokeAction({
const response = await actionsService.invoke({
id,
input,
credentials: await httpAuth.credentials(req),
@@ -247,7 +251,7 @@ describe('actionsServiceFactory', () => {
});
innerRouter.get('/actions', async (req, res) => {
const response = await actionsService.listActions({
const response = await actionsService.list({
credentials: await httpAuth.credentials(req),
});
@@ -34,19 +34,15 @@ import {
} from '@backstage/errors';
export class DefaultActionsRegistryService implements ActionsRegistryService {
private actions: Map<string, ActionsRegistryActionOptions<any, any>> =
new Map();
private constructor(
private readonly actions: Map<
string,
ActionsRegistryActionOptions<any, any>
>,
private readonly router: Router,
private readonly logger: LoggerService,
private readonly httpAuth: HttpAuthService,
private readonly auth: AuthService,
private readonly metadata: PluginMetadataService,
) {
this.bindRoutes();
}
) {}
static create({
httpAuth,
@@ -59,36 +55,14 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
auth: AuthService;
metadata: PluginMetadataService;
}): DefaultActionsRegistryService {
return new DefaultActionsRegistryService(
new Map(),
PromiseRouter(),
logger,
httpAuth,
auth,
metadata,
);
return new DefaultActionsRegistryService(logger, httpAuth, auth, metadata);
}
getRouter(): Router {
return this.router;
}
createRouter(): Router {
const router = PromiseRouter();
router.use(json());
register<TInputSchema extends ZodType, TOutputSchema extends ZodType>(
options: ActionsRegistryActionOptions<TInputSchema, TOutputSchema>,
): void {
const id = `${this.metadata.getId()}:${options.name}`;
if (this.actions.has(id)) {
throw new Error(`Action with id "${id}" is already registered`);
}
this.actions.set(id, options);
}
private bindRoutes() {
this.router.use(json());
this.router.get('/.backstage/actions/v1/actions', (_, res) => {
router.get('/.backstage/actions/v1/actions', (_, res) => {
return res.json({
actions: Array.from(this.actions.entries()).map(([id, action]) => ({
id,
@@ -105,7 +79,7 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
});
});
this.router.post(
router.post(
'/.backstage/actions/v1/actions/:actionId/invoke',
async (req, res) => {
const action = this.actions.get(req.params.actionId);
@@ -165,5 +139,18 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
}
},
);
return router;
}
register<TInputSchema extends ZodType, TOutputSchema extends ZodType>(
options: ActionsRegistryActionOptions<TInputSchema, TOutputSchema>,
): void {
const id = `${this.metadata.getId()}:${options.name}`;
if (this.actions.has(id)) {
throw new Error(`Action with id "${id}" is already registered`);
}
this.actions.set(id, options);
}
}
@@ -194,7 +194,11 @@ describe('actionsRegistryServiceFactory', () => {
name: 'test',
title: 'Test',
description: 'Test',
action: async () => ({ output: { ok: true } }),
schema: {
input: z => z.undefined(),
output: z => z.string(),
},
action: async () => ({ output: 'ok' }),
});
},
});
@@ -40,7 +40,7 @@ export const actionsRegistryServiceFactory = createServiceFactory({
metadata,
});
httpRouter.use(actionsRegistryService.getRouter());
httpRouter.use(actionsRegistryService.createRouter());
return actionsRegistryService;
},
+11 -11
View File
@@ -44,16 +44,16 @@ export type ActionsRegistryActionOptions<
name: string;
title: string;
description: string;
schema?: {
input?: (zod: typeof z) => TInputSchema;
output?: (zod: typeof z) => TOutputSchema;
schema: {
input: (zod: typeof z) => TInputSchema;
output: (zod: typeof z) => TOutputSchema;
};
action: (context: ActionsRegistryActionContext<TInputSchema>) => Promise<
TOutputSchema extends ZodType
? {
z.infer<TOutputSchema> extends void
? void
: {
output: z.infer<TOutputSchema>;
}
: void
>;
};
@@ -68,7 +68,7 @@ export interface ActionsRegistryService {
// @public (undocumented)
export interface ActionsService {
// (undocumented)
invokeAction(opts: {
invoke(opts: {
id: string;
input?: JsonObject;
credentials: BackstageCredentials;
@@ -76,7 +76,7 @@ export interface ActionsService {
output: JsonValue;
}>;
// (undocumented)
listActions: (opts: { credentials: BackstageCredentials }) => Promise<{
list: (opts: { credentials: BackstageCredentials }) => Promise<{
actions: ActionsServiceAction[];
}>;
}
@@ -87,9 +87,9 @@ export type ActionsServiceAction = {
name: string;
title: string;
description: string;
schema?: {
input?: JSONSchema7;
output?: JSONSchema7;
schema: {
input: JSONSchema7;
output: JSONSchema7;
};
};
@@ -36,14 +36,16 @@ export type ActionsRegistryActionOptions<
name: string;
title: string;
description: string;
schema?: {
input?: (zod: typeof z) => TInputSchema;
output?: (zod: typeof z) => TOutputSchema;
schema: {
input: (zod: typeof z) => TInputSchema;
output: (zod: typeof z) => TOutputSchema;
};
action: (
context: ActionsRegistryActionContext<TInputSchema>,
) => Promise<
TOutputSchema extends ZodType ? { output: z.infer<TOutputSchema> } : void
z.infer<TOutputSchema> extends void
? void
: { output: z.infer<TOutputSchema> }
>;
};
@@ -25,9 +25,9 @@ export type ActionsServiceAction = {
name: string;
title: string;
description: string;
schema?: {
input?: JSONSchema7;
output?: JSONSchema7;
schema: {
input: JSONSchema7;
output: JSONSchema7;
};
};
@@ -35,10 +35,10 @@ export type ActionsServiceAction = {
* @public
*/
export interface ActionsService {
listActions: (opts: {
list: (opts: {
credentials: BackstageCredentials;
}) => Promise<{ actions: ActionsServiceAction[] }>;
invokeAction(opts: {
invoke(opts: {
id: string;
input?: JsonObject;
credentials: BackstageCredentials;
+24
View File
@@ -3,6 +3,8 @@
> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
```ts
import { ActionsRegistryService } from '@backstage/backend-plugin-api';
import { ActionsService } from '@backstage/backend-plugin-api';
import { AuditorService } from '@backstage/backend-plugin-api';
import { AuthService } from '@backstage/backend-plugin-api';
import { Backend } from '@backstage/backend-app-api';
@@ -161,6 +163,28 @@ export function mockErrorHandler(): ErrorRequestHandler<
// @public
export namespace mockServices {
// (undocumented)
export namespace actions {
const // (undocumented)
factory: () => ServiceFactory<ActionsService, 'plugin', 'singleton'>;
const // (undocumented)
mock: (
partialImpl?: Partial<ActionsService> | undefined,
) => ServiceMock<ActionsService>;
}
// (undocumented)
export namespace actionsRegistry {
const // (undocumented)
factory: () => ServiceFactory<
ActionsRegistryService,
'plugin',
'singleton'
>;
const // (undocumented)
mock: (
partialImpl?: Partial<ActionsRegistryService> | undefined,
) => ServiceMock<ActionsRegistryService>;
}
// (undocumented)
export namespace auditor {
const // (undocumented)
@@ -53,6 +53,8 @@ import { MockRootLoggerService } from './MockRootLoggerService';
import { MockUserInfoService } from './MockUserInfoService';
import { mockCredentials } from './mockCredentials';
import { MockEventsService } from './MockEventsService';
import { actionsServiceFactory } from '@backstage/backend-defaults/actions';
import { actionsRegistryServiceFactory } from '@backstage/backend-defaults/actionsRegistry';
/** @internal */
function createLoggerMock() {
@@ -518,6 +520,20 @@ export namespace mockServices {
search: jest.fn(),
}));
}
export namespace actions {
export const factory = () => actionsServiceFactory;
export const mock = simpleMock(coreServices.actions, () => ({
list: jest.fn(),
invoke: jest.fn(),
}));
}
export namespace actionsRegistry {
export const factory = () => actionsRegistryServiceFactory;
export const mock = simpleMock(coreServices.actionsRegistry, () => ({
register: jest.fn(),
}));
}
/**
* Creates a functional mock implementation of the