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 bf0fc11e882ccb..37c06c83f87e1c 100644 --- a/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts +++ b/x-pack/plugins/remote_clusters/common/lib/cluster_serialization.ts @@ -8,13 +8,17 @@ import { PROXY_MODE } from '../constants'; // Values returned from ES GET /_remote/info +/** + * TODO: This interface needs to be updated with values from {@link RemoteInfo} provided + * by the @elastic/elasticsearch client + */ export interface ClusterInfoEs { seeds?: string[]; mode?: 'proxy' | 'sniff'; connected?: boolean; num_nodes_connected?: number; - max_connections_per_cluster?: number; - initial_connect_timeout?: string; + max_connections_per_cluster?: string | number; + initial_connect_timeout: string | number; skip_unavailable?: boolean; transport?: { ping_schedule?: string; @@ -39,8 +43,8 @@ export interface Cluster { transportPingSchedule?: string; transportCompress?: boolean; connectedNodesCount?: number; - maxConnectionsPerCluster?: number; - initialConnectTimeout?: string; + maxConnectionsPerCluster?: string | number; + initialConnectTimeout?: string | number; connectedSocketsCount?: number; hasDeprecatedProxySetting?: boolean; } diff --git a/x-pack/plugins/remote_clusters/server/lib/does_cluster_exist.ts b/x-pack/plugins/remote_clusters/server/lib/does_cluster_exist.ts index 23cbebe8ac98e3..f76854bead41dc 100644 --- a/x-pack/plugins/remote_clusters/server/lib/does_cluster_exist.ts +++ b/x-pack/plugins/remote_clusters/server/lib/does_cluster_exist.ts @@ -5,9 +5,14 @@ * 2.0. */ -export async function doesClusterExist(callAsCurrentUser: any, clusterName: string): Promise { +import { IScopedClusterClient } from 'src/core/server'; + +export async function doesClusterExist( + clusterClient: IScopedClusterClient, + clusterName: string +): Promise { try { - const clusterInfoByName = await callAsCurrentUser('cluster.remoteInfo'); + const { body: clusterInfoByName } = await clusterClient.asCurrentUser.cluster.remoteInfo(); return Boolean(clusterInfoByName && clusterInfoByName[clusterName]); } catch (err) { throw new Error('Unable to check if cluster already exists.'); diff --git a/x-pack/plugins/remote_clusters/server/plugin.ts b/x-pack/plugins/remote_clusters/server/plugin.ts index 2b8d9afe979e89..01cb0fa892316b 100644 --- a/x-pack/plugins/remote_clusters/server/plugin.ts +++ b/x-pack/plugins/remote_clusters/server/plugin.ts @@ -19,6 +19,8 @@ import { registerDeleteRoute, } from './routes/api'; +import { handleEsError } from './shared_imports'; + export interface RemoteClustersPluginSetup { isUiEnabled: boolean; } @@ -44,6 +46,9 @@ export class RemoteClustersServerPlugin config: { isCloudEnabled: Boolean(cloud?.isCloudEnabled), }, + lib: { + handleEsError, + }, }; features.registerElasticsearchFeature({ diff --git a/x-pack/plugins/remote_clusters/server/routes/api/add_route.test.ts b/x-pack/plugins/remote_clusters/server/routes/api/add_route.test.ts index 9348fd1eb20df1..b648554842a5c6 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/add_route.test.ts +++ b/x-pack/plugins/remote_clusters/server/routes/api/add_route.test.ts @@ -5,10 +5,9 @@ * 2.0. */ +import { RequestHandler } from 'src/core/server'; + import { kibanaResponseFactory } from '../../../../../../src/core/server'; -import { register } from './add_route'; -import { API_BASE_PATH } from '../../../common/constants'; -import { LicenseStatus } from '../../types'; import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; @@ -19,6 +18,16 @@ import { coreMock, } from '../../../../../../src/core/server/mocks'; +import { API_BASE_PATH } from '../../../common/constants'; + +import { handleEsError } from '../../shared_imports'; + +import { register } from './add_route'; + +import { ScopedClusterClientMock } from './types'; + +const { createApiResponse } = elasticsearchServiceMock; + // Re-implement the mock that was imported directly from `x-pack/mocks` function createCoreRequestHandlerContextMock() { return { @@ -30,264 +39,235 @@ function createCoreRequestHandlerContextMock() { const xpackMocks = { createRequestHandlerContext: createCoreRequestHandlerContextMock, }; -interface TestOptions { - licenseCheckResult?: LicenseStatus; - apiResponses?: Array<() => Promise>; - asserts: { statusCode: number; result?: Record; apiArguments?: unknown[][] }; - payload?: Record; -} describe('ADD remote clusters', () => { - const addRemoteClustersTest = ( - description: string, - { licenseCheckResult = { valid: true }, apiResponses = [], asserts, payload }: TestOptions - ) => { - test(description, async () => { - const elasticsearchMock = elasticsearchServiceMock.createLegacyClusterClient(); - - const mockRouteDependencies = { - router: httpServiceMock.createRouter(), - getLicenseStatus: () => licenseCheckResult, - elasticsearchService: elasticsearchServiceMock.createInternalSetup(), - elasticsearch: elasticsearchMock, - config: { - isCloudEnabled: false, - }, - }; + let handler: RequestHandler; + let mockRouteDependencies: ReturnType; + let mockContext: ReturnType; + let scopedClusterClientMock: ScopedClusterClientMock; + let remoteInfoMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['remoteInfo']; + let putSettingsMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['putSettings']; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); + const createMockRouteDependencies = () => ({ + router: httpServiceMock.createRouter(), + getLicenseStatus: () => ({ valid: true }), + lib: { + handleEsError, + }, + config: { + isCloudEnabled: false, + }, + }); - elasticsearchServiceMock - .createLegacyClusterClient() - .asScoped.mockReturnValue(mockScopedClusterClient); + const createMockRequest = ( + body: Record = { + name: 'test', + seeds: ['127.0.0.1:9300'], + mode: 'sniff', + skipUnavailable: false, + } + ) => + httpServerMock.createKibanaRequest({ + method: 'post', + path: API_BASE_PATH, + body, + headers: { authorization: 'foo' }, + }); - for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); - } + beforeEach(() => { + mockContext = xpackMocks.createRequestHandlerContext(); + scopedClusterClientMock = mockContext.core.elasticsearch.client; + remoteInfoMockFn = scopedClusterClientMock.asCurrentUser.cluster.remoteInfo; + putSettingsMockFn = scopedClusterClientMock.asCurrentUser.cluster.putSettings; + mockRouteDependencies = createMockRouteDependencies(); - register(mockRouteDependencies); - const [[{ validate }, handler]] = mockRouteDependencies.router.post.mock.calls; + register(mockRouteDependencies); + const [[, handlerFn]] = mockRouteDependencies.router.post.mock.calls; + handler = handlerFn; + }); - const mockRequest = httpServerMock.createKibanaRequest({ - method: 'post', - path: API_BASE_PATH, - body: payload !== undefined ? (validate as any).body.validate(payload) : undefined, - headers: { authorization: 'foo' }, - }); + describe('success', () => { + test(`adds remote cluster with "sniff" mode`, async () => { + remoteInfoMockFn.mockResolvedValueOnce(createApiResponse({ body: {} })); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + acknowledged: true, + persistent: { + cluster: { + remote: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, + }, + }, + }, + transient: {}, + }, + }) + ); - const mockContext = xpackMocks.createRequestHandlerContext(); - mockContext.core.elasticsearch.legacy.client = mockScopedClusterClient; + const mockRequest = createMockRequest(); const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - expect(response.status).toBe(asserts.statusCode); - expect(response.payload).toEqual(asserts.result); + expect(response.status).toBe(200); + expect(response.payload).toEqual({ acknowledged: true }); - if (Array.isArray(asserts.apiArguments)) { - for (const apiArguments of asserts.apiArguments) { - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); - } - }); - }; - - describe('success', () => { - addRemoteClustersTest(`adds remote cluster with "sniff" mode`, { - apiResponses: [ - async () => ({}), - async () => ({ - acknowledged: true, + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { persistent: { cluster: { remote: { test: { - connected: true, - mode: 'sniff', seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', skip_unavailable: false, + mode: 'sniff', + node_connections: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, }, }, }, }, - transient: {}, - }), - ], - payload: { - name: 'test', - seeds: ['127.0.0.1:9300'], - mode: 'sniff', - skipUnavailable: false, - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', - node_connections: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - }, - }, - }, - }, - }, - }, - ], - ], - statusCode: 200, - result: { - acknowledged: true, }, - }, + }); }); - addRemoteClustersTest(`adds remote cluster with "proxy" mode`, { - apiResponses: [ - async () => ({}), - async () => ({ - acknowledged: true, - persistent: { - cluster: { - remote: { - test: { - connected: true, - mode: 'proxy', - seeds: ['127.0.0.1:9300'], - num_sockets_connected: 1, - max_socket_connections: 18, - initial_connect_timeout: '30s', - skip_unavailable: false, + + test(`adds remote cluster with "proxy" mode`, async () => { + remoteInfoMockFn.mockResolvedValueOnce(createApiResponse({ body: {} })); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + acknowledged: true, + persistent: { + cluster: { + remote: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, }, }, }, + transient: {}, }, - transient: {}, - }), - ], - payload: { + }) + ); + + const mockRequest = createMockRequest({ name: 'test', proxyAddress: '127.0.0.1:9300', mode: 'proxy', skipUnavailable: false, serverName: 'foobar', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: null, - skip_unavailable: false, - mode: 'proxy', - node_connections: null, - proxy_address: '127.0.0.1:9300', - proxy_socket_connections: null, - server_name: 'foobar', - }, - }, - }, + }); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ acknowledged: true }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: null, + skip_unavailable: false, + mode: 'proxy', + node_connections: null, + proxy_address: '127.0.0.1:9300', + proxy_socket_connections: null, + server_name: 'foobar', }, }, }, - ], - ], - statusCode: 200, - result: { - acknowledged: true, + }, }, - }, + }); }); }); describe('failure', () => { - addRemoteClustersTest('returns 409 if remote cluster already exists', { - apiResponses: [ - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, + test('returns 409 if remote cluster already exists', async () => { + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, }, - }), - ], - payload: { - name: 'test', - seeds: ['127.0.0.1:9300'], - skipUnavailable: false, - mode: 'sniff', - }, - asserts: { - apiArguments: [['cluster.remoteInfo']], - statusCode: 409, - result: { - message: 'There is already a remote cluster with that name.', - }, - }, + }) + ); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(409); + expect(response.payload).toEqual({ + message: 'There is already a remote cluster with that name.', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).not.toHaveBeenCalled(); }); - addRemoteClustersTest('returns 400 ES did not acknowledge remote cluster', { - apiResponses: [async () => ({}), async () => ({})], - payload: { - name: 'test', - seeds: ['127.0.0.1:9300'], - skipUnavailable: false, - mode: 'sniff', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', - node_connections: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - }, - }, - }, + test('returns 400 ES did not acknowledge remote cluster', async () => { + remoteInfoMockFn.mockResolvedValueOnce(createApiResponse({ body: {} })); + putSettingsMockFn.mockResolvedValueOnce(createApiResponse({ body: {} as any })); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(400); + expect(response.payload).toEqual({ + message: 'Unable to add cluster, no response returned from ES.', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: ['127.0.0.1:9300'], + skip_unavailable: false, + mode: 'sniff', + node_connections: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, }, }, }, - ], - ], - statusCode: 400, - result: { - message: 'Unable to add cluster, no response returned from ES.', + }, }, - }, + }); }); }); }); diff --git a/x-pack/plugins/remote_clusters/server/routes/api/add_route.ts b/x-pack/plugins/remote_clusters/server/routes/api/add_route.ts index 685aee16dc665c..3b6ba618c8975b 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/add_route.ts +++ b/x-pack/plugins/remote_clusters/server/routes/api/add_route.ts @@ -14,7 +14,6 @@ import { serializeCluster, Cluster } from '../../../common/lib'; import { doesClusterExist } from '../../lib/does_cluster_exist'; import { API_BASE_PATH, PROXY_MODE, SNIFF_MODE } from '../../../common/constants'; import { licensePreRoutingFactory } from '../../lib/license_pre_routing_factory'; -import { isEsError } from '../../shared_imports'; import { RouteDependencies } from '../../types'; const bodyValidation = schema.object({ @@ -31,18 +30,23 @@ const bodyValidation = schema.object({ type RouteBody = TypeOf; export const register = (deps: RouteDependencies): void => { + const { + router, + lib: { handleEsError }, + } = deps; + const addHandler: RequestHandler = async ( ctx, request, response ) => { try { - const callAsCurrentUser = ctx.core.elasticsearch.legacy.client.callAsCurrentUser; + const { client: clusterClient } = ctx.core.elasticsearch; const { name } = request.body; // Check if cluster already exists. - const existingCluster = await doesClusterExist(callAsCurrentUser, name); + const existingCluster = await doesClusterExist(clusterClient, name); if (existingCluster) { return response.conflict({ body: { @@ -57,9 +61,11 @@ export const register = (deps: RouteDependencies): void => { } const addClusterPayload = serializeCluster(request.body as Cluster); - const updateClusterResponse = await callAsCurrentUser('cluster.putSettings', { - body: addClusterPayload, - }); + const { body: updateClusterResponse } = await clusterClient.asCurrentUser.cluster.putSettings( + { + body: addClusterPayload, + } + ); const acknowledged = get(updateClusterResponse, 'acknowledged'); const cluster = get(updateClusterResponse, `persistent.cluster.remote.${name}`); @@ -85,13 +91,10 @@ export const register = (deps: RouteDependencies): void => { }, }); } catch (error) { - if (isEsError(error)) { - return response.customError({ statusCode: error.statusCode, body: error }); - } - throw error; + return handleEsError({ error, response }); } }; - deps.router.post( + router.post( { path: API_BASE_PATH, validate: { 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 ce94f45bb84430..d83472fc564b0a 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 @@ -5,10 +5,9 @@ * 2.0. */ -import { kibanaResponseFactory } from '../../../../../../src/core/server'; +import { kibanaResponseFactory, RequestHandler } from '../../../../../../src/core/server'; import { register } from './delete_route'; import { API_BASE_PATH } from '../../../common/constants'; -import { LicenseStatus } from '../../types'; import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; @@ -19,6 +18,12 @@ import { coreMock, } from '../../../../../../src/core/server/mocks'; +import { handleEsError } from '../../shared_imports'; + +import { ScopedClusterClientMock } from './types'; + +const { createApiResponse } = elasticsearchServiceMock; + // Re-implement the mock that was imported directly from `x-pack/mocks` function createCoreRequestHandlerContextMock() { return { @@ -30,192 +35,193 @@ function createCoreRequestHandlerContextMock() { const xpackMocks = { createRequestHandlerContext: createCoreRequestHandlerContextMock, }; -interface TestOptions { - licenseCheckResult?: LicenseStatus; - apiResponses?: Array<() => Promise>; - asserts: { statusCode: number; result?: Record; apiArguments?: unknown[][] }; - params: { - nameOrNames: string; - }; -} describe('DELETE remote clusters', () => { - const deleteRemoteClustersTest = ( - description: string, - { licenseCheckResult = { valid: true }, apiResponses = [], asserts, params }: TestOptions - ) => { - test(description, async () => { - const elasticsearchMock = elasticsearchServiceMock.createLegacyClusterClient(); + let handler: RequestHandler; + let mockRouteDependencies: ReturnType; + let mockContext: ReturnType; + let scopedClusterClientMock: ScopedClusterClientMock; - const mockRouteDependencies = { - router: httpServiceMock.createRouter(), - getLicenseStatus: () => licenseCheckResult, - elasticsearchService: elasticsearchServiceMock.createInternalSetup(), - elasticsearch: elasticsearchMock, - config: { - isCloudEnabled: false, - }, - }; + let remoteInfoMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['remoteInfo']; + let getSettingsMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['getSettings']; + let putSettingsMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['putSettings']; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); + const createMockRequest = ( + body: Record = { + name: 'test', + proxyAddress: '127.0.0.1:9300', + mode: 'proxy', + skipUnavailable: false, + serverName: 'foobar', + } + ) => + httpServerMock.createKibanaRequest({ + method: 'delete', + path: `${API_BASE_PATH}/{nameOrNames}`, + params: { + nameOrNames: 'test', + }, + body, + headers: { authorization: 'foo' }, + }); - elasticsearchServiceMock - .createLegacyClusterClient() - .asScoped.mockReturnValue(mockScopedClusterClient); + const createMockRouteDependencies = () => ({ + router: httpServiceMock.createRouter(), + getLicenseStatus: () => ({ valid: true }), + lib: { + handleEsError, + }, + config: { + isCloudEnabled: false, + }, + }); - for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); - } + beforeEach(() => { + mockContext = xpackMocks.createRequestHandlerContext(); + scopedClusterClientMock = mockContext.core.elasticsearch.client; + remoteInfoMockFn = scopedClusterClientMock.asCurrentUser.cluster.remoteInfo; + getSettingsMockFn = scopedClusterClientMock.asCurrentUser.cluster.getSettings; + putSettingsMockFn = scopedClusterClientMock.asCurrentUser.cluster.putSettings; + mockRouteDependencies = createMockRouteDependencies(); - register(mockRouteDependencies); - const [[{ validate }, handler]] = mockRouteDependencies.router.delete.mock.calls; + register(mockRouteDependencies); + const [[, handlerFn]] = mockRouteDependencies.router.delete.mock.calls; + handler = handlerFn; + }); - const mockRequest = httpServerMock.createKibanaRequest({ - method: 'delete', - path: `${API_BASE_PATH}/{nameOrNames}`, - params: (validate as any).params.validate(params), - headers: { authorization: 'foo' }, - }); + describe('success', () => { + test('deletes remote cluster', async () => { + getSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: ['127.0.0.1:9300'], + skip_unavailable: false, + mode: 'sniff', + }, + }, + }, + }, + transient: {}, + }, + }) + ); + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, + }, + }) + ); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + acknowledged: true, + persistent: {}, + transient: {}, + }, + }) + ); - const mockContext = xpackMocks.createRequestHandlerContext(); - mockContext.core.elasticsearch.legacy.client = mockScopedClusterClient; - const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + const mockRequest = createMockRequest(); - expect(response.status).toBe(asserts.statusCode); - expect(response.payload).toEqual(asserts.result); + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - if (Array.isArray(asserts.apiArguments)) { - for (const apiArguments of asserts.apiArguments) { - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); - } - }); - }; + expect(response.status).toBe(200); + expect(response.payload).toEqual({ + itemsDeleted: ['test'], + errors: [], + }); - describe('success', () => { - deleteRemoteClustersTest('deletes remote cluster', { - apiResponses: [ - async () => ({ + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { persistent: { cluster: { remote: { test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', + seeds: null, + skip_unavailable: null, + mode: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, + node_connections: null, }, }, }, }, - transient: {}, - }), - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, - }, - }), - async () => ({ - acknowledged: true, - persistent: {}, - transient: {}, - }), - ], - params: { - nameOrNames: 'test', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: null, - skip_unavailable: null, - mode: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - node_connections: null, - }, - }, - }, - }, - }, - }, - ], - ], - statusCode: 200, - result: { - itemsDeleted: ['test'], - errors: [], }, - }, + }); }); }); describe('failure', () => { - deleteRemoteClustersTest( - 'returns errors array with 404 error if remote cluster does not exist', - { - apiResponses: [ - async () => ({ + test('returns errors array with 404 error if remote cluster does not exist', async () => { + getSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { persistent: { cluster: { remote: {}, }, }, transient: {}, - }), - async () => ({}), - ], - params: { - nameOrNames: 'test', - }, - asserts: { - apiArguments: [['cluster.remoteInfo']], - statusCode: 200, - result: { - errors: [ - { - error: { - options: { - body: { - message: 'There is no remote cluster with that name.', - }, - statusCode: 404, - }, - payload: { - message: 'There is no remote cluster with that name.', - }, - status: 404, + }, + }) + ); + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: {}, + }) + ); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ + errors: [ + { + error: { + options: { + body: { + message: 'There is no remote cluster with that name.', }, - name: 'test', + statusCode: 404, + }, + payload: { + message: 'There is no remote cluster with that name.', }, - ], - itemsDeleted: [], + status: 404, + }, + name: 'test', }, - }, - } - ); + ], + itemsDeleted: [], + }); - deleteRemoteClustersTest( - 'returns errors array with 400 error if ES still returns cluster information', - { - apiResponses: [ - async () => ({ + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + }); + + test('returns errors array with 400 error if ES still returns cluster information', async () => { + getSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { persistent: { cluster: { remote: { @@ -228,8 +234,12 @@ describe('DELETE remote clusters', () => { }, }, transient: {}, - }), - async () => ({ + }, + }) + ); + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { test: { connected: true, mode: 'sniff', @@ -239,8 +249,13 @@ describe('DELETE remote clusters', () => { initial_connect_timeout: '30s', skip_unavailable: false, }, - }), - async () => ({ + }, + }) + ); + + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { acknowledged: true, persistent: { cluster: { @@ -258,60 +273,57 @@ describe('DELETE remote clusters', () => { }, }, transient: {}, - }), - ], - params: { - nameOrNames: 'test', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { + }, + }) + ); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ + errors: [ + { + error: { + options: { body: { - persistent: { - cluster: { - remote: { - test: { - seeds: null, - skip_unavailable: null, - mode: null, - node_connections: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - }, - }, - }, - }, + message: 'Unable to delete cluster, information still returned from ES.', }, + statusCode: 400, }, - ], - ], - statusCode: 200, - result: { - errors: [ - { - error: { - options: { - body: { - message: 'Unable to delete cluster, information still returned from ES.', - }, - statusCode: 400, - }, - payload: { - message: 'Unable to delete cluster, information still returned from ES.', - }, - status: 400, + payload: { + message: 'Unable to delete cluster, information still returned from ES.', + }, + status: 400, + }, + name: 'test', + }, + ], + itemsDeleted: [], + }); + + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: null, + skip_unavailable: null, + mode: null, + node_connections: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, }, - name: 'test', }, - ], - itemsDeleted: [], + }, }, }, - } - ); + }); + }); }); }); 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 89df5255d19e2e..18aa9154fba421 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 @@ -15,7 +15,6 @@ import { serializeCluster } from '../../../common/lib'; import { API_BASE_PATH } from '../../../common/constants'; import { doesClusterExist } from '../../lib/does_cluster_exist'; import { licensePreRoutingFactory } from '../../lib/license_pre_routing_factory'; -import { isEsError } from '../../shared_imports'; const paramsValidation = schema.object({ nameOrNames: schema.string(), @@ -24,13 +23,18 @@ const paramsValidation = schema.object({ type RouteParams = TypeOf; export const register = (deps: RouteDependencies): void => { + const { + router, + lib: { handleEsError }, + } = deps; + const deleteHandler: RequestHandler = async ( ctx, request, response ) => { try { - const callAsCurrentUser = ctx.core.elasticsearch.legacy.client.callAsCurrentUser; + const { client: clusterClient } = ctx.core.elasticsearch; const { nameOrNames } = request.params; const names = nameOrNames.split(','); @@ -38,12 +42,12 @@ export const register = (deps: RouteDependencies): void => { const itemsDeleted: any[] = []; const errors: any[] = []; - const clusterSettings = await callAsCurrentUser('cluster.getSettings'); + const { body: clusterSettings } = await clusterClient.asCurrentUser.cluster.getSettings(); // Validator that returns an error if the remote cluster does not exist. const validateClusterDoesExist = async (name: string) => { try { - const existingCluster = await doesClusterExist(callAsCurrentUser, name); + const existingCluster = await doesClusterExist(clusterClient, name); if (!existingCluster) { return response.customError({ statusCode: 404, @@ -69,7 +73,12 @@ export const register = (deps: RouteDependencies): void => { ) => { try { const body = serializeCluster({ name, hasDeprecatedProxySetting }); - const updateClusterResponse = await callAsCurrentUser('cluster.putSettings', { body }); + + const { + body: updateClusterResponse, + } = await clusterClient.asCurrentUser.cluster.putSettings({ + body, + }); const acknowledged = get(updateClusterResponse, 'acknowledged'); const cluster = get(updateClusterResponse, `persistent.cluster.remote.${name}`); @@ -92,10 +101,7 @@ export const register = (deps: RouteDependencies): void => { }, }); } catch (error) { - if (isEsError(error)) { - return response.customError({ statusCode: error.statusCode, body: error }); - } - throw error; + return handleEsError({ error, response }); } }; @@ -129,14 +135,11 @@ export const register = (deps: RouteDependencies): void => { }, }); } catch (error) { - if (isEsError(error)) { - return response.customError({ statusCode: error.statusCode, body: error }); - } - throw error; + return handleEsError({ error, response }); } }; - deps.router.delete( + router.delete( { path: `${API_BASE_PATH}/{nameOrNames}`, validate: { diff --git a/x-pack/plugins/remote_clusters/server/routes/api/get_route.test.ts b/x-pack/plugins/remote_clusters/server/routes/api/get_route.test.ts index cfec01da943ab7..b4c8d6d304b12a 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/get_route.test.ts +++ b/x-pack/plugins/remote_clusters/server/routes/api/get_route.test.ts @@ -5,14 +5,9 @@ * 2.0. */ -import Boom from '@hapi/boom'; +import { errors } from '@elastic/elasticsearch'; -import { kibanaResponseFactory } from '../../../../../../src/core/server'; -import { register } from './get_route'; -import { API_BASE_PATH } from '../../../common/constants'; -import { LicenseStatus } from '../../types'; - -import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; +import { RequestHandler } from 'src/core/server'; import { elasticsearchServiceMock, @@ -21,6 +16,18 @@ import { coreMock, } from '../../../../../../src/core/server/mocks'; +import { kibanaResponseFactory } from '../../../../../../src/core/server'; +import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; + +import { API_BASE_PATH } from '../../../common/constants'; + +import { handleEsError } from '../../shared_imports'; + +import { register } from './get_route'; +import { ScopedClusterClientMock } from './types'; + +const { createApiResponse } = elasticsearchServiceMock; + // Re-implement the mock that was imported directly from `x-pack/mocks` function createCoreRequestHandlerContextMock() { return { @@ -32,182 +39,183 @@ function createCoreRequestHandlerContextMock() { const xpackMocks = { createRequestHandlerContext: createCoreRequestHandlerContextMock, }; -interface TestOptions { - licenseCheckResult?: LicenseStatus; - apiResponses?: Array<() => Promise>; - asserts: { statusCode: number; result?: Record; apiArguments?: unknown[][] }; -} - describe('GET remote clusters', () => { - const getRemoteClustersTest = ( - description: string, - { licenseCheckResult = { valid: true }, apiResponses = [], asserts }: TestOptions - ) => { - test(description, async () => { - const elasticsearchMock = elasticsearchServiceMock.createLegacyClusterClient(); - - const mockRouteDependencies = { - router: httpServiceMock.createRouter(), - getLicenseStatus: () => licenseCheckResult, - elasticsearchService: elasticsearchServiceMock.createInternalSetup(), - elasticsearch: elasticsearchMock, - config: { - isCloudEnabled: false, - }, - }; - - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - - elasticsearchServiceMock - .createLegacyClusterClient() - .asScoped.mockReturnValue(mockScopedClusterClient); - - for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); - } + let handler: RequestHandler; + let mockRouteDependencies: ReturnType; + let mockContext: ReturnType; + let scopedClusterClientMock: ScopedClusterClientMock; + + let remoteInfoMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['remoteInfo']; + let getSettingsMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['getSettings']; + + const createMockRequest = () => + httpServerMock.createKibanaRequest({ + method: 'get', + path: API_BASE_PATH, + headers: { authorization: 'foo' }, + }); - register(mockRouteDependencies); - const [[, handler]] = mockRouteDependencies.router.get.mock.calls; + const createMockRouteDependencies = () => ({ + router: httpServiceMock.createRouter(), + getLicenseStatus: () => ({ valid: true }), + lib: { + handleEsError, + }, + config: { + isCloudEnabled: false, + }, + }); - const mockRequest = httpServerMock.createKibanaRequest({ - method: 'get', - path: API_BASE_PATH, - headers: { authorization: 'foo' }, - }); + beforeEach(() => { + mockContext = xpackMocks.createRequestHandlerContext(); + scopedClusterClientMock = mockContext.core.elasticsearch.client; + remoteInfoMockFn = scopedClusterClientMock.asCurrentUser.cluster.remoteInfo; + getSettingsMockFn = scopedClusterClientMock.asCurrentUser.cluster.getSettings; + mockRouteDependencies = createMockRouteDependencies(); - const mockContext = xpackMocks.createRequestHandlerContext(); - mockContext.core.elasticsearch.legacy.client = mockScopedClusterClient; - - if (asserts.statusCode === 500) { - await expect(handler(mockContext, mockRequest, kibanaResponseFactory)).rejects.toThrowError( - asserts.result as Error - ); - } else { - const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - - expect(response.status).toBe(asserts.statusCode); - expect(response.payload).toEqual(asserts.result); - } - - if (Array.isArray(asserts.apiArguments)) { - for (const apiArguments of asserts.apiArguments) { - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); - } - }); - }; + register(mockRouteDependencies); + const [[, handlerFn]] = mockRouteDependencies.router.get.mock.calls; + handler = handlerFn; + }); describe('success', () => { - getRemoteClustersTest('returns remote clusters', { - apiResponses: [ - async () => ({ - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', + test('returns remote clusters', async () => { + getSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: ['127.0.0.1:9300'], + skip_unavailable: false, + mode: 'sniff', + }, }, }, }, + transient: {}, }, - transient: {}, - }), - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, - }, - }), - ], - asserts: { - apiArguments: [['cluster.getSettings'], ['cluster.remoteInfo']], - statusCode: 200, - result: [ - { - name: 'test', - seeds: ['127.0.0.1:9300'], - isConnected: true, - connectedNodesCount: 1, - maxConnectionsPerCluster: 3, - initialConnectTimeout: '30s', - skipUnavailable: false, - isConfiguredByNode: false, - mode: 'sniff', + }) + ); + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, }, - ], - }, + }) + ); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual([ + { + name: 'test', + seeds: ['127.0.0.1:9300'], + isConnected: true, + connectedNodesCount: 1, + maxConnectionsPerCluster: 3, + initialConnectTimeout: '30s', + skipUnavailable: false, + isConfiguredByNode: false, + mode: 'sniff', + }, + ]); + + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); }); - getRemoteClustersTest('returns an empty array when ES responds with an empty object', { - apiResponses: [async () => ({}), async () => ({})], - asserts: { - apiArguments: [['cluster.getSettings'], ['cluster.remoteInfo']], - statusCode: 200, - result: [], - }, + + test('returns an empty array when ES responds with an empty object', async () => { + getSettingsMockFn.mockResolvedValueOnce(createApiResponse({ body: {} as any })); + remoteInfoMockFn.mockResolvedValueOnce(createApiResponse({ body: {} })); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual([]); + + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); }); }); describe('failure', () => { - const error = Boom.notAcceptable('test error'); + const error = new errors.ResponseError({ + body: { message: 'test error' }, + statusCode: 406, + headers: {}, + meta: {} as any, + warnings: [], + }); + + test('returns an error if failure to get cluster settings', async () => { + getSettingsMockFn.mockRejectedValueOnce(error); + + const mockRequest = createMockRequest(); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - getRemoteClustersTest('returns an error if failure to get cluster settings', { - apiResponses: [ - async () => { - throw error; + expect(response.status).toBe(406); + expect(response.payload).toEqual({ + attributes: { + causes: undefined, + error: undefined, }, - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, - }, - }), - ], - asserts: { - apiArguments: [['cluster.getSettings']], - statusCode: 500, - result: error, - }, + message: 'Response Error', + }); + + expect(getSettingsMockFn).toHaveBeenCalled(); + expect(remoteInfoMockFn).not.toHaveBeenCalled(); }); - getRemoteClustersTest('returns an error if failure to get cluster remote info', { - apiResponses: [ - async () => ({ - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', + test('returns an error if failure to get cluster remote info', async () => { + getSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: ['127.0.0.1:9300'], + skip_unavailable: false, + mode: 'sniff', + }, }, }, }, + transient: {}, }, - transient: {}, - }), - async () => { - throw error; - }, - ], - asserts: { - apiArguments: [['cluster.getSettings'], ['cluster.remoteInfo']], - statusCode: 500, - result: error, - }, + }) + ); + + remoteInfoMockFn.mockRejectedValueOnce(error); + + const mockRequest = httpServerMock.createKibanaRequest({ + method: 'get', + path: API_BASE_PATH, + headers: { authorization: 'foo' }, + }); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(406); + + expect(getSettingsMockFn).toHaveBeenCalledWith(); + expect(remoteInfoMockFn).toHaveBeenCalledWith(); }); }); }); diff --git a/x-pack/plugins/remote_clusters/server/routes/api/get_route.ts b/x-pack/plugins/remote_clusters/server/routes/api/get_route.ts index fbb345203e48a8..c98f5234506860 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/get_route.ts +++ b/x-pack/plugins/remote_clusters/server/routes/api/get_route.ts @@ -11,14 +11,18 @@ import { RequestHandler } from 'src/core/server'; import { deserializeCluster } from '../../../common/lib'; import { API_BASE_PATH } from '../../../common/constants'; import { licensePreRoutingFactory } from '../../lib/license_pre_routing_factory'; -import { isEsError } from '../../shared_imports'; import { RouteDependencies } from '../../types'; export const register = (deps: RouteDependencies): void => { + const { + router, + lib: { handleEsError }, + } = deps; + const allHandler: RequestHandler = async (ctx, request, response) => { try { - const callAsCurrentUser = await ctx.core.elasticsearch.legacy.client.callAsCurrentUser; - const clusterSettings = await callAsCurrentUser('cluster.getSettings'); + const { client: clusterClient } = ctx.core.elasticsearch; + const { body: clusterSettings } = await clusterClient.asCurrentUser.cluster.getSettings(); const transientClusterNames = Object.keys( get(clusterSettings, 'transient.cluster.remote') || {} @@ -27,7 +31,7 @@ export const register = (deps: RouteDependencies): void => { get(clusterSettings, 'persistent.cluster.remote') || {} ); - const clustersByName = await callAsCurrentUser('cluster.remoteInfo'); + const { body: clustersByName } = await clusterClient.asCurrentUser.cluster.remoteInfo(); const clusterNames = (clustersByName && Object.keys(clustersByName)) || []; const body = clusterNames.map((clusterName: string): any => { @@ -60,14 +64,11 @@ export const register = (deps: RouteDependencies): void => { return response.ok({ body }); } catch (error) { - if (isEsError(error)) { - return response.customError({ statusCode: error.statusCode, body: error }); - } - throw error; + return handleEsError({ error, response }); } }; - deps.router.get( + router.get( { path: API_BASE_PATH, validate: false, diff --git a/x-pack/plugins/remote_clusters/server/routes/api/types.ts b/x-pack/plugins/remote_clusters/server/routes/api/types.ts new file mode 100644 index 00000000000000..5a2fd20ba4d355 --- /dev/null +++ b/x-pack/plugins/remote_clusters/server/routes/api/types.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { elasticsearchServiceMock } from '../../../../../../src/core/server/mocks'; + +export type ScopedClusterClientMock = ReturnType< + typeof elasticsearchServiceMock.createScopedClusterClient +>; diff --git a/x-pack/plugins/remote_clusters/server/routes/api/update_route.test.ts b/x-pack/plugins/remote_clusters/server/routes/api/update_route.test.ts index 22c87786a585c4..129326dea95ec4 100644 --- a/x-pack/plugins/remote_clusters/server/routes/api/update_route.test.ts +++ b/x-pack/plugins/remote_clusters/server/routes/api/update_route.test.ts @@ -4,13 +4,8 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - +import { RequestHandler } from 'src/core/server'; import { kibanaResponseFactory } from '../../../../../../src/core/server'; -import { register } from './update_route'; -import { API_BASE_PATH } from '../../../common/constants'; -import { LicenseStatus } from '../../types'; - -import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; import { elasticsearchServiceMock, @@ -19,6 +14,17 @@ import { coreMock, } from '../../../../../../src/core/server/mocks'; +import { licensingMock } from '../../../../../plugins/licensing/server/mocks'; + +import { API_BASE_PATH } from '../../../common/constants'; + +import { handleEsError } from '../../shared_imports'; + +import { register } from './update_route'; +import { ScopedClusterClientMock } from './types'; + +const { createApiResponse } = elasticsearchServiceMock; + // Re-implement the mock that was imported directly from `x-pack/mocks` function createCoreRequestHandlerContextMock() { return { @@ -30,315 +36,296 @@ function createCoreRequestHandlerContextMock() { const xpackMocks = { createRequestHandlerContext: createCoreRequestHandlerContextMock, }; -interface TestOptions { - licenseCheckResult?: LicenseStatus; - apiResponses?: Array<() => Promise>; - asserts: { statusCode: number; result?: Record; apiArguments?: unknown[][] }; - payload?: Record; - params: { - name: string; - }; -} describe('UPDATE remote clusters', () => { - const updateRemoteClustersTest = ( - description: string, - { - licenseCheckResult = { valid: true }, - apiResponses = [], - asserts, - payload, - params, - }: TestOptions - ) => { - test(description, async () => { - const elasticsearchMock = elasticsearchServiceMock.createLegacyClusterClient(); - - const mockRouteDependencies = { - router: httpServiceMock.createRouter(), - getLicenseStatus: () => licenseCheckResult, - elasticsearchService: elasticsearchServiceMock.createInternalSetup(), - elasticsearch: elasticsearchMock, - config: { - isCloudEnabled: false, - }, - }; + let handler: RequestHandler; + let mockRouteDependencies: ReturnType; + let mockContext: ReturnType; + let scopedClusterClientMock: ScopedClusterClientMock; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); + let remoteInfoMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['remoteInfo']; + let putSettingsMockFn: ScopedClusterClientMock['asCurrentUser']['cluster']['putSettings']; - elasticsearchServiceMock - .createLegacyClusterClient() - .asScoped.mockReturnValue(mockScopedClusterClient); + const createMockRequest = ( + body: Record = { seeds: ['127.0.0.1:9300'], skipUnavailable: true, mode: 'sniff' } + ) => + httpServerMock.createKibanaRequest({ + method: 'put', + path: `${API_BASE_PATH}/{name}`, + params: { + name: 'test', + }, + body, + headers: { authorization: 'foo' }, + }); - for (const apiResponse of apiResponses) { - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(apiResponse); - } + const createMockRouteDependencies = () => ({ + router: httpServiceMock.createRouter(), + getLicenseStatus: () => ({ valid: true }), + lib: { + handleEsError, + }, + config: { + isCloudEnabled: false, + }, + }); - register(mockRouteDependencies); - const [[{ validate }, handler]] = mockRouteDependencies.router.put.mock.calls; + beforeEach(() => { + mockContext = xpackMocks.createRequestHandlerContext(); + scopedClusterClientMock = mockContext.core.elasticsearch.client; + remoteInfoMockFn = scopedClusterClientMock.asCurrentUser.cluster.remoteInfo; + putSettingsMockFn = scopedClusterClientMock.asCurrentUser.cluster.putSettings; + mockRouteDependencies = createMockRouteDependencies(); - const mockRequest = httpServerMock.createKibanaRequest({ - method: 'put', - path: `${API_BASE_PATH}/{name}`, - params: (validate as any).params.validate(params), - body: payload !== undefined ? (validate as any).body.validate(payload) : undefined, - headers: { authorization: 'foo' }, - }); + register(mockRouteDependencies); + const [[, handlerFn]] = mockRouteDependencies.router.put.mock.calls; + handler = handlerFn; + }); - const mockContext = xpackMocks.createRequestHandlerContext(); - mockContext.core.elasticsearch.legacy.client = mockScopedClusterClient; - const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + describe('success', () => { + test('updates remote cluster', async () => { + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, + }, + }) + ); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + acknowledged: true, + persistent: { + cluster: { + remote: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: true, + }, + }, + }, + }, + transient: {}, + }, + }) + ); - expect(response.status).toBe(asserts.statusCode); - expect(response.payload).toEqual(asserts.result); + const mockRequest = createMockRequest(); - if (Array.isArray(asserts.apiArguments)) { - for (const apiArguments of asserts.apiArguments) { - expect(mockScopedClusterClient.callAsCurrentUser).toHaveBeenCalledWith(...apiArguments); - } - } else { - expect(mockScopedClusterClient.callAsCurrentUser).not.toHaveBeenCalled(); - } - }); - }; + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); - describe('success', () => { - updateRemoteClustersTest('updates remote cluster', { - apiResponses: [ - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, - }, - }), - async () => ({ - acknowledged: true, + expect(response.status).toBe(200); + expect(response.payload).toEqual({ + connectedNodesCount: 1, + initialConnectTimeout: '30s', + isConfiguredByNode: false, + isConnected: true, + maxConnectionsPerCluster: 3, + name: 'test', + seeds: ['127.0.0.1:9300'], + skipUnavailable: true, + mode: 'sniff', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { persistent: { cluster: { remote: { test: { - connected: true, - mode: 'sniff', seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', skip_unavailable: true, + mode: 'sniff', + node_connections: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, }, }, }, }, - transient: {}, - }), - ], - params: { - name: 'test', - }, - payload: { - seeds: ['127.0.0.1:9300'], - skipUnavailable: true, - mode: 'sniff', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: true, - mode: 'sniff', - node_connections: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - }, - }, + }, + }); + }); + + test('updates v1 proxy cluster', async () => { + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + initial_connect_timeout: '30s', + skip_unavailable: false, + seeds: ['127.0.0.1:9300'], + max_connections_per_cluster: 20, + num_nodes_connected: 1, + }, + }, + }) + ); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + acknowledged: true, + persistent: { + cluster: { + remote: { + test: { + connected: true, + proxy_address: '127.0.0.1:9300', + initial_connect_timeout: '30s', + skip_unavailable: true, + mode: 'proxy', + proxy_socket_connections: 18, }, }, }, }, - ], - ], - statusCode: 200, - result: { - connectedNodesCount: 1, - initialConnectTimeout: '30s', - isConfiguredByNode: false, - isConnected: true, - maxConnectionsPerCluster: 3, - name: 'test', - seeds: ['127.0.0.1:9300'], - skipUnavailable: true, - mode: 'sniff', - }, - }, - }); - updateRemoteClustersTest('updates v1 proxy cluster', { - apiResponses: [ - async () => ({ - test: { - connected: true, - initial_connect_timeout: '30s', - skip_unavailable: false, - seeds: ['127.0.0.1:9300'], + transient: {}, }, - }), - async () => ({ - acknowledged: true, + }) + ); + + const mockRequest = createMockRequest({ + proxyAddress: '127.0.0.1:9300', + skipUnavailable: true, + mode: 'proxy', + hasDeprecatedProxySetting: true, + serverName: '', + proxySocketConnections: 18, + }); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(200); + expect(response.payload).toEqual({ + initialConnectTimeout: '30s', + isConfiguredByNode: false, + isConnected: true, + proxyAddress: '127.0.0.1:9300', + name: 'test', + skipUnavailable: true, + mode: 'proxy', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { persistent: { cluster: { remote: { test: { - connected: true, proxy_address: '127.0.0.1:9300', - initial_connect_timeout: '30s', skip_unavailable: true, mode: 'proxy', + node_connections: null, + seeds: null, proxy_socket_connections: 18, + server_name: null, + proxy: null, }, }, }, }, - transient: {}, - }), - ], - params: { - name: 'test', - }, - payload: { - proxyAddress: '127.0.0.1:9300', - skipUnavailable: true, - mode: 'proxy', - hasDeprecatedProxySetting: true, - serverName: '', - proxySocketConnections: 18, - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - proxy_address: '127.0.0.1:9300', - skip_unavailable: true, - mode: 'proxy', - node_connections: null, - seeds: null, - proxy_socket_connections: 18, - server_name: null, - proxy: null, - }, - }, - }, - }, - }, - }, - ], - ], - statusCode: 200, - result: { - initialConnectTimeout: '30s', - isConfiguredByNode: false, - isConnected: true, - proxyAddress: '127.0.0.1:9300', - name: 'test', - skipUnavailable: true, - mode: 'proxy', }, - }, + }); }); }); describe('failure', () => { - updateRemoteClustersTest('returns 404 if remote cluster does not exist', { - apiResponses: [async () => ({})], - payload: { + test('returns 404 if remote cluster does not exist', async () => { + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: {} as any, + }) + ); + + const mockRequest = createMockRequest({ seeds: ['127.0.0.1:9300'], skipUnavailable: false, mode: 'sniff', - }, - params: { - name: 'test', - }, - asserts: { - apiArguments: [['cluster.remoteInfo']], - statusCode: 404, - result: { - message: 'There is no remote cluster with that name.', - }, - }, + }); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(404); + expect(response.payload).toEqual({ + message: 'There is no remote cluster with that name.', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).not.toHaveBeenCalled(); }); - updateRemoteClustersTest('returns 400 if ES did not acknowledge remote cluster', { - apiResponses: [ - async () => ({ - test: { - connected: true, - mode: 'sniff', - seeds: ['127.0.0.1:9300'], - num_nodes_connected: 1, - max_connections_per_cluster: 3, - initial_connect_timeout: '30s', - skip_unavailable: false, + test('returns 400 if ES did not acknowledge remote cluster', async () => { + remoteInfoMockFn.mockResolvedValueOnce( + createApiResponse({ + body: { + test: { + connected: true, + mode: 'sniff', + seeds: ['127.0.0.1:9300'], + num_nodes_connected: 1, + max_connections_per_cluster: 3, + initial_connect_timeout: '30s', + skip_unavailable: false, + }, }, - }), - async () => ({}), - ], - payload: { + }) + ); + putSettingsMockFn.mockResolvedValueOnce( + createApiResponse({ + body: {} as any, + }) + ); + + const mockRequest = createMockRequest({ seeds: ['127.0.0.1:9300'], skipUnavailable: false, mode: 'sniff', - }, - params: { - name: 'test', - }, - asserts: { - apiArguments: [ - ['cluster.remoteInfo'], - [ - 'cluster.putSettings', - { - body: { - persistent: { - cluster: { - remote: { - test: { - seeds: ['127.0.0.1:9300'], - skip_unavailable: false, - mode: 'sniff', - node_connections: null, - proxy_address: null, - proxy_socket_connections: null, - server_name: null, - }, - }, - }, + }); + + const response = await handler(mockContext, mockRequest, kibanaResponseFactory); + + expect(response.status).toBe(400); + expect(response.payload).toEqual({ + message: 'Unable to edit cluster, no response returned from ES.', + }); + + expect(remoteInfoMockFn).toHaveBeenCalledWith(); + expect(putSettingsMockFn).toHaveBeenCalledWith({ + body: { + persistent: { + cluster: { + remote: { + test: { + seeds: ['127.0.0.1:9300'], + skip_unavailable: false, + mode: 'sniff', + node_connections: null, + proxy_address: null, + proxy_socket_connections: null, + server_name: null, }, }, }, - ], - ], - statusCode: 400, - result: { - message: 'Unable to edit cluster, no response returned from ES.', + }, }, - }, + }); }); }); }); 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 99fb7dd01adb13..efe259b40ec251 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 @@ -15,7 +15,6 @@ import { serializeCluster, deserializeCluster, Cluster, ClusterInfoEs } from '.. import { doesClusterExist } from '../../lib/does_cluster_exist'; import { RouteDependencies } from '../../types'; import { licensePreRoutingFactory } from '../../lib/license_pre_routing_factory'; -import { isEsError } from '../../shared_imports'; const bodyValidation = schema.object({ skipUnavailable: schema.boolean(), @@ -37,18 +36,23 @@ type RouteParams = TypeOf; type RouteBody = TypeOf; export const register = (deps: RouteDependencies): void => { + const { + router, + lib: { handleEsError }, + } = deps; + const updateHandler: RequestHandler = async ( ctx, request, response ) => { try { - const callAsCurrentUser = ctx.core.elasticsearch.legacy.client.callAsCurrentUser; + const { client: clusterClient } = ctx.core.elasticsearch; const { name } = request.params; // Check if cluster does exist. - const existingCluster = await doesClusterExist(callAsCurrentUser, name); + const existingCluster = await doesClusterExist(clusterClient, name); if (!existingCluster) { return response.notFound({ body: { @@ -65,9 +69,11 @@ export const register = (deps: RouteDependencies): void => { // Update cluster as new settings const updateClusterPayload = serializeCluster({ ...request.body, name } as Cluster); - const updateClusterResponse = await callAsCurrentUser('cluster.putSettings', { - body: updateClusterPayload, - }); + const { body: updateClusterResponse } = await clusterClient.asCurrentUser.cluster.putSettings( + { + body: updateClusterPayload, + } + ); const acknowledged = get(updateClusterResponse, 'acknowledged'); const cluster = get( @@ -97,14 +103,11 @@ export const register = (deps: RouteDependencies): void => { }, }); } catch (error) { - if (isEsError(error)) { - return response.customError({ statusCode: error.statusCode, body: error }); - } - throw error; + return handleEsError({ error, response }); } }; - deps.router.put( + router.put( { path: `${API_BASE_PATH}/{name}`, validate: { diff --git a/x-pack/plugins/remote_clusters/server/shared_imports.ts b/x-pack/plugins/remote_clusters/server/shared_imports.ts index df9b3dd53cc1f7..7f55d189457c70 100644 --- a/x-pack/plugins/remote_clusters/server/shared_imports.ts +++ b/x-pack/plugins/remote_clusters/server/shared_imports.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { isEsError } from '../../../../src/plugins/es_ui_shared/server'; +export { handleEsError } from '../../../../src/plugins/es_ui_shared/server'; diff --git a/x-pack/plugins/remote_clusters/server/types.ts b/x-pack/plugins/remote_clusters/server/types.ts index 00b73d614ec962..89d1ebfb5cf4ef 100644 --- a/x-pack/plugins/remote_clusters/server/types.ts +++ b/x-pack/plugins/remote_clusters/server/types.ts @@ -6,10 +6,13 @@ */ import { IRouter } from 'kibana/server'; + import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server'; import { LicensingPluginSetup } from '../../licensing/server'; import { CloudSetup } from '../../cloud/server'; +import { handleEsError } from './shared_imports'; + export interface Dependencies { licensing: LicensingPluginSetup; cloud: CloudSetup; @@ -22,6 +25,9 @@ export interface RouteDependencies { config: { isCloudEnabled: boolean; }; + lib: { + handleEsError: typeof handleEsError; + }; } export interface LicenseStatus {