[Remote clusters] Fix handling of remote clusters with deprecated pro…

…xy setting (elastic#61126) (elastic#61335)

x-pack/plugins/remote_clusters/common/lib/cluster_serialization.test.ts

@@ -124,6 +124,7 @@ describe('cluster_serialization', () => {
         skipUnavailable: false,
         transportPingSchedule: '-1',
         transportCompress: false,
+        serverName: 'localhost',
       });
     });
 
@@ -155,6 +156,38 @@ describe('cluster_serialization', () => {
       expect(() => serializeCluster('foo')).toThrowError();
     });
 
+    it('should serialize a cluster that has a deprecated proxy setting', () => {
+      expect(
+        serializeCluster({
+          name: 'test_cluster',
+          proxyAddress: 'localhost:9300',
+          mode: 'proxy',
+          isConnected: true,
+          skipUnavailable: false,
+          proxySocketConnections: 18,
+          serverName: 'localhost',
+          hasDeprecatedProxySetting: true,
+        })
+      ).toEqual({
+        persistent: {
+          cluster: {
+            remote: {
+              test_cluster: {
+                mode: 'proxy',
+                proxy_socket_connections: 18,
+                proxy_address: 'localhost:9300',
+                skip_unavailable: false,
+                server_name: 'localhost',
+                proxy: null,
+                seeds: null,
+                node_connections: null,
+              },
+            },
+          },
+        },
+      });
+    });
+
     it('should serialize a complete cluster object to only dynamic properties', () => {
       expect(
         serializeCluster({
@@ -167,13 +200,14 @@ describe('cluster_serialization', () => {
           skipUnavailable: false,
           transportPingSchedule: '-1',
           transportCompress: false,
+          mode: 'sniff',
         })
       ).toEqual({
         persistent: {
           cluster: {
             remote: {
               test_cluster: {
-                mode: null,
+                mode: 'sniff',
                 node_connections: null,
                 proxy_address: null,
                 proxy_socket_connections: null,

x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts

@@ -6,7 +6,8 @@
 
 import { PROXY_MODE } from '../constants';
 
-export interface ClusterEs {
+// Values returned from ES GET /_remote/info
+export interface ClusterInfoEs {
   seeds?: string[];
   mode?: 'proxy' | 'sniff';
   connected?: boolean;
@@ -42,27 +43,31 @@ export interface Cluster {
   connectedSocketsCount?: number;
   hasDeprecatedProxySetting?: boolean;
 }
-export interface ClusterPayload {
+
+interface ClusterPayloadEs {
+  skip_unavailable?: boolean | null;
+  mode?: 'sniff' | 'proxy' | null;
+  proxy_address?: string | null;
+  proxy_socket_connections?: number | null;
+  server_name?: string | null;
+  seeds?: string[] | null;
+  node_connections?: number | null;
+  proxy?: null;
+}
+// Payload expected from ES PUT /_cluster/settings
+export interface ClusterSettingsPayloadEs {
   persistent: {
     cluster: {
       remote: {
-        [key: string]: {
-          skip_unavailable?: boolean | null;
-          mode?: 'sniff' | 'proxy' | null;
-          proxy_address?: string | null;
-          proxy_socket_connections?: number | null;
-          server_name?: string | null;
-          seeds?: string[] | null;
-          node_connections?: number | null;
-        };
+        [key: string]: ClusterPayloadEs;
       };
     };
   };
 }
 
 export function deserializeCluster(
   name: string,
-  esClusterObject: ClusterEs,
+  esClusterObject: ClusterInfoEs,
   deprecatedProxyAddress?: string | undefined
 ): Cluster {
   if (!name || !esClusterObject || typeof esClusterObject !== 'object') {
@@ -112,12 +117,16 @@ export function deserializeCluster(
   // If a user has a remote cluster with the deprecated proxy setting,
   // we transform the data to support the new implementation and also flag the deprecation
   if (deprecatedProxyAddress) {
+    // Create server name (address, without port), since field doesn't exist in deprecated implementation
+    const defaultServerName = deprecatedProxyAddress.split(':')[0];
+
     deserializedClusterObject = {
       ...deserializedClusterObject,
       proxyAddress: deprecatedProxyAddress,
       seeds: undefined,
       hasDeprecatedProxySetting: true,
       mode: PROXY_MODE,
+      serverName: defaultServerName,
     };
   }
 
@@ -131,7 +140,7 @@ export function deserializeCluster(
   return deserializedClusterObject;
 }
 
-export function serializeCluster(deserializedClusterObject: Cluster): ClusterPayload {
+export function serializeCluster(deserializedClusterObject: Cluster): ClusterSettingsPayloadEs {
   if (!deserializedClusterObject || typeof deserializedClusterObject !== 'object') {
     throw new Error('Unable to serialize cluster');
   }
@@ -145,22 +154,31 @@ export function serializeCluster(deserializedClusterObject: Cluster): ClusterPay
     proxyAddress,
     proxySocketConnections,
     serverName,
+    hasDeprecatedProxySetting,
   } = deserializedClusterObject;
 
+  const clusterData: ClusterPayloadEs = {
+    skip_unavailable: typeof skipUnavailable === 'boolean' ? skipUnavailable : null,
+    mode: mode || null,
+    proxy_address: proxyAddress || null,
+    proxy_socket_connections: proxySocketConnections || null,
+    server_name: serverName || null,
+    seeds: seeds || null,
+    node_connections: nodeConnections || null,
+  };
+
+  // This is only applicable in edit mode
+  // In order to "upgrade" an existing remote cluster to use the new proxy mode settings, we need to set the old proxy setting to null
+  if (hasDeprecatedProxySetting) {
+    clusterData.proxy = null;
+  }
+
   return {
     // Background on why we only save as persistent settings detailed here: https://github.com/elastic/kibana/pull/26067#issuecomment-441848124
     persistent: {
       cluster: {
         remote: {
-          [name]: {
-            skip_unavailable: typeof skipUnavailable === 'boolean' ? skipUnavailable : null,
-            mode: mode || null,
-            proxy_address: proxyAddress || null,
-            proxy_socket_connections: proxySocketConnections || null,
-            server_name: serverName || null,
-            seeds: seeds || null,
-            node_connections: nodeConnections || null,
-          },
+          [name]: clusterData,
         },
       },
     },

x-pack/plugins/remote_clusters/common/lib/index.ts

@@ -4,4 +4,9 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { deserializeCluster, serializeCluster, Cluster, ClusterEs } from './cluster_serialization';
+export {
+  deserializeCluster,
+  serializeCluster,
+  Cluster,
+  ClusterInfoEs,
+} from './cluster_serialization';

x-pack/plugins/remote_clusters/public/application/sections/components/remote_cluster_form/remote_cluster_form.js

@@ -135,6 +135,7 @@ export class RemoteClusterForm extends Component {
         skipUnavailable,
       },
     } = this.state;
+    const { fields } = this.props;
 
     let modeSettings;
 
@@ -155,6 +156,7 @@ export class RemoteClusterForm extends Component {
       name,
       skipUnavailable,
       mode,
+      hasDeprecatedProxySetting: fields.hasDeprecatedProxySetting,
       ...modeSettings,
     };
   }

x-pack/plugins/remote_clusters/server/routes/api/delete_route.test.ts

@@ -84,6 +84,20 @@ describe('DELETE remote clusters', () => {
   describe('success', () => {
     deleteRemoteClustersTest('deletes remote cluster', {
       apiResponses: [
+        async () => ({
+          persistent: {
+            cluster: {
+              remote: {
+                test: {
+                  seeds: ['127.0.0.1:9300'],
+                  skip_unavailable: false,
+                  mode: 'sniff',
+                },
+              },
+            },
+          },
+          transient: {},
+        }),
         async () => ({
           test: {
             connected: true,
@@ -143,7 +157,17 @@ describe('DELETE remote clusters', () => {
     deleteRemoteClustersTest(
       'returns errors array with 404 error if remote cluster does not exist',
       {
-        apiResponses: [async () => ({})],
+        apiResponses: [
+          async () => ({
+            persistent: {
+              cluster: {
+                remote: {},
+              },
+            },
+            transient: {},
+          }),
+          async () => ({}),
+        ],
         params: {
           nameOrNames: 'test',
         },
@@ -178,6 +202,20 @@ describe('DELETE remote clusters', () => {
       'returns errors array with 400 error if ES still returns cluster information',
       {
         apiResponses: [
+          async () => ({
+            persistent: {
+              cluster: {
+                remote: {
+                  test: {
+                    seeds: ['127.0.0.1:9300'],
+                    skip_unavailable: false,
+                    mode: 'sniff',
+                  },
+                },
+              },
+            },
+            transient: {},
+          }),
           async () => ({
             test: {
               connected: true,

x-pack/plugins/remote_clusters/server/routes/api/delete_route.ts

@@ -37,6 +37,8 @@ export const register = (deps: RouteDependencies): void => {
       const itemsDeleted: any[] = [];
       const errors: any[] = [];
 
+      const clusterSettings = await callAsCurrentUser('cluster.getSettings');
+
       // Validator that returns an error if the remote cluster does not exist.
       const validateClusterDoesExist = async (name: string) => {
         try {
@@ -60,9 +62,12 @@ export const register = (deps: RouteDependencies): void => {
       };
 
       // Send the request to delete the cluster and return an error if it could not be deleted.
-      const sendRequestToDeleteCluster = async (name: string) => {
+      const sendRequestToDeleteCluster = async (
+        name: string,
+        hasDeprecatedProxySetting: boolean
+      ) => {
         try {
-          const body = serializeCluster({ name });
+          const body = serializeCluster({ name, hasDeprecatedProxySetting });
           const updateClusterResponse = await callAsCurrentUser('cluster.putSettings', { body });
           const acknowledged = get(updateClusterResponse, 'acknowledged');
           const cluster = get(updateClusterResponse, `persistent.cluster.remote.${name}`);
@@ -98,8 +103,12 @@ export const register = (deps: RouteDependencies): void => {
         let error: any = await validateClusterDoesExist(clusterName);
 
         if (!error) {
+          // Check if cluster contains deprecated proxy setting
+          const hasDeprecatedProxySetting = Boolean(
+            get(clusterSettings, `persistent.cluster.remote[${clusterName}].proxy`, undefined)
+          );
           // Delete the cluster.
-          error = await sendRequestToDeleteCluster(clusterName);
+          error = await sendRequestToDeleteCluster(clusterName, hasDeprecatedProxySetting);
         }
 
         if (error) {

x-pack/plugins/remote_clusters/server/routes/api/update_route.ts

@@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n';
 import { RequestHandler } from 'src/core/server';
 
 import { API_BASE_PATH, SNIFF_MODE, PROXY_MODE } from '../../../common/constants';
-import { serializeCluster, deserializeCluster, Cluster, ClusterEs } from '../../../common/lib';
+import { serializeCluster, deserializeCluster, Cluster, ClusterInfoEs } from '../../../common/lib';
 import { doesClusterExist } from '../../lib/does_cluster_exist';
 import { RouteDependencies } from '../../types';
 import { licensePreRoutingFactory } from '../../lib/license_pre_routing_factory';
@@ -68,7 +68,10 @@ export const register = (deps: RouteDependencies): void => {
       });
 
       const acknowledged = get(updateClusterResponse, 'acknowledged');
-      const cluster = get(updateClusterResponse, `persistent.cluster.remote.${name}`) as ClusterEs;
+      const cluster = get(
+        updateClusterResponse,
+        `persistent.cluster.remote.${name}`
+      ) as ClusterInfoEs;
 
       if (acknowledged && cluster) {
         const body = {