refactor(core): Simplify ExternalSecretsProxy setup and move it to core (#16021)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2025-06-05 12:49:08 +02:00
committed by GitHub
parent 3e91f3253b
commit 2258a74518
20 changed files with 227 additions and 118 deletions

View File

@@ -4,6 +4,7 @@ import { ExecutionRepository } from '@n8n/db';
import { WorkflowRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import { ExternalSecretsProxy } from 'n8n-core';
import type { IWorkflowBase } from 'n8n-workflow';
import type {
IExecuteWorkflowInfo,
@@ -23,7 +24,6 @@ import {
SubworkflowPolicyChecker,
} from '@/executions/pre-execution-checks';
import { ExternalHooks } from '@/external-hooks';
import { SecretsHelper } from '@/secrets-helpers.ee';
import { UrlService } from '@/services/url.service';
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
import { Telemetry } from '@/telemetry';
@@ -86,12 +86,12 @@ describe('WorkflowExecuteAdditionalData', () => {
const variablesService = mockInstance(VariablesService);
variablesService.getAllCached.mockResolvedValue([]);
const credentialsHelper = mockInstance(CredentialsHelper);
const secretsHelper = mockInstance(SecretsHelper);
const externalSecretsProxy = mockInstance(ExternalSecretsProxy);
const eventService = mockInstance(EventService);
mockInstance(ExternalHooks);
Container.set(VariablesService, variablesService);
Container.set(CredentialsHelper, credentialsHelper);
Container.set(SecretsHelper, secretsHelper);
Container.set(ExternalSecretsProxy, externalSecretsProxy);
const executionRepository = mockInstance(ExecutionRepository);
mockInstance(Telemetry);
const workflowRepository = mockInstance(WorkflowRepository);
@@ -306,7 +306,7 @@ describe('WorkflowExecuteAdditionalData', () => {
userId: undefined,
setExecutionStatus: expect.any(Function),
variables: mockVariables,
secretsHelpers: secretsHelper,
externalSecretsProxy,
startRunnerTask: expect.any(Function),
logAiEvent: expect.any(Function),
});

View File

@@ -11,6 +11,7 @@ import {
ObjectStoreService,
DataDeduplicationService,
ErrorReporter,
ExternalSecretsProxy,
} from 'n8n-core';
import { ensureError, sleep, UserError } from 'n8n-workflow';
@@ -278,6 +279,7 @@ export abstract class BaseCommand extends Command {
async initExternalSecrets() {
const secretsManager = Container.get(ExternalSecretsManager);
await secretsManager.init();
Container.get(ExternalSecretsProxy).setManager(secretsManager);
}
initWorkflowHistory() {

View File

@@ -6,7 +6,7 @@ import { Container } from '@n8n/di';
import Csrf from 'csrf';
import type { Response } from 'express';
import { captor, mock } from 'jest-mock-extended';
import { Cipher, type InstanceSettings } from 'n8n-core';
import { Cipher, type InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import nock from 'nock';
@@ -19,7 +19,6 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { ExternalHooks } from '@/external-hooks';
import type { OAuthRequest } from '@/requests';
import { SecretsHelper } from '@/secrets-helpers.ee';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { mockInstance } from '@test/mocking';
@@ -28,7 +27,7 @@ jest.mock('@/workflow-execute-additional-data');
describe('OAuth1CredentialController', () => {
mockInstance(Logger);
mockInstance(ExternalHooks);
mockInstance(SecretsHelper);
mockInstance(ExternalSecretsProxy);
mockInstance(VariablesService, {
getAllCached: async () => [],
});

View File

@@ -6,7 +6,7 @@ import { Container } from '@n8n/di';
import Csrf from 'csrf';
import { type Response } from 'express';
import { captor, mock } from 'jest-mock-extended';
import { Cipher, type InstanceSettings } from 'n8n-core';
import { Cipher, type InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import nock from 'nock';
@@ -19,7 +19,6 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { ExternalHooks } from '@/external-hooks';
import type { OAuthRequest } from '@/requests';
import { SecretsHelper } from '@/secrets-helpers.ee';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { mockInstance } from '@test/mocking';
@@ -27,7 +26,7 @@ jest.mock('@/workflow-execute-additional-data');
describe('OAuth2CredentialController', () => {
mockInstance(Logger);
mockInstance(SecretsHelper);
mockInstance(ExternalSecretsProxy);
mockInstance(VariablesService, {
getAllCached: async () => [],
});

View File

@@ -327,8 +327,6 @@ export class CredentialsHelper extends ICredentialsHelper {
return decryptedDataOriginal;
}
await additionalData?.secretsHelpers?.waitForInit();
return await this.applyDefaultsAndOverwrites(
additionalData,
decryptedDataOriginal,

View File

@@ -4,6 +4,7 @@ import { Container } from '@n8n/di';
import axios from 'axios';
import type { AxiosRequestConfig, Method } from 'axios';
import { Agent as HTTPSAgent } from 'https';
import { ExternalSecretsProxy } from 'n8n-core';
import { jsonParse, MessageEventBusDestinationTypeNames } from 'n8n-workflow';
import type {
MessageEventBusDestinationOptions,
@@ -14,7 +15,6 @@ import type {
} from 'n8n-workflow';
import { CredentialsHelper } from '@/credentials-helper';
import { SecretsHelper } from '@/secrets-helpers.ee';
import { MessageEventBusDestination } from './message-event-bus-destination.ee';
import { eventMessageGenericDestinationTestEvent } from '../event-message-classes/event-message-generic';
@@ -103,7 +103,7 @@ export class MessageEventBusDestinationWebhook
if (foundCredential) {
const credentialsDecrypted = await this.credentialsHelper?.getDecrypted(
{
secretsHelpers: Container.get(SecretsHelper),
externalSecretsProxy: Container.get(ExternalSecretsProxy),
} as unknown as IWorkflowExecuteAdditionalData,
foundCredential[1],
foundCredential[0],

View File

@@ -352,10 +352,10 @@ describe('External Secrets Manager', () => {
expect(manager.getSecretNames('dummy')).toEqual(['test1', 'test2']);
});
test('should return undefined when provider does not exist', async () => {
test('should return an empty array when provider does not exist', async () => {
await manager.init();
expect(manager.getSecretNames('nonexistent')).toBeUndefined();
expect(manager.getSecretNames('nonexistent')).toBeEmptyArray();
});
});

View File

@@ -2,7 +2,7 @@ import { Logger } from '@n8n/backend-common';
import { SettingsRepository } from '@n8n/db';
import { OnPubSubEvent, OnShutdown } from '@n8n/decorators';
import { Service } from '@n8n/di';
import { Cipher } from 'n8n-core';
import { Cipher, type IExternalSecretsManager } from 'n8n-core';
import { jsonParse, type IDataObject, ensureError, UnexpectedError } from 'n8n-workflow';
import { EventService } from '@/events/event.service';
@@ -19,7 +19,7 @@ import { ExternalSecretsConfig } from './external-secrets.config';
import type { ExternalSecretsSettings, SecretsProvider, SecretsProviderSettings } from './types';
@Service()
export class ExternalSecretsManager {
export class ExternalSecretsManager implements IExternalSecretsManager {
private providers: Record<string, SecretsProvider> = {};
private initializingPromise?: Promise<void>;
@@ -211,7 +211,7 @@ export class ExternalSecretsManager {
return provider in this.providers;
}
getProviderNames(): string[] | undefined {
getProviderNames(): string[] {
return Object.keys(this.providers);
}
@@ -223,8 +223,8 @@ export class ExternalSecretsManager {
return this.getProvider(provider)?.hasSecret(name) ?? false;
}
getSecretNames(provider: string): string[] | undefined {
return this.getProvider(provider)?.getSecretNames();
getSecretNames(provider: string): string[] {
return this.getProvider(provider)?.getSecretNames() ?? [];
}
getAllSecretNames(): Record<string, string[]> {

View File

@@ -3,6 +3,7 @@ import type { IExecutionResponse } from '@n8n/db';
import type { ExecutionRepository } from '@n8n/db';
import { mock } from 'jest-mock-extended';
import type { WorkflowExecute as ActualWorkflowExecute } from 'n8n-core';
import { ExternalSecretsProxy } from 'n8n-core';
import { mockInstance } from 'n8n-core/test/utils';
import type { IPinData, ITaskData, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import { Workflow, type IRunExecutionData, type WorkflowExecuteMode } from 'n8n-workflow';
@@ -11,7 +12,6 @@ import { CredentialsHelper } from '@/credentials-helper';
import { VariablesService } from '@/environments.ee/variables/variables.service.ee';
import { ExternalHooks } from '@/external-hooks';
import type { ManualExecutionService } from '@/manual-execution.service';
import { SecretsHelper } from '@/secrets-helpers.ee';
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
@@ -23,7 +23,7 @@ mockInstance(VariablesService, {
getAllCached: jest.fn().mockResolvedValue([]),
});
mockInstance(CredentialsHelper);
mockInstance(SecretsHelper);
mockInstance(ExternalSecretsProxy);
mockInstance(WorkflowStaticDataService);
mockInstance(WorkflowStatisticsService);
mockInstance(ExternalHooks);

View File

@@ -1,42 +0,0 @@
import { Service } from '@n8n/di';
import type { SecretsHelpersBase } from 'n8n-workflow';
import { ExternalSecretsManager } from './external-secrets.ee/external-secrets-manager.ee';
@Service()
export class SecretsHelper implements SecretsHelpersBase {
constructor(private service: ExternalSecretsManager) {}
async update() {
if (!this.service.initialized) {
await this.service.init();
}
await this.service.updateSecrets();
}
async waitForInit() {
if (!this.service.initialized) {
await this.service.init();
}
}
getSecret(provider: string, name: string) {
return this.service.getSecret(provider, name);
}
hasSecret(provider: string, name: string): boolean {
return this.service.hasSecret(provider, name);
}
hasProvider(provider: string): boolean {
return this.service.hasProvider(provider);
}
listProviders(): string[] {
return this.service.getProviderNames() ?? [];
}
listSecrets(provider: string): string[] {
return this.service.getSecretNames(provider) ?? [];
}
}

View File

@@ -7,7 +7,7 @@ import { Logger } from '@n8n/backend-common';
import { GlobalConfig } from '@n8n/config';
import { ExecutionRepository, WorkflowRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { WorkflowExecute } from 'n8n-core';
import { ExternalSecretsProxy, WorkflowExecute } from 'n8n-core';
import { UnexpectedError, Workflow } from 'n8n-workflow';
import type {
IDataObject,
@@ -46,7 +46,6 @@ import {
import type { UpdateExecutionPayload } from '@/interfaces';
import { NodeTypes } from '@/node-types';
import { Push } from '@/push';
import { SecretsHelper } from '@/secrets-helpers.ee';
import { UrlService } from '@/services/url.service';
import { TaskRequester } from '@/task-runners/task-managers/task-requester';
import { findSubworkflowStart } from '@/utils';
@@ -388,7 +387,7 @@ export async function getBase(
userId,
setExecutionStatus,
variables,
secretsHelpers: Container.get(SecretsHelper),
externalSecretsProxy: Container.get(ExternalSecretsProxy),
async startRunnerTask(
additionalData: IWorkflowExecuteAdditionalData,
jobType: string,

View File

@@ -2,7 +2,7 @@ import type { WebhookEntity } from '@n8n/db';
import { WorkflowRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import { InstanceSettings } from 'n8n-core';
import { InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
import { NodeApiError, Workflow } from 'n8n-workflow';
@@ -19,7 +19,6 @@ import { ExecutionService } from '@/executions/execution.service';
import { ExternalHooks } from '@/external-hooks';
import { NodeTypes } from '@/node-types';
import { Push } from '@/push';
import { SecretsHelper } from '@/secrets-helpers.ee';
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
import { WebhookService } from '@/webhooks/webhook.service';
import * as AdditionalData from '@/workflow-execute-additional-data';
@@ -33,7 +32,7 @@ import { mockInstance } from '../shared/mocking';
mockInstance(ActiveExecutions);
mockInstance(Push);
mockInstance(SecretsHelper);
mockInstance(ExternalSecretsProxy);
mockInstance(ExecutionService);
mockInstance(WorkflowService);