Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kibana request headers #79218

Merged
merged 14 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/core/server/elasticsearch/client/client_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { duration } from 'moment';
import { ElasticsearchClientConfig, parseClientOptions } from './client_config';
import { DEFAULT_HEADERS } from '../default_headers';

const createConfig = (
parts: Partial<ElasticsearchClientConfig> = {}
Expand All @@ -36,6 +37,18 @@ const createConfig = (
};

describe('parseClientOptions', () => {
it('includes headers designing the HTTP request as originating from Kibana by default', () => {
const config = createConfig({});

expect(parseClientOptions(config, false)).toEqual(
expect.objectContaining({
headers: {
...DEFAULT_HEADERS,
},
})
);
});

describe('basic options', () => {
it('`customHeaders` option', () => {
const config = createConfig({
Expand All @@ -48,13 +61,33 @@ describe('parseClientOptions', () => {
expect(parseClientOptions(config, false)).toEqual(
expect.objectContaining({
headers: {
...DEFAULT_HEADERS,
foo: 'bar',
hello: 'dolly',
},
})
);
});

it('`customHeaders` take precedence to default kibana headers', () => {
const customHeader = {
[Object.keys(DEFAULT_HEADERS)[0]]: 'foo',
};
const config = createConfig({
customHeaders: {
...customHeader,
},
});

expect(parseClientOptions(config, false)).toEqual(
expect.objectContaining({
headers: {
...customHeader,
},
})
);
});

it('`keepAlive` option', () => {
expect(parseClientOptions(createConfig({ keepAlive: true }), false)).toEqual(
expect.objectContaining({ agent: { keepAlive: true } })
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/elasticsearch/client/client_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { URL } from 'url';
import { Duration } from 'moment';
import { ClientOptions, NodeOptions } from '@elastic/elasticsearch';
import { ElasticsearchConfig } from '../elasticsearch_config';
import { DEFAULT_HEADERS } from '../default_headers';

/**
* Configuration options to be used to create a {@link IClusterClient | cluster client} using the
Expand Down Expand Up @@ -61,7 +62,10 @@ export function parseClientOptions(
const clientOptions: ClientOptions = {
sniffOnStart: config.sniffOnStart,
sniffOnConnectionFault: config.sniffOnConnectionFault,
headers: config.customHeaders,
headers: {
...DEFAULT_HEADERS,
...config.customHeaders,
},
};

if (config.pingTimeout != null) {
Expand Down
62 changes: 57 additions & 5 deletions src/core/server/elasticsearch/client/cluster_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { GetAuthHeaders } from '../../http';
import { elasticsearchClientMock } from './mocks';
import { ClusterClient } from './cluster_client';
import { ElasticsearchClientConfig } from './client_config';
import { DEFAULT_HEADERS } from '../default_headers';

const createConfig = (
parts: Partial<ElasticsearchClientConfig> = {}
Expand Down Expand Up @@ -127,7 +128,7 @@ describe('ClusterClient', () => {

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { foo: 'bar', 'x-opaque-id': expect.any(String) },
headers: { ...DEFAULT_HEADERS, foo: 'bar', 'x-opaque-id': expect.any(String) },
});
});

Expand All @@ -147,7 +148,7 @@ describe('ClusterClient', () => {

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { authorization: 'auth', 'x-opaque-id': expect.any(String) },
headers: { ...DEFAULT_HEADERS, authorization: 'auth', 'x-opaque-id': expect.any(String) },
});
});

Expand All @@ -171,7 +172,7 @@ describe('ClusterClient', () => {

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { authorization: 'auth', 'x-opaque-id': expect.any(String) },
headers: { ...DEFAULT_HEADERS, authorization: 'auth', 'x-opaque-id': expect.any(String) },
});
});

Expand All @@ -193,6 +194,7 @@ describe('ClusterClient', () => {
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
...DEFAULT_HEADERS,
foo: 'bar',
hello: 'dolly',
'x-opaque-id': expect.any(String),
Expand All @@ -214,6 +216,7 @@ describe('ClusterClient', () => {
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
...DEFAULT_HEADERS,
'x-opaque-id': 'my-fake-id',
},
});
Expand All @@ -239,6 +242,7 @@ describe('ClusterClient', () => {
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
...DEFAULT_HEADERS,
foo: 'auth',
hello: 'dolly',
'x-opaque-id': expect.any(String),
Expand Down Expand Up @@ -266,13 +270,60 @@ describe('ClusterClient', () => {
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
...DEFAULT_HEADERS,
foo: 'request',
hello: 'dolly',
'x-opaque-id': expect.any(String),
},
});
});

it('respect the precedence of config headers over default headers', () => {
const headerKey = Object.keys(DEFAULT_HEADERS)[0];
const config = createConfig({
customHeaders: {
[headerKey]: 'foo',
},
});
getAuthHeaders.mockReturnValue({});

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

clusterClient.asScoped(request);

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
[headerKey]: 'foo',
'x-opaque-id': expect.any(String),
},
});
});

it('respect the precedence of request headers over default headers', () => {
const headerKey = Object.keys(DEFAULT_HEADERS)[0];
const config = createConfig({
requestHeadersWhitelist: [headerKey],
});
getAuthHeaders.mockReturnValue({});

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

clusterClient.asScoped(request);

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
[headerKey]: 'foo',
'x-opaque-id': expect.any(String),
},
});
});

it('respect the precedence of x-opaque-id header over config headers', () => {
const config = createConfig({
customHeaders: {
Expand All @@ -292,6 +343,7 @@ describe('ClusterClient', () => {
expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: {
...DEFAULT_HEADERS,
'x-opaque-id': 'from request',
},
});
Expand All @@ -315,7 +367,7 @@ describe('ClusterClient', () => {

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

Expand All @@ -339,7 +391,7 @@ describe('ClusterClient', () => {

expect(scopedClient.child).toHaveBeenCalledTimes(1);
expect(scopedClient.child).toHaveBeenCalledWith({
headers: { foo: 'bar' },
headers: { ...DEFAULT_HEADERS, foo: 'bar' },
});
});
});
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/elasticsearch/client/cluster_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ElasticsearchClient } from './types';
import { configureClient } from './configure_client';
import { ElasticsearchClientConfig } from './client_config';
import { ScopedClusterClient, IScopedClusterClient } from './scoped_cluster_client';
import { DEFAULT_HEADERS } from '../default_headers';

const noop = () => undefined;

Expand Down Expand Up @@ -108,6 +109,7 @@ export class ClusterClient implements ICustomClusterClient {
}

return {
...DEFAULT_HEADERS,
...this.config.customHeaders,
...scopedHeaders,
};
Expand Down
27 changes: 27 additions & 0 deletions src/core/server/elasticsearch/default_headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { deepFreeze } from '@kbn/std';

export const DEFAULT_HEADERS = deepFreeze({
// Elasticsearch uses this to identify when a request is coming from Kibana, to allow Kibana to
// access system indices using the standard ES APIs without logging a warning. After migrating to
// use the new system index APIs, this header can be removed.
'x-elastic-product-origin': 'kibana',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think someone reading this code would benefit from a comment that explains the intention? E.g.

Elasticsearch uses this to identify when a request is coming from Kibana,
to allow Kibana deprecated access to system indices logging a warning.
We'll migrate to use new system index APIs in the future, which will allow
us to remove this header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, adding that!

});
Loading