Skip to content

Commit

Permalink
fix(server): sslmode not working (#15587)
Browse files Browse the repository at this point in the history 
* parse db url before passing it to the driver

* don't be lazy

* simplify

* simplify

* add tests

* update sql sync script

* update mock

* remove unused import

* remove unused imports
  • Loading branch information
@mertalev
mertalev authored Jan 24, 2025
1 parent f5a3d7b commit ba01b40
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 50 deletions.
16 changes: 15 additions & 1 deletion server/src/app.module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import { Inject, Module, OnModuleDestroy, OnModuleInit, ValidationPipe } from '@
import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE, ModuleRef } from '@nestjs/core';
import { ScheduleModule, SchedulerRegistry } from '@nestjs/schedule';
import { TypeOrmModule } from '@nestjs/typeorm';
import { PostgresJSDialect } from 'kysely-postgres-js';
import { ClsModule } from 'nestjs-cls';
import { KyselyModule } from 'nestjs-kysely';
import { OpenTelemetryModule } from 'nestjs-otel';
import postgres from 'postgres';
import { commands } from 'src/commands';
import { IWorker } from 'src/constants';
import { controllers } from 'src/controllers';
Expand Down Expand Up @@ -57,7 +59,19 @@ const imports = [
},
}),
TypeOrmModule.forFeature(entities),
KyselyModule.forRoot(database.config.kysely),
KyselyModule.forRoot({
dialect: new PostgresJSDialect({ postgres: postgres(database.config.kysely) }),
log(event) {
if (event.level === 'error') {
console.error('Query failed :', {
durationMs: event.queryDurationMillis,
error: event.error,
sql: event.query.sql,
params: event.query.parameters,
});
}
},
}),
];

class BaseModule implements OnModuleInit, OnModuleDestroy {
Expand Down
4 changes: 3 additions & 1 deletion server/src/bin/sync-sql.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import { Reflector } from '@nestjs/core';
import { SchedulerRegistry } from '@nestjs/schedule';
import { Test } from '@nestjs/testing';
import { TypeOrmModule } from '@nestjs/typeorm';
import { PostgresJSDialect } from 'kysely-postgres-js';
import { KyselyModule } from 'nestjs-kysely';
import { OpenTelemetryModule } from 'nestjs-otel';
import { mkdir, rm, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import postgres from 'postgres';
import { format } from 'sql-formatter';
import { GENERATE_SQL_KEY, GenerateSqlQueries } from 'src/decorators';
import { entities } from 'src/entities';
Expand Down Expand Up @@ -84,7 +86,7 @@ class SqlGenerator {
const moduleFixture = await Test.createTestingModule({
imports: [
KyselyModule.forRoot({
...database.config.kysely,
dialect: new PostgresJSDialect({ postgres: postgres(database.config.kysely) }),
log: (event) => {
if (event.level === 'query') {
this.sqlLogger.logQuery(event.query.sql);
Expand Down
78 changes: 73 additions & 5 deletions server/src/repositories/config.repository.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { PostgresJSDialect } from 'kysely-postgres-js';
import { ImmichTelemetry } from 'src/enum';
import { clearEnvCache, ConfigRepository } from 'src/repositories/config.repository';

Expand Down Expand Up @@ -81,10 +80,13 @@ describe('getEnv', () => {
const { database } = getEnv();
expect(database).toEqual({
config: {
kysely: {
dialect: expect.any(PostgresJSDialect),
log: expect.any(Function),
},
kysely: expect.objectContaining({
host: 'database',
port: 5432,
database: 'immich',
username: 'postgres',
password: 'postgres',
}),
typeorm: expect.objectContaining({
type: 'postgres',
host: 'database',
Expand All @@ -104,6 +106,72 @@ describe('getEnv', () => {
const { database } = getEnv();
expect(database).toMatchObject({ skipMigrations: true });
});

it('should use DB_URL', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich';
const { database } = getEnv();
expect(database.config.kysely).toMatchObject({
host: 'database1',
password: 'postgres2',
user: 'postgres1',
port: 54_320,
database: 'immich',
});
});

it('should handle sslmode=require', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?sslmode=require';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: {} });
});

it('should handle sslmode=prefer', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?sslmode=prefer';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: {} });
});

it('should handle sslmode=verify-ca', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?sslmode=verify-ca';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: {} });
});

it('should handle sslmode=verify-full', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?sslmode=verify-full';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: {} });
});

it('should handle sslmode=no-verify', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?sslmode=no-verify';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: { rejectUnauthorized: false } });
});

it('should handle ssl=true', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?ssl=true';

const { database } = getEnv();

expect(database.config.kysely).toMatchObject({ ssl: true });
});

it('should reject invalid ssl', () => {
process.env.DB_URL = 'postgres://postgres1:postgres2@database1:54320/immich?ssl=invalid';

expect(() => getEnv()).toThrowError('Invalid ssl option: invalid');
});
});

describe('redis', () => {
Expand Down
75 changes: 47 additions & 28 deletions server/src/repositories/config.repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import { plainToInstance } from 'class-transformer';
import { validateSync } from 'class-validator';
import { Request, Response } from 'express';
import { RedisOptions } from 'ioredis';
import { KyselyConfig } from 'kysely';
import { PostgresJSDialect } from 'kysely-postgres-js';
import { CLS_ID, ClsModuleOptions } from 'nestjs-cls';
import { OpenTelemetryModuleOptions } from 'nestjs-otel/lib/interfaces';
import { join, resolve } from 'node:path';
import postgres, { Notice } from 'postgres';
import { parse } from 'pg-connection-string';
import { Notice } from 'postgres';
import { citiesFile, excludePaths, IWorker } from 'src/constants';
import { Telemetry } from 'src/decorators';
import { EnvDto } from 'src/dtos/env.dto';
Expand All @@ -20,6 +19,20 @@ import { QueueName } from 'src/interfaces/job.interface';
import { setDifference } from 'src/utils/set';
import { PostgresConnectionOptions } from 'typeorm/driver/postgres/PostgresConnectionOptions.js';

type Ssl = 'require' | 'allow' | 'prefer' | 'verify-full' | boolean | object;
type PostgresConnectionConfig = {
host?: string;
password?: string;
user?: string;
port?: number;
database?: string;
client_encoding?: string;
ssl?: Ssl;
application_name?: string;
fallback_application_name?: string;
options?: string;
};

export interface EnvData {
host?: string;
port: number;
Expand Down Expand Up @@ -53,7 +66,7 @@ export interface EnvData {
};

database: {
config: { typeorm: PostgresConnectionOptions & DatabaseConnectionParams; kysely: KyselyConfig };
config: { typeorm: PostgresConnectionOptions & DatabaseConnectionParams; kysely: PostgresConnectionConfig };
skipMigrations: boolean;
vectorExtension: VectorExtension;
};
Expand Down Expand Up @@ -124,6 +137,9 @@ const asSet = <T>(value: string | undefined, defaults: T[]) => {
return new Set(values.length === 0 ? defaults : (values as T[]));
};

const isValidSsl = (ssl?: string | boolean | object): ssl is Ssl =>
typeof ssl !== 'string' || ssl === 'require' || ssl === 'allow' || ssl === 'prefer' || ssl === 'verify-full';

const getEnv = (): EnvData => {
const dto = plainToInstance(EnvDto, process.env);
const errors = validateSync(dto);
Expand Down Expand Up @@ -185,6 +201,31 @@ const getEnv = (): EnvData => {
}
}

const parts = {
connectionType: 'parts',
host: dto.DB_HOSTNAME || 'database',
port: dto.DB_PORT || 5432,
username: dto.DB_USERNAME || 'postgres',
password: dto.DB_PASSWORD || 'postgres',
database: dto.DB_DATABASE_NAME || 'immich',
} as const;

let parsedOptions: PostgresConnectionConfig = parts;
if (dto.DB_URL) {
const parsed = parse(dto.DB_URL);
if (!isValidSsl(parsed.ssl)) {
throw new Error(`Invalid ssl option: ${parsed.ssl}`);
}

parsedOptions = {
...parsed,
ssl: parsed.ssl,
host: parsed.host ?? undefined,
port: parsed.port ? Number(parsed.port) : undefined,
database: parsed.database ?? undefined,
};
}

const driverOptions = {
onnotice: (notice: Notice) => {
if (notice['severity'] !== 'NOTICE') {
Expand All @@ -206,17 +247,9 @@ const getEnv = (): EnvData => {
serialize: (value: number) => value.toString(),
},
},
...parsedOptions,
};

const parts = {
connectionType: 'parts',
host: dto.DB_HOSTNAME || 'database',
port: dto.DB_PORT || 5432,
username: dto.DB_USERNAME || 'postgres',
password: dto.DB_PASSWORD || 'postgres',
database: dto.DB_DATABASE_NAME || 'immich',
} as const;

return {
host: dto.IMMICH_HOST,
port: dto.IMMICH_PORT || 2283,
Expand Down Expand Up @@ -282,21 +315,7 @@ const getEnv = (): EnvData => {
parseInt8: true,
...(databaseUrl ? { connectionType: 'url', url: databaseUrl } : parts),
},
kysely: {
dialect: new PostgresJSDialect({
postgres: databaseUrl ? postgres(databaseUrl, driverOptions) : postgres({ ...parts, ...driverOptions }),
}),
log(event) {
if (event.level === 'error') {
console.error('Query failed :', {
durationMs: event.queryDurationMillis,
error: event.error,
sql: event.query.sql,
params: event.query.parameters,
});
}
},
},
kysely: driverOptions,
},

skipMigrations: dto.DB_SKIP_MIGRATIONS ?? false,
Expand Down
22 changes: 15 additions & 7 deletions server/src/services/database.service.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { PostgresJSDialect } from 'kysely-postgres-js';
import {
DatabaseExtension,
EXTENSION_NAMES,
Expand Down Expand Up @@ -62,8 +61,11 @@ describe(DatabaseService.name, () => {
database: {
config: {
kysely: {
dialect: expect.any(PostgresJSDialect),
log: ['error'],
host: 'database',
port: 5432,
user: 'postgres',
password: 'postgres',
database: 'immich',
},
typeorm: {
connectionType: 'parts',
Expand Down Expand Up @@ -298,8 +300,11 @@ describe(DatabaseService.name, () => {
database: {
config: {
kysely: {
dialect: expect.any(PostgresJSDialect),
log: ['error'],
host: 'database',
port: 5432,
user: 'postgres',
password: 'postgres',
database: 'immich',
},
typeorm: {
connectionType: 'parts',
Expand Down Expand Up @@ -328,8 +333,11 @@ describe(DatabaseService.name, () => {
database: {
config: {
kysely: {
dialect: expect.any(PostgresJSDialect),
log: ['error'],
host: 'database',
port: 5432,
user: 'postgres',
password: 'postgres',
database: 'immich',
},
typeorm: {
connectionType: 'parts',
Expand Down
9 changes: 1 addition & 8 deletions server/test/repositories/config.repository.mock.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { PostgresJSDialect } from 'kysely-postgres-js';
import postgres from 'postgres';
import { ImmichEnvironment, ImmichWorker } from 'src/enum';
import { DatabaseExtension } from 'src/interfaces/database.interface';
import { EnvData } from 'src/repositories/config.repository';
Expand All @@ -24,12 +22,7 @@ const envData: EnvData = {

database: {
config: {
kysely: {
dialect: new PostgresJSDialect({
postgres: postgres({ database: 'immich', host: 'database', port: 5432 }),
}),
log: ['error'],
},
kysely: { database: 'immich', host: 'database', port: 5432 },
typeorm: {
connectionType: 'parts',
database: 'immich',
Expand Down

0 comments on commit ba01b40

Please sign in to comment.