From d0e80a5ba2ae43dc1238077c5fb9b77be318137a Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Fri, 14 Mar 2025 19:00:03 +0100 Subject: [PATCH] [Fix] Class validator native `isSemver` does not handle v-prefix (#10907) # Introduction Under the hood class-validator isSemver uses https://github.com/validatorjs/validator.js/blob/master/src/lib/isSemVer.js which does not cover all semVer use cases ## Even tho Had a discussion with @charles was about to store in db ws version as `vx.y.z`. We felt like we wanted it to be stored as `x.y.z`, in my opinion `APP_VERSION` should reflect the tag used to be build the instance and not be updated But we could extract only `x.y.z` from it at runtime Also handling the `v` extraction in CD is IMO not the most reliable ## Env var logging refactor Now not stopping on first error log ```ts Successfully compiled: 2128 files with swc (185.34ms) Watching for file changes. [Nest] 52686 - 03/14/2025, 6:28:33 PM ERROR PG_DATABASE_URL should not be null or undefined PG_DATABASE_URL must be a URL address [Nest] 52686 - 03/14/2025, 6:28:33 PM ERROR APP_VERSION must be a valid semantic version (e.g., 1.0.0) /Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts:1019 throw new Error("Environment variables validation failed") ^ Error: Environment variables validation failed at Object.validate (/Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts:1019:11) at Function.forRoot (/Users/paulrastoin/ws/twenty/node_modules/@nestjs/config/dist/config.module.js:67:45) at Object. (/Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment.module.ts:11:18) at Module._compile (node:internal/modules/cjs/loader:1256:14) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1310:10) at Module.load (node:internal/modules/cjs/loader:1119:32) at Function.Module._load (node:internal/modules/cjs/loader:960:12) at Module.require (node:internal/modules/cjs/loader:1143:19) at require (node:internal/modules/cjs/helpers:121:18) at Object. (/Users/paulrastoin/ws/twenty/packages/twenty-server/dist/src/database/typeorm/typeorm.module.js:14:28) ``` --- .../decorators/is-twenty-semver.decorator.ts | 33 ++++++++++++++++ .../environment/environment-variables.ts | 38 ++++++++++++------- 2 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/environment/decorators/is-twenty-semver.decorator.ts diff --git a/packages/twenty-server/src/engine/core-modules/environment/decorators/is-twenty-semver.decorator.ts b/packages/twenty-server/src/engine/core-modules/environment/decorators/is-twenty-semver.decorator.ts new file mode 100644 index 000000000..e40746675 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/environment/decorators/is-twenty-semver.decorator.ts @@ -0,0 +1,33 @@ +import { + registerDecorator, + ValidationArguments, + ValidationOptions, + ValidatorConstraint, + ValidatorConstraintInterface, +} from 'class-validator'; +import semver from 'semver'; + +@ValidatorConstraint({ async: false }) +export class IsTwentySemVerValidator implements ValidatorConstraintInterface { + validate(version: string) { + const parsed = semver.parse(version); + + return parsed !== null; + } + + defaultMessage(args: ValidationArguments) { + return `${args.property} must be a valid semantic version (e.g., 1.0.0)`; + } +} + +export const IsTwentySemVer = + (validationOptions?: ValidationOptions) => + (object: object, propertyName: string) => { + registerDecorator({ + target: object.constructor, + propertyName: propertyName, + options: validationOptions, + constraints: [], + validator: IsTwentySemVerValidator, + }); + }; diff --git a/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts b/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts index d07bb35fd..2a814f30a 100644 --- a/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts @@ -7,12 +7,13 @@ import { IsEnum, IsNumber, IsOptional, - IsSemVer, IsString, IsUrl, ValidateIf, + ValidationError, validateSync, } from 'class-validator'; +import { isDefined } from 'twenty-shared'; import { EmailDriver } from 'src/engine/core-modules/email/interfaces/email.interface'; import { AwsRegion } from 'src/engine/core-modules/environment/interfaces/aws-region.interface'; @@ -30,12 +31,12 @@ import { IsAWSRegion } from 'src/engine/core-modules/environment/decorators/is-a import { IsDuration } from 'src/engine/core-modules/environment/decorators/is-duration.decorator'; import { IsOptionalOrEmptyString } from 'src/engine/core-modules/environment/decorators/is-optional-or-empty-string.decorator'; import { IsStrictlyLowerThan } from 'src/engine/core-modules/environment/decorators/is-strictly-lower-than.decorator'; +import { IsTwentySemVer } from 'src/engine/core-modules/environment/decorators/is-twenty-semver.decorator'; import { EnvironmentVariablesGroup } from 'src/engine/core-modules/environment/enums/environment-variables-group.enum'; import { ExceptionHandlerDriver } from 'src/engine/core-modules/exception-handler/interfaces'; import { StorageDriverType } from 'src/engine/core-modules/file-storage/interfaces'; import { LoggerDriverType } from 'src/engine/core-modules/logger/interfaces'; import { ServerlessDriverType } from 'src/engine/core-modules/serverless/serverless.interface'; -import { assert } from 'src/utils/assert'; export class EnvironmentVariables { @EnvironmentVariablesMetadata({ @@ -986,7 +987,7 @@ export class EnvironmentVariables { description: 'Twenty server version', }) @IsOptionalOrEmptyString() - @IsSemVer() + @IsTwentySemVer() APP_VERSION?: string; } @@ -995,21 +996,32 @@ export const validate = ( ): EnvironmentVariables => { const validatedConfig = plainToClass(EnvironmentVariables, config); - const errors = validateSync(validatedConfig, { strictGroups: true }); + const validationErrors = validateSync(validatedConfig, { + strictGroups: true, + }); - const warnings = validateSync(validatedConfig, { groups: ['warning'] }); - - if (warnings.length > 0) { - warnings.forEach((warning) => { - if (warning.constraints && warning.property) { - Object.values(warning.constraints).forEach((message) => { - Logger.warn(message); - }); + const validationWarnings = validateSync(validatedConfig, { + groups: ['warning'], + }); + const logValidatonErrors = ( + errorCollection: ValidationError[], + type: 'error' | 'warn', + ) => + errorCollection.forEach((error) => { + if (!isDefined(error.constraints) || !isDefined(error.property)) { + return; } + Logger[type](Object.values(error.constraints).join('\n')); }); + + if (validationWarnings.length > 0) { + logValidatonErrors(validationWarnings, 'warn'); } - assert(!errors.length, errors.toString()); + if (validationErrors.length > 0) { + logValidatonErrors(validationErrors, 'error'); + throw new Error('Environment variables validation failed'); + } return validatedConfig; };