From d38d9114e48bc00cb5853031cf0ed8bbf9dbd43f Mon Sep 17 00:00:00 2001 From: restrry Date: Fri, 5 Jul 2019 12:23:39 +0200 Subject: [PATCH 1/3] createCluster created with partial config --- .../elasticsearch/elasticsearch_service.test.ts | 2 +- .../server/elasticsearch/elasticsearch_service.ts | 15 ++++++++++----- src/legacy/core_plugins/elasticsearch/index.js | 5 +---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/core/server/elasticsearch/elasticsearch_service.test.ts b/src/core/server/elasticsearch/elasticsearch_service.test.ts index a0f71801293823..cbb7be280459ef 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.test.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.test.ts @@ -115,7 +115,7 @@ describe('#setup', () => { expect(clusterClient).toBe(mockClusterClientInstance); expect(MockClusterClient).toHaveBeenCalledWith( - mockConfig, + expect.objectContaining(mockConfig), expect.objectContaining({ context: ['elasticsearch', 'some-custom-type'] }), expect.any(Function) ); diff --git a/src/core/server/elasticsearch/elasticsearch_service.ts b/src/core/server/elasticsearch/elasticsearch_service.ts index 4e90ff2e2baeca..16430a13b69b8b 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.ts @@ -18,7 +18,8 @@ */ import { ConnectableObservable, Observable, Subscription } from 'rxjs'; -import { filter, map, publishReplay, switchMap } from 'rxjs/operators'; +import { filter, first, map, publishReplay, switchMap } from 'rxjs/operators'; +import { merge } from 'lodash'; import { CoreService } from '../../types'; import { CoreContext } from '../core_context'; import { Logger } from '../logging'; @@ -45,7 +46,10 @@ export interface ElasticsearchServiceSetup { readonly config$: Observable; }; - readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient; + readonly createClient: ( + type: string, + config: Partial + ) => ClusterClient; readonly adminClient$: Observable; readonly dataClient$: Observable; } @@ -100,15 +104,16 @@ export class ElasticsearchService implements CoreService; this.subscription = clients$.connect(); - + const config = await this.config$.pipe(first()).toPromise(); return { legacy: { config$: clients$.pipe(map(clients => clients.config)) }, adminClient$: clients$.pipe(map(clients => clients.adminClient)), dataClient$: clients$.pipe(map(clients => clients.dataClient)), - createClient: (type: string, clientConfig: ElasticsearchClientConfig) => { - return this.createClusterClient(type, clientConfig, deps.http.auth.getAuthHeaders); + createClient: (type: string, clientConfig: Partial) => { + const finalConfig = merge({}, config, clientConfig); + return this.createClusterClient(type, finalConfig, deps.http.auth.getAuthHeaders); }, }; } diff --git a/src/legacy/core_plugins/elasticsearch/index.js b/src/legacy/core_plugins/elasticsearch/index.js index 8dc9c77e79d994..4ee567d2148a33 100644 --- a/src/legacy/core_plugins/elasticsearch/index.js +++ b/src/legacy/core_plugins/elasticsearch/index.js @@ -80,10 +80,7 @@ export default function (kibana) { // We fill all the missing properties in the `clientConfig` using the default // Elasticsearch config so that we don't depend on default values set and // controlled by underlying Elasticsearch JS client. - const cluster = new Cluster(server.newPlatform.setup.core.elasticsearch.createClient(name, { - ...esConfig, - ...clientConfig, - })); + const cluster = new Cluster(server.newPlatform.setup.core.elasticsearch.createClient(name, clientConfig)); clusters.set(name, cluster); From 42f41f87c5276901ab8337e956d3b529f6cda255 Mon Sep 17 00:00:00 2001 From: restrry Date: Mon, 8 Jul 2019 10:38:43 +0200 Subject: [PATCH 2/3] add tests --- .../elasticsearch_service.test.ts | 102 ++++++++++++++---- .../elasticsearch/elasticsearch_service.ts | 23 +++- .../core_plugins/elasticsearch/index.js | 3 - 3 files changed, 101 insertions(+), 27 deletions(-) diff --git a/src/core/server/elasticsearch/elasticsearch_service.test.ts b/src/core/server/elasticsearch/elasticsearch_service.test.ts index cbb7be280459ef..e84ddb2baa30cd 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.test.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.test.ts @@ -39,8 +39,12 @@ const deps = { configService.atPath.mockReturnValue( new BehaviorSubject({ hosts: ['http://1.2.3.4'], - healthCheck: {}, - ssl: {}, + healthCheck: { + delay: 2000, + }, + ssl: { + verificationMode: 'none', + }, } as any) ); @@ -57,7 +61,7 @@ beforeEach(() => { afterEach(() => jest.clearAllMocks()); describe('#setup', () => { - test('returns legacy Elasticsearch config as a part of the contract', async () => { + it('returns legacy Elasticsearch config as a part of the contract', async () => { const setupContract = await elasticsearchService.setup(deps); await expect(setupContract.legacy.config$.pipe(first()).toPromise()).resolves.toBeInstanceOf( @@ -65,7 +69,7 @@ describe('#setup', () => { ); }); - test('returns data and admin client observables as a part of the contract', async () => { + it('returns data and admin client observables as a part of the contract', async () => { const mockAdminClusterClientInstance = { close: jest.fn() }; const mockDataClusterClientInstance = { close: jest.fn() }; MockClusterClient.mockImplementationOnce( @@ -103,27 +107,83 @@ describe('#setup', () => { expect(mockDataClusterClientInstance.close).not.toHaveBeenCalled(); }); - test('returns `createClient` as a part of the contract', async () => { - const setupContract = await elasticsearchService.setup(deps); - - const mockClusterClientInstance = { close: jest.fn() }; - MockClusterClient.mockImplementation(() => mockClusterClientInstance); - - const mockConfig = { logQueries: true }; - const clusterClient = setupContract.createClient('some-custom-type', mockConfig as any); - - expect(clusterClient).toBe(mockClusterClientInstance); - - expect(MockClusterClient).toHaveBeenCalledWith( - expect.objectContaining(mockConfig), - expect.objectContaining({ context: ['elasticsearch', 'some-custom-type'] }), - expect.any(Function) - ); + describe('#createClient', () => { + it('allows to specify config properties', async () => { + const setupContract = await elasticsearchService.setup(deps); + + const mockClusterClientInstance = { close: jest.fn() }; + MockClusterClient.mockImplementation(() => mockClusterClientInstance); + + const customConfig = { logQueries: true }; + const clusterClient = setupContract.createClient('some-custom-type', customConfig); + + expect(clusterClient).toBe(mockClusterClientInstance); + + expect(MockClusterClient).toHaveBeenCalledWith( + expect.objectContaining(customConfig), + expect.objectContaining({ context: ['elasticsearch', 'some-custom-type'] }), + expect.any(Function) + ); + }); + + it('falls back to elasticsearch default config values if property not specified', async () => { + const setupContract = await elasticsearchService.setup(deps); + // reset all mocks called during setup phase + MockClusterClient.mockClear(); + + const customConfig = { + hosts: ['http://8.8.8.8'], + logQueries: true, + ssl: { certificate: 'certificate-value' }, + }; + setupContract.createClient('some-custom-type', customConfig); + + const config = MockClusterClient.mock.calls[0][0]; + expect(config).toMatchInlineSnapshot(` +Object { + "healthCheckDelay": 2000, + "hosts": Array [ + "http://8.8.8.8", + ], + "logQueries": true, + "requestHeadersWhitelist": Array [ + undefined, + ], + "ssl": Object { + "certificate": "certificate-value", + "verificationMode": "none", + }, +} +`); + }); + it('falls back to elasticsearch config if custom config not passed', async () => { + const setupContract = await elasticsearchService.setup(deps); + // reset all mocks called during setup phase + MockClusterClient.mockClear(); + + setupContract.createClient('another-type'); + + const config = MockClusterClient.mock.calls[0][0]; + expect(config).toMatchInlineSnapshot(` +Object { + "healthCheckDelay": 2000, + "hosts": Array [ + "http://1.2.3.4", + ], + "requestHeadersWhitelist": Array [ + undefined, + ], + "ssl": Object { + "verificationMode": "none", + }, +} +`); + }); }); }); describe('#stop', () => { - test('stops both admin and data clients', async () => { + it('stops both admin and data clients', async () => { const mockAdminClusterClientInstance = { close: jest.fn() }; const mockDataClusterClientInstance = { close: jest.fn() }; MockClusterClient.mockImplementationOnce( diff --git a/src/core/server/elasticsearch/elasticsearch_service.ts b/src/core/server/elasticsearch/elasticsearch_service.ts index 16430a13b69b8b..bd34523db69754 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.ts @@ -45,10 +45,25 @@ export interface ElasticsearchServiceSetup { readonly legacy: { readonly config$: Observable; }; - + /** + * Create application specific Elasticsearch cluster API client with customized config. + * + * @param type Unique identifier of the client + * @param config Valid sub-set of default Elasticsearch config. + * We fill all the missing properties in the `clientConfig` using the default + * Elasticsearch config so that we don't depend on default values set and + * controlled by underlying Elasticsearch JS client. + * We don't run validation against passed config expect it to be valid. + * + * @example + * ``` + * const client = elasticsearch.createCluster('my-app-name', config); + * const data = await client.callAsInternalUser(); + * ``` + */ readonly createClient: ( type: string, - config: Partial + config?: Partial ) => ClusterClient; readonly adminClient$: Observable; readonly dataClient$: Observable; @@ -104,14 +119,16 @@ export class ElasticsearchService implements CoreService; this.subscription = clients$.connect(); + const config = await this.config$.pipe(first()).toPromise(); + return { legacy: { config$: clients$.pipe(map(clients => clients.config)) }, adminClient$: clients$.pipe(map(clients => clients.adminClient)), dataClient$: clients$.pipe(map(clients => clients.dataClient)), - createClient: (type: string, clientConfig: Partial) => { + createClient: (type: string, clientConfig: Partial = {}) => { const finalConfig = merge({}, config, clientConfig); return this.createClusterClient(type, finalConfig, deps.http.auth.getAuthHeaders); }, diff --git a/src/legacy/core_plugins/elasticsearch/index.js b/src/legacy/core_plugins/elasticsearch/index.js index 4ee567d2148a33..51dac9e3a46826 100644 --- a/src/legacy/core_plugins/elasticsearch/index.js +++ b/src/legacy/core_plugins/elasticsearch/index.js @@ -77,9 +77,6 @@ export default function (kibana) { throw new Error(`cluster '${name}' already exists`); } - // We fill all the missing properties in the `clientConfig` using the default - // Elasticsearch config so that we don't depend on default values set and - // controlled by underlying Elasticsearch JS client. const cluster = new Cluster(server.newPlatform.setup.core.elasticsearch.createClient(name, clientConfig)); clusters.set(name, cluster); From 6ea0cb722880114c8245990aba119a9689be1251 Mon Sep 17 00:00:00 2001 From: restrry Date: Mon, 8 Jul 2019 10:45:57 +0200 Subject: [PATCH 3/3] re-genereate docs --- ...bana-plugin-server.elasticsearchclientconfig.md | 1 + ...erver.elasticsearchservicesetup.createclient.md | 14 +++++++++++++- ...bana-plugin-server.elasticsearchservicesetup.md | 2 +- .../elasticsearch/elasticsearch_client_config.ts | 2 +- .../server/elasticsearch/elasticsearch_service.ts | 7 ++++--- src/core/server/server.api.md | 3 +-- 6 files changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-server.elasticsearchclientconfig.md b/docs/development/core/server/kibana-plugin-server.elasticsearchclientconfig.md index cd42a84f621a91..39e6ac428292be 100644 --- a/docs/development/core/server/kibana-plugin-server.elasticsearchclientconfig.md +++ b/docs/development/core/server/kibana-plugin-server.elasticsearchclientconfig.md @@ -4,6 +4,7 @@ ## ElasticsearchClientConfig type + Signature: ```typescript diff --git a/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.createclient.md b/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.createclient.md index 8b1ce883c4a147..c29d3fbbf69ab8 100644 --- a/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.createclient.md +++ b/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.createclient.md @@ -4,8 +4,20 @@ ## ElasticsearchServiceSetup.createClient property +Create application specific Elasticsearch cluster API client with customized config. + Signature: ```typescript -readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient; +readonly createClient: (type: string, clientConfig?: Partial) => ClusterClient; ``` + +## Example + + +```js +const client = elasticsearch.createCluster('my-app-name', config); +const data = await client.callAsInternalUser(); + +``` + diff --git a/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.md b/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.md index 3197869d6d6634..a295022971a147 100644 --- a/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.md +++ b/docs/development/core/server/kibana-plugin-server.elasticsearchservicesetup.md @@ -16,7 +16,7 @@ export interface ElasticsearchServiceSetup | Property | Type | Description | | --- | --- | --- | | [adminClient$](./kibana-plugin-server.elasticsearchservicesetup.adminclient$.md) | Observable<ClusterClient> | | -| [createClient](./kibana-plugin-server.elasticsearchservicesetup.createclient.md) | (type: string, config: ElasticsearchClientConfig) => ClusterClient | | +| [createClient](./kibana-plugin-server.elasticsearchservicesetup.createclient.md) | (type: string, clientConfig?: Partial<ElasticsearchClientConfig>) => ClusterClient | Create application specific Elasticsearch cluster API client with customized config. | | [dataClient$](./kibana-plugin-server.elasticsearchservicesetup.dataclient$.md) | Observable<ClusterClient> | | | [legacy](./kibana-plugin-server.elasticsearchservicesetup.legacy.md) | {
readonly config$: Observable<ElasticsearchConfig>;
} | | diff --git a/src/core/server/elasticsearch/elasticsearch_client_config.ts b/src/core/server/elasticsearch/elasticsearch_client_config.ts index 9fe56bf8c7e582..dcc09f711abbeb 100644 --- a/src/core/server/elasticsearch/elasticsearch_client_config.ts +++ b/src/core/server/elasticsearch/elasticsearch_client_config.ts @@ -28,7 +28,7 @@ import { Logger } from '../logging'; import { ElasticsearchConfig } from './elasticsearch_config'; /** - * @internalremarks Config that consumers can pass to the Elasticsearch JS client is complex and includes + * @privateRemarks Config that consumers can pass to the Elasticsearch JS client is complex and includes * not only entries from standard `elasticsearch.*` yaml config, but also some Elasticsearch JS * client specific options like `keepAlive` or `plugins` (that eventually will be deprecated). * diff --git a/src/core/server/elasticsearch/elasticsearch_service.ts b/src/core/server/elasticsearch/elasticsearch_service.ts index bd34523db69754..38a0d19b1ae3f2 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.ts @@ -49,21 +49,22 @@ export interface ElasticsearchServiceSetup { * Create application specific Elasticsearch cluster API client with customized config. * * @param type Unique identifier of the client - * @param config Valid sub-set of default Elasticsearch config. + * @param clientConfig A config consists of Elasticsearch JS client options and + * valid sub-set of Elasticsearch service config. * We fill all the missing properties in the `clientConfig` using the default * Elasticsearch config so that we don't depend on default values set and * controlled by underlying Elasticsearch JS client. * We don't run validation against passed config expect it to be valid. * * @example - * ``` + * ```js * const client = elasticsearch.createCluster('my-app-name', config); * const data = await client.callAsInternalUser(); * ``` */ readonly createClient: ( type: string, - config?: Partial + clientConfig?: Partial ) => ClusterClient; readonly adminClient$: Observable; readonly dataClient$: Observable; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index b1fdc1289151ce..79d784224eccd7 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -142,8 +142,7 @@ export class ElasticsearchErrorHelpers { export interface ElasticsearchServiceSetup { // (undocumented) readonly adminClient$: Observable; - // (undocumented) - readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient; + readonly createClient: (type: string, clientConfig?: Partial) => ClusterClient; // (undocumented) readonly dataClient$: Observable; // (undocumented)