From 33a2703429d9eaa41f72d2e7d2da5be60b6c620f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 14 Aug 2024 09:44:19 +0200 Subject: [PATCH] feat(core): Allow overriding npm registry for community packages (#10325) --- packages/@n8n/config/src/configs/nodes.ts | 4 ++ packages/@n8n/config/test/config.test.ts | 1 + packages/cli/src/License.ts | 4 ++ packages/cli/src/constants.ts | 1 + .../cli/src/controllers/e2e.controller.ts | 1 + .../communityPackages.service.test.ts | 47 ++++++++++++++----- .../src/services/communityPackages.service.ts | 29 +++++++++--- 7 files changed, 68 insertions(+), 19 deletions(-) diff --git a/packages/@n8n/config/src/configs/nodes.ts b/packages/@n8n/config/src/configs/nodes.ts index 3837b0a99b258..cf8407762cb17 100644 --- a/packages/@n8n/config/src/configs/nodes.ts +++ b/packages/@n8n/config/src/configs/nodes.ts @@ -26,6 +26,10 @@ class CommunityPackagesConfig { @Env('N8N_COMMUNITY_PACKAGES_ENABLED') enabled: boolean = true; + /** NPM registry URL to pull community packages from */ + @Env('N8N_COMMUNITY_PACKAGES_REGISTRY') + registry: string = 'https://registry.npmjs.org'; + /** Whether to reinstall any missing community packages */ @Env('N8N_REINSTALL_MISSING_PACKAGES') reinstallMissing: boolean = false; diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 83edcc25afc43..b8e89d0ab7f0d 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -108,6 +108,7 @@ describe('GlobalConfig', () => { nodes: { communityPackages: { enabled: true, + registry: 'https://registry.npmjs.org', reinstallMissing: false, }, errorTriggerType: 'n8n-nodes-base.errorTrigger', diff --git a/packages/cli/src/License.ts b/packages/cli/src/License.ts index 643da18fd26e9..fc0d2cd45a235 100644 --- a/packages/cli/src/License.ts +++ b/packages/cli/src/License.ts @@ -305,6 +305,10 @@ export class License { return this.isFeatureEnabled(LICENSE_FEATURES.PROJECT_ROLE_VIEWER); } + isCustomNpmRegistryEnabled() { + return this.isFeatureEnabled(LICENSE_FEATURES.COMMUNITY_NODES_CUSTOM_REGISTRY); + } + getCurrentEntitlements() { return this.manager?.getCurrentEntitlements() ?? []; } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 41c95cb01ace6..dfb072576aa9f 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -90,6 +90,7 @@ export const LICENSE_FEATURES = { PROJECT_ROLE_ADMIN: 'feat:projectRole:admin', PROJECT_ROLE_EDITOR: 'feat:projectRole:editor', PROJECT_ROLE_VIEWER: 'feat:projectRole:viewer', + COMMUNITY_NODES_CUSTOM_REGISTRY: 'feat:communityNodes:customRegistry', } as const; export const LICENSE_QUOTAS = { diff --git a/packages/cli/src/controllers/e2e.controller.ts b/packages/cli/src/controllers/e2e.controller.ts index 3954bb1caf891..20224795052e8 100644 --- a/packages/cli/src/controllers/e2e.controller.ts +++ b/packages/cli/src/controllers/e2e.controller.ts @@ -87,6 +87,7 @@ export class E2EController { [LICENSE_FEATURES.PROJECT_ROLE_ADMIN]: false, [LICENSE_FEATURES.PROJECT_ROLE_EDITOR]: false, [LICENSE_FEATURES.PROJECT_ROLE_VIEWER]: false, + [LICENSE_FEATURES.COMMUNITY_NODES_CUSTOM_REGISTRY]: false, }; private numericFeatures: Record = { diff --git a/packages/cli/src/services/__tests__/communityPackages.service.test.ts b/packages/cli/src/services/__tests__/communityPackages.service.test.ts index 8d3289a8695b2..ec7ce61ba9c1c 100644 --- a/packages/cli/src/services/__tests__/communityPackages.service.test.ts +++ b/packages/cli/src/services/__tests__/communityPackages.service.test.ts @@ -20,6 +20,7 @@ import { InstalledNodesRepository } from '@db/repositories/installedNodes.reposi import { InstalledPackagesRepository } from '@db/repositories/installedPackages.repository'; import { InstalledNodes } from '@db/entities/InstalledNodes'; import type { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; +import type { License } from '@/License'; import { mockInstance } from '@test/mocking'; import { COMMUNITY_NODE_VERSION, COMMUNITY_PACKAGE_VERSION } from '@test-integration/constants'; @@ -39,19 +40,20 @@ const execMock = ((...args) => { }) as typeof exec; describe('CommunityPackagesService', () => { + const license = mock(); const globalConfig = mock({ nodes: { communityPackages: { reinstallMissing: false, + registry: 'some.random.host', }, }, }); const loadNodesAndCredentials = mock(); + const nodeName = randomName(); const installedNodesRepository = mockInstance(InstalledNodesRepository); installedNodesRepository.create.mockImplementation(() => { - const nodeName = randomName(); - return Object.assign(new InstalledNodes(), { name: nodeName, type: nodeName, @@ -74,6 +76,7 @@ describe('CommunityPackagesService', () => { mock(), loadNodesAndCredentials, mock(), + license, globalConfig, ); @@ -374,25 +377,23 @@ describe('CommunityPackagesService', () => { }; describe('updateNpmModule', () => { - const packageDirectoryLoader = mock(); + const installedPackage = mock({ packageName: mockPackageName() }); + const packageDirectoryLoader = mock({ + loadedNodes: [{ name: nodeName, version: 1 }], + }); beforeEach(async () => { jest.clearAllMocks(); loadNodesAndCredentials.loadPackage.mockResolvedValue(packageDirectoryLoader); + mocked(exec).mockImplementation(execMock); }); - test('should call `exec` with the correct command ', async () => { + test('should call `exec` with the correct command and registry', async () => { // // ARRANGE // - const nodeName = randomName(); - packageDirectoryLoader.loadedNodes = [{ name: nodeName, version: 1 }]; - - const installedPackage = new InstalledPackages(); - installedPackage.packageName = mockPackageName(); - - mocked(exec).mockImplementation(execMock); + license.isCustomNpmRegistryEnabled.mockReturnValue(true); // // ACT @@ -406,10 +407,32 @@ describe('CommunityPackagesService', () => { expect(exec).toHaveBeenCalledTimes(1); expect(exec).toHaveBeenNthCalledWith( 1, - `npm install ${installedPackage.packageName}@latest`, + `npm install ${installedPackage.packageName}@latest --registry=some.random.host`, expect.any(Object), expect.any(Function), ); }); + + test('should throw when not licensed', async () => { + // + // ARRANGE + // + license.isCustomNpmRegistryEnabled.mockReturnValue(false); + + // + // ACT + // + const promise = communityPackagesService.updatePackage( + installedPackage.packageName, + installedPackage, + ); + + // + // ASSERT + // + await expect(promise).rejects.toThrow( + 'Your license does not allow for feat:communityNodes:customRegistry.', + ); + }); }); }); diff --git a/packages/cli/src/services/communityPackages.service.ts b/packages/cli/src/services/communityPackages.service.ts index b1f46d52fb434..985fb44fabdc7 100644 --- a/packages/cli/src/services/communityPackages.service.ts +++ b/packages/cli/src/services/communityPackages.service.ts @@ -14,16 +14,21 @@ import { toError } from '@/utils'; import { InstalledPackagesRepository } from '@db/repositories/installedPackages.repository'; import type { InstalledPackages } from '@db/entities/InstalledPackages'; import { + LICENSE_FEATURES, NODE_PACKAGE_PREFIX, NPM_COMMAND_TOKENS, NPM_PACKAGE_STATUS_GOOD, RESPONSE_ERROR_MESSAGES, UNKNOWN_FAILURE_REASON, } from '@/constants'; +import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; import type { CommunityPackages } from '@/Interfaces'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { Logger } from '@/Logger'; import { OrchestrationService } from './orchestration.service'; +import { License } from '@/License'; + +const DEFAULT_REGISTRY = 'https://registry.npmjs.org'; const { PACKAGE_NAME_NOT_PROVIDED, @@ -57,10 +62,9 @@ export class CommunityPackagesService { private readonly installedPackageRepository: InstalledPackagesRepository, private readonly loadNodesAndCredentials: LoadNodesAndCredentials, private readonly orchestrationService: OrchestrationService, - globalConfig: GlobalConfig, - ) { - this.reinstallMissingPackages = globalConfig.nodes.communityPackages.reinstallMissing; - } + private readonly license: License, + private readonly globalConfig: GlobalConfig, + ) {} get hasMissingPackages() { return this.missingPackages.length > 0; @@ -279,7 +283,8 @@ export class CommunityPackagesService { if (missingPackages.size === 0) return; - if (this.reinstallMissingPackages) { + const { reinstallMissing } = this.globalConfig.nodes.communityPackages; + if (reinstallMissing) { this.logger.info('Attempting to reinstall missing packages', { missingPackages }); try { // Optimistic approach - stop if any installation fails @@ -321,13 +326,21 @@ export class CommunityPackagesService { await this.orchestrationService.publish('community-package-uninstall', { packageName }); } + private getNpmRegistry() { + const { registry } = this.globalConfig.nodes.communityPackages; + if (registry !== DEFAULT_REGISTRY && !this.license.isCustomNpmRegistryEnabled()) { + throw new FeatureNotLicensedError(LICENSE_FEATURES.COMMUNITY_NODES_CUSTOM_REGISTRY); + } + return registry; + } + private async installOrUpdatePackage( packageName: string, options: { version?: string } | { installedPackage: InstalledPackages }, ) { const isUpdate = 'installedPackage' in options; const packageVersion = isUpdate || !options.version ? 'latest' : options.version; - const command = `npm install ${packageName}@${packageVersion}`; + const command = `npm install ${packageName}@${packageVersion} --registry=${this.getNpmRegistry()}`; try { await this.executeNpmCommand(command); @@ -379,7 +392,9 @@ export class CommunityPackagesService { } async installOrUpdateNpmPackage(packageName: string, packageVersion: string) { - await this.executeNpmCommand(`npm install ${packageName}@${packageVersion}`); + await this.executeNpmCommand( + `npm install ${packageName}@${packageVersion} --registry=${this.getNpmRegistry()}`, + ); await this.loadNodesAndCredentials.loadPackage(packageName); await this.loadNodesAndCredentials.postProcessLoaders(); this.logger.info(`Community package installed: ${packageName}`);