diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts index f3a3b15ec9..c5d437d57b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.test.ts @@ -23,17 +23,14 @@ import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha' describe('DefaultTemplateActionRegistry', () => { let registry: DefaultTemplateActionRegistry; let mockActionsService: ReturnType; - let mockAuthService: ReturnType; let mockLogger: ReturnType; beforeEach(() => { mockActionsService = actionsRegistryServiceMock(); - mockAuthService = mockServices.auth(); mockLogger = mockServices.logger.mock(); registry = new DefaultTemplateActionRegistry( mockActionsService, - mockAuthService, mockLogger, ); }); @@ -80,7 +77,9 @@ describe('DefaultTemplateActionRegistry', () => { }); registry.register(action); - const result = await registry.get('test-action'); + const result = await registry.get('test-action', { + credentials: mockCredentials.user(), + }); expect(result).toBe(action); }); @@ -101,7 +100,9 @@ describe('DefaultTemplateActionRegistry', () => { action: async () => ({ output: {} }), }); - const result = await registry.get('test:service-action'); + const result = await registry.get('test:service-action', { + credentials: mockCredentials.user(), + }); expect(result.id).toBe('test:service-action'); expect(result.description).toBe('Service action'); @@ -109,28 +110,26 @@ describe('DefaultTemplateActionRegistry', () => { }); it('should throw NotFoundError when action is not found', async () => { - await expect(registry.get('non-existent-action')).rejects.toThrow( - NotFoundError, - ); - await expect(registry.get('non-existent-action')).rejects.toThrow( + await expect( + registry.get('non-existent-action', { + credentials: mockCredentials.user(), + }), + ).rejects.toThrow(NotFoundError); + await expect( + registry.get('non-existent-action', { + credentials: mockCredentials.user(), + }), + ).rejects.toThrow( "Template action with ID 'non-existent-action' is not registered", ); }); - - it('should use own service credentials when getting action', async () => { - const spy = jest.spyOn(mockActionsService, 'list'); - - await expect(registry.get('non-existent')).rejects.toThrow(); - - expect(spy).toHaveBeenCalledWith({ - credentials: await mockAuthService.getOwnServiceCredentials(), - }); - }); }); describe('list', () => { it('should return empty map when no actions are registered', async () => { - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); expect(result).toBeInstanceOf(Map); expect(result.size).toBe(0); @@ -152,7 +151,9 @@ describe('DefaultTemplateActionRegistry', () => { registry.register(action1); registry.register(action2); - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); expect(result.size).toBe(2); expect(result.get('action-1')).toBe(action1); @@ -175,7 +176,9 @@ describe('DefaultTemplateActionRegistry', () => { action: async () => ({ output: {} }), }); - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); expect(result.size).toBe(1); const action = result.get('test:service-action'); @@ -204,7 +207,9 @@ describe('DefaultTemplateActionRegistry', () => { }); registry.register(localAction); - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); expect(result.get('test:same-id')).toBe(localAction); expect(mockLogger.warn).toHaveBeenCalledWith( @@ -212,25 +217,6 @@ describe('DefaultTemplateActionRegistry', () => { ); }); - it('should use provided credentials when listing actions', async () => { - const credentials = mockCredentials.user(); - const spy = jest.spyOn(mockActionsService, 'list'); - - await registry.list({ credentials }); - - expect(spy).toHaveBeenCalledWith({ credentials }); - }); - - it('should use own service credentials when no credentials provided', async () => { - const spy = jest.spyOn(mockActionsService, 'list'); - - await registry.list(); - - expect(spy).toHaveBeenCalledWith({ - credentials: await mockAuthService.getOwnServiceCredentials(), - }); - }); - it('should set supportsDryRun to false for destructive actions', async () => { mockActionsService.register({ name: 'destructive-action', @@ -247,7 +233,9 @@ describe('DefaultTemplateActionRegistry', () => { action: async () => ({ output: {} }), }); - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); const action = result.get('test:destructive-action'); expect(action!.supportsDryRun).toBe(false); @@ -269,7 +257,9 @@ describe('DefaultTemplateActionRegistry', () => { action: async () => ({ output: {} }), }); - const result = await registry.list(); + const result = await registry.list({ + credentials: mockCredentials.user(), + }); const action = result.get('test:non-readonly-action'); expect(action!.supportsDryRun).toBe(false); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts index 7bcf120646..dab1447eee 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts @@ -18,7 +18,6 @@ import { ConflictError, NotFoundError } from '@backstage/errors'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { ActionsService } from '@backstage/backend-plugin-api/alpha'; import { - AuthService, BackstageCredentials, LoggerService, } from '@backstage/backend-plugin-api'; @@ -31,9 +30,12 @@ import { JsonObject } from '@backstage/types'; */ export interface TemplateActionRegistry { register(action: TemplateAction): void; - get(actionId: string): Promise>; - list(options?: { - credentials?: BackstageCredentials; + get( + actionId: string, + options: { credentials: BackstageCredentials }, + ): Promise>; + list(options: { + credentials: BackstageCredentials; }): Promise>>; } @@ -45,7 +47,6 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry { constructor( private readonly actionsRegistry: ActionsService, - private readonly auth: AuthService, private readonly logger: LoggerService, ) {} @@ -59,8 +60,11 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry { this.actions.set(action.id, action); } - async get(actionId: string): Promise> { - const action = (await this.list()).get(actionId); + async get( + actionId: string, + options: { credentials: BackstageCredentials }, + ): Promise> { + const action = (await this.list(options)).get(actionId); if (!action) { throw new NotFoundError( `Template action with ID '${actionId}' is not registered. See https://backstage.io/docs/features/software-templates/builtin-actions/ on how to add a new action module.`, @@ -69,14 +73,13 @@ export class DefaultTemplateActionRegistry implements TemplateActionRegistry { return action; } - async list(options?: { - credentials?: BackstageCredentials; + async list(options: { + credentials: BackstageCredentials; }): Promise>> { const ret = new Map(this.actions); const { actions } = await this.actionsRegistry.list({ - credentials: - options?.credentials ?? (await this.auth.getOwnServiceCredentials()), + credentials: options.credentials, }); for (const action of actions) { diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts index b3f4b5e2cf..edbe28a0f5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/DecoratedActionsRegistry.ts @@ -31,9 +31,12 @@ export class DecoratedActionsRegistry implements TemplateActionRegistry { } } - async get(actionId: string): Promise { + async get( + actionId: string, + options: { credentials: BackstageCredentials }, + ): Promise { try { - return await this.innerRegistry.get(actionId); + return await this.innerRegistry.get(actionId, options); } catch (e) { if (!this.innerActions.has(actionId)) { throw e; @@ -42,8 +45,8 @@ export class DecoratedActionsRegistry implements TemplateActionRegistry { } } - async list(options?: { - credentials?: BackstageCredentials; + async list(options: { + credentials: BackstageCredentials; }): Promise>> { const inner = await this.innerRegistry.list(options); return new Map>([ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 886ddda36f..2c661fab56 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -110,7 +110,6 @@ describe('NunjucksWorkflowRunner', () => { actionRegistry = new DefaultTemplateActionRegistry( actionsRegistryServiceMock(), - mockServices.auth(), mockServices.logger.mock(), ); fakeActionHandler = jest.fn(); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index d13fa58a94..4d070daa52 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -33,7 +33,7 @@ import { SecureTemplater, SecureTemplateRenderer, } from '../../lib/templating/SecureTemplater'; -import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts'; +import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; import { generateExampleOutput, isTruthy } from './helper'; import { TaskTrackType, WorkflowResponse, WorkflowRunner } from './types'; @@ -245,7 +245,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { return; } const action: TemplateAction = - await this.options.actionRegistry.get(step.action); + await this.options.actionRegistry.get(step.action, { + credentials: await task.getInitiatorCredentials(), + }); const { taskLogger } = createStepLogger({ task, step, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 7b85dd1270..adfdf0b553 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -25,7 +25,7 @@ import { TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; import PQueue from 'p-queue'; -import { TemplateActionRegistry } from '../actions/TemplateActionRegistry.ts'; +import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { WorkflowRunner } from './types'; import { setTimeout } from 'timers/promises'; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index bcdcedb70d..3894ffb876 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -271,7 +271,6 @@ export async function createRouter( const actionRegistry = new DefaultTemplateActionRegistry( actionsRegistry, - auth, logger, );