refactor(core): Centralize module management (#16464)

Co-authored-by: Danny Martini <danny@n8n.io>
This commit is contained in:
Iván Ovejero
2025-06-19 14:32:31 +02:00
committed by GitHub
parent d0eb7a45ad
commit bb7c68f6bf
24 changed files with 328 additions and 134 deletions

View File

@@ -196,8 +196,8 @@ export interface FrontendSettings {
quota: number;
};
/** Backend modules that were loaded during startup based on user configuration and pre-init check. */
loadedModules: string[];
/** Backend modules that were initialized during startup. */
activeModules: string[];
}
export type FrontendModuleSettings = {

View File

@@ -72,19 +72,7 @@ export abstract class BaseCommand extends Command {
protected needsTaskRunner = false;
protected async loadModules() {
for (const moduleName of this.modulesConfig.modules) {
// add module to the registry for dependency injection
try {
await import(`../modules/${moduleName}/${moduleName}.module`);
} catch {
await import(`../modules/${moduleName}.ee/${moduleName}.module`);
}
this.modulesConfig.addLoadedModule(moduleName);
this.logger.debug(`Loaded module "${moduleName}"`);
}
this.moduleRegistry.addEntities();
await this.moduleRegistry.loadModules();
}
async init(): Promise<void> {

View File

@@ -0,0 +1,186 @@
import type { LicenseState } from '@n8n/backend-common';
import type { ModuleInterface, ModuleMetadata } from '@n8n/decorators';
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import { ModuleConfusionError } from '../errors/module-confusion.error';
import { ModuleRegistry } from '../module-registry';
import { MODULE_NAMES } from '../modules.config';
beforeEach(() => {
jest.resetAllMocks();
process.env = {};
Container.reset();
});
describe('eligibleModules', () => {
it('should consider all default modules eligible', () => {
expect(Container.get(ModuleRegistry).eligibleModules).toEqual(MODULE_NAMES);
});
it('should consider a module ineligible if it was disabled via env var', () => {
process.env.N8N_DISABLED_MODULES = 'insights';
expect(Container.get(ModuleRegistry).eligibleModules).toEqual(['external-secrets']);
});
it('should throw `ModuleConfusionError` if a module is both enabled and disabled', () => {
process.env.N8N_ENABLED_MODULES = 'insights';
process.env.N8N_DISABLED_MODULES = 'insights';
expect(() => Container.get(ModuleRegistry).eligibleModules).toThrow(ModuleConfusionError);
});
});
describe('loadModules', () => {
it('should load entities defined by modules', async () => {
const FirstEntity = class FirstEntityClass {};
const SecondEntity = class SecondEntityClass {};
const ModuleClass = {
entities: jest.fn().mockReturnValue([FirstEntity, SecondEntity]),
};
const moduleMetadata = mock<ModuleMetadata>({
getClasses: jest.fn().mockReturnValue([ModuleClass]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
await moduleRegistry.loadModules([]);
expect(moduleRegistry.entities).toEqual([FirstEntity, SecondEntity]);
});
it('should load no entities if none are defined by modules', async () => {
const ModuleClass = { entities: jest.fn().mockReturnValue([]) };
const moduleMetadata = mock<ModuleMetadata>({
getClasses: jest.fn().mockReturnValue([ModuleClass]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
await moduleRegistry.loadModules([]);
expect(moduleRegistry.entities).toEqual([]);
});
});
describe('initModules', () => {
it('should init module if it has no feature flag', async () => {
const ModuleClass = { init: jest.fn() };
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest
.fn()
.mockReturnValue([['test-module', { licenseFlag: undefined, class: ModuleClass }]]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
await moduleRegistry.initModules();
expect(ModuleClass.init).toHaveBeenCalled();
});
it('should init module if it is licensed', async () => {
const ModuleClass = { init: jest.fn() };
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest
.fn()
.mockReturnValue([
['test-module', { licenseFlag: 'feat:testFeature', class: ModuleClass }],
]),
});
const licenseState = mock<LicenseState>({ isLicensed: jest.fn().mockReturnValue(true) });
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), licenseState, mock(), mock());
await moduleRegistry.initModules();
expect(ModuleClass.init).toHaveBeenCalled();
});
it('should skip init for unlicensed module', async () => {
const ModuleClass = { init: jest.fn() };
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest
.fn()
.mockReturnValue([
['test-module', { licenseFlag: 'feat:testFeature', class: ModuleClass }],
]),
});
const licenseState = mock<LicenseState>({ isLicensed: jest.fn().mockReturnValue(false) });
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), licenseState, mock(), mock());
await moduleRegistry.initModules();
expect(ModuleClass.init).not.toHaveBeenCalled();
});
it('should accept module without `init` method', async () => {
const ModuleClass = {};
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest
.fn()
.mockReturnValue([['test-module', { licenseFlag: undefined, class: ModuleClass }]]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
await moduleRegistry.initModules();
await expect(moduleRegistry.initModules()).resolves.not.toThrow();
});
it('registers settings', async () => {
// ARRANGE
const moduleName = 'test-module';
const moduleSettings = { foo: 1 };
const ModuleClass: ModuleInterface = {
init: jest.fn(),
settings: jest.fn().mockReturnValue(moduleSettings),
};
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest.fn().mockReturnValue([[moduleName, { class: ModuleClass }]]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
// ACT
await moduleRegistry.initModules();
// ASSERT
expect(ModuleClass.settings).toHaveBeenCalled();
expect(moduleRegistry.settings.has(moduleName)).toBe(true);
expect(moduleRegistry.settings.get(moduleName)).toBe(moduleSettings);
});
it('activates the module', async () => {
// ARRANGE
const moduleName = 'test-module';
const moduleSettings = { foo: 1 };
const ModuleClass: ModuleInterface = {
init: jest.fn(),
settings: jest.fn().mockReturnValue(moduleSettings),
};
const moduleMetadata = mock<ModuleMetadata>({
getEntries: jest.fn().mockReturnValue([[moduleName, { class: ModuleClass }]]),
});
Container.get = jest.fn().mockReturnValue(ModuleClass);
const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock());
// ACT
await moduleRegistry.initModules();
// ASSERT
expect(moduleRegistry.isActive(moduleName as any)).toBe(true);
expect(moduleRegistry.getActiveModules()).toEqual([moduleName]);
});
});

View File

@@ -1,51 +1,20 @@
import { Container } from '@n8n/di';
import { UnexpectedError } from 'n8n-workflow';
import { UnknownModuleError } from '../errors/unknown-module.error';
import { ModulesConfig } from '../modules.config';
describe('ModulesConfig', () => {
beforeEach(() => {
jest.resetAllMocks();
process.env = {};
Container.reset();
});
it('should initialize with insights modules if no environment variable is set', () => {
const config = Container.get(ModulesConfig);
expect(config.modules).toEqual(['insights', 'external-secrets']);
});
it('should parse valid module names from environment variable', () => {
process.env.N8N_ENABLED_MODULES = 'insights';
const config = Container.get(ModulesConfig);
expect(config.modules).toEqual(['insights', 'external-secrets']);
});
it('should disable valid module names from environment variable', () => {
process.env.N8N_DISABLED_MODULES = 'insights';
const config = Container.get(ModulesConfig);
expect(config.modules).toEqual(['external-secrets']);
});
it('should throw UnexpectedError for invalid module names', () => {
process.env.N8N_ENABLED_MODULES = 'invalidModule';
expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError);
});
it('should throw UnexpectedError if any module is both enabled and disabled', () => {
process.env.N8N_ENABLED_MODULES = 'insights';
process.env.N8N_DISABLED_MODULES = 'insights';
const config = Container.get(ModulesConfig);
expect(() => config.modules).toThrow(UnexpectedError);
});
it('should throw UnexpectedError if any enabled module name is invalid', () => {
process.env.N8N_ENABLED_MODULES = 'insights,invalidModule';
expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError);
});
it('should throw UnexpectedError if any disabled module name is invalid', () => {
process.env.N8N_DISABLED_MODULES = 'insights,invalidModule';
expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError);
});
beforeEach(() => {
jest.resetAllMocks();
process.env = {};
Container.reset();
});
it('should throw `UnknownModuleError` if any enabled module name is invalid', () => {
process.env.N8N_ENABLED_MODULES = 'insights,invalidModule';
expect(() => Container.get(ModulesConfig)).toThrowError(UnknownModuleError);
});
it('should throw `UnknownModuleError` if any disabled module name is invalid', () => {
process.env.N8N_DISABLED_MODULES = 'insights,invalidModule';
expect(() => Container.get(ModulesConfig)).toThrowError(UnknownModuleError);
});

View File

@@ -0,0 +1,9 @@
import { UserError } from 'n8n-workflow';
export class MissingModuleError extends UserError {
constructor(moduleName: string, errorMsg: string) {
super(
`Failed to load module "${moduleName}": ${errorMsg}. Please review the module's entrypoint file name and the module's directory name.`,
);
}
}

View File

@@ -0,0 +1,11 @@
import { UserError } from 'n8n-workflow';
export class ModuleConfusionError extends UserError {
constructor(moduleNames: string[]) {
const modules = moduleNames.length > 1 ? 'modules' : 'a module';
super(
`Found ${modules} listed in both \`N8N_ENABLED_MODULES\` and \`N8N_DISABLED_MODULES\`: ${moduleNames.join(', ')}. Please review your environment variables, as a module cannot be both enabled and disabled.`,
);
}
}

View File

@@ -0,0 +1,7 @@
import { UnexpectedError } from 'n8n-workflow';
export class UnknownModuleError extends UnexpectedError {
constructor(moduleName: string) {
super(`Unknown module "${moduleName}"`, { level: 'fatal' });
}
}

View File

@@ -27,7 +27,7 @@ import { InsightsCollectionService } from '../insights-collection.service';
import { InsightsConfig } from '../insights.config';
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
});

View File

@@ -20,7 +20,7 @@ import { InsightsCompactionService } from '../insights-compaction.service';
import { InsightsConfig } from '../insights.config';
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
});

View File

@@ -19,7 +19,7 @@ import { InsightsPruningService } from '../insights-pruning.service';
import { InsightsConfig } from '../insights.config';
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
});

View File

@@ -31,7 +31,7 @@ import { InsightsConfig } from '../insights.config';
import { InsightsService } from '../insights.service';
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
});

View File

@@ -14,7 +14,7 @@ import type { TypeUnit } from '../insights-shared';
let insightsRawRepository: InsightsRawRepository;
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
insightsRawRepository = Container.get(InsightsRawRepository);
});

View File

@@ -12,7 +12,7 @@ import { InsightsByPeriodRepository } from '../insights-by-period.repository';
describe('InsightsByPeriodRepository', () => {
beforeAll(async () => {
await testModules.load(['insights']);
await testModules.loadModules(['insights']);
await testDb.init();
});

View File

@@ -13,6 +13,11 @@ import type {
Workflow,
} from 'n8n-workflow';
import { MissingModuleError } from './errors/missing-module.error';
import { ModuleConfusionError } from './errors/module-confusion.error';
import { ModulesConfig } from './modules.config';
import type { ModuleName } from './modules.config';
@Service()
export class ModuleRegistry {
readonly entities: EntityClass[] = [];
@@ -24,8 +29,63 @@ export class ModuleRegistry {
private readonly lifecycleMetadata: LifecycleMetadata,
private readonly licenseState: LicenseState,
private readonly logger: Logger,
private readonly modulesConfig: ModulesConfig,
) {}
private readonly defaultModules: ModuleName[] = ['insights', 'external-secrets'];
private readonly activeModules: string[] = [];
get eligibleModules(): ModuleName[] {
const { enabledModules, disabledModules } = this.modulesConfig;
const doubleListed = enabledModules.filter((m) => disabledModules.includes(m));
if (doubleListed.length > 0) throw new ModuleConfusionError(doubleListed);
const defaultPlusEnabled = [...new Set([...this.defaultModules, ...enabledModules])];
return defaultPlusEnabled.filter((m) => !disabledModules.includes(m));
}
/**
* Loads [module name].module.ts for each eligible module.
* This only registers the database entities for module and should be done
* before instantiating the datasource.
*
* This will not register routes or do any other kind of module related
* setup.
*/
async loadModules(modules?: ModuleName[]) {
for (const moduleName of modules ?? this.eligibleModules) {
try {
await import(`../modules/${moduleName}/${moduleName}.module`);
} catch {
try {
await import(`../modules/${moduleName}.ee/${moduleName}.module`);
} catch (error) {
throw new MissingModuleError(moduleName, error instanceof Error ? error.message : '');
}
}
}
for (const ModuleClass of this.moduleMetadata.getClasses()) {
const entities = Container.get(ModuleClass).entities?.();
if (!entities || entities.length === 0) continue;
this.entities.push(...entities);
}
}
/**
* Calls `init` on each eligible module.
*
* This will do things like registering routes, setup timers or other module
* specific setup.
*
* `ModuleRegistry.loadModules` must have been called before.
*/
async initModules() {
for (const [moduleName, moduleEntry] of this.moduleMetadata.getEntries()) {
const { licenseFlag, class: ModuleClass } = moduleEntry;
@@ -34,6 +94,7 @@ export class ModuleRegistry {
this.logger.debug(`Skipped init for unlicensed module "${moduleName}"`);
continue;
}
await Container.get(ModuleClass).init?.();
const moduleSettings = await Container.get(ModuleClass).settings?.();
@@ -41,17 +102,19 @@ export class ModuleRegistry {
if (!moduleSettings) continue;
this.settings.set(moduleName, moduleSettings);
this.logger.debug(`Initialized module "${moduleName}"`);
this.activeModules.push(moduleName);
}
}
addEntities() {
for (const ModuleClass of this.moduleMetadata.getClasses()) {
const entities = Container.get(ModuleClass).entities?.();
isActive(moduleName: ModuleName) {
return this.activeModules.includes(moduleName);
}
if (!entities || entities.length === 0) continue;
this.entities.push(...entities);
}
getActiveModules() {
return this.activeModules;
}
registerLifecycleHooks(hooks: ExecutionLifecycleHooks) {

View File

@@ -1,58 +1,28 @@
import { CommaSeparatedStringArray, Config, Env } from '@n8n/config';
import type { InstanceSettings } from 'n8n-core';
import { UnexpectedError } from 'n8n-workflow';
export type ModulePreInitContext = {
instance: InstanceSettings;
};
import { UnknownModuleError } from './errors/unknown-module.error';
export type ModulePreInit = {
shouldLoadModule: (ctx: ModulePreInitContext) => boolean;
};
export const MODULE_NAMES = ['insights', 'external-secrets'] as const;
const moduleNames = ['insights', 'external-secrets'] as const;
export type ModuleName = (typeof moduleNames)[number];
export type ModuleName = (typeof MODULE_NAMES)[number];
class Modules extends CommaSeparatedStringArray<ModuleName> {
class ModuleArray extends CommaSeparatedStringArray<ModuleName> {
constructor(str: string) {
super(str);
for (const moduleName of this) {
if (!moduleNames.includes(moduleName)) {
throw new UnexpectedError(`Unknown module name ${moduleName}`, { level: 'fatal' });
}
if (!MODULE_NAMES.includes(moduleName)) throw new UnknownModuleError(moduleName);
}
}
}
@Config
export class ModulesConfig {
/** Comma-separated list of all modules enabled */
/** Comma-separated list of all enabled modules. */
@Env('N8N_ENABLED_MODULES')
enabledModules: Modules = [];
enabledModules: ModuleArray = [];
/** Comma-separated list of all disabled modules */
/** Comma-separated list of all disabled modules. */
@Env('N8N_DISABLED_MODULES')
disabledModules: Modules = [];
// Default modules are always enabled unless explicitly disabled
private readonly defaultModules: ModuleName[] = ['insights', 'external-secrets'];
// Loaded modules are the ones that have been loaded so far by the instance
readonly loadedModules = new Set<ModuleName>();
// Get all modules by merging default and enabled, and filtering out disabled modules
get modules(): ModuleName[] {
if (this.enabledModules.some((module) => this.disabledModules.includes(module))) {
throw new UnexpectedError('Module cannot be both enabled and disabled', { level: 'fatal' });
}
const enabledModules = Array.from(new Set(this.defaultModules.concat(this.enabledModules)));
return enabledModules.filter((module) => !this.disabledModules.includes(module));
}
addLoadedModule(module: ModuleName) {
this.loadedModules.add(module);
}
disabledModules: ModuleArray = [];
}

View File

@@ -18,7 +18,6 @@ import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee';
import { License } from '@/license';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { ModuleRegistry } from '@/modules/module-registry';
import { ModulesConfig } from '@/modules/modules.config';
import { isApiEnabled } from '@/public-api';
import { PushConfig } from '@/push/push.config';
import type { CommunityPackagesService } from '@/services/community-packages.service';
@@ -49,7 +48,6 @@ export class FrontendService {
private readonly instanceSettings: InstanceSettings,
private readonly urlService: UrlService,
private readonly securityConfig: SecurityConfig,
private readonly modulesConfig: ModulesConfig,
private readonly pushConfig: PushConfig,
private readonly binaryDataConfig: BinaryDataConfig,
private readonly licenseState: LicenseState,
@@ -256,7 +254,7 @@ export class FrontendService {
evaluation: {
quota: this.licenseState.getMaxWorkflowsWithEvaluations(),
},
loadedModules: this.modulesConfig.modules,
activeModules: this.moduleRegistry.getActiveModules(),
};
}

View File

@@ -1,15 +1,8 @@
import { Container } from '@n8n/di';
import { ModuleRegistry } from '@/modules/module-registry';
import type { ModuleName } from '@/modules/modules.config';
export async function load(moduleNames: string[]) {
for (const moduleName of moduleNames) {
try {
await import(`../../../src/modules/${moduleName}/${moduleName}.module`);
} catch {
await import(`../../../src/modules/${moduleName}.ee/${moduleName}.module`);
}
}
Container.get(ModuleRegistry).addEntities();
export async function loadModules(moduleNames: ModuleName[]) {
await Container.get(ModuleRegistry).loadModules(moduleNames);
}

View File

@@ -125,7 +125,7 @@ export const setupTestServer = ({
// eslint-disable-next-line complexity
beforeAll(async () => {
if (modules) await testModules.load(modules);
if (modules) await testModules.loadModules(modules);
await testDb.init();
config.set('userManagement.jwtSecret', 'My JWT secret');

View File

@@ -148,5 +148,5 @@ export const defaultSettings: FrontendSettings = {
evaluation: {
quota: 0,
},
loadedModules: [],
activeModules: [],
};

View File

@@ -112,7 +112,7 @@ const mainMenuItems = computed(() => [
position: 'bottom',
route: { to: { name: VIEWS.INSIGHTS } },
available:
settingsStore.settings.loadedModules.includes('insights') &&
settingsStore.settings.activeModules.includes('insights') &&
hasPermission(['rbac'], { rbac: { scope: 'insights:list' } }),
},
{

View File

@@ -19,7 +19,7 @@ export const useInsightsStore = defineStore('insights', () => {
);
const isInsightsEnabled = computed(() =>
settingsStore.settings.loadedModules.includes('insights'),
settingsStore.settings.activeModules.includes('insights'),
);
const isDashboardEnabled = computed(

View File

@@ -185,7 +185,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => {
const isDevRelease = computed(() => settings.value.releaseChannel === 'dev');
const loadedModules = computed(() => settings.value.loadedModules);
const activeModules = computed(() => settings.value.activeModules);
const setSettings = (newSettings: FrontendSettings) => {
settings.value = newSettings;
@@ -426,7 +426,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => {
getSettings,
setSettings,
initialize,
loadedModules,
activeModules,
getModuleSettings,
moduleSettings,
};

View File

@@ -46,7 +46,7 @@ let telemetry: ReturnType<typeof useTelemetry>;
describe('SigninView', () => {
const signInWithValidUser = async () => {
settingsStore.isCloudDeployment = false;
settingsStore.loadedModules = [];
settingsStore.activeModules = [];
usersStore.loginWithCreds.mockResolvedValueOnce();
const { getByRole, queryByTestId, container } = renderComponent();

View File

@@ -145,7 +145,7 @@ const login = async (form: LoginRequestDto) => {
}
await settingsStore.getSettings();
if (settingsStore.loadedModules.length > 0) {
if (settingsStore.activeModules.length > 0) {
await settingsStore.getModuleSettings();
}