From 3a407f38f57ad3237f1f7c31b48c6570569f23c2 Mon Sep 17 00:00:00 2001 From: Alexander Lukin Date: Tue, 21 Nov 2023 15:21:12 +0400 Subject: [PATCH 1/2] refactor!: environment variables validation Refactor environment variable validation rules. Apply consistent patterns to env validation. The main motivation for this change was as follows: 1. Make env validation more strict and consistent. Stick to the same validation patterns in different projects. 2. Provide consistent fallback options for optional variables. Here are the main changes: 1. Previously if a user set an empty string as the variable name in the .env file like this: ``` VARIABLE_NAME= ``` not all variable initializers assigned a correct default value to the variable in this case. Now it is fixed. For the purpose of this improvement, now all optional variable initializers always have the `@Transform` decorator that provides a default value for the variable. Also for this purpose, the `toBoolean` function has been refactored to be more similar to the `toNumber` function. 2. Make validation of boolean variables more strict. CAUTION: This might be a breaking change for users who use values like "yes" to initialize boolean variables. 3. Implement the new `toArrayOfUrls()` function to provide a reasonable default value for variables that should have a list of URLs in their values. This function also fixes a bug with default values for such variables. Previously, if a user set an empty string as a value of such variables, the validation worked incorrectly. 4. Now we validate that values in `PROVIDERS_URLS` and `CL_API_URLS` are indeed URLs. 5. The `CHAIN_ID` variable now supports only a pre-defined list of testnet IDs. Add the appropriate `Network` enum for this purpose. CAUTION: Currently the Keys API doesn't officially support testnets other than Goerli, Holesky, and Mainnet. But this might be a breaking change if some users use Keys API for other testnets. 6. More strict validation for the `PORT` and `DB_PORT` values. CAUTION: This might be a breaking change. 7. Add the `@IsNotEmpty` decorator to the required variables. 8. Fix confusing validation rules for the `DB_PASSWORD` variable. Add default value. --- src/common/config/env.validation.ts | 128 +++++++++++------- .../interfaces/environment.interface.ts | 6 + 2 files changed, 86 insertions(+), 48 deletions(-) diff --git a/src/common/config/env.validation.ts b/src/common/config/env.validation.ts index 18a92697..9e424644 100644 --- a/src/common/config/env.validation.ts +++ b/src/common/config/env.validation.ts @@ -1,64 +1,72 @@ import { plainToClass, Transform } from 'class-transformer'; import { - IsEnum, - IsString, - IsOptional, - validateSync, - Min, - IsArray, ArrayMinSize, - IsInt, + IsArray, IsBoolean, - ValidateIf, + IsEnum, + IsInt, IsNotEmpty, + IsOptional, IsPositive, + IsString, + IsUrl, + Max, + Min, + ValidateIf, + validateSync, } from 'class-validator'; -import { Environment, LogLevel, LogFormat } from './interfaces'; +import { Environment, LogLevel, LogFormat, Network } from './interfaces'; import { NonEmptyArray } from '@lido-nestjs/execution/dist/interfaces/non-empty-array'; const toNumber = ({ defaultValue }) => ({ value }) => { - if (value === '' || value == null || value == undefined) return defaultValue; + if (value === '' || value == null) return defaultValue; return Number(value); }; -export const toBoolean = (value: any): boolean => { - if (typeof value === 'boolean') { - return value; - } +const toBoolean = ({ defaultValue }) => { + return function({ value }) { + if (value == null || value === '') { + return defaultValue; + } - if (typeof value === 'number') { - return !!value; - } + if (typeof value === 'boolean') { + return value; + } - if (!(typeof value === 'string')) { - return false; - } + const str = value.toString().toLowerCase().trim(); - switch (value.toLowerCase().trim()) { - case 'true': - case 'yes': - case '1': + if (str === 'true') { return true; - case 'false': - case 'no': - case '0': - case null: - return false; - default: + } + + if (str === 'false') { return false; + } + + return value; + } +} + +const toArrayOfUrls = (url: string | null): string[] => { + if (url == null || url === '') { + return []; } + + return url.split(',').map((str) => str.trim().replace(/\/$/, '')); }; export class EnvironmentVariables { @IsOptional() @IsEnum(Environment) + @Transform(({ value }) => value || Environment.development) NODE_ENV: Environment = Environment.development; @IsOptional() @IsInt() - @Min(1) + @Min(1025) + @Max(65535) @Transform(toNumber({ defaultValue: 3000 })) PORT = 3000; @@ -68,19 +76,19 @@ export class EnvironmentVariables { @IsOptional() @IsInt() - @Min(1) + @IsPositive() @Transform(toNumber({ defaultValue: 5 })) GLOBAL_THROTTLE_TTL = 5; @IsOptional() @IsInt() - @Min(1) + @IsPositive() @Transform(toNumber({ defaultValue: 100 })) GLOBAL_THROTTLE_LIMIT = 100; @IsOptional() @IsInt() - @Min(1) + @IsPositive() @Transform(toNumber({ defaultValue: 1 })) GLOBAL_CACHE_TTL = 1; @@ -98,78 +106,102 @@ export class EnvironmentVariables { @Transform(({ value }) => value || LogFormat.json) LOG_FORMAT: LogFormat = LogFormat.json; + @IsNotEmpty() @IsArray() @ArrayMinSize(1) - @Transform(({ value }) => value.split(',').map((url) => url.replace(/\/$/, ''))) + @IsUrl({ + require_protocol: true + }, { + each: true, + }) + @Transform(({ value }) => toArrayOfUrls(value)) PROVIDERS_URLS!: NonEmptyArray; - @IsInt() + @IsNotEmpty() + @IsEnum(Network) @Transform(({ value }) => parseInt(value, 10)) - CHAIN_ID!: number; + CHAIN_ID!: Network; + @IsNotEmpty() @IsString() DB_HOST!: string; + @IsNotEmpty() @IsString() DB_USER!: string; @IsOptional() @IsString() - @IsNotEmpty() - DB_PASSWORD!: string; + DB_PASSWORD = ''; @ValidateIf((e) => !e.DB_PASSWORD) @IsString() @IsNotEmpty() DB_PASSWORD_FILE!: string; + @IsNotEmpty() @IsString() DB_NAME!: string; + @IsNotEmpty() @IsInt() + @Min(1025) + @Max(65535) @Transform(({ value }) => parseInt(value, 10)) DB_PORT!: number; @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10)) + @IsPositive() + @Transform(toNumber({ defaultValue: 100 })) PROVIDER_JSON_RPC_MAX_BATCH_SIZE = 100; @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10)) + @IsPositive() + @Transform(toNumber({ defaultValue: 5 })) PROVIDER_CONCURRENT_REQUESTS = 5; @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10)) + @IsPositive() + @Transform(toNumber({ defaultValue: 10 })) PROVIDER_BATCH_AGGREGATION_WAIT_MS = 10; // Enable endpoints that use CL API for ejector @IsOptional() @IsBoolean() - @Transform(({ value }) => toBoolean(value)) + @Transform(toBoolean({ defaultValue: true })) VALIDATOR_REGISTRY_ENABLE = true; - @ValidateIf((e) => e.VALIDATOR_REGISTRY_ENABLE === true) + @ValidateIf((e) => e.VALIDATOR_REGISTRY_ENABLE) + @IsNotEmpty() @IsArray() @ArrayMinSize(1) - @Transform(({ value }) => value.split(',').map((url) => url.replace(/\/$/, ''))) + @IsUrl({ + require_protocol: true, + }, { + each: true, + }) + @Transform(({ value }) => toArrayOfUrls(value)) CL_API_URLS: string[] = []; @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10)) + @IsPositive() + @Transform(toNumber({ defaultValue: 5000 })) UPDATE_KEYS_INTERVAL_MS = 5000; @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10)) + @IsPositive() + @Transform(toNumber({ defaultValue: 10000 })) UPDATE_VALIDATORS_INTERVAL_MS = 10000; @IsOptional() + @IsInt() @IsPositive() - @Transform(({ value }) => parseInt(value, 10)) + @Transform(toNumber({ defaultValue: 1100 })) KEYS_FETCH_BATCH_SIZE = 1100; } diff --git a/src/common/config/interfaces/environment.interface.ts b/src/common/config/interfaces/environment.interface.ts index 1b9198f1..38a03af9 100644 --- a/src/common/config/interfaces/environment.interface.ts +++ b/src/common/config/interfaces/environment.interface.ts @@ -17,3 +17,9 @@ export enum LogFormat { json = 'json', simple = 'simple', } + +export enum Network { + Mainnet = 1, + Goerli = 5, + Holesky = 17000, +} From 6b59254882ffb3bf4726cc21d9abfed1977bd3aa Mon Sep 17 00:00:00 2001 From: Alexander Lukin Date: Fri, 24 Nov 2023 20:46:09 +0400 Subject: [PATCH 2/2] refactor!: environment variables validation Resolve review comments. The main changes are: 1. Loosen validation rules for boolean variables. Now numbers "1" and "0" and constants "yes" and "no" are accepted as valid boolean values. 2. Make DB password value required. Remove the default empty DB password value. 3. Rename the `Network` interface to `Chain`. --- src/common/config/env.validation.ts | 29 +++++++++++-------- .../interfaces/environment.interface.ts | 2 +- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/common/config/env.validation.ts b/src/common/config/env.validation.ts index 9e424644..4a1dd0bc 100644 --- a/src/common/config/env.validation.ts +++ b/src/common/config/env.validation.ts @@ -15,7 +15,7 @@ import { ValidateIf, validateSync, } from 'class-validator'; -import { Environment, LogLevel, LogFormat, Network } from './interfaces'; +import { Environment, LogLevel, LogFormat, Chain } from './interfaces'; import { NonEmptyArray } from '@lido-nestjs/execution/dist/interfaces/non-empty-array'; const toNumber = @@ -37,15 +37,20 @@ const toBoolean = ({ defaultValue }) => { const str = value.toString().toLowerCase().trim(); - if (str === 'true') { - return true; - } + switch (str) { + case 'true': + case 'yes': + case '1': + return true; - if (str === 'false') { - return false; - } + case 'false': + case 'no': + case '0': + return false; - return value; + default: + return value; + } } } @@ -118,9 +123,9 @@ export class EnvironmentVariables { PROVIDERS_URLS!: NonEmptyArray; @IsNotEmpty() - @IsEnum(Network) + @IsEnum(Chain) @Transform(({ value }) => parseInt(value, 10)) - CHAIN_ID!: Network; + CHAIN_ID!: Chain; @IsNotEmpty() @IsString() @@ -130,9 +135,9 @@ export class EnvironmentVariables { @IsString() DB_USER!: string; - @IsOptional() + @IsNotEmpty() @IsString() - DB_PASSWORD = ''; + DB_PASSWORD!: string; @ValidateIf((e) => !e.DB_PASSWORD) @IsString() diff --git a/src/common/config/interfaces/environment.interface.ts b/src/common/config/interfaces/environment.interface.ts index 38a03af9..5a86d4a2 100644 --- a/src/common/config/interfaces/environment.interface.ts +++ b/src/common/config/interfaces/environment.interface.ts @@ -18,7 +18,7 @@ export enum LogFormat { simple = 'simple', } -export enum Network { +export enum Chain { Mainnet = 1, Goerli = 5, Holesky = 17000,