fix: use CLUSTER NODES hostname over raw ip for cluster discovery

redisinsight/api/src/__mocks__/redis-info.ts

@@ -104,16 +104,6 @@ export const mockRedisSentinelMasterResponse: Array<string[]> = [
   mockSentinelMasterInOkState,
 ];
 
-// eslint-disable-next-line max-len
-export const mockRedisClusterNodesResponse: string =
-  '07c37dfeb235213a872192d90877d0cd55635b91 127.0.0.1:30004@31004 slave e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 0 1426238317239 4 connected\n' +
-  'e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 127.0.0.1:30001@31001 myself,master - 0 0 1 connected 0-16383';
-
-// eslint-disable-next-line max-len
-export const mockRedisClusterNodesResponseIPv6: string =
-  '07c37dfeb235213a872192d90877d0cd55635b91 2001:db8::1:7001@17001 slave e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 0 1426238317239 4 connected\n' +
-  'e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 2001:db8::2:7002@17002 myself,master - 0 0 1 connected 0-16383';
-
 export const mockStandaloneRedisInfoReply: string = `${
   mockRedisServerInfoResponse
 }\r\n${mockRedisClientsInfoResponse}\r\n${mockRedisMemoryInfoResponse}\r\n${

redisinsight/api/src/models/redis-cluster.ts

@@ -15,15 +15,3 @@ export interface IRedisClusterNodeAddress {
   host: string;
   port: number;
 }
-
-export interface IRedisClusterNode extends IRedisClusterNodeAddress {
-  id: string;
-  replicaOf: string;
-  linkState: RedisClusterNodeLinkState;
-  slot: string;
-}
-
-export enum RedisClusterNodeLinkState {
-  Connected = 'connected',
-  Disconnected = 'disconnected',
-}

redisinsight/api/src/modules/cluster-monitor/strategies/__tests__/cluster-shards.factory.ts

@@ -34,17 +34,24 @@ export const toNodeReplyArray = (obj: Record<string, any>): any[] => {
 };
 
 export const clusterShardNodeRawFactory = Factory.define<ClusterShardNodeRaw>(
-  () => ({
-    id: faker.string.hexadecimal({ length: 40, prefix: '' }),
-    port: faker.number.int({ min: 6379, max: 6400 }),
-    ip: faker.internet.ipv4(),
-    endpoint: faker.internet.ipv4(),
-    hostname: '',
-    role: 'master',
-    'replication-offset': faker.number.int({ min: 0, max: 200000 }),
-    health: faker.helpers.arrayElement(Object.values(HealthStatus)),
-    'tls-port': undefined,
-  }),
+  () => {
+    const ip = faker.internet.ipv4();
+
+    return {
+      id: faker.string.hexadecimal({ length: 40, prefix: '' }),
+      port: faker.number.int({ min: 6379, max: 6400 }),
+      ip,
+      // `cluster-preferred-endpoint-type ip` is Redis's own default, so the
+      // preferred endpoint equals the ip unless a test explicitly overrides
+      // it to simulate a `hostname` or unknown-endpoint configuration.
+      endpoint: ip,
+      hostname: '',
+      role: 'master',
+      'replication-offset': faker.number.int({ min: 0, max: 200000 }),
+      health: faker.helpers.arrayElement(Object.values(HealthStatus)),
+      'tls-port': undefined,
+    };
+  },
 );
 
 export const clusterShardRawFactory = Factory.define<ClusterShardRaw>(

redisinsight/api/src/modules/cluster-monitor/strategies/cluster-shards.info.strategy.spec.ts

@@ -26,7 +26,7 @@ describe('ClusterShardsInfoStrategy', () => {
   });
 
   describe('getClusterNodesFromRedis', () => {
-    it('should return cluster info with ip when hostname is empty', async () => {
+    it('should use the ip when it is the preferred endpoint (cluster-preferred-endpoint-type ip, the default)', async () => {
       const node1 = clusterShardNodeRawFactory.build();
       const node2 = clusterShardNodeRawFactory.build();
       const shard1 = clusterShardRawFactory.build(
@@ -51,18 +51,21 @@ describe('ClusterShardsInfoStrategy', () => {
       expect(info[1].slots).toEqual([formatSlotRange(5001, 16383)]);
     });
 
-    it('should use hostname when cluster-announce-hostname is configured', async () => {
+    it('should use the endpoint when the preferred endpoint type is hostname', async () => {
       const hostname1 = 'redis-node-1.example.com';
       const hostname2 = 'redis-node-2.example.com';
       const master1 = clusterShardNodeRawFactory.build({
         hostname: hostname1,
+        endpoint: hostname1,
       });
       const replica1 = clusterShardNodeRawFactory.build({
         hostname: hostname1,
+        endpoint: hostname1,
         role: 'slave',
       });
       const master2 = clusterShardNodeRawFactory.build({
         hostname: hostname2,
+        endpoint: hostname2,
       });
 
       const shard1 = clusterShardRawFactory.build(
@@ -89,10 +92,31 @@ describe('ClusterShardsInfoStrategy', () => {
       expect(info[1].host).toBe(hostname2);
     });
 
+    it('should use the ip, not an announced hostname, when the preferred endpoint type is ip', async () => {
+      // Regression test for https://github.com/redis/RedisInsight/pull/6180#discussion_r3546234089:
+      // a node may announce a hostname purely as metadata while
+      // cluster-preferred-endpoint-type still resolves to ip.
+      const node = clusterShardNodeRawFactory.build({
+        hostname: 'redis-node-1.example.com',
+      });
+      const shard = clusterShardRawFactory.build(
+        {},
+        { transient: { slotStart: 0, slotEnd: 16383, nodes: [node] } },
+      );
+      const reply = [shard].map(toClusterShardsReplyEntry);
+      when(clusterClient.sendCommand).mockResolvedValue(reply);
+
+      const info = await service.getClusterNodesFromRedis(clusterClient);
+
+      expect(info).toHaveLength(1);
+      expect(info[0].host).toBe(node.ip);
+    });
+
     it('should use tls-port when regular port is not available', async () => {
       const tlsPort = 6380;
       const node = clusterShardNodeRawFactory.build({
         hostname: 'redis-tls.example.com',
+        endpoint: 'redis-tls.example.com',
         port: undefined,
         'tls-port': tlsPort,
       });
@@ -114,7 +138,7 @@ describe('ClusterShardsInfoStrategy', () => {
   describe('processShardNodes', () => {
     const slots = ['0-5000'];
 
-    it('should use ip as host when hostname is empty string', () => {
+    it('should use ip as host when it is the preferred endpoint (default)', () => {
       const raw = clusterShardNodeRawFactory.build();
 
       const result = ClusterShardsInfoStrategy.processShardNodes(
@@ -125,9 +149,12 @@ describe('ClusterShardsInfoStrategy', () => {
       expect(result[0]).toHaveProperty('ip', raw.ip);
     });
 
-    it('should prefer hostname over ip when hostname is set', () => {
+    it('should use the endpoint when the preferred endpoint type is hostname', () => {
       const hostname = 'redis.example.com';
-      const raw = clusterShardNodeRawFactory.build({ hostname });
+      const raw = clusterShardNodeRawFactory.build({
+        hostname,
+        endpoint: hostname,
+      });
 
       const result = ClusterShardsInfoStrategy.processShardNodes(
         [toNodeReplyArray(raw)],
@@ -137,7 +164,19 @@ describe('ClusterShardsInfoStrategy', () => {
       expect(result[0]).toHaveProperty('ip', raw.ip);
     });
 
-    it('should fall back to ip when hostname is not present in reply', () => {
+    it('should use ip, not an announced hostname, when the preferred endpoint type is ip', () => {
+      // Regression test for https://github.com/redis/RedisInsight/pull/6180#discussion_r3546234089
+      const hostname = 'redis.example.com';
+      const raw = clusterShardNodeRawFactory.build({ hostname });
+
+      const result = ClusterShardsInfoStrategy.processShardNodes(
+        [toNodeReplyArray(raw)],
+        slots,
+      );
+      expect(result[0].host).toBe(raw.ip);
+    });
+
+    it('should fall back to ip when endpoint is not present in reply', () => {
       const ip = '10.0.0.99';
       const shardNodes = [
         [
@@ -162,6 +201,51 @@ describe('ClusterShardsInfoStrategy', () => {
       expect(result[0]).toHaveProperty('ip', ip);
     });
 
+    it('should fall back to ip when endpoint is an empty string', () => {
+      const raw = clusterShardNodeRawFactory.build({ endpoint: '' });
+
+      const result = ClusterShardsInfoStrategy.processShardNodes(
+        [toNodeReplyArray(raw)],
+        slots,
+      );
+      expect(result[0].host).toBe(raw.ip);
+    });
+
+    it('should use fallbackHost when endpoint is null (unknown endpoint) and no ip is present', () => {
+      const shardNodes = [
+        [
+          'id',
+          'n1',
+          'port',
+          6379,
+          'endpoint',
+          null,
+          'role',
+          'master',
+          'health',
+          'online',
+        ],
+      ];
+
+      const result = ClusterShardsInfoStrategy.processShardNodes(
+        shardNodes,
+        slots,
+        '203.0.113.10',
+      );
+      expect(result[0].host).toBe('203.0.113.10');
+    });
+
+    it('should fall back to ip when endpoint is the "?" misconfigured marker', () => {
+      const raw = clusterShardNodeRawFactory.build({ endpoint: '?' });
+
+      const result = ClusterShardsInfoStrategy.processShardNodes(
+        [toNodeReplyArray(raw)],
+        slots,
+        '203.0.113.10', // must be ignored - '?' is not necessarily this node
+      );
+      expect(result[0].host).toBe(raw.ip);
+    });
+
     it('should use tls-port when port is missing', () => {
       const tlsPort = 6380;
       const raw = clusterShardNodeRawFactory.build({
@@ -230,11 +314,13 @@ describe('ClusterShardsInfoStrategy', () => {
       const tlsPort = 6390;
       const primary = clusterShardNodeRawFactory.build({
         hostname,
+        endpoint: hostname,
         port: undefined,
         'tls-port': tlsPort,
       });
       const replica = clusterShardNodeRawFactory.build({
         hostname,
+        endpoint: hostname,
         port: undefined,
         'tls-port': tlsPort + 1,
         role: 'slave',

redisinsight/api/src/modules/cluster-monitor/strategies/cluster-shards.info.strategy.ts

@@ -4,7 +4,10 @@ import {
   ClusterNodeDetails,
   NodeRole,
 } from 'src/modules/cluster-monitor/models';
-import { convertArrayReplyToObject } from 'src/modules/redis/utils';
+import {
+  convertArrayReplyToObject,
+  resolvePreferredEndpoint,
+} from 'src/modules/redis/utils';
 import { RedisClient } from 'src/modules/redis/client';
 
 export class ClusterShardsInfoStrategy extends AbstractInfoStrategy {
@@ -17,7 +20,11 @@ export class ClusterShardsInfoStrategy extends AbstractInfoStrategy {
       ...resp.map((shardArray) => {
         const shard = convertArrayReplyToObject(shardArray);
         const slots = ClusterShardsInfoStrategy.calculateSlots(shard.slots);
-        return ClusterShardsInfoStrategy.processShardNodes(shard.nodes, slots);
+        return ClusterShardsInfoStrategy.processShardNodes(
+          shard.nodes,
+          slots,
+          client.options?.host,
+        );
       }),
     );
   }
@@ -35,13 +42,21 @@ export class ClusterShardsInfoStrategy extends AbstractInfoStrategy {
   static processShardNodes(
     shardNodes: any[],
     slots: string[],
+    fallbackHost?: string,
   ): Partial<ClusterNodeDetails>[] {
     let primary;
     const nodes = shardNodes.map((nodeArray) => {
       const nodeObj = convertArrayReplyToObject(nodeArray);
       const node = {
         id: nodeObj.id,
-        host: nodeObj.hostname || nodeObj.ip,
+        // `endpoint` is CLUSTER SHARDS' server-resolved preferred address
+        // (respects `cluster-preferred-endpoint-type`) - `hostname` alone is
+        // only supplementary metadata and must not drive this on its own.
+        // Fall back to `ip` for an unknown/misconfigured endpoint since this
+        // is a display-only value, not a connection target.
+        host:
+          resolvePreferredEndpoint(nodeObj.endpoint, fallbackHost) ||
+          nodeObj.ip,
         ip: nodeObj.ip,
         port: nodeObj.port || nodeObj['tls-port'],
         tlsPort: nodeObj['tls-port'],