Skip to content

Commit

Permalink
Remove generated facade, use ES Client directly
Browse files Browse the repository at this point in the history
  • Loading branch information
@pgayvallet
pgayvallet committed Jul 3, 2020
1 parent 1a68ea1 commit f956fb4
Show file tree
Hide file tree
Showing 16 changed files with 279 additions and 3,160 deletions.
416 changes: 0 additions & 416 deletions src/core/server/elasticsearch/client/client_facade.mock.ts
View file

This file was deleted.

1,858 changes: 0 additions & 1,858 deletions src/core/server/elasticsearch/client/client_facade.ts
View file

This file was deleted.

5 changes: 0 additions & 5 deletions src/core/server/elasticsearch/client/cluster_client.test.mocks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,3 @@ export const configureClientMock = jest.fn();
jest.doMock('./configure_client', () => ({
configureClient: configureClientMock,
}));

export const getClientFacadeMock = jest.fn();
jest.doMock('./get_client_facade', () => ({
getClientFacade: getClientFacadeMock,
}));
167 changes: 103 additions & 64 deletions src/core/server/elasticsearch/client/cluster_client.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,14 @@
* under the License.
*/

import type { Client } from '@elastic/elasticsearch';
import { configureClientMock, getClientFacadeMock } from './cluster_client.test.mocks';
import { configureClientMock } from './cluster_client.test.mocks';
import { loggingSystemMock } from '../../logging/logging_system.mock';
import { httpServerMock } from '../../http/http_server.mocks';
import { GetAuthHeaders } from '../../http';
import { elasticsearchClientMock } from './mocks';
import { ClusterClient } from './cluster_client';
import { ElasticsearchClientConfig } from './client_config';

const createClientMock = (): jest.Mocked<Client> => {
return ({
close: jest.fn(),
} as unknown) as jest.Mocked<Client>;
};

const createConfig = (
parts: Partial<ElasticsearchClientConfig> = {}
): ElasticsearchClientConfig => {
Expand All @@ -50,15 +43,13 @@ const createConfig = (
describe('ClusterClient', () => {
let logger: ReturnType<typeof loggingSystemMock.createLogger>;
let getAuthHeaders: jest.MockedFunction<GetAuthHeaders>;
let internalClient: jest.Mocked<Client>;
let scopedClient: jest.Mocked<Client>;
let internalFacade: ReturnType<typeof elasticsearchClientMock.createFacade>;
let internalClient: ReturnType<typeof elasticsearchClientMock.createInternalClient>;
let scopedClient: ReturnType<typeof elasticsearchClientMock.createInternalClient>;

beforeEach(() => {
logger = loggingSystemMock.createLogger();
internalClient = createClientMock();
scopedClient = createClientMock();
internalFacade = elasticsearchClientMock.createFacade();
internalClient = elasticsearchClientMock.createInternalClient();
scopedClient = elasticsearchClientMock.createInternalClient();
getAuthHeaders = jest.fn().mockImplementation(() => ({
authorization: 'auth',
foo: 'bar',
Expand All @@ -67,18 +58,10 @@ describe('ClusterClient', () => {
configureClientMock.mockImplementation((config, { scoped = false }) => {
return scoped ? scopedClient : internalClient;
});

getClientFacadeMock.mockImplementation((client) => {
if (client === internalClient) {
return internalFacade;
}
return elasticsearchClientMock.createFacade();
});
});

afterEach(() => {
configureClientMock.mockReset();
getClientFacadeMock.mockReset();
});

it('creates a single internal and scoped client during initialization', () => {
Expand All @@ -89,19 +72,13 @@ describe('ClusterClient', () => {
expect(configureClientMock).toHaveBeenCalledTimes(2);
expect(configureClientMock).toHaveBeenCalledWith(config, { logger });
expect(configureClientMock).toHaveBeenCalledWith(config, { logger, scoped: true });

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(internalClient);
});

describe('#asInternalUser', () => {
it('returns the facade using the internal client', () => {
it('returns the internal client', () => {
const clusterClient = new ClusterClient(createConfig(), logger, getAuthHeaders);

getClientFacadeMock.mockClear();

expect(clusterClient.asInternalUser()).toBe(internalFacade);
expect(getClientFacadeMock).not.toHaveBeenCalled();
expect(clusterClient.asInternalUser()).toBe(internalClient);
});
});

Expand All @@ -110,33 +87,29 @@ describe('ClusterClient', () => {
const clusterClient = new ClusterClient(createConfig(), logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest();

getClientFacadeMock.mockClear();

const scopedClusterClient = clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, expect.any(Object));
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({ headers: expect.any(Object) });

expect(scopedClusterClient.asInternalUser()).toBe(clusterClient.asInternalUser());
expect(scopedClusterClient.asCurrentUser()).toBe(getClientFacadeMock.mock.results[0].value);
expect(scopedClusterClient.asCurrentUser()).toBe(scopedClient.child.mock.results[0].value);
});

it('returns a distinct facade on each call', () => {
it('returns a distinct scoped cluster client on each call', () => {
const clusterClient = new ClusterClient(createConfig(), logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest();

getClientFacadeMock.mockClear();

const scopedClusterClient1 = clusterClient.asScoped(request);
const scopedClusterClient2 = clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(2);
expect(scopedClient.child).toHaveBeenCalledTimes(2);

expect(scopedClusterClient1).not.toBe(scopedClusterClient2);
expect(scopedClusterClient1.asInternalUser()).toBe(scopedClusterClient2.asInternalUser());
});

it('creates a scoped facade with filtered request headers', () => {
it('creates a scoped client with filtered request headers', () => {
const config = createConfig({
requestHeadersWhitelist: ['foo'],
});
Expand All @@ -150,13 +123,11 @@ describe('ClusterClient', () => {
},
});

getClientFacadeMock.mockClear();

clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, {
foo: 'bar',
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { foo: 'bar' },
});
});

Expand All @@ -172,13 +143,11 @@ describe('ClusterClient', () => {
const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest({});

getClientFacadeMock.mockClear();

clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, {
authorization: 'auth',
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { authorization: 'auth' },
});
});

Expand All @@ -198,13 +167,87 @@ describe('ClusterClient', () => {
},
});

getClientFacadeMock.mockClear();
clusterClient.asScoped(request);

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { authorization: 'auth' },
});
});

it('includes the `customHeaders` from the config when creating the child client', () => {
const config = createConfig({
customHeaders: {
foo: 'bar',
hello: 'dolly',
},
requestHeadersWhitelist: ['authorization'],
});
getAuthHeaders.mockReturnValue({});

const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest({});

clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, {
authorization: 'auth',
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
foo: 'bar',
hello: 'dolly',
},
});
});

it('respect the precedence of auth headers over config headers', () => {
const config = createConfig({
customHeaders: {
foo: 'config',
hello: 'dolly',
},
requestHeadersWhitelist: ['foo'],
});
getAuthHeaders.mockReturnValue({
foo: 'auth',
});

const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest({});

clusterClient.asScoped(request);

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
foo: 'auth',
hello: 'dolly',
},
});
});

it('respect the precedence of request headers over config headers', () => {
const config = createConfig({
customHeaders: {
foo: 'config',
hello: 'dolly',
},
requestHeadersWhitelist: ['foo'],
});
getAuthHeaders.mockReturnValue({});

const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
const request = httpServerMock.createKibanaRequest({
headers: { foo: 'request' },
});

clusterClient.asScoped(request);

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
foo: 'request',
hello: 'dolly',
},
});
});

Expand All @@ -222,13 +265,11 @@ describe('ClusterClient', () => {
},
};

getClientFacadeMock.mockClear();

clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, {
authorization: 'auth',
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { authorization: 'auth' },
});
});

Expand All @@ -248,13 +289,11 @@ describe('ClusterClient', () => {
},
};

getClientFacadeMock.mockClear();

clusterClient.asScoped(request);

expect(getClientFacadeMock).toHaveBeenCalledTimes(1);
expect(getClientFacadeMock).toHaveBeenCalledWith(scopedClient, {
foo: 'bar',
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { foo: 'bar' },
});
});
});
Expand Down
44 changes: 26 additions & 18 deletions src/core/server/elasticsearch/client/cluster_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import { Logger } from '../../logging';
import { GetAuthHeaders, isRealRequest, Headers } from '../../http';
import { ensureRawRequest, filterHeaders } from '../../http/router';
import { ScopeableRequest } from '../types';
import { getClientFacade } from './get_client_facade';
import { ClientFacade } from './client_facade';
import { ElasticSearchClient } from './types';
import { configureClient } from './configure_client';
import { ElasticsearchClientConfig } from './client_config';
import { ScopedClusterClient, IScopedClusterClient } from './scoped_cluster_client';
Expand All @@ -39,9 +38,9 @@ const noop = () => undefined;
**/
export interface IClusterClient {
/**
* Returns a {@link ClientFacade | facade} to be used to query the ES cluster on behalf of the Kibana internal user
* Returns a {@link ElasticSearchClient | client} to be used to query the ES cluster on behalf of the Kibana internal user
*/
asInternalUser: () => ClientFacade;
asInternalUser: () => ElasticSearchClient;
/**
* Creates a {@link IScopedClusterClient | scoped cluster client} bound to given {@link ScopeableRequest | request}
*/
Expand All @@ -64,9 +63,8 @@ export interface ICustomClusterClient extends IClusterClient {
/** @internal **/
export class ClusterClient implements IClusterClient, ICustomClusterClient {
private readonly internalClient: Client;
private readonly scopedClient: Client;
private readonly rootScopedClient: Client;

private readonly internalFacade: ClientFacade;
private isClosed = false;

constructor(
Expand All @@ -75,18 +73,19 @@ export class ClusterClient implements IClusterClient, ICustomClusterClient {
private readonly getAuthHeaders: GetAuthHeaders = noop
) {
this.internalClient = configureClient(config, { logger });
this.internalFacade = getClientFacade(this.internalClient);
this.scopedClient = configureClient(config, { logger, scoped: true });
this.rootScopedClient = configureClient(config, { logger, scoped: true });
}

asInternalUser() {
return this.internalFacade;
return this.internalClient;
}

asScoped(request: ScopeableRequest) {
const headers = this.getScopedHeaders(request);
const scopedWrapper = getClientFacade(this.scopedClient, headers);
return new ScopedClusterClient(this.internalFacade, scopedWrapper);
const scopedHeaders = this.getScopedHeaders(request);
const scopedClient = this.rootScopedClient.child({
headers: scopedHeaders,
});
return new ScopedClusterClient(this.internalClient, scopedClient);
}

public close() {
Expand All @@ -96,16 +95,25 @@ export class ClusterClient implements IClusterClient, ICustomClusterClient {

this.isClosed = true;
this.internalClient.close();
this.scopedClient.close();
this.rootScopedClient.close();
}

private getScopedHeaders(request: ScopeableRequest): Headers {
if (!isRealRequest(request)) {
return filterHeaders(request?.headers ?? {}, this.config.requestHeadersWhitelist);
let scopedHeaders: Headers;
if (isRealRequest(request)) {
const authHeaders = this.getAuthHeaders(request);
const requestHeaders = ensureRawRequest(request).headers;
scopedHeaders = filterHeaders(
{ ...requestHeaders, ...authHeaders },
this.config.requestHeadersWhitelist
);
} else {
scopedHeaders = filterHeaders(request?.headers ?? {}, this.config.requestHeadersWhitelist);
}
const authHeaders = this.getAuthHeaders(request);
const headers = ensureRawRequest(request).headers;

return filterHeaders({ ...headers, ...authHeaders }, this.config.requestHeadersWhitelist);
return {
...this.config.customHeaders,
...scopedHeaders,
};
}
}
Loading

0 comments on commit f956fb4

Please sign in to comment.