refactor(core): Port nodes config (no-changelog) (#10140)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <[email protected]>
n8n-io · Jul 23, 2024 · 95b85dd · 95b85dd
1 parent d2a3a4a
commit 95b85dd
Show file tree

Hide file tree

Showing 14 changed files with 95 additions and 65 deletions.
diff --git a/packages/@n8n/config/src/configs/nodes.ts b/packages/@n8n/config/src/configs/nodes.ts
@@ -0,0 +1,46 @@
+import { Config, Env, Nested } from '../decorators';
+
+function isStringArray(input: unknown): input is string[] {
+	return Array.isArray(input) && input.every((item) => typeof item === 'string');
+}
+
+class JsonStringArray extends Array<string> {
+	constructor(str: string) {
+		super();
+
+		let parsed: unknown;
+
+		try {
+			parsed = JSON.parse(str);
+		} catch {
+			return [];
+		}
+
+		return isStringArray(parsed) ? parsed : [];
+	}
+}
+
+@Config
+class CommunityPackagesConfig {
+	/** Whether to enable community packages */
+	@Env('N8N_COMMUNITY_PACKAGES_ENABLED')
+	enabled: boolean = true;
+}
+
+@Config
+export class NodesConfig {
+	/** Node types to load. Includes all if unspecified. @example '["n8n-nodes-base.hackerNews"]' */
+	@Env('NODES_INCLUDE')
+	readonly include: JsonStringArray = [];
+
+	/** Node types not to load. Excludes none if unspecified. @example '["n8n-nodes-base.hackerNews"]' */
+	@Env('NODES_EXCLUDE')
+	readonly exclude: JsonStringArray = [];
+
+	/** Node type to use as error trigger */
+	@Env('NODES_ERROR_TRIGGER_TYPE')
+	readonly errorTriggerType: string = 'n8n-nodes-base.errorTrigger';
+
+	@Nested
+	readonly communityPackages: CommunityPackagesConfig;
+}
diff --git a/packages/@n8n/config/src/decorators.ts b/packages/@n8n/config/src/decorators.ts
@@ -4,6 +4,7 @@ import { Container, Service } from 'typedi';
 
 // eslint-disable-next-line @typescript-eslint/ban-types
 type Class = Function;
+type Constructable<T = unknown> = new (rawValue: string) => T;
 type PropertyKey = string | symbol;
 interface PropertyMetadata {
 	type: unknown;
@@ -46,6 +47,8 @@ export const Config: ClassDecorator = (ConfigClass: Class) => {
 					} else {
 						value = value === 'true';
 					}
+				} else if (type !== String && type !== Object) {
+					value = new (type as Constructable)(value as string);
 				}
 
 				if (value !== undefined) {

diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts
@@ -7,6 +7,7 @@ import { PublicApiConfig } from './configs/public-api';
 import { ExternalSecretsConfig } from './configs/external-secrets';
 import { TemplatesConfig } from './configs/templates';
 import { EventBusConfig } from './configs/event-bus';
+import { NodesConfig } from './configs/nodes';
 
 @Config
 class UserManagementConfig {
@@ -39,4 +40,7 @@ export class GlobalConfig {
 
 	@Nested
 	eventBus: EventBusConfig;
+
+	@Nested
+	readonly nodes: NodesConfig;
 }
diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts
@@ -17,7 +17,7 @@ describe('GlobalConfig', () => {
 		process.env = originalEnv;
 	});
 
-	const defaultConfig = {
+	const defaultConfig: GlobalConfig = {
 		database: {
 			logging: {
 				enabled: false,
@@ -100,6 +100,14 @@ describe('GlobalConfig', () => {
 			preferGet: false,
 			updateInterval: 300,
 		},
+		nodes: {
+			communityPackages: {
+				enabled: true,
+			},
+			errorTriggerType: 'n8n-nodes-base.errorTrigger',
+			include: [],
+			exclude: [],
+		},
 		publicApi: {
 			disabled: false,
 			path: 'api',
@@ -128,6 +136,7 @@ describe('GlobalConfig', () => {
 			DB_POSTGRESDB_HOST: 'some-host',
 			DB_POSTGRESDB_USER: 'n8n',
 			DB_TABLE_PREFIX: 'test_',
+			NODES_INCLUDE: '["n8n-nodes-base.hackerNews"]',
 		};
 		const config = Container.get(GlobalConfig);
 		expect(config).toEqual({
@@ -144,11 +153,15 @@ describe('GlobalConfig', () => {
 				tablePrefix: 'test_',
 				type: 'sqlite',
 			},
+			nodes: {
+				...defaultConfig.nodes,
+				include: ['n8n-nodes-base.hackerNews'],
+			},
 		});
 		expect(mockFs.readFileSync).not.toHaveBeenCalled();
 	});
 
-	it('should use values from env variables when defined and convert them to the correct type', () => {
+	it('should read values from files using _FILE env variables', () => {
 		const passwordFile = '/path/to/postgres/password';
 		process.env = {
 			DB_POSTGRESDB_PASSWORD_FILE: passwordFile,

diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts
@@ -19,7 +19,6 @@ import type {
 } from 'n8n-workflow';
 import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
 
-import config from '@/config';
 import {
 	CUSTOM_API_CALL_KEY,
 	CUSTOM_API_CALL_NAME,
@@ -28,6 +27,7 @@ import {
 	inE2ETests,
 } from '@/constants';
 import { Logger } from '@/Logger';
+import { GlobalConfig } from '@n8n/config';
 
 interface LoadedNodesAndCredentials {
 	nodes: INodeTypeData;
@@ -44,15 +44,16 @@ export class LoadNodesAndCredentials {
 
 	loaders: Record<string, DirectoryLoader> = {};
 
-	excludeNodes = config.getEnv('nodes.exclude');
+	excludeNodes = this.globalConfig.nodes.exclude;
 
-	includeNodes = config.getEnv('nodes.include');
+	includeNodes = this.globalConfig.nodes.include;
 
 	private postProcessors: Array<() => Promise<void>> = [];
 
 	constructor(
 		private readonly logger: Logger,
 		private readonly instanceSettings: InstanceSettings,
+		private readonly globalConfig: GlobalConfig,
 	) {}
 
 	async init() {

diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts
@@ -120,7 +120,7 @@ export class Server extends AbstractServer {
 			await Container.get(LdapService).init();
 		}
 
-		if (config.getEnv('nodes.communityPackages.enabled')) {
+		if (this.globalConfig.nodes.communityPackages.enabled) {
 			await import('@/controllers/communityPackages.controller');
 		}
 

diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts
@@ -72,8 +72,7 @@ import { UrlService } from './services/url.service';
 import { WorkflowExecutionService } from './workflows/workflowExecution.service';
 import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus';
 import { EventService } from './eventbus/event.service';
-
-const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType');
+import { GlobalConfig } from '@n8n/config';
 
 export function objectToError(errorObject: unknown, workflow: Workflow): Error {
 	// TODO: Expand with other error types
@@ -176,6 +175,7 @@ export function executeErrorWorkflow(
 			};
 		}
 
+		const { errorTriggerType } = Container.get(GlobalConfig).nodes;
 		// Run the error workflow
 		// To avoid an infinite loop do not run the error workflow again if the error-workflow itself failed and it is its own error-workflow.
 		const { errorWorkflow } = workflowData.settings ?? {};
@@ -220,7 +220,7 @@ export function executeErrorWorkflow(
 		} else if (
 			mode !== 'error' &&
 			workflowId !== undefined &&
-			workflowData.nodes.some((node) => node.type === ERROR_TRIGGER_TYPE)
+			workflowData.nodes.some((node) => node.type === errorTriggerType)
 		) {
 			logger.verbose('Start internal error workflow', { executionId, workflowId });
 			void Container.get(OwnershipService)

diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts
@@ -252,16 +252,15 @@ export class Start extends BaseCommand {
 			config.set(setting.key, jsonParse(setting.value, { fallbackValue: setting.value }));
 		});
 
-		const areCommunityPackagesEnabled = config.getEnv('nodes.communityPackages.enabled');
+		const globalConfig = Container.get(GlobalConfig);
 
-		if (areCommunityPackagesEnabled) {
+		if (globalConfig.nodes.communityPackages.enabled) {
 			const { CommunityPackagesService } = await import('@/services/communityPackages.service');
 			await Container.get(CommunityPackagesService).setMissingPackages({
 				reinstallMissingPackages: flags.reinstallMissingPackages,
 			});
 		}
 
-		const globalConfig = Container.get(GlobalConfig);
 		const { type: dbType } = globalConfig.database;
 		if (dbType === 'sqlite') {
 			const shouldRunVacuum = globalConfig.database.sqlite.executeVacuumOnStartup;

diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts
@@ -2,18 +2,9 @@ import path from 'path';
 import convict from 'convict';
 import { Container } from 'typedi';
 import { InstanceSettings } from 'n8n-core';
-import { LOG_LEVELS, jsonParse } from 'n8n-workflow';
+import { LOG_LEVELS } from 'n8n-workflow';
 import { ensureStringArray } from './utils';
 
-convict.addFormat({
-	name: 'json-string-array',
-	coerce: (rawStr: string) =>
-		jsonParse<string[]>(rawStr, {
-			errorMessage: `Expected this value "${rawStr}" to be valid JSON`,
-		}),
-	validate: ensureStringArray,
-});
-
 convict.addFormat({
 	name: 'comma-separated-list',
 	coerce: (rawStr: string) => rawStr.split(','),
@@ -615,35 +606,6 @@ export const schema = {
 		env: 'EXTERNAL_HOOK_FILES',
 	},
 
-	nodes: {
-		include: {
-			doc: 'Nodes to load',
-			format: 'json-string-array',
-			default: undefined,
-			env: 'NODES_INCLUDE',
-		},
-		exclude: {
-			doc: 'Nodes not to load',
-			format: 'json-string-array',
-			default: undefined,
-			env: 'NODES_EXCLUDE',
-		},
-		errorTriggerType: {
-			doc: 'Node Type to use as Error Trigger',
-			format: String,
-			default: 'n8n-nodes-base.errorTrigger',
-			env: 'NODES_ERROR_TRIGGER_TYPE',
-		},
-		communityPackages: {
-			enabled: {
-				doc: 'Allows you to disable the usage of community packages for nodes',
-				format: Boolean,
-				default: true,
-				env: 'N8N_COMMUNITY_PACKAGES_ENABLED',
-			},
-		},
-	},
-
 	logs: {
 		level: {
 			doc: 'Log output level',

diff --git a/packages/cli/src/config/types.ts b/packages/cli/src/config/types.ts
@@ -75,8 +75,6 @@ type ToReturnType<T extends ConfigOptionPath> = T extends NumericPath
 type ExceptionPaths = {
 	'queue.bull.redis': RedisOptions;
 	binaryDataManager: BinaryData.Config;
-	'nodes.exclude': string[] | undefined;
-	'nodes.include': string[] | undefined;
 	'userManagement.isInstanceOwnerSetUp': boolean;
 	'ui.banners.dismissed': string[] | undefined;
 };

diff --git a/packages/cli/src/security-audit/risk-reporters/InstanceRiskReporter.ts b/packages/cli/src/security-audit/risk-reporters/InstanceRiskReporter.ts
@@ -88,15 +88,17 @@ export class InstanceRiskReporter implements RiskReporter {
 		const settings: Record<string, unknown> = {};
 
 		settings.features = {
-			communityPackagesEnabled: config.getEnv('nodes.communityPackages.enabled'),
+			communityPackagesEnabled: this.globalConfig.nodes.communityPackages.enabled,
 			versionNotificationsEnabled: this.globalConfig.versionNotifications.enabled,
 			templatesEnabled: this.globalConfig.templates.enabled,
 			publicApiEnabled: isApiEnabled(),
 		};
 
+		const { exclude, include } = this.globalConfig.nodes;
+
 		settings.nodes = {
-			nodesExclude: config.getEnv('nodes.exclude') ?? 'none',
-			nodesInclude: config.getEnv('nodes.include') ?? 'none',
+			nodesExclude: exclude.length === 0 ? 'none' : exclude.join(', '),
+			nodesInclude: include.length === 0 ? 'none' : include.join(', '),
 		};
 
 		settings.telemetry = {

diff --git a/packages/cli/src/security-audit/risk-reporters/NodesRiskReporter.ts b/packages/cli/src/security-audit/risk-reporters/NodesRiskReporter.ts
@@ -1,7 +1,6 @@
 import * as path from 'path';
 import glob from 'fast-glob';
 import { Service } from 'typedi';
-import config from '@/config';
 import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
 import { getNodeTypes } from '@/security-audit/utils';
 import {
@@ -14,12 +13,14 @@ import {
 import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
 import type { Risk, RiskReporter } from '@/security-audit/types';
 import { CommunityPackagesService } from '@/services/communityPackages.service';
+import { GlobalConfig } from '@n8n/config';
 
 @Service()
 export class NodesRiskReporter implements RiskReporter {
 	constructor(
 		private readonly loadNodesAndCredentials: LoadNodesAndCredentials,
 		private readonly communityPackagesService: CommunityPackagesService,
+		private readonly globalConfig: GlobalConfig,
 	) {}
 
 	async report(workflows: WorkflowEntity[]) {
@@ -85,7 +86,7 @@ export class NodesRiskReporter implements RiskReporter {
 	}
 
 	private async getCommunityNodeDetails() {
-		if (!config.getEnv('nodes.communityPackages.enabled')) return [];
+		if (!this.globalConfig.nodes.communityPackages.enabled) return [];
 
 		const installedPackages = await this.communityPackagesService.getAllInstalledPackages();
 

diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts
@@ -57,7 +57,7 @@ export class FrontendService {
 
 		this.initSettings();
 
-		if (config.getEnv('nodes.communityPackages.enabled')) {
+		if (this.globalConfig.nodes.communityPackages.enabled) {
 			void import('@/services/communityPackages.service').then(({ CommunityPackagesService }) => {
 				this.communityPackagesService = Container.get(CommunityPackagesService);
 			});
@@ -154,7 +154,7 @@ export class FrontendService {
 				latestVersion: 1,
 				path: this.globalConfig.publicApi.path,
 				swaggerUi: {
-					enabled: !Container.get(GlobalConfig).publicApi.swaggerUiDisabled,
+					enabled: !this.globalConfig.publicApi.swaggerUiDisabled,
 				},
 			},
 			workflowTagsDisabled: config.getEnv('workflowTagsDisabled'),
@@ -166,7 +166,7 @@ export class FrontendService {
 			},
 			executionMode: config.getEnv('executions.mode'),
 			pushBackend: config.getEnv('push.backend'),
-			communityNodesEnabled: config.getEnv('nodes.communityPackages.enabled'),
+			communityNodesEnabled: this.globalConfig.nodes.communityPackages.enabled,
 			deployment: {
 				type: config.getEnv('deployment.type'),
 			},