From 223ec2d9c99c7633f7fc99b53af0471d41d21892 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: Thu, 27 Feb 2025 09:21:45 +0100 Subject: [PATCH] refactor(core): Prevent a server from starting if it's configured to use S3, but the license does not allow it (#13532) --- packages/cli/src/commands/base-command.ts | 46 +++---------------- packages/cli/src/license.ts | 21 +-------- .../__tests__/object-store.service.test.ts | 18 -------- .../object-store/object-store.service.ee.ts | 26 +---------- .../src/binary-data/object-store/utils.ts | 4 -- 5 files changed, 10 insertions(+), 105 deletions(-) diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index d9e3356ac8..d5b9427ed2 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -148,7 +148,7 @@ export abstract class BaseCommand extends Command { const isSelected = config.getEnv('binaryDataManager.mode') === 's3'; const isAvailable = config.getEnv('binaryDataManager.availableModes').includes('s3'); - if (!isSelected && !isAvailable) return; + if (!isSelected) return; if (isSelected && !isAvailable) { throw new UserError( @@ -157,51 +157,19 @@ export abstract class BaseCommand extends Command { } const isLicensed = Container.get(License).isFeatureEnabled(LICENSE_FEATURES.BINARY_DATA_S3); - - if (isSelected && isAvailable && isLicensed) { - this.logger.debug( - 'License found for external storage - object store to init in read-write mode', + if (!isLicensed) { + this.logger.error( + 'No license found for S3 storage. \n Either set `N8N_DEFAULT_BINARY_DATA_MODE` to something else, or upgrade to a license that supports this feature.', ); - - await this._initObjectStoreService(); - - return; + return this.exit(1); } - if (isSelected && isAvailable && !isLicensed) { - this.logger.debug( - 'No license found for external storage - object store to init with writes blocked. To enable writes, please upgrade to a license that supports this feature.', - ); - - await this._initObjectStoreService({ isReadOnly: true }); - - return; - } - - if (!isSelected && isAvailable) { - this.logger.debug( - 'External storage unselected but available - object store to init with writes unused', - ); - - await this._initObjectStoreService(); - - return; - } - } - - private async _initObjectStoreService(options = { isReadOnly: false }) { - const objectStoreService = Container.get(ObjectStoreService); - - this.logger.debug('Initializing object store service'); - + this.logger.debug('License found for external storage - Initializing object store service'); try { - await objectStoreService.init(); - objectStoreService.setReadonly(options.isReadOnly); - + await Container.get(ObjectStoreService).init(); this.logger.debug('Object store init completed'); } catch (e) { const error = e instanceof Error ? e : new Error(`${e}`); - this.logger.debug('Object store init failed', { error }); } } diff --git a/packages/cli/src/license.ts b/packages/cli/src/license.ts index 3ae36e9a3c..694433000d 100644 --- a/packages/cli/src/license.ts +++ b/packages/cli/src/license.ts @@ -2,7 +2,7 @@ import { GlobalConfig } from '@n8n/config'; import { Container, Service } from '@n8n/di'; import type { TEntitlement, TFeatures, TLicenseBlock } from '@n8n_io/license-sdk'; import { LicenseManager } from '@n8n_io/license-sdk'; -import { InstanceSettings, ObjectStoreService, Logger } from 'n8n-core'; +import { InstanceSettings, Logger } from 'n8n-core'; import config from '@/config'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; @@ -106,7 +106,6 @@ export class License { const features = this.manager.getFeatures(); this.checkIsLicensedForMultiMain(features); - this.checkIsLicensedForBinaryDataS3(features); this.logger.debug('License initialized'); } catch (error: unknown) { @@ -135,7 +134,6 @@ export class License { this.logger.debug('License feature change detected', _features); this.checkIsLicensedForMultiMain(_features); - this.checkIsLicensedForBinaryDataS3(_features); if (this.instanceSettings.isMultiMain && !this.instanceSettings.isLeader) { this.logger @@ -405,23 +403,6 @@ export class License { } } - /** - * Ensures that the instance is licensed for binary data S3 if S3 is selected and available - */ - private checkIsLicensedForBinaryDataS3(features: TFeatures) { - const isS3Selected = config.getEnv('binaryDataManager.mode') === 's3'; - const isS3Available = config.getEnv('binaryDataManager.availableModes').includes('s3'); - const isS3Licensed = features['feat:binaryDataS3']; - - if (isS3Selected && isS3Available && !isS3Licensed) { - this.logger.debug( - 'License changed with no support for external storage - blocking writes on object store. To restore writes, please upgrade to a license that supports this feature.', - ); - - Container.get(ObjectStoreService).setReadonly(true); - } - } - enableAutoRenewals() { this.manager?.enableAutoRenewals(); } diff --git a/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts b/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts index ca8f4c0c94..03cfc5b979 100644 --- a/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts +++ b/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts @@ -4,7 +4,6 @@ import { mock } from 'jest-mock-extended'; import { Readable } from 'stream'; import { ObjectStoreService } from '@/binary-data/object-store/object-store.service.ee'; -import { writeBlockedMessage } from '@/binary-data/object-store/utils'; jest.mock('axios'); @@ -128,23 +127,6 @@ describe('put()', () => { }); }); - it('should block if read-only', async () => { - objectStoreService.setReadonly(true); - - const metadata = { fileName: 'file.txt', mimeType: 'text/plain' }; - - const promise = objectStoreService.put(fileId, mockBuffer, metadata); - - await expect(promise).resolves.not.toThrow(); - - const result = await promise; - - expect(result.status).toBe(403); - expect(result.statusText).toBe('Forbidden'); - - expect(result.data).toBe(writeBlockedMessage(fileId)); - }); - it('should throw an error on request failure', async () => { const metadata = { fileName: 'file.txt', mimeType: 'text/plain' }; diff --git a/packages/core/src/binary-data/object-store/object-store.service.ee.ts b/packages/core/src/binary-data/object-store/object-store.service.ee.ts index 5561bd61db..b557146872 100644 --- a/packages/core/src/binary-data/object-store/object-store.service.ee.ts +++ b/packages/core/src/binary-data/object-store/object-store.service.ee.ts @@ -3,7 +3,7 @@ import { Service } from '@n8n/di'; import { sign } from 'aws4'; import type { Request as Aws4Options } from 'aws4'; import axios from 'axios'; -import type { AxiosRequestConfig, AxiosResponse, InternalAxiosRequestConfig, Method } from 'axios'; +import type { AxiosRequestConfig, Method } from 'axios'; import { ApplicationError } from 'n8n-workflow'; import { createHash } from 'node:crypto'; import type { Readable } from 'stream'; @@ -11,7 +11,7 @@ import type { Readable } from 'stream'; import { Logger } from '@/logging/logger'; import type { ListPage, MetadataResponseHeaders, RawListPage, RequestOptions } from './types'; -import { isStream, parseXml, writeBlockedMessage } from './utils'; +import { isStream, parseXml } from './utils'; import type { BinaryData } from '../types'; @Service() @@ -20,8 +20,6 @@ export class ObjectStoreService { private isReady = false; - private isReadOnly = false; - constructor( private readonly logger: Logger, private readonly s3Config: S3Config, @@ -48,10 +46,6 @@ export class ObjectStoreService { this.setReady(true); } - setReadonly(newState: boolean) { - this.isReadOnly = newState; - } - setReady(newState: boolean) { this.isReady = newState; } @@ -73,8 +67,6 @@ export class ObjectStoreService { * @doc https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html */ async put(filename: string, buffer: Buffer, metadata: BinaryData.PreWriteMetadata = {}) { - if (this.isReadOnly) return await this.blockWrite(filename); - const headers: Record = { 'Content-Length': buffer.length, 'Content-MD5': createHash('md5').update(buffer).digest('base64'), @@ -204,20 +196,6 @@ export class ObjectStoreService { return page as ListPage; } - private async blockWrite(filename: string): Promise { - const logMessage = writeBlockedMessage(filename); - - this.logger.warn(logMessage); - - return { - status: 403, - statusText: 'Forbidden', - data: logMessage, - headers: {}, - config: {} as InternalAxiosRequestConfig, - }; - } - private async request( method: Method, rawPath = '', diff --git a/packages/core/src/binary-data/object-store/utils.ts b/packages/core/src/binary-data/object-store/utils.ts index 6dc1e4df77..16ed264eb3 100644 --- a/packages/core/src/binary-data/object-store/utils.ts +++ b/packages/core/src/binary-data/object-store/utils.ts @@ -14,7 +14,3 @@ export async function parseXml(xml: string): Promise { valueProcessors: [parseNumbers, parseBooleans], }) as Promise); } - -export function writeBlockedMessage(filename: string) { - return `Request to write file "${filename}" to object storage was blocked because S3 storage is not available with your current license. Please upgrade to a license that supports this feature, or set N8N_DEFAULT_BINARY_DATA_MODE to an option other than "s3".`; -}