Skip to content

Commit 2258a74

Browse files
netroynetroy
authored
refactor(core): Simplify ExternalSecretsProxy setup and move it to core (#16021)
1 parent 3e91f32 commit 2258a74

20 files changed

+227
-118
lines changed

‎packages/cli/src/__tests__/workflow-execute-additional-data.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ExecutionRepository } from '@n8n/db';
44
import { WorkflowRepository } from '@n8n/db';
55
import { Container } from '@n8n/di';
66
import { mock } from 'jest-mock-extended';
7+
import { ExternalSecretsProxy } from 'n8n-core';
78
import type { IWorkflowBase } from 'n8n-workflow';
89
import type {
910
IExecuteWorkflowInfo,
@@ -23,7 +24,6 @@ import {
2324
SubworkflowPolicyChecker,
2425
} from '@/executions/pre-execution-checks';
2526
import { ExternalHooks } from '@/external-hooks';
26-
import { SecretsHelper } from '@/secrets-helpers.ee';
2727
import { UrlService } from '@/services/url.service';
2828
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
2929
import { Telemetry } from '@/telemetry';
@@ -86,12 +86,12 @@ describe('WorkflowExecuteAdditionalData', () => {
8686
const variablesService = mockInstance(VariablesService);
8787
variablesService.getAllCached.mockResolvedValue([]);
8888
const credentialsHelper = mockInstance(CredentialsHelper);
89-
const secretsHelper = mockInstance(SecretsHelper);
89+
const externalSecretsProxy = mockInstance(ExternalSecretsProxy);
9090
const eventService = mockInstance(EventService);
9191
mockInstance(ExternalHooks);
9292
Container.set(VariablesService, variablesService);
9393
Container.set(CredentialsHelper, credentialsHelper);
94-
Container.set(SecretsHelper, secretsHelper);
94+
Container.set(ExternalSecretsProxy, externalSecretsProxy);
9595
const executionRepository = mockInstance(ExecutionRepository);
9696
mockInstance(Telemetry);
9797
const workflowRepository = mockInstance(WorkflowRepository);
@@ -306,7 +306,7 @@ describe('WorkflowExecuteAdditionalData', () => {
306306
userId: undefined,
307307
setExecutionStatus: expect.any(Function),
308308
variables: mockVariables,
309-
secretsHelpers: secretsHelper,
309+
externalSecretsProxy,
310310
startRunnerTask: expect.any(Function),
311311
logAiEvent: expect.any(Function),
312312
});

‎packages/cli/src/commands/base-command.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
ObjectStoreService,
1212
DataDeduplicationService,
1313
ErrorReporter,
14+
ExternalSecretsProxy,
1415
} from 'n8n-core';
1516
import { ensureError, sleep, UserError } from 'n8n-workflow';
1617

@@ -278,6 +279,7 @@ export abstract class BaseCommand extends Command {
278279
async initExternalSecrets() {
279280
const secretsManager = Container.get(ExternalSecretsManager);
280281
await secretsManager.init();
282+
Container.get(ExternalSecretsProxy).setManager(secretsManager);
281283
}
282284

283285
initWorkflowHistory() {

‎packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Container } from '@n8n/di';
66
import Csrf from 'csrf';
77
import type { Response } from 'express';
88
import { captor, mock } from 'jest-mock-extended';
9-
import { Cipher, type InstanceSettings } from 'n8n-core';
9+
import { Cipher, type InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
1010
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
1111
import nock from 'nock';
1212

@@ -19,7 +19,6 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error';
1919
import { NotFoundError } from '@/errors/response-errors/not-found.error';
2020
import { ExternalHooks } from '@/external-hooks';
2121
import type { OAuthRequest } from '@/requests';
22-
import { SecretsHelper } from '@/secrets-helpers.ee';
2322
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
2423
import { mockInstance } from '@test/mocking';
2524

@@ -28,7 +27,7 @@ jest.mock('@/workflow-execute-additional-data');
2827
describe('OAuth1CredentialController', () => {
2928
mockInstance(Logger);
3029
mockInstance(ExternalHooks);
31-
mockInstance(SecretsHelper);
30+
mockInstance(ExternalSecretsProxy);
3231
mockInstance(VariablesService, {
3332
getAllCached: async () => [],
3433
});

‎packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Container } from '@n8n/di';
66
import Csrf from 'csrf';
77
import { type Response } from 'express';
88
import { captor, mock } from 'jest-mock-extended';
9-
import { Cipher, type InstanceSettings } from 'n8n-core';
9+
import { Cipher, type InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
1010
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
1111
import nock from 'nock';
1212

@@ -19,15 +19,14 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error';
1919
import { NotFoundError } from '@/errors/response-errors/not-found.error';
2020
import { ExternalHooks } from '@/external-hooks';
2121
import type { OAuthRequest } from '@/requests';
22-
import { SecretsHelper } from '@/secrets-helpers.ee';
2322
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
2423
import { mockInstance } from '@test/mocking';
2524

2625
jest.mock('@/workflow-execute-additional-data');
2726

2827
describe('OAuth2CredentialController', () => {
2928
mockInstance(Logger);
30-
mockInstance(SecretsHelper);
29+
mockInstance(ExternalSecretsProxy);
3130
mockInstance(VariablesService, {
3231
getAllCached: async () => [],
3332
});

‎packages/cli/src/credentials-helper.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,6 @@ export class CredentialsHelper extends ICredentialsHelper {
327327
return decryptedDataOriginal;
328328
}
329329

330-
await additionalData?.secretsHelpers?.waitForInit();
331-
332330
return await this.applyDefaultsAndOverwrites(
333331
additionalData,
334332
decryptedDataOriginal,

‎packages/cli/src/eventbus/message-event-bus-destination/message-event-bus-destination-webhook.ee.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Container } from '@n8n/di';
44
import axios from 'axios';
55
import type { AxiosRequestConfig, Method } from 'axios';
66
import { Agent as HTTPSAgent } from 'https';
7+
import { ExternalSecretsProxy } from 'n8n-core';
78
import { jsonParse, MessageEventBusDestinationTypeNames } from 'n8n-workflow';
89
import type {
910
MessageEventBusDestinationOptions,
@@ -14,7 +15,6 @@ import type {
1415
} from 'n8n-workflow';
1516

1617
import { CredentialsHelper } from '@/credentials-helper';
17-
import { SecretsHelper } from '@/secrets-helpers.ee';
1818

1919
import { MessageEventBusDestination } from './message-event-bus-destination.ee';
2020
import { eventMessageGenericDestinationTestEvent } from '../event-message-classes/event-message-generic';
@@ -103,7 +103,7 @@ export class MessageEventBusDestinationWebhook
103103
if (foundCredential) {
104104
const credentialsDecrypted = await this.credentialsHelper?.getDecrypted(
105105
{
106-
secretsHelpers: Container.get(SecretsHelper),
106+
externalSecretsProxy: Container.get(ExternalSecretsProxy),
107107
} as unknown as IWorkflowExecuteAdditionalData,
108108
foundCredential[1],
109109
foundCredential[0],

‎packages/cli/src/external-secrets.ee/__tests__/external-secrets-manager.ee.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,10 @@ describe('External Secrets Manager', () => {
352352
expect(manager.getSecretNames('dummy')).toEqual(['test1', 'test2']);
353353
});
354354

355-
test('should return undefined when provider does not exist', async () => {
355+
test('should return an empty array when provider does not exist', async () => {
356356
await manager.init();
357357

358-
expect(manager.getSecretNames('nonexistent')).toBeUndefined();
358+
expect(manager.getSecretNames('nonexistent')).toBeEmptyArray();
359359
});
360360
});
361361

‎packages/cli/src/external-secrets.ee/external-secrets-manager.ee.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Logger } from '@n8n/backend-common';
22
import { SettingsRepository } from '@n8n/db';
33
import { OnPubSubEvent, OnShutdown } from '@n8n/decorators';
44
import { Service } from '@n8n/di';
5-
import { Cipher } from 'n8n-core';
5+
import { Cipher, type IExternalSecretsManager } from 'n8n-core';
66
import { jsonParse, type IDataObject, ensureError, UnexpectedError } from 'n8n-workflow';
77

88
import { EventService } from '@/events/event.service';
@@ -19,7 +19,7 @@ import { ExternalSecretsConfig } from './external-secrets.config';
1919
import type { ExternalSecretsSettings, SecretsProvider, SecretsProviderSettings } from './types';
2020

2121
@Service()
22-
export class ExternalSecretsManager {
22+
export class ExternalSecretsManager implements IExternalSecretsManager {
2323
private providers: Record<string, SecretsProvider> = {};
2424

2525
private initializingPromise?: Promise<void>;
@@ -211,7 +211,7 @@ export class ExternalSecretsManager {
211211
return provider in this.providers;
212212
}
213213

214-
getProviderNames(): string[] | undefined {
214+
getProviderNames(): string[] {
215215
return Object.keys(this.providers);
216216
}
217217

@@ -223,8 +223,8 @@ export class ExternalSecretsManager {
223223
return this.getProvider(provider)?.hasSecret(name) ?? false;
224224
}
225225

226-
getSecretNames(provider: string): string[] | undefined {
227-
return this.getProvider(provider)?.getSecretNames();
226+
getSecretNames(provider: string): string[] {
227+
return this.getProvider(provider)?.getSecretNames() ?? [];
228228
}
229229

230230
getAllSecretNames(): Record<string, string[]> {

‎packages/cli/src/scaling/__tests__/job-processor.service.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { IExecutionResponse } from '@n8n/db';
33
import type { ExecutionRepository } from '@n8n/db';
44
import { mock } from 'jest-mock-extended';
55
import type { WorkflowExecute as ActualWorkflowExecute } from 'n8n-core';
6+
import { ExternalSecretsProxy } from 'n8n-core';
67
import { mockInstance } from 'n8n-core/test/utils';
78
import type { IPinData, ITaskData, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
89
import { Workflow, type IRunExecutionData, type WorkflowExecuteMode } from 'n8n-workflow';
@@ -11,7 +12,6 @@ import { CredentialsHelper } from '@/credentials-helper';
1112
import { VariablesService } from '@/environments.ee/variables/variables.service.ee';
1213
import { ExternalHooks } from '@/external-hooks';
1314
import type { ManualExecutionService } from '@/manual-execution.service';
14-
import { SecretsHelper } from '@/secrets-helpers.ee';
1515
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
1616
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
1717
import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
@@ -23,7 +23,7 @@ mockInstance(VariablesService, {
2323
getAllCached: jest.fn().mockResolvedValue([]),
2424
});
2525
mockInstance(CredentialsHelper);
26-
mockInstance(SecretsHelper);
26+
mockInstance(ExternalSecretsProxy);
2727
mockInstance(WorkflowStaticDataService);
2828
mockInstance(WorkflowStatisticsService);
2929
mockInstance(ExternalHooks);

‎packages/cli/src/secrets-helpers.ee.ts

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎packages/cli/src/workflow-execute-additional-data.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Logger } from '@n8n/backend-common';
77
import { GlobalConfig } from '@n8n/config';
88
import { ExecutionRepository, WorkflowRepository } from '@n8n/db';
99
import { Container } from '@n8n/di';
10-
import { WorkflowExecute } from 'n8n-core';
10+
import { ExternalSecretsProxy, WorkflowExecute } from 'n8n-core';
1111
import { UnexpectedError, Workflow } from 'n8n-workflow';
1212
import type {
1313
IDataObject,
@@ -46,7 +46,6 @@ import {
4646
import type { UpdateExecutionPayload } from '@/interfaces';
4747
import { NodeTypes } from '@/node-types';
4848
import { Push } from '@/push';
49-
import { SecretsHelper } from '@/secrets-helpers.ee';
5049
import { UrlService } from '@/services/url.service';
5150
import { TaskRequester } from '@/task-runners/task-managers/task-requester';
5251
import { findSubworkflowStart } from '@/utils';
@@ -388,7 +387,7 @@ export async function getBase(
388387
userId,
389388
setExecutionStatus,
390389
variables,
391-
secretsHelpers: Container.get(SecretsHelper),
390+
externalSecretsProxy: Container.get(ExternalSecretsProxy),
392391
async startRunnerTask(
393392
additionalData: IWorkflowExecuteAdditionalData,
394393
jobType: string,

‎packages/cli/test/integration/active-workflow-manager.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { WebhookEntity } from '@n8n/db';
22
import { WorkflowRepository } from '@n8n/db';
33
import { Container } from '@n8n/di';
44
import { mock } from 'jest-mock-extended';
5-
import { InstanceSettings } from 'n8n-core';
5+
import { InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
66
import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
77
import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
88
import { NodeApiError, Workflow } from 'n8n-workflow';
@@ -19,7 +19,6 @@ import { ExecutionService } from '@/executions/execution.service';
1919
import { ExternalHooks } from '@/external-hooks';
2020
import { NodeTypes } from '@/node-types';
2121
import { Push } from '@/push';
22-
import { SecretsHelper } from '@/secrets-helpers.ee';
2322
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
2423
import { WebhookService } from '@/webhooks/webhook.service';
2524
import * as AdditionalData from '@/workflow-execute-additional-data';
@@ -33,7 +32,7 @@ import { mockInstance } from '../shared/mocking';
3332

3433
mockInstance(ActiveExecutions);
3534
mockInstance(Push);
36-
mockInstance(SecretsHelper);
35+
mockInstance(ExternalSecretsProxy);
3736
mockInstance(ExecutionService);
3837
mockInstance(WorkflowService);
3938

‎packages/core/src/execution-engine/__tests__/execution-lifecycle-hooks.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type {
1313

1414
import type {
1515
ExecutionLifecycleHookName,
16-
ExecutionLifecyleHookHandlers,
16+
ExecutionLifecycleHookHandlers,
1717
} from '../execution-lifecycle-hooks';
1818
import { ExecutionLifecycleHooks } from '../execution-lifecycle-hooks';
1919

@@ -46,12 +46,14 @@ describe('ExecutionLifecycleHooks', () => {
4646
describe('addHandler()', () => {
4747
const hooksHandlers =
4848
mock<{
49-
[K in keyof ExecutionLifecyleHookHandlers]: ExecutionLifecyleHookHandlers[K][number];
49+
[K in keyof ExecutionLifecycleHookHandlers]: ExecutionLifecycleHookHandlers[K][number];
5050
}>();
5151

5252
const testCases: Array<{
5353
hook: ExecutionLifecycleHookName;
54-
args: Parameters<ExecutionLifecyleHookHandlers[keyof ExecutionLifecyleHookHandlers][number]>;
54+
args: Parameters<
55+
ExecutionLifecycleHookHandlers[keyof ExecutionLifecycleHookHandlers][number]
56+
>;
5557
}> = [
5658
{ hook: 'nodeExecuteBefore', args: ['testNode', mock<ITaskStartedData>()] },
5759
{

0 commit comments

Comments
 (0)