Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
62533bd
#RI-4813 - [BE] Do not duplicate Cloud databases
egor-zalenski Sep 18, 2023
690fa44
#RI-4813 - [BE] Do not duplicate Cloud databases
egor-zalenski Sep 19, 2023
a0a15aa
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
026956a
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
5bf22f4
#RI-4813 - fix pr comments
egor-zalenski Sep 19, 2023
1789ff7
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
0d5e752
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
b515651
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
31e41a9
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
3cc9e58
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
089ef3d
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
30223a5
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
3700d8d
#RI-4813 - [FE] Do not duplicate Cloud databases
egor-zalenski Sep 19, 2023
65f379a
#RI-4813 - fix tests
egor-zalenski Sep 19, 2023
abd5e58
#RI-4813_remove_descripbe.only
egor-zalenski Sep 20, 2023
b1a36bc
Merge pull request #2600 from RedisInsight/be/feature/RI-4813_not_dup…
egor-zalenski Sep 20, 2023
3c30161
Merge pull request #2602 from RedisInsight/fe/feature/RI-4813_not_dup…
egor-zalenski Sep 20, 2023
723e727
* #RI-4975 - DB tries to connect if user added the db for the second …
egor-zalenski Sep 21, 2023
6561ebf
* #RI-4975 - DB tries to connect if user added the db for the second …
egor-zalenski Sep 21, 2023
45f1eea
Merge pull request #2611 from RedisInsight/fe/bugfix/RI-4813
egor-zalenski Sep 21, 2023
37e6c74
Merge branch 'main' into feature/RI-4813_not_duplicate_cloud_database
egor-zalenski Sep 21, 2023
2078d24
add test for RI-4813 Do not duplicate Cloud databases
mariasergeenko Sep 21, 2023
cbc5b8f
fix for cloudBdbId
mariasergeenko Sep 21, 2023
10decfb
fix for only
mariasergeenko Sep 21, 2023
fdf9eda
Merge pull request #2614 from RedisInsight/e2e/feature/RI-4813_not_du…
mariasergeenko Sep 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions redisinsight/api/src/constants/custom-error-codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ export enum CustomErrorCodes {
CloudTaskNotFound = 11_112,
CloudJobNotFound = 11_113,
CloudSubscriptionAlreadyExistsFree = 11_114,

// General database errors [11200, 11299]
DatabaseAlreadyExists = 11_200,
}
1 change: 1 addition & 0 deletions redisinsight/api/src/constants/error-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export default {
REDIS_CLOUD_FORBIDDEN: 'Error fetching account details.',

DATABASE_IS_INACTIVE: 'The database is inactive.',
DATABASE_ALREADY_EXISTS: 'The database already exists.',

INCORRECT_CLUSTER_CURSOR_FORMAT: 'Incorrect cluster cursor format.',
REMOVING_MULTIPLE_ELEMENTS_NOT_SUPPORT: () => 'Removing multiple elements is available for Redis databases v. 6.2 or later.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('DatabaseImportService', () => {
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
provider: 'RE_CLOUD',
new: true,
});
}, false);
});
it('should create standalone with created name', async () => {
await service['createDatabase']({
Expand All @@ -180,7 +180,7 @@ describe('DatabaseImportService', () => {
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
name: `${mockDatabase.host}:${mockDatabase.port}`,
new: true,
});
}, false);
});
it('should create standalone with none compressor', async () => {
await service['createDatabase']({
Expand All @@ -192,7 +192,7 @@ describe('DatabaseImportService', () => {
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
compressor: Compressor.NONE,
new: true,
});
}, false);
});
it('should create standalone with compressor', async () => {
await service['createDatabase']({
Expand All @@ -204,7 +204,7 @@ describe('DatabaseImportService', () => {
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
compressor: Compressor.GZIP,
new: true,
});
}, false);
});
it('should create cluster database', async () => {
await service['createDatabase']({
Expand All @@ -217,7 +217,7 @@ describe('DatabaseImportService', () => {
...pick(mockDatabase, ['host', 'port', 'name', 'timeout', 'compressor', 'modules']),
connectionType: ConnectionType.CLUSTER,
new: true,
});
}, false);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export class DatabaseImportService {

const database = classToClass(Database, dto);

await this.databaseRepository.create(database);
await this.databaseRepository.create(database, false);

return {
index,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class DatabaseController {
async create(
@Body() dto: CreateDatabaseDto,
): Promise<Database> {
return await this.service.create(dto);
return await this.service.create(dto, true);
}

@UseInterceptors(ClassSerializerInterceptor)
Expand Down
5 changes: 3 additions & 2 deletions redisinsight/api/src/modules/database/database.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,16 @@ export class DatabaseService {
/**
* Create new database with auto-detection of database type, modules, etc.
* @param dto
* @param uniqueCheck
*/
async create(dto: CreateDatabaseDto): Promise<Database> {
async create(dto: CreateDatabaseDto, uniqueCheck = false): Promise<Database> {
try {
this.logger.log('Creating new database.');

const database = await this.repository.create({
...await this.databaseFactory.createDatabaseModel(classToClass(Database, dto)),
new: true,
});
}, uniqueCheck);

// todo: clarify if we need this and if yes - rethink implementation
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { HttpException, HttpExceptionOptions, HttpStatus } from '@nestjs/common';
import { CustomErrorCodes } from 'src/constants';
import ERROR_MESSAGES from 'src/constants/error-messages';

export class DatabaseAlreadyExistsException extends HttpException {
constructor(databaseId: string, message = ERROR_MESSAGES.DATABASE_ALREADY_EXISTS, options?: HttpExceptionOptions) {
const response = {
message,
statusCode: HttpStatus.CONFLICT,
error: 'DatabaseAlreadyExists',
errorCode: CustomErrorCodes.DatabaseAlreadyExists,
resource: {
databaseId,
},
};

super(response, response.statusCode, options);
}
}
1 change: 1 addition & 0 deletions redisinsight/api/src/modules/database/exeptions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './database-already-exists.exception';
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ export abstract class DatabaseRepository {
/**
* Create database
* @param database
* @param uniqueCheck
*/
abstract create(database: Database): Promise<Database>;
abstract create(database: Database, uniqueCheck: boolean): Promise<Database>;

/**
* Update database with new data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import { CaCertificateRepository } from 'src/modules/certificate/repositories/ca
import { ClientCertificateRepository } from 'src/modules/certificate/repositories/client-certificate.repository';
import { cloneClassInstance } from 'src/utils';
import { SshOptionsEntity } from 'src/modules/ssh/entities/ssh-options.entity';
import ERROR_MESSAGES from 'src/constants/error-messages';
import { DatabaseAlreadyExistsException } from 'src/modules/database/exeptions';

const listFields = [
'id', 'name', 'host', 'port', 'db', 'timeout',
Expand Down Expand Up @@ -251,7 +253,7 @@ describe('LocalDatabaseRepository', () => {

describe('create', () => {
it('should create standalone database', async () => {
const result = await service.create(mockDatabase);
const result = await service.create(mockDatabase, false);

expect(result).toEqual(mockDatabase);
expect(caCertRepository.create).not.toHaveBeenCalled();
Expand All @@ -261,7 +263,7 @@ describe('LocalDatabaseRepository', () => {
it('should create standalone database with cloud details', async () => {
repository.save.mockResolvedValue(mockDatabaseEntityWithCloudDetails);

const result = await service.create(mockDatabaseWithCloudDetails);
const result = await service.create(mockDatabaseWithCloudDetails, false);

expect(result).toEqual(mockDatabaseWithCloudDetails);
expect(caCertRepository.create).not.toHaveBeenCalled();
Expand All @@ -271,7 +273,7 @@ describe('LocalDatabaseRepository', () => {
it('should create standalone database (with existing certificates)', async () => {
repository.save.mockResolvedValueOnce(mockDatabaseWithTlsAuthEntity);

const result = await service.create(mockDatabaseWithTlsAuth);
const result = await service.create(mockDatabaseWithTlsAuth, false);

expect(result).toEqual(mockDatabaseWithTlsAuth);
expect(caCertRepository.create).not.toHaveBeenCalled();
Expand All @@ -283,12 +285,26 @@ describe('LocalDatabaseRepository', () => {

const result = await service.create(
omit(cloneClassInstance(mockDatabaseWithTlsAuth), 'caCert.id', 'clientCert.id'),
false,
);

expect(result).toEqual(mockDatabaseWithTlsAuth);
expect(caCertRepository.create).toHaveBeenCalled();
expect(clientCertRepository.create).toHaveBeenCalled();
});

it('should throw an error if create called with cloud details and have the same entity', async () => {
repository.findOneBy.mockResolvedValueOnce(mockDatabaseEntity);
try {
await service.create(mockDatabaseEntityWithCloudDetails, true);
fail();
} catch (e) {
expect(e).toBeInstanceOf(DatabaseAlreadyExistsException);
expect(e.message).toEqual(ERROR_MESSAGES.DATABASE_ALREADY_EXISTS);
expect(e.response?.resource?.databaseId).toEqual(mockDatabaseEntity.id);
expect(repository.save).not.toHaveBeenCalled();
}
});
});

describe('update', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Injectable } from '@nestjs/common';
import { DatabaseRepository } from 'src/modules/database/repositories/database.repository';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { FindOptionsWhere, Repository } from 'typeorm';
import { get, set } from 'lodash';
import { DatabaseEntity } from 'src/modules/database/entities/database.entity';
import { Database } from 'src/modules/database/models/database';
import { classToClass } from 'src/utils';
Expand All @@ -10,13 +11,24 @@ import { ModelEncryptor } from 'src/modules/encryption/model.encryptor';
import { CaCertificateRepository } from 'src/modules/certificate/repositories/ca-certificate.repository';
import { ClientCertificateRepository } from 'src/modules/certificate/repositories/client-certificate.repository';
import { SshOptionsEntity } from 'src/modules/ssh/entities/ssh-options.entity';
import { DatabaseAlreadyExistsException } from 'src/modules/database/exeptions';

@Injectable()
export class LocalDatabaseRepository extends DatabaseRepository {
private readonly modelEncryptor: ModelEncryptor;

private readonly sshModelEncryptor: ModelEncryptor;

private uniqFieldsForCloudDatabase: string[] = [
'host',
'port',
'username',
'password',
'caCert.certificate',
'clientCert.certificate',
'clientCert.key',
];

constructor(
@InjectRepository(DatabaseEntity)
private readonly repository: Repository<DatabaseEntity>,
Expand Down Expand Up @@ -88,8 +100,12 @@ export class LocalDatabaseRepository extends DatabaseRepository {
/**
* Create database with encrypted sensitive fields
* @param database
* @param uniqueCheck
*/
public async create(database: Database): Promise<Database> {
public async create(database: Database, uniqueCheck: boolean): Promise<Database> {
if (uniqueCheck) {
await this.checkUniqueness(database);
}
const entity = classToClass(DatabaseEntity, await this.populateCertificates(database));
return classToClass(
Database,
Expand Down Expand Up @@ -205,4 +221,39 @@ export class LocalDatabaseRepository extends DatabaseRepository {

return decryptedEntity;
}

/**
* Check uniqueness of the database
* @param database
* @private
* @throws DatabaseAlreadyExistsException
*/
private async checkUniqueness(database: Database): Promise<void> {
// Do not create a connection if it triggered from cloud and have the same fields
if (database.cloudDetails?.cloudId) {
const entity = await this.encryptEntity(classToClass(DatabaseEntity, { ...database }));

if (entity.caCert) {
entity.caCert = await (new ModelEncryptor(this.encryptionService, [
'certificate',
])).encryptEntity(entity.caCert);
}

if (entity.clientCert) {
entity.clientCert = await (new ModelEncryptor(this.encryptionService, [
'certificate', 'key',
])).encryptEntity(entity.clientCert);
}

const query: FindOptionsWhere<DatabaseEntity> = {};
this.uniqFieldsForCloudDatabase.forEach((field) => {
set(query, field, get(entity, field));
});

const existingDatabase = await this.repository.findOneBy(query);
if (existingDatabase) {
throw new DatabaseAlreadyExistsException(existingDatabase.id);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class StackDatabasesRepository extends LocalDatabaseRepository implements
verifyServerCert: false,
connectionType: ConnectionType.STANDALONE,
lastConnection: null,
});
}, false);
}
this.logger.log(`Succeed to set predefined database ${id}`);
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class ModelEncryptor {
async encryptEntity<T>(entity: T): Promise<T> {
const encryptedEntity = cloneClassInstance(entity);

// TODO: implement support depth in field, 'obj.field'
await Promise.all(this.fields.map(async (field) => {
if (entity[field]) {
const { data, encryption } = await this.encryptionService.encrypt(entity[field]);
Expand Down
Loading