From 008d3944c00b309a86f5bd2cf8e2b3c2fb461003 Mon Sep 17 00:00:00 2001 From: Alison Goryachev Date: Wed, 25 Mar 2020 14:33:33 -0400 Subject: [PATCH] [Remote clusters] Fix handling of remote clusters with deprecated proxy setting (#61126) --- .../common/lib/cluster_serialization.test.ts | 36 ++++++++++- .../common/lib/cluster_serialization.ts | 62 ++++++++++++------- .../remote_clusters/common/lib/index.ts | 7 ++- .../remote_cluster_form.js | 2 + .../server/routes/api/delete_route.test.ts | 40 +++++++++++- .../server/routes/api/delete_route.ts | 15 ++++- .../server/routes/api/update_route.ts | 7 ++- 7 files changed, 139 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.test.ts b/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.test.ts index 10b3dbbd9b452..7fd8b4a894989 100644 --- a/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.test.ts +++ b/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, diff --git a/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts b/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts index d0898fda93a41..3d8ffa13b8218 100644 --- a/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts +++ b/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,19 +43,23 @@ 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; }; }; }; @@ -62,7 +67,7 @@ export interface ClusterPayload { 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, }, }, }, diff --git a/x-pack/plugins/remote_clusters/common/lib/index.ts b/x-pack/plugins/remote_clusters/common/lib/index.ts index 52a0536bfd55b..43cf004becff9 100644 --- a/x-pack/plugins/remote_clusters/common/lib/index.ts +++ b/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'; diff --git a/x-pack/plugins/remote_clusters/public/application/sections/components/remote_cluster_form/remote_cluster_form.js b/x-pack/plugins/remote_clusters/public/application/sections/components/remote_cluster_form/remote_cluster_form.js index 94d6ca4ebb648..7d3ed0708ae54 100644 --- a/x-pack/plugins/remote_clusters/public/application/sections/components/remote_cluster_form/remote_cluster_form.js +++ b/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, }; } diff --git a/x-pack/plugins/remote_clusters/server/routes/api/delete_route.test.ts b/x-pack/plugins/remote_clusters/server/routes/api/delete_route.test.ts index cf14f8a67054e..ec14336da08d4 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/delete_route.test.ts +++ b/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, diff --git a/x-pack/plugins/remote_clusters/server/routes/api/delete_route.ts b/x-pack/plugins/remote_clusters/server/routes/api/delete_route.ts index 742780ffed309..07b0fa3fd9cd7 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/delete_route.ts +++ b/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) { diff --git a/x-pack/plugins/remote_clusters/server/routes/api/update_route.ts b/x-pack/plugins/remote_clusters/server/routes/api/update_route.ts index 14b161b6f26b5..47b0d4ad2def2 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/update_route.ts +++ b/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 = {