Make host/port editable for non-managed databases

redisinsight/api/src/constants/error-messages.ts

@@ -27,6 +27,8 @@ export default {
   UNDEFINED_INSTANCE_ID: 'Undefined redis database instance id.',
   NO_CONNECTION_TO_REDIS_DB: 'No connection to the Redis Database.',
   WRONG_DATABASE_TYPE: 'Wrong database type.',
+  HOST_PORT_NOT_EDITABLE_FOR_MANAGED_DATABASE:
+    'Host and port cannot be changed for a database managed by a cloud provider.',
   CONNECTION_TIMEOUT:
     'The connection has timed out, please check the connection details.',
   DB_CONNECTION_TIMEOUT:

redisinsight/api/src/modules/database/database.service.spec.ts

@@ -1,4 +1,5 @@
 import {
+  BadRequestException,
   InternalServerErrorException,
   NotFoundException,
 } from '@nestjs/common';
@@ -361,6 +362,163 @@ describe('DatabaseService', () => {
         ),
       ).rejects.toThrow(NotFoundException);
     });
+
+    describe('managed databases endpoint guard', () => {
+      it('should throw BadRequest when changing host of a cloud-managed database', async () => {
+        databaseRepository.get.mockResolvedValueOnce(
+          mockDatabaseWithCloudDetails,
+        );
+
+        await expect(
+          service.update(
+            mockSessionMetadata,
+            mockDatabase.id,
+            classToClass(UpdateDatabaseDto, { host: 'new-host' }),
+            true,
+          ),
+        ).rejects.toThrow(
+          new BadRequestException(
+            ERROR_MESSAGES.HOST_PORT_NOT_EDITABLE_FOR_MANAGED_DATABASE,
+          ),
+        );
+        expect(databaseRepository.update).not.toHaveBeenCalled();
+      });
+
+      it('should throw BadRequest when changing port of an Azure-managed database', async () => {
+        databaseRepository.get.mockResolvedValueOnce(
+          mockDatabaseWithProviderDetails,
+        );
+
+        await expect(
+          service.update(
+            mockSessionMetadata,
+            mockDatabase.id,
+            classToClass(UpdateDatabaseDto, { port: 6380 }),
+            true,
+          ),
+        ).rejects.toThrow(
+          new BadRequestException(
+            ERROR_MESSAGES.HOST_PORT_NOT_EDITABLE_FOR_MANAGED_DATABASE,
+          ),
+        );
+        expect(databaseRepository.update).not.toHaveBeenCalled();
+      });
+
+      it('should allow updating other fields of a managed database when the endpoint is unchanged', async () => {
+        databaseRepository.get.mockResolvedValueOnce(
+          mockDatabaseWithCloudDetails,
+        );
+        databaseRepository.update.mockReturnValue({
+          ...mockDatabaseWithCloudDetails,
+          name: 'new-name',
+        });
+
+        await service.update(
+          mockSessionMetadata,
+          mockDatabase.id,
+          classToClass(UpdateDatabaseDto, {
+            name: 'new-name',
+            host: mockDatabaseWithCloudDetails.host,
+            port: mockDatabaseWithCloudDetails.port,
+          }),
+          true,
+        );
+
+        expect(databaseRepository.update).toHaveBeenCalled();
+      });
+
+      it('should allow changing host of a non-managed database', async () => {
+        databaseRepository.update.mockReturnValue({
+          ...mockDatabase,
+          host: 'new-host',
+        });
+
+        await service.update(
+          mockSessionMetadata,
+          mockDatabase.id,
+          classToClass(UpdateDatabaseDto, { host: 'new-host' }),
+          true,
+        );
+
+        expect(databaseRepository.update).toHaveBeenCalled();
+      });
+    });
+
+    describe('endpoint name sync', () => {
+      const HOST = '127.0.100.1';
+      const PORT = 6379;
+
+      // Fresh object per call: merge mutates the returned database, and the
+      // shared mockDatabase can be mutated by other update tests.
+      const defaultNamedDatabase = () => ({
+        ...mockDatabase,
+        host: HOST,
+        port: PORT,
+        name: `${HOST}:${PORT}`,
+      });
+
+      it('should sync the name to the new endpoint when the name was the default host:port', async () => {
+        databaseRepository.get.mockResolvedValueOnce(defaultNamedDatabase());
+
+        await service.update(
+          mockSessionMetadata,
+          mockDatabase.id,
+          classToClass(UpdateDatabaseDto, { host: 'new-host' }),
+          true,
+        );
+
+        expect(databaseFactory.createDatabaseModel).toHaveBeenCalledWith(
+          mockSessionMetadata,
+          expect.objectContaining({
+            host: 'new-host',
+            name: `new-host:${PORT}`,
+          }),
+        );
+      });
+
+      it('should not override an explicit name provided in the update', async () => {
+        databaseRepository.get.mockResolvedValueOnce(defaultNamedDatabase());
+
+        await service.update(
+          mockSessionMetadata,
+          mockDatabase.id,
+          classToClass(UpdateDatabaseDto, {
+            host: 'new-host',
+            name: 'custom-name',
+          }),
+          true,
+        );
+
+        expect(databaseFactory.createDatabaseModel).toHaveBeenCalledWith(
+          mockSessionMetadata,
+          expect.objectContaining({ host: 'new-host', name: 'custom-name' }),
+        );
+      });
+
+      it('should not change a custom name when the endpoint changes', async () => {
+        databaseRepository.get.mockResolvedValueOnce({
+          ...mockDatabase,
+          host: HOST,
+          port: PORT,
+          name: 'my-custom-alias',
+        });
+
+        await service.update(
+          mockSessionMetadata,
+          mockDatabase.id,
+          classToClass(UpdateDatabaseDto, { host: 'new-host' }),
+          true,
+        );
+
+        expect(databaseFactory.createDatabaseModel).toHaveBeenCalledWith(
+          mockSessionMetadata,
+          expect.objectContaining({
+            host: 'new-host',
+            name: 'my-custom-alias',
+          }),
+        );
+      });
+    });
   });
 
   describe('test', () => {

redisinsight/api/src/modules/database/database.service.ts

@@ -1,4 +1,5 @@
 import {
+  BadRequestException,
   Injectable,
   InternalServerErrorException,
   Logger,
@@ -85,6 +86,39 @@
     );
   }
 
+  /**
+   * Checks whether the endpoint (host/port) in the dto differs from the stored one.
+   * Unlike isEndpointAffected, this compares values so an unchanged host/port
+   * present in the payload is not treated as a change.
+   */
+  static isEndpointChanged(
+    dto: UpdateDatabaseDto,
+    database: Database,
+  ): boolean {
+    return (
+      (dto.host !== undefined && dto.host !== database.host) ||
+      (dto.port !== undefined && dto.port !== database.port)
+    );
+  }
+
+  /**
+   * A database is considered managed when its endpoint is owned by a cloud
+   * provider (Redis Cloud subscription or Azure). For such databases the
+   * host/port are tied to provider metadata (cloudDetails/providerDetails) that
+   * would become stale if the endpoint were edited manually.
+   */
+  static isManagedDatabase(database: Database): boolean {
+    return !!database.cloudDetails?.cloudId || !!database.providerDetails;
+  }
+
+  /**
+   * Whether the database name is still the default "host:port" derived from its
+   * current endpoint (i.e. the user never set a custom alias).
+   */
+  static hasDefaultEndpointName(database: Database): boolean {
+    return database.name === `${database.host}:${database.port}`;
+  }
+
   private async merge(
     database: Database,
     dto: UpdateDatabaseDto,
@@ -246,10 +280,34 @@
     this.logger.debug(`Updating database: ${id}`, sessionMetadata);
     const oldDatabase = await this.get(sessionMetadata, id, true);
 
+    if (
+      DatabaseService.isEndpointChanged(dto, oldDatabase) &&
+      DatabaseService.isManagedDatabase(oldDatabase)
+    ) {
+      throw new BadRequestException(
+        ERROR_MESSAGES.HOST_PORT_NOT_EDITABLE_FOR_MANAGED_DATABASE,
+      );
+    }
+
+    // When the name is still the default "host:port" and the endpoint changes,
+    // keep the name in sync with the new endpoint. Computed before merge (which
+    // mutates oldDatabase) and skipped when the caller sets a name explicitly or
+    // the user has a custom alias.
+    const syncedName =
+      dto.name === undefined &&
+      DatabaseService.isEndpointChanged(dto, oldDatabase) &&
+      DatabaseService.hasDefaultEndpointName(oldDatabase)
+        ? `${dto.host ?? oldDatabase.host}:${dto.port ?? oldDatabase.port}`
+        : undefined;
+
     let database: Database;
     try {
       database = await this.merge(oldDatabase, dto);
 
+      if (syncedName !== undefined) {
+        database.name = syncedName;
+      }
+
       if (DatabaseService.isConnectionAffected(dto)) {
         if (DatabaseService.isEndpointAffected(dto)) {
           database.provider = undefined;

redisinsight/api/test/api/database/PATCH-databases-id.test.ts

@@ -9,6 +9,7 @@ import {
   _,
   it,
   validateApiCall,
+  before,
   after,
 } from '../deps';
 import { Joi } from '../../helpers/test';
@@ -253,6 +254,95 @@ describe(`PATCH /databases/:id`, () => {
       },
     ].map(mainCheckFn);
   });
+  describe('Managed databases (cloud) endpoint guard', () => {
+    const managedHostPortMessage =
+      'Host and port cannot be changed for a database managed by a cloud provider.';
+    // Dedicated managed instance so we never mutate the shared TEST_INSTANCE_ID
+    // (its cloudDetails would otherwise leak into later tests).
+    const MANAGED_ID = 'cloud0000-0000-4000-8000-managed000001';
+    const managedEndpoint = () => endpoint(MANAGED_ID);
+    const managedName = constants.getRandomString();
+
+    // Seed once directly via the repository: cloudDetails marks the database as
+    // managed, and the guard rejects endpoint changes before any connection, so
+    // no real connectivity is required.
+    before(async () => {
+      const rep = await localDb.getRepository(localDb.repositories.DATABASE);
+      await rep.save({
+        id: MANAGED_ID,
+        name: 'cloud-managed-db',
+        host: constants.TEST_REDIS_HOST,
+        port: constants.TEST_REDIS_PORT,
+        connectionType: constants.STANDALONE,
+        tls: false,
+        verifyServerCert: false,
+        modules: '[]',
+        version: '7.0',
+        cloudDetails: {
+          cloudId: constants.TEST_CLOUD_ID,
+          subscriptionType: 'fixed',
+        },
+      });
+    });
+
+    after(async () => {
+      const rep = await localDb.getRepository(localDb.repositories.DATABASE);
+      await rep.delete(MANAGED_ID);
+    });
+
+    [
+      {
+        name: 'Should reject host change for a cloud-managed database',
+        endpoint: managedEndpoint,
+        data: {
+          host: constants.getRandomString(),
+        },
+        statusCode: 400,
+        responseBody: {
+          statusCode: 400,
+          error: 'Bad Request',
+          message: managedHostPortMessage,
+        },
+        after: async () => {
+          // endpoint must remain unchanged
+          const db = await localDb.getInstanceById(MANAGED_ID);
+          expect(db?.host).to.eq(constants.TEST_REDIS_HOST);
+          expect(db?.port).to.eq(constants.TEST_REDIS_PORT);
+        },
+      },
+      {
+        name: 'Should reject port change for a cloud-managed database',
+        endpoint: managedEndpoint,
+        data: {
+          port: 1234,
+        },
+        statusCode: 400,
+        responseBody: {
+          statusCode: 400,
+          error: 'Bad Request',
+          message: managedHostPortMessage,
+        },
+        after: async () => {
+          const db = await localDb.getInstanceById(MANAGED_ID);
+          expect(db?.port).to.eq(constants.TEST_REDIS_PORT);
+        },
+      },
+      {
+        name: 'Should allow non-endpoint change (name) for a cloud-managed database',
+        endpoint: managedEndpoint,
+        data: {
+          name: managedName,
+        },
+        responseBody: {
+          name: managedName,
+        },
+        after: async () => {
+          const db = await localDb.getInstanceById(MANAGED_ID);
+          expect(db?.name).to.eq(managedName);
+        },
+      },
+    ].map(mainCheckFn);
+  });
   describe('TAGS', () => {
     const newTagsDto1 = [
       {