Skip to content

Commit

Permalink
createCluster requires a partial elasticsearch config (#40405)
Browse files Browse the repository at this point in the history
* createCluster created with partial config

* add tests

* re-genereate docs
  • Loading branch information
mshustov authored Jul 9, 2019
1 parent ba1f7e3 commit 9920c87
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

## ElasticsearchClientConfig type


<b>Signature:</b>

```typescript
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,20 @@

## ElasticsearchServiceSetup.createClient property

Create application specific Elasticsearch cluster API client with customized config.

<b>Signature:</b>

```typescript
readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient;
readonly createClient: (type: string, clientConfig?: Partial<ElasticsearchClientConfig>) => ClusterClient;
```

## Example


```js
const client = elasticsearch.createCluster('my-app-name', config);
const data = await client.callAsInternalUser();

```

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface ElasticsearchServiceSetup
| Property | Type | Description |
| --- | --- | --- |
| [adminClient$](./kibana-plugin-server.elasticsearchservicesetup.adminclient$.md) | <code>Observable&lt;ClusterClient&gt;</code> | |
| [createClient](./kibana-plugin-server.elasticsearchservicesetup.createclient.md) | <code>(type: string, config: ElasticsearchClientConfig) =&gt; ClusterClient</code> | |
| [createClient](./kibana-plugin-server.elasticsearchservicesetup.createclient.md) | <code>(type: string, clientConfig?: Partial&lt;ElasticsearchClientConfig&gt;) =&gt; ClusterClient</code> | Create application specific Elasticsearch cluster API client with customized config. |
| [dataClient$](./kibana-plugin-server.elasticsearchservicesetup.dataclient$.md) | <code>Observable&lt;ClusterClient&gt;</code> | |
| [legacy](./kibana-plugin-server.elasticsearchservicesetup.legacy.md) | <code>{</code><br/><code> readonly config$: Observable&lt;ElasticsearchConfig&gt;;</code><br/><code> }</code> | |

Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
Expand Down
102 changes: 81 additions & 21 deletions src/core/server/elasticsearch/elasticsearch_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);

Expand All @@ -57,15 +61,15 @@ 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(
ElasticsearchConfig
);
});

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(
Expand Down Expand Up @@ -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(
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(
Expand Down
33 changes: 28 additions & 5 deletions src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -44,8 +45,27 @@ export interface ElasticsearchServiceSetup {
readonly legacy: {
readonly config$: Observable<ElasticsearchConfig>;
};

readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient;
/**
* Create application specific Elasticsearch cluster API client with customized config.
*
* @param type Unique identifier of the client
* @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,
clientConfig?: Partial<ElasticsearchClientConfig>
) => ClusterClient;
readonly adminClient$: Observable<ClusterClient>;
readonly dataClient$: Observable<ClusterClient>;
}
Expand Down Expand Up @@ -101,14 +121,17 @@ export class ElasticsearchService implements CoreService<ElasticsearchServiceSet

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<ElasticsearchClientConfig> = {}) => {
const finalConfig = merge({}, config, clientConfig);
return this.createClusterClient(type, finalConfig, deps.http.auth.getAuthHeaders);
},
};
}
Expand Down
3 changes: 1 addition & 2 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ export class ElasticsearchErrorHelpers {
export interface ElasticsearchServiceSetup {
// (undocumented)
readonly adminClient$: Observable<ClusterClient>;
// (undocumented)
readonly createClient: (type: string, config: ElasticsearchClientConfig) => ClusterClient;
readonly createClient: (type: string, clientConfig?: Partial<ElasticsearchClientConfig>) => ClusterClient;
// (undocumented)
readonly dataClient$: Observable<ClusterClient>;
// (undocumented)
Expand Down
8 changes: 1 addition & 7 deletions src/legacy/core_plugins/elasticsearch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,7 @@ 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, {
...esConfig,
...clientConfig,
}));
const cluster = new Cluster(server.newPlatform.setup.core.elasticsearch.createClient(name, clientConfig));

clusters.set(name, cluster);

Expand Down

0 comments on commit 9920c87

Please sign in to comment.